Skip to content

Add runtime-aware gain map routing predicate - #462

Merged
DichenZhang1 merged 6 commits into
google:mainfrom
gregbenz:codex/gainmap-routing-predicate
Sep 29, 2026
Merged

DichenZhang1 merged 6 commits into
google:mainfrom
gregbenz:codex/gainmap-routing-predicate

Conversation

@gregbenz

@gregbenz gregbenz commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This adds a routing helper for image loaders that already have a complete encoded image in memory:

int uhdr_is_supported_gainmap_image(const void* data, size_t size);

A return of 1 means the loader can try libultrahdr. A return of 0 means it should use another route, not necessarily that the image has no gain map. A positive result is a routing hint, not a decode guarantee.

The helper checks gain-map structure and, for AVIF/HEIF, decoder availability without decoding pixels or copying the complete encoded stream. Existing is_uhdr_image() and uhdr_dec_probe() behavior is unchanged.

Behavior and compatibility

  • JPEG input must contain supported gain-map metadata and an MPF directory associating the secondary JPEG. The stricter association checks apply only to the new API; existing Apple primary-size bookkeeping is accepted.
  • AVIF/HEIF input must have an associated gain-map item with valid metadata and directly coded AV1/HEVC images supported by the runtime.
  • Alpha is supported on the primary image and the gain map. Directly coded alpha dependencies are checked for supported dimensions/layouts and decoder availability. Derived, unresolved, duplicate, or nested alpha dependencies return 0; inspection is limited to 16 auxiliary items per alpha-bearing image.
  • The libheif item-inspection API is optional. Without it, the new helper returns 0 for AVIF/HEIF, while existing encoding, decoding, and JPEG routing remain available.

These changes address the review concerns about preserving the legacy probe, keeping system libheif builds usable without item inspection, and accepting supported alpha rather than rejecting it outright.

Callers should retain cheap format prefilters and normal decode-error handling. This requires a complete buffer, not a prefix or stream. Downstreams should check for the new symbol until a released version provides it; adapting a JPEG-only loader to AVIF/HEIF remains separate work.

Validation

At e92c3c9, the local AVIF-enabled suite passed with 1,106 tests passed, 229 skipped, and no failures. The fork's Linux, Windows, macOS, and Android workflows also passed at that revision.

Additional checks cover builds without item inspection, JPEG-only builds with exceptions disabled, malformed MPF associations, and focused ASan/UBSan tests. Sanitizer validation did not instrument libheif, and LeakSanitizer was unavailable locally.

Alpha coverage includes primary-alpha routing and SDR RGBA decode, plus a generated gain-map-side alpha regression. Reinstating blanket alpha rejection locally makes the new test fail. The gain-map-side test verifies routing, not decoded alpha fidelity. Real mixed-codec alpha and a comprehensive malformed-alpha graph matrix were not tested.

Performance

Warm-call measurements on arm64 macOS, using the final AVIF-only release build:

Input New helper Legacy probe
1.21 MB gain-map JPEG 108 us 163 us
1.34 MB gain-map AVIF 12 us 34 us
938-byte AVIF with gain-map-side alpha 9.5 us 9.5 us

These are medians of five 1,000-call batch averages after warmup, excluding file I/O and pixel decoding. They are not end-to-end loader measurements or worst-case bounds. JPEG inspection still scans encoded bytes, and container parsing can allocate memory; avoiding a full-stream copy does not make the operation allocation-free or constant-time.

The release version is left unchanged for maintainers to decide.

@google-cla

google-cla Bot commented Aug 27, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gregbenz
gregbenz marked this pull request as ready for review August 27, 2026 01:20
@gregbenz
gregbenz force-pushed the codex/gainmap-routing-predicate branch from f6f8571 to 816c3ba Compare September 4, 2026 01:15
@gregbenz
gregbenz force-pushed the codex/gainmap-routing-predicate branch from 816c3ba to af90073 Compare September 10, 2026 03:08
@DichenZhang1

Copy link
Copy Markdown
Collaborator

Thanks for the contribution!

This change generally looks good to me, with a minor comment: could you please split the version bump (2.0.2 -> 2.1.0) in CMakeLists.txt / version headers into a separate standalone PR?
Rationale:

  1. Separation of Concerns: Keeping code changes separate from release metadata ensures clean atomic commits.
  2. Cherry-picking: Isolating functional changes allows us to cleanly backport or cherry-pick this fix onto maintenance/patch branches if needed without prematurely bumping the version.
  3. SemVer & Release Cadence: Version increments to 2.1.0 should be landed as a dedicated release commit once all target features for the minor milestone are finalized.

Thanks!

@gregbenz

Copy link
Copy Markdown
Contributor Author

@DichenZhang1 Thanks! Removed the version bump in dd6405d. This PR now contains only the routing changes, with the version remaining at 2.0.2. All checks pass.

@DichenZhang1

Copy link
Copy Markdown
Collaborator

Thanks for adding this — a cheap "can this runtime actually decode it?" probe is genuinely useful, and the implementation is careful (RAII handles, bounds-checked MPF parsing, fuzzer coverage).

Before it lands, I'd point out three blocking issues:

  1. is_uhdr_image() behavior change. Reimplementing it on top of inspect_gainmap_image(..., kStructural) makes JPEG stricter (MPF association now required, so some previously-accepted files return 0) while making HEIF more permissive (no dimension/colorspace/decoder checks). That's a bidirectional semantic change to a shipped API, and the new header comment only describes the loosening. Can we leave is_uhdr_image() on its existing path and keep uhdr_is_supported_gainmap_image() purely additive — or at minimum document the MPF strictness?

  2. Probe forces a hard dependency on libheif's item API. Gating the build on LIBHEIF_HAS_ITEM_API means a system libheif with gain-map support but no item API loses all HEIF/AVIF support under UHDR_BUILD_DEPS=OFF, encode included. That's a steep price for one helper — could the heif_item_get_item_type call be #ifdef-guarded so the probe degrades gracefully instead?

  3. Alpha rejection is a false negative. get_direct_image_decoder_format() bails on heif_image_handle_has_alpha_channel(), but our encoder emits alpha (add_alpha_plane / save_alpha_channel = 1, Preserve alpha in raw AVIF and HEIF encoding #455) and the decoder requests heif_chroma_interleaved_RGBA unconditionally. We'd report "unsupported" for files we write and read fine — I think this check can be dropped.

Let me know if you think these are reasonable or you'd like me providing more info. Thank you!

Preserve the legacy probe implementation, make item inspection optional for system libheif, and validate direct alpha decoder dependencies. Update the public contract and compatibility regressions.
@gregbenz

Copy link
Copy Markdown
Contributor Author

@DichenZhang1 Thanks for the review. Sounds good on all three points. I’ve updated the PR:

  1. Restored the original is_uhdr_image() implementation. The stricter JPEG MPF checks now apply only to the new predicate.
  2. Made libheif item inspection optional. Builds with gain-map support but no item API retain HEIF/AVIF encoding and decoding; only the new predicate returns 0 for those formats.
  3. Removed the blanket alpha rejection. Regression tests now cover alpha on the primary image and, separately, on the gain-map item. The primary-alpha test also verifies SDR RGBA decode.

The alpha check still verifies that the directly associated alpha item has a supported layout and an available decoder. Derived, unresolved, duplicate, or nested alpha dependencies conservatively return 0, without recursively walking the item graph. This keeps the runtime-support check without rejecting alpha itself.

Local tests passed, including compatibility checks for builds without item inspection or HEIF support, plus sanitizer checks. Linux, Windows, macOS, and Android CI also passed on the fork at e92c3c9. The version is unchanged.

@DichenZhang1 DichenZhang1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good to me. Thank you for PR.

@DichenZhang1
DichenZhang1 merged commit 66821e0 into google:main Sep 29, 2026
29 checks passed
@gregbenz
gregbenz deleted the codex/gainmap-routing-predicate branch September 29, 2026 00:08
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.

2 participants