Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tone-mapping should be stable when mDCv and cLLi are static #319

Open
palemieux opened this issue Jun 12, 2023 · 21 comments
Open

Tone-mapping should be stable when mDCv and cLLi are static #319

palemieux opened this issue Jun 12, 2023 · 21 comments
Assignees

Comments

@palemieux
Copy link
Contributor

If present, mDCv and cLLi SHALL completely define the tone mapping algorithm used by the decoder when rendering the image to a display.

mDCv and cLLi SHOULD be set. This is particularly important when drawing a temporal sequence of images. If mDCv and cLLi are not set, the tone mapping algorithm can vary over the sequence, resulting in temporal artifacts.

@palemieux palemieux self-assigned this Jun 12, 2023
@ProgramMax
Copy link
Collaborator

Just to be clear, I think "...completely define the tone mapping algorithm..." might just mean the decoder's choice of tone mapping is now set, right? IE we aren't defining what tone mapping algorithm the decoder should use. Just that it should be consistent with itself.

(The intention being that the decoder doesn't change which tone mapping algorithm it uses based on the image, for example...just the metadata.)

@fintelia
Copy link

Does "SHALL completely define" contradict "Ancillary chunks may be ignored by a decoder."?

@ProgramMax
Copy link
Collaborator

It shouldn't.
It's more like when mDCv and cLLI are present, the decoder has everything it needs to make a consistent decision about which tone mapping to use. I think that is the intent of the "shall completely define" phrasing. (And we want to enforce that it will indeed be consistent with itself.)

But those are still ancillary chunks.

@fintelia
Copy link

If a decoder sees that mDCv and cLLI are present but ignores them and does tone mapping based on the image contents instead, has it violated the spec?

@palemieux
Copy link
Contributor Author

If a decoder sees that mDCv and cLLI are present but ignores them and does tone mapping based on the image contents instead, has it violated the spec?

I think it should be strongly discouraged at the very least.

@palemieux
Copy link
Contributor Author

IE we aren't defining what tone mapping algorithm the decoder should use. Just that it should be consistent with itself.

Yes, the idea is that the HDR metadata, if present, should set all internal parameters of the algorithm, no matter what they are.

@ProgramMax
Copy link
Collaborator

It is a little tricky because the HDR metadata in cLLi is itself dependent on the image contents.

I could imagine a scenario where a new tone mapping algorithm is developed and the mDCv and cLLi chunks are not sufficient. So even though they are provided in an image, the decoder might ignore them.

However, that theoretical future decoder will run into the problem that spawned this issue: Two images next to each other or in a sequence like a flipbook might have a jarring, unintentional jump if they use different tone mapping algorithms.

The goal is to make the decoder consistent with itself. It can use the new tone mapping algorithm both times. That's fine. Said another way: So long as the decoder ignores both chunks consistently and always applies its tone mapping algorithm, that is fine.

@palemieux
Copy link
Contributor Author

Said another way: So long as the decoder ignores both chunks consistently and always applies its tone mapping algorithm, that is fine.

I think it should be a little bit more stringent: it should be possible for the author to strongly hint that multiple images should be tone-mapped ignoring their contents.

@palemieux
Copy link
Contributor Author

It is a little tricky because the HDR metadata in cLLi is itself dependent on the image contents.

cLLi is for an image sequence, not an individual image.

@svgeesus
Copy link
Contributor

It is a little tricky because the HDR metadata in cLLi is itself dependent on the image contents.

cLLi is for an image sequence, not an individual image.

It's a good thing the PNG spec defines some terms, so that we have the option of using them consistently.

A PNG datastream consists of a static image and, optionally, a frame-based sequence. ( which may or may not include the static image as the first frame).
https://w3c.github.io/PNG-spec/#static-and-animated-images
https://w3c.github.io/PNG-spec/#dfn-frame

cLLi is defined on frames. We don't need the term 'image sequence'.

I just noticed that the casual reader might conclude that a static (non-animatedPNG has no frames and thus, that they can't use cLLI to define the brightest pixel in a static PNG.

The spec is actually clear on that, because the MaxCLL is defined on "the entire playback sequence" but I think we could be a bit more explicit that MaxCLL can indeed be defined on a static PNG.

@simontWork
Copy link
Contributor

Do we need to define what happens if a user creates and mDCV tag that differs from the monitor specified in the specification? For example, sRGB has a 80nit monitor in the spec, Adobe RGB has a 160nit monitor, BT.709 and BT.2020 both have 100 nit defined in BT.2035 and HLG has a variable monitor brightness with a corrective gamma adjustment. How do you actually use the mDCV tag to maintain subjective appearance?

@jbowler
Copy link

jbowler commented Jan 31, 2024

If a decoder sees that mDCv and cLLI are present but ignores them and does tone mapping based on the image contents instead, has it violated the spec?

I think it should be strongly discouraged at the very least.

Then the chunks should be MDCv and CLLI (etc): this is the precise difference between an ancillary chunk and a critical chunk. An ancillary chunk may be ignored, a critical chunk shall not be ignored.

@svgeesus
Copy link
Contributor

svgeesus commented Feb 5, 2024

this is the precise difference between an ancillary chunk and a critical chunk. An ancillary chunk may be ignored, a critical chunk shall not be ignored.

True, although the PNG spec has always interpreted critical to mean "no image of any sort can be displayed without it", not "needed to display the image correctly". Which is why we have only 4 critical chunks: IHDR, IDAT, IEND and PLTE - and the last of those, as originally defined, was only sort-of-critical hence the need to add sPLT to fix that.

Notably, chunks which are needed to make the image display correctly, like gAMA (if that is the only colorspace-related info) or iCCP are ancillary. So I certainly don't think mDCv and cLLi need to become critical.

@palemieux
Copy link
Contributor Author

So I certainly don't think mDCv and cLLi need to become critical.

+1

@jbowler
Copy link

jbowler commented Feb 5, 2024

So I certainly don't think mDCv and cLLi need to become critical.

+1

I can't see ChrisL's original comment here on github however my point was not that the chunks should be critical (they shouldn't) but that unless they are critical all decoders are completely free to ignore them; it's just a QoI issue. At least the use of the word SHALL in the OP comment creates a massive conflict in the specification; the checks are ancillary but the decoder "shall" use them to control tone mapping. In effect it's making them backdoor critical chunks. Hence my comment.

@svgeesus
Copy link
Contributor

We have plenty of existing cases where an ancillary chunk includes normative wording (shall, must, should). In general, this seems to mean "you can get some sort of image without this, so it is ancillary" and also "if you use this chunk (readers) or create this chunk (writers) then ...". A few examples:

tRNS

Each entry indicates that pixels of the corresponding palette index shall be treated as having the specified alpha value.

Encoders should set the other bits to 0, and decoders must mask the other bits to 0 before the value is used.

A tRNS chunk shall not appear for color types 4 and 6, since a full alpha channel is already present in those cases.

iCCP

When the iCCP chunk is present, PNG decoders that recognize it and are capable of color management shall ignore the gAMA and cHRM chunks and use the iCCP chunk instead and interpret it according to [ICC].

Unless a cICP chunk exists, a PNG datastream should contain at most one embedded profile, whether specified explicitly with an iCCP or implicitly with an sRGB chunk.

sBIT

Each depth specified in sBIT shall be greater than zero and less than or equal to the sample depth (which is 8 for indexed-color images, and the bit depth given in IHDR for other color types).

tEXt

Newlines in the text string should be represented by a single linefeed character (decimal 10).

@svgeesus
Copy link
Contributor

If present, mDCv and cLLi SHALL completely define the tone mapping algorithm used by the decoder when rendering the image to a display.

I just realized that a display system with an ambient light detector (eg an HDR TV) whose tone mapping responds to the current HDR headroom, would not comply with that requirement. Also, in some cases it is the display not the decoder which does the tone mapping, and the display does not know what PNG chunks were in the image (or even that it was a PNG image).

@jbowler
Copy link

jbowler commented Mar 6, 2024

I just realized that a display system with an ambient light detector

Chris, honestly; what on earth does this have to do with PNG? Like, dude, what on earth does tone mapping have to do with PNG?

@digitaltvguy
Copy link
Contributor

digitaltvguy commented Mar 6, 2024

I just realized that a display system with an ambient light detector

Chris, honestly; what on earth does this have to do with PNG? Like, dude, what on earth does tone mapping have to do with PNG?

PNG's that are HDR can be displayed on an SDR display (and often will be), so tone mapping is important. This occurs in MacOS eDR right now so you can place HDR and SDR images in different application windows.

@svgeesus
Copy link
Contributor

svgeesus commented Mar 6, 2024

Chris, honestly; what on earth does this have to do with PNG? Like, dude, what on earth does tone mapping have to do with PNG?

John, honestly, you must have heard of HDR? You may have missed that PNG now supports HDR images as well as SDR ones. Here is an explainer

@simontWork
Copy link
Contributor

simontWork commented Mar 7, 2024

If present, mDCv and cLLi SHALL completely define the tone mapping algorithm used by the decoder when rendering the image to a display.

I just realized that a display system with an ambient light detector (eg an HDR TV) whose tone mapping responds to the current HDR headroom, would not comply with that requirement. Also, in some cases it is the display not the decoder which does the tone mapping, and the display does not know what PNG chunks were in the image (or even that it was a PNG image).

You may find that even without an ambient light detector, it doesn't comply. TV manufacturers want their products to look different in a show room to allow the consumer to choose based on look and most will need to follow regional power saving regulations. There are a number of initiatives to get a more standardised look.

As well as HDR to SDR tone-mapping, PQ HDR systems require HDR to HDR tone mapping to adjust a display referred signal to a lower capability monitor, which is where the metadata added to PNG originated.

There was some discussion in the W3C Color on the Web group on a minimum viable tone-mapping and we presented a relatively simple technique for HDR-SDR mapping complete with ambient light adaptation: https://bbc.github.io/w3c-tone-mapping-demo/ This could obviously be built upon, for example a better gamut reduction algorithm could be included, which I know is something that @svgeesus has been investigating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants