jpegrutils: set use_base_cg for Apple gain maps - #484
Open
tejas-dbx-public wants to merge 2 commits into
Open
tejas-dbx-public wants to merge 2 commits into
tejas-dbx-public wants to merge 2 commits into
Conversation
The Apple decode path in getMetadataFromXMP() returns early without setting use_base_cg, so it stays false. Decoding is unaffected, but re-encoding the decoded base image, gain map and metadata (e.g. to resize an Apple HDR JPEG) then fails in encodeJPEGR with "ICC marker in gainmap jpeg is missing": use_base_cg == false tells the encoder the gain map lives in an alternate color space and must carry its own ICC profile, which Apple gain-map images do not. Apple applies the gain map in the base image's color space, so set use_base_cg = true, matching the ISO and Adobe XMP paths. Test: decodeApple now asserts use_base_cg for both Apple fixtures (it fails without this change).
Re-encode the decoded base image, gain map and metadata with uhdr_enc_set_compressed_image + uhdr_enc_set_gainmap_image and check the result decodes. Without the use_base_cg fix this fails because the Apple gain-map image has no ICC profile.
Contributor
|
@tejas-dbx-public Thanks for clarifying on the other PR, and for adding the round-trip tests for both Apple samples. That covers the failure I had in mind. This updated change looks good to me. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Apple decode path in
getMetadataFromXMP()(added in #385) returns early without settinguse_base_cg, so it staysfalse.Decoding is unaffected. But re-encoding the decoded base image, gain map, and metadata through
uhdr_enc_set_compressed_image+uhdr_enc_set_gainmap_imagefails inJpegR::encodeJPEGR:use_base_cg == falsetells the encoder the gain map is applied in an alternate color space and must carry its own ICC profile. Apple gain-map images don't have one. Apple applies the gain map in the base image's color space, souse_base_cgshould betrue, which is what the ISO and Adobe XMP paths already set.In practice this shows up when an app resizes an Apple HDR JPEG while preserving its gain map (for example ImageMagick's
UHDR:coder): the transcode fails instead of producing a smaller Ultra HDR JPEG.Decoded output doesn't change. Apple gain-map images carry no ICC, so
applyGainMapresolves the gain map's gamut to the base's, and both gamut-conversion paths are identity whichever wayuse_base_cgis set. I confirmed that decoding both Apple fixtures to linear HDR gives byte-identical output with and without this change. Only the encode check is affected.Test
JpegRTest.decodeApplenow also assertsuse_base_cgfor both Apple fixtures (apple_gainmap_old.jpg,apple_gainmap_new.jpg), and round-trips each one: it re-encodes the decoded base image, gain map and metadata throughuhdr_enc_set_compressed_image+uhdr_enc_set_gainmap_image, then decodes the result. Without this change the re-encode fails with the ICC error above; with it, both fixtures pass. The fullultrahdr_unit_testsuite passes (1093 tests).Related discussion: #365.