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

png_set_cHRM() fails when using ACEScg coordinates #578

Closed
mbechard opened this issue Aug 30, 2024 · 18 comments
Closed

png_set_cHRM() fails when using ACEScg coordinates #578

mbechard opened this issue Aug 30, 2024 · 18 comments

Comments

@mbechard
Copy link

I guess this may be going away with the newer upcoming HDR workflows, but is it expected that png_set_cHRM() fails when it's given the ACEScg coordinates?

Specifically it's failing due to
if (xy->redy < 0 || xy->redy > PNG_FP_1-xy->redx) return 1;

Where redx = 71300 and redy =29300

Thanks

@jbowler
Copy link
Contributor

jbowler commented Sep 3, 2024

I don't think so; in CIExyY x and y and z must all be >= 0 however by definition x+y+z == 1 therefore x+y <= 1 (or z with be <0) to y<=1-x. In fact x,y or z being 0 is impossible too because, in practice, they are all positive, but rounding might give a zero.

Can you post the whole cHRM chunk you are using; i.e. red, green, blue and white plus, if you have it available, the CIEXYZ triple of the endpoints.

EDIT: also post the values in cICP, assuming you have it in your PNG.

@mbechard
Copy link
Author

mbechard commented Sep 3, 2024

It's exactly the values from the AP1 columns in the table in:
https://en.wikipedia.org/wiki/Academy_Color_Encoding_System#ACES_Color_Spaces

So redx = 71300, redy = 29300, so:
29300 > 100000 - 71300, and the check fails.

ACEScg isn't included in the list of cICP values.

@jbowler
Copy link
Contributor

jbowler commented Sep 3, 2024

Yeah. They're using imaginary colours for the end points. In the chromaticity diagram on the wikipedia page you can see that both AP0 and AP1 use imaginary end-points.

It's easy to relax the test; I made it conservative because PNG was well established as a final image format which meant that the endpoints had to be real colours and the test was put in because lcms crashed when the end-points were linear in the chromaticity space (because the conversion to XYZ is impossible.)

However if the test is relaxed the math in png_get_cHRM_XYZ_fixed is probably going to fail and that will cause png_get_cHRM_XYZ to fail too. It requires extensive testing and, even then, results with negative values for X, Y or Z will be output.

There's a similar problem with cICP; it can encode negative values for R, G and B. In both cases the current solution is for the app to handle it but that means relaxing the test and "not-a-bug" to any app that crashes as a result (the conversion routines can be made to png_error).

@ctruta: judgement?

@jbowler
Copy link
Contributor

jbowler commented Sep 3, 2024

The primaries are not documented on the wikipedia page and the official(?) documentation is broken:

https://github.com/ampas/aces-docs/blob/dev/python/TB-2018-001/aces_wp.py

That link is quoted here, "Derivation of CIE chromaticity coordinates":

https://docs.acescentral.com/tb/white-point#derivation-of-cie-chromaticity-coordinates

Imaginary colours with negative XYZ coordinates can be used to span the human gamut, but they can also be used to crash other software.

@mbechard
Copy link
Author

mbechard commented Sep 3, 2024

I think here is the Official doc page for it:
https://docs.acescentral.com/specifications/acescg/#color-space

The python link does work if you change the branch to main, but ya the link is busted pointing to it.
It seems like officially ACES only really talks about using EXR and MXF as formats the use that colorspace, I was just making sure I wasn't missing coverage in PNG if it was possible under the hood.

@jbowler
Copy link
Contributor

jbowler commented Sep 3, 2024

PNG if it was possible

It's nothing to do with PNG; PNG supports all manner of stuff pretty much without limit. cICP explicitly defines negative values for R,G,B (check the definition of "narrow range" images). The libpng stuff is there because lcms crashed and someone (not me) decided that had to be fixed in libpng.

ACEScg uses ACES AP1 and that, as you noticed, has a negative "z" chromaticity for "R" therefore (given that x and y are positive) it has a CIEXYZ with a negative Z ("G" and "B" are all positive).

Any value of x,y,z is well defined and any set of endpoints are well defined unless they are co-linear in xyz space. Negative x,y are within the human gamut - they don't show you the third quadrant of CIExyY but it also describes the human perceptual gamut, just with all negative values.

So the checks can be removed but @ctruta has to accept that this will probably result in some code crashing; the libpng checks are there to prevent crashes in other peoples' code. The libpng XYZ output code will also have to start generating png_error because the math will sometimes overflow in fixed point. The co-linear check should be ok to implement in fixed point (I think it's just a dot product or two.)

But you still haven't answered my question; what CIEXYZ endpoints are you using?

@jbowler
Copy link
Contributor

jbowler commented Sep 4, 2024

Ok, so I wrote the tests so that they are safe under overflow and underflow conditions (I think) and therefore testing for in-gamut endpoints is just a convenience. That means that the actual tests should work to impose the libpng limits even if the theoretical tests on x and y would fail.

@mbechard please pull and review this:

#579

Since it deletes the lines I'm pretty sure they won't cause a problem :-)

@jbowler
Copy link
Contributor

jbowler commented Sep 4, 2024

@mbechard: how are you going to handle the data? In general ACES seems similar to PQ (it's scene referenced) but it has much greater dynamic range. PQ maxes out at 2000cd/m2, in photographic terms that's a scene light level of around 14EV100 but the actual light levels in such a scene will span a range; up to at least 10x the EV, and routinely EVs go up to 16 (snow/desert/beach). 16EV100 corresponds to a scene "average" of 8000cd/m2 so a max of maybe 80,000cd/m2 (1E5cd/m2).

The range of PQ can't be increased because of the mandated math to scale to different bit depths, so 16-bit PQ has the same max as 10-bit PQ; the scaling adds spurious accuracy not range.

Dumping the floating point values into 16-bit PNG is an option but the floating point (_Float16) bit format is signed; I would suggest at least using an offset notation (so 0 is 32768 and positive values are greater than negative ones along with some explicit statements about the denormalized values (or avoid them...)

@mbechard
Copy link
Author

mbechard commented Sep 4, 2024

But you still haven't answered my question; what CIEXYZ endpoints are you using?

So far I've not used CIEXYZ endpoints directly in my work incorporating colorspace awareness, and only using CIE xy values. Every library I've worked with seems to work just using those.
So for this case it's

Red: (0.713, 0.293)
Green: (0.165, 0.830)
Blue: (0.128, 0.044)
White: (0.32156, 0.33767)

At present I was planning on just using a linear transfer function, and leaving the data as fixed point. So no implied light level. I feel like there is value in being able to save data in wider-gamuts for consumption in SDR workflows. Things such as LED walls used in concerts don't use HDR data, but can output wider gamuts.

The ACES standard saves out EXR files using 16-bit signed floats in AP0, so I think if I was to use floats in PNG it would be fine to used signed in there too. The precision seems sufficient so far (Nuke does it's ACEScg work in 16-bit signed float as well). In that case I would also use linear transfer.

I'll test this later today, thanks!

@jbowler
Copy link
Contributor

jbowler commented Sep 4, 2024

Long: there are a lot of approaches possible with AEScg.

The CIEXYZ problem arose on Windows around 2001 where the colour management software required XYZ endpoints and (to make it worse) they had to be adapted to a D50 white point (i.e. be the colours of the end points as would be seen by a user adapted to D50). It was very annoying - no chromaticity support at all.

What you are planning is unambiguously compatible with all versions of PNG and, in fact, the issue with the negative z in the red endpoint can be worked round without any perceivable loss of color. However there is a lot of rope...

To go from floating point which, in _Float16, has about a 33EV dynamic range to 16-bit linear with a 655:1 dynamic range, just 9.35EV, means you end up having to do some tone-mapping. 16-bit linear is more than adequate for SDR but it's also the case that 16-bit non-linear (power law encoded) can be used to handle a 33EV dynamic range with at least 1% precision across the range. I was wrong about PQ; 1.0 corresponds to 10,000cd/m2. The ACES document implies that 16 corresponds to 1cd/m2, although it's not completely clear (it says 16 corresponds to "1" without giving units but elsewhere it uses nits.) That means the peak in ACES is 65504/16cd/m2 or 4096cd/m2; within the range of PQ.

PQ (defined in table 3 value 16 (https://www.itu.int/rec/T-REC-H.273-202309-T/en but that's a REC, it may change) uses a modified power law. Pure log (log to base 1.01) requires 2299 values, 11.2 bits, to encode 33EV, 12 bits gives more than 58EV and 53EV is enough to cover the whole of human vision plus the brightness of the sun's disk (1.7E9cd/m2) - enough for a lightdome.

Using just gamma encoding, i.e. so that following holds, where L is a light level (R,G,B or X,Y,Z) value and E is the encoded value):

E = L^(1/gamma)

The largest dynamic range with 16 bits is achieved with a gamma of about 240; this gives 24119 as the lowest value such that the next value is no more than 1% higher and a dynamic range of 346EV. To achieve a dynamic range of 33EV requires a gamma of between 4.5 and 5. A gamma of 3 (as in Lab) gives 23EV.

The PNG iCCP chunk (an embedded ICC profile) allows pretty much arbitrary encodings. My understanding is that the latest ICC profile standards support profiles which use floating point input data. IMO if someone puts one of those into a PNG (they are not disallowed by any version of the spec) it would indicate unambiguously that the 16-bit channel data is floating point and common sense means that it is IEEE754 binary16; including a sign bit in the top bit.

Lots of rope :-) All these things are possible; the trick to dynamic range is use at least 12 bits and choose a precision to match; at 0.1% precision the best power law dynamic range with a full 16 bits is 34.8EV using gamma 24 but the log encoding (log base 1.001) gives about 94.5EV. The only catch here is that negative numbers cannot be encoded using a power law; it is necessary to store the sign separately and this reduces the number of bits to (effectively) 15. This kills the dynamic range!

As for the cHRM chunk and ACEScg (AP1) you could simply round the red x,y chromaticities to 2dp :-)

value x y z
Red 0.713 0.293 -0.006
Green 0.165 0.83 0.005
Blue 0.128 0.044 0.828
White 0.32168 0.33767 0.34065

Rounding gives a 0.42% error in the redx value and a 1.03% error in redy; the actual errors introduced in any R, G or B values will be less that 1% in practice (so below the limits of human perception) and, in fact, most of the values in any image will be unaffected; just those right at the boundary will change.

Of course it is better to remove the limit; the above comment only applies to ACEScg which uses ACES AP1 and I'm only suggesting this as a temporary work-round for the libpng limitation (it's nothing to do with PNG). ACES AP0 is different; it uses a negative redy and that violates all current versions of the PNG specification (x and y must be positive, z may be negative). I think this restriction should be relaxed and, indeed, accommodated in libpng regardless.

@mbechard
Copy link
Author

mbechard commented Sep 6, 2024

Thanks so much for the detailed response! Yeah with ACES only really defining EXR and MXF as formats to hold the data, it seems like there is value is supporting other formats though still, and thinking about what transfers make sense with it, beyond linear.
Re: ACES and nits. Using OpenColorIO if you convert ACEScg (1,1,1) to Rec2100 PQ without any tone mapping, you get 0.508, which implies they treat 1.0 as 100 nits. I'm not sure if that's a ACES standard though, or just paper white value the transform in the OpenColorIO profile was setup to use.

@jbowler
Copy link
Contributor

jbowler commented Sep 7, 2024

Using OpenColorIO if you convert ACEScg (1,1,1) to Rec2100 PQ without any tone mapping, you get 0.508, which implies they treat 1.0 as 100 nits.

That seems like a weird scaling factor but then so does 16. A range of +/-65504 should be fine with just nits (cd/m2) and no scaling particularly as the ACES reference viewing environment is described as follows:

The luminance level of the adapting field shall be consistent with luminance levels typical of daylight-illuminated outdoor scenes, in which a perfect reflecting diffuser would have a luminance level of at least 1 600 cd/m2.

Oh well, it will become more clear over time I hope.

@jbowler
Copy link
Contributor

jbowler commented Sep 7, 2024

BTW the change is in libpng16 now so it should be released in libpng 1.6.44

@jbowler
Copy link
Contributor

jbowler commented Sep 8, 2024

@ctruta: resolved by #579

@jbowler
Copy link
Contributor

jbowler commented Sep 12, 2024

@ctruta: possibly unresolved by issue #587

@ctruta
Copy link
Member

ctruta commented Sep 12, 2024

@ctruta: possibly unresolved by issue #587

Well, I landed #588. And if you have more fixes coming, you know the drill 😉

@jbowler
Copy link
Contributor

jbowler commented Oct 13, 2024

@mbechard is this fixed now? I.e. in 1.6.44. I believe it is but I can't close bugs here so....

@mbechard
Copy link
Author

Yep sorry, it's working well for me now.

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

3 participants