-
Notifications
You must be signed in to change notification settings - Fork 633
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 v3: Support for mDCV and cLLI #635
Conversation
This adds APIs to get/set the two remaining new PNG-v3 colour space chunks. The mDCV API matches that of cHRM. Both chunks support floating point APIs (all values in the two chunks are real numbers). Both chunk have a new encoded type, a 4dp fixed point number, which cannot be represented in the existing png_fixed_point type so a png_uint_32 is used. Test examples for cICP, cLLI and mDCV are now in 'pngtest.png' and a necessary change to the pngunknown test has been made to accomodate the additions. Signed-off-by: John Bowler <[email protected]>
This should apply ok to libpng18 too. As the CI message says the API for mDCV matches that for cHRM. This is essential because if a PNG with just gAMA+cHRM is converted to cICP using a different encoding, e.g. converting sRGB to REC 2020 (any variant) the cHRM chunk provides the values for mDCV. It would be error prone, very inconvenient and totally unnecessary if apps had to convert from one chromaticity encoding to another. Fortunately mDCV is a subset of cHRM so the use of the cHRM data types (in particular png_fixed_point) merely requires range checks in png_set_mDCV_fixed (range checks where are necessary for floating point anyway.) Tested with the new pngtest.png which revealed that pngunknown needs to be updated for new chunks too... |
Apart from the png.h change these files are machine generated. Signed-off-by: John Bowler <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks, @jbowler. But before we move on, here are my off-topic post-review comments.
@@ -3346,7 +3346,7 @@ PNG_EXPORT(244, int, png_set_option, (png_structrp png_ptr, int option, | |||
* one to use is one more than this.) | |||
*/ | |||
#ifdef PNG_EXPORT_LAST_ORDINAL | |||
PNG_EXPORT_LAST_ORDINAL(251); | |||
PNG_EXPORT_LAST_ORDINAL(259); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we'll git-merge all this into the libpng18 branch as planned, but beyond that, these ordinals gotta go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PNG_EXPORT_LAST_ORDINAL(259); | |
PNG_EXPORT_LAST_ORDINAL(259); |
At least the script (only in the configure build) that checks the ordinals then faults the build unless the machine-generated files have been updated too.
1.8; a simple change that is unlikely to cause merge conflicts against 1.6 since I believe it's only a few lines. Remove the symbol-by-ordinal support. Throw that PoS against the wall and see what breaks. But don't make any other change to those damned macros because if it has to be reintroduced the result would be a nightmare bug farm.
Completely off topic: the same applies to pnglibconf.h.prebuilt, but generate that in the tarball for the "approved" Linux config. Just don't check it in autoconf or cmake builds that use it.
I think you said this years ago, but if it wasn't you it wasn't me or Glenn either yet it is still correct: The machine generated files should not be in the Git.
png_get_mDCV @256 | ||
png_get_mDCV_fixed @257 | ||
png_set_mDCV @258 | ||
png_set_mDCV_fixed @259 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... this @259
and all of its friends must be no more by the time we publish libpng-1.8.0.
This adds APIs to get/set the two remaining new PNG-v3 colour space
chunks. The mDCV API matches that of cHRM. Both chunks support
floating point APIs (all values in the two chunks are real numbers).
Both chunk have a new encoded type, a 4dp fixed point number, which
cannot be represented in the existing png_fixed_point type so a
png_uint_32 is used.
Test examples for cICP, cLLI and mDCV are now in 'pngtest.png' and a
necessary change to the pngunknown test has been made to accomodate the
additions.
Signed-off-by: John Bowler [email protected]