Skip to content

api: reject target display peak brightness equal to SDR white - #507

Open
KavyaC11 wants to merge 1 commit into
google:mainfrom
KavyaC11:fix-target-peak-brightness-sdr-white
Open

KavyaC11 wants to merge 1 commit into
google:mainfrom
KavyaC11:fix-target-peak-brightness-sdr-white

Conversation

@KavyaC11

Copy link
Copy Markdown

Fixes #505.

uhdr_enc_set_target_display_peak_brightness() accepted the documented range [203, 10000] inclusive. At exactly 203 nits the encoder writes hdr_capacity_max = 1.0 = hdr_capacity_min, which the library's own metadata validation rejects, so the encode succeeds but the output cannot be read back.

This makes the lower bound exclusive in the setter and in the matching JpegR check, and changes the documented range (header and ultrahdr_app help) to (203, 10000]. It adds the regression test UltraHdrTargetBrightnessTest.SdrWhiteIsRejectedAndValuesAboveItRoundTrip; the test fails on 47d9f00 and passes with this change.

Tested at 47d9f00 with ASan and UBSan: the rest of ultrahdr_unit_test is unchanged (1104 passed, 231 skipped, the same 6 UBSan enum failures as without this change). This narrows the accepted interval, so it needs maintainer review; an alternative is to keep 203 valid and relax the metadata check, and I can switch if you prefer that.

uhdr_enc_set_target_display_peak_brightness() accepted the documented
range [203, 10000] inclusive. At exactly 203 nits the encoder writes
hdr_capacity_max = 203 / kSdrWhiteNits = 1.0 = hdr_capacity_min, which the
library's own metadata validation rejects ("hdr capacity max ... expects
to be > hdr capacity min"). The encode succeeds, but the output is
rejected by uhdr_dec_probe() and uhdr_is_supported_gainmap_image() for
JPEG and AVIF output, both presets, with or without an SDR intent.

Make the lower bound exclusive in the setter and in the matching check in
JpegR, and update the documented range to (203, 10000]. Values just above
203 (e.g. 203.0001) already produce valid streams.

Add a regression test.
@gregbenz

Copy link
Copy Markdown
Contributor

This looks like a reasonable minimal fix. Rejecting 203 nits up front avoids reporting successful encoding for output the decoder subsequently rejects. Optional test improvement: include std::nextafter(203.0f, 204.0f) to cover the smallest accepted float above the boundary. I don’t consider that a blocker, more of a theoretical cleanup.

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

Successfully merging this pull request may close these issues.

Target display peak brightness of 203 nits (documented as valid) produces a stream the decoder rejects

2 participants