Build break: fix write order of colourspace chunks

[jbowler]

cICP was written after PLTE, not before. The other chunks were output
in an order which does not match the new PNG-v3 "priority" order. This
change outputs all chunks in the "priority" order; highest precedence
first. This means that the PNGs so written conform to PNG v3 (cICP) and
allow a streaming app to handle chunks in order without buffering data
which may later be overridden. Note that PNV-v3 establishes the idea of
dropping ancillary chunks which are inconveniently ordered in the
definition of how APNG chunks are handled.

Signed-off-by: John Bowler [email protected]

[jbowler]

Should also apply to 1.8

[ctruta]

Highly appreciated -- thank you very much.

I just noticed the regression myself, also. Travis CI is down, and that's ok. But the CMake verification is passing while the configure verification is the only one that failed, and that's suspicious.

[jbowler]

IRC pngtest "passes" when the --strict option is not supplied. I think it simply doesn't do the binary compare. It's the memcmp on line 1736. If there's an "unsupported" chunk in there or if 'strict' is zero it returns zero.

[ctruta]

IRC pngtest "passes" when the --strict option is not supplied.

I think you are correct. Which means, it's the CMake build that needs "fixing", by adding OPTIONS --strict to COMMAND pngtest in the CMake file.

I fixed a typo, I added a few commas here and there, and I edited your commit subject line. This wasn't just a build fix, but an actual fix to the actual libpng code. Many thanks @jbowler!

[jbowler]

Yeah, I created the branch before I worked out just how bad it was; I could see the test image was wrong.

Off topic: I'm working on mDCV, cLLI. Trivial, just typing then a test case (I'll use the sRGB reference monitor).

[ctruta]

Off topic: I'm working on mDCV, cLLI. Trivial, just typing then a test case (I'll use the sRGB reference monitor).

Two emoji thumbs up to that 👍👍

So @jbowler would you agree that cICP is ready to go with libpng-1.6.45?

[jbowler]

cICP hasn't been tested and mDCV and cLLI are availble now in. Chris Seeger seemed to think he could get some testing done. At present with have one test case in pngtest.png (which requires the pngtest.png in #635) and suggests quite strongly that the roundtrip works for all three chunks; we aren't changing the data on read-then-write (i.e. pngtest --strict pngtest.png). For a limited test you can open that file in Chrome:

https://github.com/jbowler/libpng/blob/pngv3-mDCV%2BcLLI/pngtest.png

compare with the 'old' one which is currently in libpng18:HEAD:

https://github.com/pnggroup/libpng/blob/libpng18/pngtest.png

They are identical but I don't know if Chrome handles that particular cICP, it's cICP(1,13,0,1) - i.e. it's just sRGB - so there should be absolutely no change.

The only other test file we have is the one in contrib/testpngs/png-3 but as I said I find this file highly suspicious. I'll author test files but this takes longer. It should be easier now I have full support in libpng and that will help test the code.

It looks like the TV/monitor pipeline only supports "HLG". That has a limited dynamic range (it only has 10 bits per channel).

[ctruta]

For a limited test you can open that file in Chrome:

https://github.com/jbowler/libpng/blob/pngv3-mDCV%2BcLLI/pngtest.png

Thanks. I just pulled the branch pngv3-mDCV+cLLI from your repo, locally, so, there, I stole it 🤡

[jbowler]

"gh pr checkout 635" does the same thing :-)