feat(hdr): HDR image support, HDR10 metadata, and regression test harness#314
Conversation
7d4cb98 to
ad5f458
Compare
95985dc to
064f24f
Compare
1be168d to
fd14349
Compare
064f24f to
04106c6
Compare
fd14349 to
00cdaeb
Compare
04106c6 to
d758123
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
Automated review (Claude Code)
Reviewed against CLAUDE.md. Stack-level meta on #268.
Critical
- Silent failure on HDR image extraction —
packages/producer/src/services/renderOrchestrator.ts:~1236-1244. IfrunFfmpegfails (non-HDR codec, multi-frame GIF/AVIF, unsupported pix_fmt), the code onlylog.debugs and still callshdrFrameDirs.set(img.id, frameDir)pointing at an empty directory. Downstream blit reads missingframe_0001.pngand either crashes or silently skips depending on consumer. Promote tolog.warn, and either skiphdrFrameDirs.set()on failure or throw. At minimum remove the id fromnativeHdrVideoIdsso the blit path doesn't try to use it. - Silent failure when HDR image path doesn't exist —
renderOrchestrator.ts:~878-881.if (!existsSync(imgPath)) return;with no log. A typo insrcsilently drops the image from HDR detection and the render proceeds as SDR with no user-facing indication. Addlog.warn.
Important
Promise.allshort-circuits on one bad image — same probe block.extractVideoMetadata(imgPath)is not wrapped in try/catch — one corrupt image rejects the wholePromise.all, aborting the render. Existing video path probes tolerate this; match the behavior.imagesonCompiledComposition/CompositionMetadatais a breaking change —packages/producer/src/services/htmlCompiler.ts:45,renderOrchestrator.ts:221. The fields are non-optional. Any downstream TS consumer constructing these (tests, external plugins) fails to compile. Makeimages?: ImageElement[]optional or default to[]and document.- Multi-frame source (GIF/AVIF) silently picks frame 1 —
-frames:v 1on an animated source yields frame 0. DOM plays the animation, composite renders a static poster → desync invisible in logs. Detectnb_frames > 1at probe time and warn/refuse, or document "HDR img supports still images only".
Suggestions
hdrVideoStartTimesnaming now misleading — images share the map. Rename tohdrLayerStartTimes.- Test coverage gaps on
parseImageElements— currently covers happy path + no-duration + auto-id. Missing: zero-<img>case;data-duration="0"/ negative /NaN(the<= 0guard is correct but untested); missingdata-start(defaults to 0 but untested); dedupe-by-id when two imgs share an id. - Dir collision
hdr_<id>— bothextractHdrVideoFramesAtTimesand the new image block write toframesDir/hdr_<id>/.parseImageElementsgenerateshf-img-Nbut a video author could writeid="hf-img-0". Namespace tohdr_img_<id>. - Decode caching — blitting the same decoded 16-bit PNG every frame the image is visible is wasteful. Decode-once + memcpy. Worth a TODO; will dominate CPU for long stills.
parseImageElementsmutates DOM viael.setAttribute("id", id)but the rewritten tree is discarded — the mutation has no effect. Either remove the side effect, or have the function return the rewritten HTML (mirrors whatparseVideoElementsneeds for id stability).
Strengths
- Reuses
extractVideoMetadata+isHdrColorSpace+blitHdrVideoLayerwithout introducing a parallel probe/blit code path — correct. skipSdrConversionflag with inline "why Chrome's sRGB misinterpretation matters" comment is exactly the kind of "why" comment that saves future debugging.- Parallel probing via
Promise.allis right for I/O-bound ffprobe calls. - Tests cover happy path + two important edge cases (no
data-duration, auto-id).
🤖 Automated review. See stack meta-comment on #268.
- Make CompiledComposition.images optional so external TS consumers constructing this interface don't fail to compile; default to [] at the single call site that needs a concrete array. - Rename hdrVideoStartTimes → hdrLayerStartTimes: the map holds both video and image layer start times since #314. - Warn when an HDR <img> source is animated (fps > 0 or duration > 0.05s). ffmpeg -frames:v 1 only extracts the poster frame, so the composite drifts out of sync with what the DOM plays. HDR img is stills-only for now. - Fix parseImageElements: NaN data-duration was slipping through because `NaN <= 0` is false. Gate on Number.isFinite first. Also NaN-guard start, defaulting to 0 on parse failure. - Add PNG filter decode tests (None/Sub/Up/Average/Paeth) — Chrome's libpng picks filters heuristically per scanline, so regressions in any filter produce garbage for HDR DOM overlays. - Add explicit-rejection tests for Adam7 interlace and missing-IHDR PNGs. - Expand parseImageElements coverage: empty input, zero/negative/NaN data-duration, missing data-start. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks. Fixes landed. Addressed:
Nothing from the original findings on this PR is unaddressed. |
d758123 to
b4a73fe
Compare
53a839b to
1f0c851
Compare
b4a73fe to
abaccfc
Compare
1f0c851 to
f2c320d
Compare
abaccfc to
f19714e
Compare
6606e55 to
ebfb7e2
Compare
f19714e to
51616e6
Compare
Adds the test cases the PR #314 review flagged as missing: - empty image list / no <img> elements - duration="0", negative, NaN, and Infinity rejection - missing data-start defaults to 0 - duplicate ids are preserved (documents current contract) Made-with: Cursor
Adds the test cases the PR #314 review flagged as missing: - empty image list / no <img> elements - duration="0", negative, NaN, and Infinity rejection - missing data-start defaults to 0 - duplicate ids are preserved (documents current contract) Made-with: Cursor
052878e to
40cca3b
Compare
7005b6a to
5893c6f
Compare
Adds the test cases the PR #314 review flagged as missing: - empty image list / no <img> elements - duration="0", negative, NaN, and Infinity rejection - missing data-start defaults to 0 - duplicate ids are preserved (documents current contract) Made-with: Cursor
5893c6f to
99d4129
Compare
CI fix: typecheck regression in
|
ab78ab4 to
63a1723
Compare
Cleanup (rebase, no functional change)Per request, dropped two stray commits that didn't belong on this PR:
Performed via Local typecheck + lint + format checks pass on the rebased head. Pushed and waiting on CI to confirm regression shards are still green. |
4f6e967 to
6f97988
Compare
Adds the test cases the PR #314 review flagged as missing: - empty image list / no <img> elements - duration="0", negative, NaN, and Infinity rejection - missing data-start defaults to 0 - duplicate ids are preserved (documents current contract) Made-with: Cursor
6f97988 to
e2d1dfd
Compare
9c14888 to
c48ef97
Compare
a7e1d86 to
6da105c
Compare
ImageElement type and parseImageElements() parser for <img> elements. When --hdr is set, images are probed alongside videos for HDR color space. HDR images get single-frame 16-bit PNG extraction and route through the existing blitHdrVideoLayer path. SDR images unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Tolerate per-image HDR probe failures; warn on missing paths. - Warn when HDR <img> source is animated (stills-only for now). - Namespace HDR image frame dirs as hdr_img_<id> to avoid collisions. - Drop image id from HDR layer set when extraction fails. - Promote HDR frame extraction failure from info to warn. - Rename hdrVideoStartTimes → hdrLayerStartTimes. - Make CompiledComposition.images optional. - Fix NaN data-duration slipping through parseImageElements. - Remove dead setAttribute mutation in parseImageElements. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements the decode-once cache claimed in the PR review reply: HDR image layers now reuse decoded rgb48le buffers across the frames they're visible. Cache is render-scoped (cleared per job) and keyed by framePath::sourceTransfer::targetTransfer because convertTransfer mutates the buffer in-place. Only image layers receive the cache. Video layers would bloat memory (every frame has a unique path: ~37 MB × 300 frames at 1080p ≈ 11 GB). Images decode once and are blitted on every visible frame. Also promotes the blitHdrVideoLayer catch block from log.debug to log.warn — a blit failure means a missing/dropped HDR layer, which is user-visible and shouldn't be silent at the default log level. Made-with: Cursor
Adds the test cases the PR #314 review flagged as missing: - empty image list / no <img> elements - duration="0", negative, NaN, and Infinity rejection - missing data-start defaults to 0 - duplicate ids are preserved (documents current contract) Made-with: Cursor
Writes to ffmpeg's stdin pipe that race with ffmpeg's exit emit EINVAL or EPIPE as unhandled 'error' events on the Socket, crashing the process. Add a no-op error handler on stdin — the exit handler already captures the failure via the result object. Surfaced during HDR regression renders where the last frame's write coincides with ffmpeg finishing input processing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ogger test Adds a standalone smoke harness for the HDR encode pipeline plus a guard against the silent-decode-failure regression in blitHdrVideoLayer. - packages/producer/scripts/hdr-smoke.ts: end-to-end harness that renders the hdr-pq and mixed-sdr-hdr regression fixtures, then ffprobes both stream-level and frame-level side-data so we can catch missing MasteringDisplay / MaxCLL SEI in CI without hand-running ffprobe. Frame probe uses -show_frames -read_intervals %+#1 since x265 emits HDR metadata as in-band SEI prefix NAL units, not container-level boxes. - renderOrchestrator.test.ts: new "logs decode errors via the supplied logger" test pinning blitHdrVideoLayer's failure path to log.warn (matches production behaviour — silent failures were the original PR-314 review concern). - renderOrchestrator.ts: doc-only — clarify that blitHdrVideoLayer is exported so the time→frame math, last-frame freeze, border- radius detection, and affine-vs-region branch can be unit tested without spinning up the full producer. - .gitignore: ignore packages/producer/tests/hdr-regression/_renders and the generated hdr-full-demo workdir so smoke runs don't pollute git status.
…R composite
DOM overlays composited onto HDR video frames were oversaturated because
`blitRgba8OverRgb48le` mapped sRGB 8-bit values directly through an
HDR OETF (HLG or PQ) without first converting from BT.709 to BT.2020
color primaries. Treating sRGB values as if they already lived in the
much wider BT.2020 gamut pushed saturated colors well past the
designer's intent — e.g. sRGB pure blue (0,0,255) landed on BT.2020
blue, which is far more vivid than what was specified.
Replace `buildSrgbToHdrLut` with the full pipeline:
sRGB 8-bit
→ linear BT.709 (sRGB EOTF, 256-entry LUT)
→ linear BT.2020 (3×3 BT.2087-0 primary matrix)
→ HDR signal 16-bit (HLG/PQ OETF, 4096-entry LUT)
The matrix rows sum to 1.0, so neutral content (R=G=B) is invariant —
text and grayscale UI render identically. Chromatic content (icons,
accent colors, progress bars) is now color-accurate against BT.2020
HDR video. PQ scales relative to 203 nits SDR white per BT.2408 so
SDR overlays sit at conventional brightness inside the HDR frame.
Verified end-to-end with hdr-smoke (sdr-baseline, hdr-pq,
mixed-sdr-hdr): sampled red overlay pixels (#C1121F) in the rendered
PQ output match the calculated post-conversion 16-bit values
(~30837/18894/15630), versus the old buggy pipeline's
~33820/10529/13663 (visibly oversaturated).
Removes the `getSrgbToHdrLut` public export — the LUT is now an
internal pipeline stage, not a single-step conversion.
Made-with: Cursor
x265 emits HDR10 mastering display + content light level metadata as
in-band HEVC SEI messages, but FFmpeg's `mov` muxer doesn't extract
those into the container-level `mdcv` (Mastering Display Color Volume)
and `clli` (Content Light Level Info) boxes that ingest pipelines
read. Without them, YouTube, Apple AirPlay, and most HDR TVs see only
stream-level color tagging (`colr`) and treat the file as SDR BT.2020,
silently tone-mapping the output.
Add `mp4HdrBoxes.ts`, which surgically inserts `mdcv` + `clli` boxes
inside the HEVC sample entry (`hvc1`/`hev1`), bumps every parent box's
size, and rewrites every `stco`/`co64` chunk offset that points past
the insertion site so the file stays decodable. Reference: ISO/IEC
14496-15 (NAL-structured video) and ISO/IEC 23001-8 (CICP).
Wire the injection in two places:
- `streamingEncoder.ts`: post-encode for direct HDR renders, so
standalone calls into the engine emit YouTube-ready files.
- `renderOrchestrator.ts`: post-mux for the producer pipeline,
because FFmpeg's mp4 muxer rebuilds the container during
mux/faststart and drops the boxes we injected into the
intermediate video-only file.
Failures degrade to a warning — the file is still playable; only HDR
recognition on strict ingests is affected. Covered by 50+ unit tests
in `mp4HdrBoxes.test.ts` (parser fuzzing, box layout, stco/co64
rewriting, malformed-input rejection).
Made-with: Cursor
`page.evaluate` callbacks with nested `function` declarations crashed with `ReferenceError: __name is not defined` whenever the host was bundled by tsx/bun. esbuild's `keepNames` mode wraps every function declaration — including ones inside the body of an evaluate callback — with a `__name(fn, "name")` call to preserve `Function.prototype.name`. The helper is injected into the host bundle but never serialized into the function string Puppeteer ships to the browser, so the browser context sees a free reference and throws. Install a no-op identity `globalThis.__name` shim via `page.evaluateOnNewDocument` during session init. We pass a string literal (not a function) because esbuild does not transform string contents — defining the shim inline would itself get wrapped with `__name(...)` and produce a use-before-define cycle. Running it via `evaluateOnNewDocument` guarantees the shim is in place before any page script (including subsequent `page.evaluate` callbacks) executes. Document the constraint at the call site in `videoFrameInjector.ts` so future edits don't try to redefine the shim inside an evaluate callback. Made-with: Cursor
A six-scene composition that exercises every native HDR compositing feature in one render so regressions in any one path get caught by the existing `bun run hdr-smoke` flow: - scene 1: pure HDR video + sRGB DOM badge overlay - scene 2: HDR background + SDR picture-in-picture + sRGB DOM headline - scene 3: 16-bit HDR PNG image + sRGB DOM caption - scene 4: two HDR videos masked by border-radius (circle + rounded) - scene 5: three z-ordered overlapping cards (HDR, SDR, HDR) with sRGB tags - scene 6: three transformed clips (translate/rotate, scale/skew, opacity/translateY) — exercises per-element compositor layers The five inter-scene transitions cover four shader programs (domain-warp, cross-warp-morph, flash-through-white, gravitational-lens) so the engine-render-mode init path in hyper-shader.ts gets coverage too. Asset symlinks reuse source media that hdr-pq and sdr-baseline already ship, so this does not add any new binary blobs to the repo. The vendored shader-transitions.js is a local build of @hyperframes/shader-transitions@0.4.6 — the published CDN bundle at this version is missing the window.__hf.transitions write the engine needs for native HDR compositing. Once a fixed build ships the vendor file goes away and the HTML can pull from CDN again. Wires the fixture into hdr-smoke.ts with the same probe expectations as the other HDR fixtures (yuv420p10le, smpte2084, bt2020, requireHdrSideData), and adds **/vendor/ to the oxlint ignore patterns so vendored library bundles don't trip lint. Made-with: Cursor
Adds a focused 6-second composition that runs the same opacity timeline on an HDR clip and an SDR clip side-by-side: a `tl.set` to opacity 0, an entry fade-in (0→1), then an opacity yoyo (1↔0.15). Both wrappers use the same selectors and share the same tween, so any divergence between the HDR (compositor) path and the SDR (DOM-injected <img>) path is immediately visible as a left/right asymmetry. This fixture would have caught the SDR-opacity regression fixed in the previous commit — without it, the issue only surfaced as a vague "opacity isn't doing anything" report on the larger hdr-feature-stack composition. The fixture is also wired into hdr-smoke so CI exercises the same code path on every change. Assets are symlinked to the existing hdr-feature-stack videos to avoid duplicating ~25 MB of binary content. Made-with: Cursor
Expands the regression composition to better cover the SDR-opacity fix and to read more clearly when reviewing the rendered output: * Replace the previous 36-byte placeholder media (Git LFS pointers that were never resolved on disk) with real BT.2020 PQ clips drawn from a YouTube HDR demo (M8hv1Oah2uQ) and BT.709 SDR clips drawn from a YouTube SDR sample (SnUBb-FAlCY). Total ~40 MB of binaries — kept inline rather than tracked through LFS so the fixture matches the rest of the hdr-regression suite. * Add `data-start` / `data-duration` to every HUD slate, caption, and overlay element so they actually reach the layered compositor during their owning scene window. Without these the elements were computed off-stage by the renderer and the slates / pip captions in scenes 1 and 2 never appeared in the final video. * Paint scene 6 on a white field so an opacity yoyo on a framed clip is visible — on the default near-black root the dip-to-translucent reads as no change at all. * Drop opacity from the scene-6 transform yoyo on `#s6-f3`. Earlier scenes already cover the opacity-yoyo path; scene 6 is now a dedicated transform showcase (entry opacity fade-in stays so the clip enters cleanly). Made-with: Cursor
Commit eac396a renamed hdrVideoStartTimes → hdrLayerStartTimes across the file, but it predated the diagnostic-block typo fix on feat/hdr-layered-compositing (4f6e967). After the restack, line 1470 ended up using the (now non-existent) hdrVideoStartTimes name and broke the producer typecheck/build on this branch. Restore the rename so the diagnostic block matches the local declaration. Made-with: Cursor
Add a colocated README describing the five HDR regression fixtures, the three layers of HDR coverage we have today (engine vitests, the hdr-smoke metadata script, and the partial visual harness integration), and the known gaps - notably that hdr-smoke is not in CI and there are no committed pixel goldens, which is why the SDR opacity yoyo bug on <video>-backed clips slipped through initially. Add a pointer from CONTRIBUTING.md so HDR-touching contributors find it from the standard "Running Tests" section. Made-with: Cursor
c48ef97 to
79d3a7d
Compare

Summary
Top of the HDR stack. Lands native HDR
<img>rendering plus the polish needed to ship HDR end-to-end: HDR10 container metadata, sRGB→BT.2020 overlay color, encoder/render fixes, and a regression test harness (hdr-smoke) with three fixtures so the stack stays green.What's in this PR
1. HDR image support —
<img>rendering with full HDR fidelity249675dc feat(hdr): add HDR image support — probe, extract, and compositeImageElementtype +parseImageElements()invideoFrameExtractor.tscollects<img>elements withdata-start/data-durationfrom compositions.--hdris set, each image'ssrcis probed with ffprobe for HDR color space (bt2020 + PQ/HLG). Runs in parallel alongside video probes.ffmpeg -frames:v 1 -pix_fmt rgb48leat the element's layout dimensions. Output goes tohdr_<id>/frame_0001.png— same directory structure as video frame extraction, just one frame.blitHdrVideoLayerpath. The image's single frame gets blitted every frame the image is visible. Cross-transfer conversion (HLG↔PQ) applies automatically.CompiledComposition.images—htmlCompiler.tsnow collects images into a typed array alongside videos and audios.The HDR video pipeline already handled z-ordered layer compositing, GSAP transforms, border-radius masks, sRGB→HDR LUT conversion, and cross-transfer conversion — an HDR image is just a video with one frame, so the entire blit path reuses unchanged.
2. HDR10 container metadata for player compatibility
10bea303 feat(hdr): inject mdcv/clli MP4 container boxes for HDR10 outputsYouTube, QuickTime, and Safari only treat a file as HDR10 when the MP4 carries
mdcv(Mastering Display Color Volume) andclli(Content Light Level Info) boxes. We now inject both during muxing — without them PQ files were detected as SDR by most consumer players. Includesmp4HdrBoxes.test.tswith 398 lines of round-trip coverage.3. Color-accurate DOM overlays in HDR composites
b78a8df2 fix(engine): convert sRGB DOM overlays to BT.2020 primaries before HDR compositeDOM overlays were being baked into HDR frames with their sRGB primaries unchanged, so reds/greens shifted noticeably under PQ encoding. Overlays now go through a 3×3 sRGB→BT.2020 primary conversion before the OETF, matching what real HDR content looks like next to text/icons.
4. Engine stability
8e3dc4bd fix(engine): install __name shim before page scripts run— esbuild-emitted bundles call a global__name()helper for class display names. Pages that loaded before our injection threw__name is not defined. Shim is now installed in the early stub.5e90cd5e fix(engine): handle stdin EINVAL on streaming encoder pipe close— when ffmpeg exits between frames the next write was crashing the producer; now we detectEINVAL/EPIPE, log once, and let the encoder shutdown path handle it.039dda8c fix(producer): add HDR image decode cache + promote blit failure to warn— HDR images were being decoded once per frame (~4 ms × 30 fps × 5 s = 600 ms wasted per clip). Cached now. Blit failures degrade to a warning so a single bad layer doesn't kill the whole render.5. Regression test harness —
hdr-smoke161d4dbb test(producer): add hdr-smoke regression script + blitHdrVideoLayer logger testpackages/producer/scripts/hdr-smoke.ts— single entry point that renders every fixture intests/hdr-regression/, verifies the output withffprobe(yuv420p10le,smpte2084,bt2020,requireHdrSideData), and reports per-fixture pass/fail. Used both locally and in CI.Three fixtures cover the HDR stack:
hdr-feature-stackopacity-mixed-fadefeat/hdr-layered-compositing.parseImageElementscoverage749cfd96— extra unit-test cases for the new image parser.6. Cleanup
5279501e fix: restore .gitignore entries removed during rebase5a8ba3d7 fix(hdr): address review feedback across stack5893c6f3 chore: ignore studio scene thumbnails cacheFiles changed (vs.
feat/hdr-layered-compositing)How to test
For ad-hoc HDR image testing:
ffmpeg -ss 2 -i hdr-video.mp4 -frames:v 1 -pix_fmt rgb48le -c:v png hdr-photo.png<img id="bg" class="clip" data-start="0" data-duration="5" src="hdr-photo.png" style="..." />--hdr: verify H.265 10-bit PQ/BT.2020 output, image visible at HDR quality--hdr: verify SDR output, image renders through Chrome, no probe overheadStack position
7 of 7 — top of the HDR stack. Stacked on #268 (layered compositing).
Full stack: #258 → #265 → #288 → #289 → #290 → #268 → #314
🤖 Generated with Claude Code