Closed jbowler closed 3 months ago
Those changes come from
I think I agree that private color types should still be a thing. The fix would be just adding it to that list.
I don't think this allows private bit depth, or filter type though. Am I missing something? Although, it should allow those for the same reason that private color types should be acknowledged, right? (Simply adding them to the list would fix that. And it would confirm they work via the private bit 7.)
Rather than list out the possibly private fields, should we just say all fields can be private using that private bit? That would match the previous definition, right? I think the problem there is non-enum type fields. IHDR's width field shouldn't follow this.
So I think explicitly listing them out is the correct move. It cannot be all fields. We just need to update the list.
I think we are only talking about fields in IHDR here. Of which, I believe all but width and height can have private values: Bit depth, Colour type, Compression method, Filter method, Interlace method Which means the current list has 3 of the 5 fields. And the fix is just to add the other two fields (bit depth & colour type), right?
We could also consider filter type, which is a sub-value of filter method. I don't think this is strictly required. A user could create filter method 128 which has the same filter types as filter method 0, but additionally includes their new filter type.
If all private values are in IHDR, I'm tempted to not include private filter types.
(As a side note, the spec is VERY difficult to follow when trying to get information about filter types. We should improve that later.)
The second edition spec says:
Filter method 0 specifies exactly this set of five filter types and this shall not be extended. This ensures that decoders need not decompress the data to determine whether it contains unsupported filter types: it is sufficient to check the filter method in IHDR.
Probably best to say you need a private filter method if you want to add more filter types
Probably best to say you need a private filter method if you want to add more filter types
"Filter method" and "filter type" are used interchangeably to refer to the value of the byte in the IHDR chunk. This leads to endless confusion; within the libpng API definition (png.h) "filter_method" is used when setting/getting the IHDR values (this is a foolish consistency if ever there was one) but the API to get the value from a PNG that has been read is "png_get_filter_type". The actual filter algorithm is not necessarily a single number; a new filter type/method might, for example, use different filter algorithms on different image channels.
In fact libpng is worse that that; the declaration of png_get_IHDR uses "method" throughout, the definition uses "type" throughout. That has caused me considerably confusion at least once.
Nevertheless you are correct; MNG defines a value "64" to be used in the IHDR of a PNG which uses "intra-pixel differencing" for filtering.
Note that MNG uses a non-private value because the context (a PNG in a MNG stream) allows MNG to extend the public definitions. The choice of "64" was to give some space so that PNG can separately add filter types without forcing MNG to fork the PNG spec (well, to stop taking updates...)
"Private" just means "all bets are off"; if a private value appears then it is necessary to look elsewhere for information about how to handle the stream. For chunks this is a bit like XML; the chunk signature functions the same was as an XML schema, but a critical chunk, a signature change or, indeed, embedding in a different stream (as in MNG, which does all three) suffice.
In general my conclusions for private values in IHDR are that while any of the encoding control bytes (color type, bit depth, interlace method, compression method, filter type) can use private or effectively private values (e.g. use 0 for bit depth) I would put a critical chunk either before or immediately after the IHDR. The latter, immediately after, I would do when just changing compression method or filter type because those changes don't alter the canonical frame-buffer format; an application does not need to concern itself with compression method or filter type because the library handles that. It does need to take action if it gets a color type or a bit depth or, indeed, an interlace method that it doesn't support.
BTW I see no problem with lying; PNG supports "RGB" or "luminance" data with an option per-pixel coverage value. Well, ok, if I have YCbCr I can change the filter type or the compression method to say that and I can still decode it to RGB. If my luma channel has 14 bits then I can still decode that to RGB 16-bit channels. If my coverage channel has only 3-bits (all that is required) I can still produce 8 if you really must have that :-) If I have an OpenEXR with (RA,GA,BA) and a depth map too that's just a different filter type and, if you're me, compression method. You can't take the private bit away from me.
v2 12.11.2:
Becomes v12.10.2:
I feel this is excessive, but that's not an issue. Rather the issue is the extra specification:
So the bug of consequence is that this disallows private "color types" which were explicitly allowed in PNGv2; section 5.8 is new.
It also allows private big depth, which was not allowed before, but also it allows addition of compression method, filter method, filter type and interlace method without using the private bit flag (bit 7). The bit flag is there to make it clear to everyone that this is a private, non-approved, extension/change.