w3c / PNG-spec

Maintenance of the PNG specification
https://w3c.github.io/PNG-spec/
Other
43 stars 10 forks source link

PNGv2 12.11.2 -> PNGv3 12.10.2 transcription issue #446

Closed jbowler closed 3 weeks ago

jbowler commented 2 months ago

v2 12.11.2:

12.11.2 Private type and method codes This specification defines the meaning of only some of the possible values of some fields. For example, only compression method 0 and filter types 0 through 4 are defined in this International Standard. Numbers greater than 127 shall be used when inventing experimental or private definitions of values for any of these fields. Numbers below 128 are reserved for possible public extensions of this specification through future standardization (see 4.9 Extension and registration). The use of private type codes may render a datastream unreadable by standard decoders. Such codes are strongly discouraged except for experimental purposes, and should not appear in publicly available software or datastreams.

Becomes v12.10.2:

12.10.2 Use of non-reserved field values Encoders MAY use non-reserved field values for experimental or private use.

I feel this is excessive, but that's not an issue. Rather the issue is the extra specification:

5.8 Private field values Values greater than or equal to 128 in the following fields are private field values:

compression method interlace method filter method These private field values are neither defined nor reserved by this specification.

Private field values MAY be used for experimental or private semantics.

Private field values SHOULD NOT appear in publicly available software or datastreams since they can result in datastreams that are unreadable by PNG decoders as detailed at 13. PNG decoders and viewers.

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.

svgeesus commented 2 months ago

Those changes come from

ProgramMax commented 1 month ago

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.)

fintelia commented 1 month ago

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

jbowler commented 1 month ago

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.