Repository navigation
fix(core): hlg footage tone-maps to sdr at the bt.2408 reference white - #7234
Merged
Merged
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Approving at 5aa9358b. No blockers.
What I checked (CI's pinned ffmpeg n8.0.1 BtbN build, sha-verified):
- Fails without the fix: I set
BT2408_HLG_REFERENCE_WHITE_NITSback to 100 and rebuilt core.hdrToneMap.test.ts > puts mid grey and reference white where Apple's SDR conversion doesand the 203-nit pin test both fail. The engine'smatches Studio's HLG tone map and isolates transformed cache entriesfails too, with a pixel buffer mismatch. All pass at this head. - Suites at this head: core
hdrToneMap.test.ts7/7, enginevideoFrameExtractor.test.ts229/229, studio-serverproxyTranscoder+mediaMetadata90/90. - SPS trace on real files: I encoded HLG HEVC (
.mov, hvc1) and HLG H.264 (.mp4) and rankeyframeTransferArgs. Each printedtransfer_characteristics … = 18once, from extradata, which parses toarib-std-b67. The decision logic is right: only all-HLG gets 203, and empty, failed or mixed reads get 100. Only an HLG first frame pays for the extra read, and the nestedinExtractionSlotis safe because the first slot is released infinallybefore the.thenruns.
Non-blocking:
- A failed keyframe read is cached in the engine.
withKeyframeTransfersturns a failed read intokeyframeTransfers: [], and that resolves the cached probe. So a transient failure pins the file to 100 nits for the rest of the process. The first-frame read deliberately drops failures from the cache. Studio's proxy re-probes on each call, so after one bad read, preview (203) and export (100) can differ, which is what this PR set out to prevent. Consider evicting the entry when!result.success, as the first read does. - The grey test depends on the ffmpeg/zimg version. On Debian's ffmpeg 5.1 with the fix, white comes out at 174 against Apple's 161, so it fails the ±8 tolerance (mid grey is exact at 105). It passes on the pinned 8.0 (165). CI and the shipped binaries use 8.x, so this only affects contributors running against a system ffmpeg. A comment or a version guard would save them a confusing red test.
- Stacked JSDoc:
/** The first shown frame's colour tags … */now sits abovewithKeyframeTransfers, on top of that function's own doc. It belongs onreadFirstFrameColour. Commentsis red:comment-ratchetreportscore/src/hdrToneMap.tsgoing from 6.8% to 7.4% (3 comment lines added). It isn't onmain's required list.- Comment numbers: the PR text and test comment say "ours were 138 and 202" at 100 nits. Those are raw limited-range luma. After the test's own full-range conversion they are about 142 and 216. That doesn't change the conclusion.
CI (CI, regression, Windows render, CodeQL) was still running when I reviewed.
Contributor
…LG's white An SPS with an unspecified transfer takes the stream's tag, as its frames do; a failed keyframe read is not cached; the grey test needs ffmpeg 8.
Contributor
There was a problem hiding this comment.
Approving at e6c6d0c0. All five of my earlier notes are addressed, and I found no blockers.
Checked against my notes:
- Failed keyframe read:
readKeyframeTransfersreturnsnullon failure, andforget()drops the cached probe, the same way the first-frame read does. The newreads an HLG source's keyframes again after that read failedtest pins it with{ firstFrame: 2, keyframes: 2 }. - Grey test on older ffmpeg: it now skips below ffmpeg 8. My Debian
5.1.9parses as major 5, so it skips there. - Stacked JSDoc: fixed; the doc is back on
readFirstFrameColour. Comments: the extra doc line onToneMapFrameColouris gone.- Comment numbers: now 142 and 216.
New behaviour, tested with CI's pinned ffmpeg 8.0.1:
readsKeyframeTransfersnow also reads keyframes when the first frame's transfer is missing or unknown. An SPS reporting transfer2(unspecified) now takes the stream's tag. On a real file (x265 colour description with transfer unspecified, probedarib-std-b67), the SPS read gives['unspecified'], and the filter maps it atnpl=203.- Suites at this head: core
hdrToneMap7/7; enginevideoFrameExtractor.test.ts+.concurrency.test.ts235/235 (the8 + 2 * 2count passes); studio-serverproxyTranscoder+mediaMetadata90/90.
Non-blocking:
- SPS with no colour description: an HEVC whose SPS has no VUI colour description prints no
transfer_characteristicsline, sokeyframeTransferscomes back[]and the file stays at 100. That holds even when the probed stream says HLG and the first frame is untagged. I couldn't get ffmpeg 8 to write a container-onlycolrHLG tag with-c copyto confirm such files occur in practice. If they do, treating an empty read as the stream's tag (when the probed tag is HLG) would cover them. - Brightness can change mid-export: with failures now uncached, a transient failure in the first-frame or keyframe read tone-maps that cut at 100. Later cuts re-probe and may get 203, so one export could change brightness between cuts. It needs a local ffmpeg header read to fail, so it's rare. It's the trade-off of the retry I suggested, and I'm noting it here rather than asking for a change.
SanHsien
added a commit
to SanHsien/hyperframes
that referenced
this pull request
Nov 6, 2026
…eview Adopts or partially adopts 11 groups: multi-worker tween replay behind the timeline stand-in (heygen-com#7231), template entries in the shared compile stage (heygen-com#7217), HYPERFRAMES_CACHE_DIR and HYPERFRAMES_CONFIG_DIR for every per-user cache and settings file (heygen-com#7224, heygen-com#7229; HeyGen credentials stay under HEYGEN_CONFIG_DIR), WebKit audio clock (heygen-com#7225), packet-based video length (heygen-com#7233), HLG tone mapping (heygen-com#7234), fetch network cause (heygen-com#7236), .hyperframesignore in lint (heygen-com#7239), info --media (heygen-com#7235), lint priority (heygen-com#7244), check --budget (heygen-com#7245) and the unknown-command answer (heygen-com#7247). bun.lock gains only the lint `ignore` edge from heygen-com#7239, already resolved. Watermark advances to 0a6746e. Co-authored-by: Jerrai <[email protected]> Co-authored-by: Miguel Ángel <[email protected]> Co-authored-by: WaterrrForever <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
5 tasks done
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.
What
HLG footage forced to SDR (Desktop's export,
snapshot, Studio's preview proxy) now maps its mid-tones where Apple's own SDR conversion does. The tone map takes BT.2408's 203-nit reference white for a stream whose every SPS is HLG. PQ, SDR and mixed streams keep today's 100 nits.Why
iPhone video is HLG (often with Dolby Vision 8.4 on top). Exported to SDR, it came out about 30% brighter in the mid-tones than Apple's own conversion of the same clip, so footage looked washed out next to Photos or QuickTime.
Measured on a GitHub
macos-latestrunner with the ffmpeg Desktop ships, againstavconvert --preset PresetHighestQualityof the same clips. The score is CIEDE2000 on fixed patches, the worst patch reported:hevc/dv84.movsample)Dolby Vision metadata changes nothing in Apple's result, so the gap is the HLG curve. On the grey steps, 50% signal was 105 from Apple and 142 from us. Other curves (mobius, reinhard, clip) and converting primaries before the tone map all scored worse or the same. PQ gets worse at 203, so it stays at 100. Its own gap to Apple is separate and not changed here.
Related work
None open on this path.
How
core/src/hdrToneMap.tsowns the decision:toneMapReferenceNits(keyframeTransfers)is 203 only when every transfer read is HLG, else 100.hdrToSdrToneMapFiltertakes the transfers with the first frame's tags.keyframeTransferArgsreads the transfer of every SPS, in the file header and in keyframes, without decoding a picture:-discard nokey, and for H.264 and HEVC only SPS units (filter_units), intotrace_headers. A codec it cannot trace, or a failed read, gives 100 nits.setparams, so an HLG clip whose stream VUI says "unspecified" (only the container tags HLG) still gets 203, like its fully tagged copy.snapshot's colour read and the preview proxy's. Per file rather than per extraction window keeps preview and export identical and the probe-once rule. It is also stricter: a mixed file never gets 203.measure probe --normalizetakes the same white from its probed transfer.Time it adds
On the same Mac with the app's ffmpeg, a 3-minute 4K 10-bit HLG clip with a keyframe every second (180 keyframes):
For scale, extracting a 40-second 1080p HLG clip for an
--sdrexport took 43.3 s on that runner. The same read on a stream that switches from HLG to PQ reported both transfers, matching what the decoded frames carry.Size
One decision, used by every caller that tone-maps HDR to SDR.
Test plan
Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug
fails without the fix: with HLG's white at 100,
hdrToSdrToneMapFilter on HLG grey > puts mid grey and reference white where Apple's SDR conversion doesfails (ffmpeg tone-maps two 10-bit HLG greys; Apple's are 105 and 161, ours were 142 and 216), and so does the engine's real-fileforced-SDR HDR extraction > matches Studio's HLG tone map. Both pass here.Pinned: an all-HLG stream gets 203. HLG with PQ, HLG with BT.709, PQ alone, no reading and a failed read all keep 100. An unspecified SPS transfer follows the stream: 203 under HLG, 100 under PQ. A failed keyframe read is not cached: the next extraction reads again (
reads an HLG source's keyframes again after that read failedfails without it). The engine's two real mixed HLG and PQ tests still pass unchanged: each PQ frame equals the PQ-only file's.3 runs in a row on Linux:
core/src/hdrToneMap.test.ts(7),engine/.../videoFrameExtractor.test.tsand.concurrency.test.ts(235), the Studio proxy and media metadata tests (90). Once:clisnapshot and measure unit tests (187), all of core (5811) and Studio's helpers (646). With the app's own Linux ffmpeg 8.1.2, the engine's forced-SDR tests (13, the untagged-transfer HLG case included, which skips on older ffprobe) and the core tests pass. The grey test is skipped below ffmpeg 8, whose zimg puts HLG white near 174.tscis clean for core, engine and studio-server, and so are oxfmt and oxlint on the changed files.Not covered: Apple's PQ conversion (left as is), VP9 or AV1 HLG (no traced SPS transfer, so they keep 100), and a real 10-minute iPhone clip (the 10-minute figures scale the 3-minute measurement).