Commit Graph
22 Commits
Author SHA1 Message Date
James Russo 2339757377 fix: bound HDR and video extraction resources (#2955)
* fix: bound HDR and video extraction resources

* fix: trim negative video extraction preroll

* fix: skip invisible video extraction windows

* fix: preserve negative-start loop and held tails

* fix: cap finite video slots to source duration

* fix: bound held-tail frame extraction

* fix: plan from playable video duration

* fix: preserve open-ended held video tails

* fix: resolve held tails from decoded frames

* fix: normalize final-frame probe timestamps

* fix: handle unseekable final-frame sources

* fix: dedupe final-frame probes per render

* refactor: clarify output dynamic range contract
2026-08-02 20:21:54 -07:00
Vance IngallsandClaude Opus 5 9e275423e2 fix(engine): reject stdin input, decode stdout correctly, bound its size
Three issues in runFfprobe's process and stream handling.

A filePath of exactly "-" hung for 30 seconds. `--` stops option parsing,
so "-intro.mp4" is safe, but ffprobe rewrites "-" to `fd:` AFTER option
parsing and reads stdin — and stdin was an inherited pipe the parent
never writes to and never ends. The probe ran to the deadline and failed
with an empty diagnostic, because ffprobe never errored so stderr was
blank: 30010 ms and no message, against 28 ms for a normal missing-file
error. Rejected up front, and the child now gets stdio ["ignore", ...]
so no future invocation can block on stdin either.

stdout was decoded per chunk. `stdout += data.toString()` decodes each
64 KiB pipe chunk independently, so a multi-byte character straddling a
boundary became U+FFFD on both sides — verified: 200 KB of 3-byte
characters produced 15 replacements and a string 9 characters longer
than the source. -show_format output above ~64 KiB with non-ASCII tag
text returns silently mangled values, since JSON.parse still succeeds.
Now accumulated through StringDecoder.

Note on testing that one: U+FFFD is valid JSON string content, and
nothing on extractMediaMetadata's public surface exposes a tag value, so
there is no assertion that fails against the old implementation. Rather
than add a test that cannot fail, it is stated here and the bound below
is what the new test covers.

stdout was unbounded. stderr is capped by ManagedChildProcess but stdout
was not, and analyzeKeyframeIntervals emits one line per frame — an
all-intra ProRes proxy can produce an arbitrarily large string. Capped
at 8M characters, which real -show_streams JSON is nowhere near.

Tests: "-" rejected without spawning, the stdio shape, and the size
bound. Reverting the stdin guards fails 1. The first draft of the bound
checked before appending, so a single oversized chunk passed — the test
caught it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 19:53:01 -07:00
Vance IngallsandClaude Opus 5 361fd49926 fix(engine): allowlist AAC-LC for the packet refinement, not deny HE-AAC
The previous gate was a HE-AAC DENYLIST, so every other profile still
got the 1024-sample formula. ffprobe reports codec_name "aac" for all of
them; the framing lives in the profile:

  LC            1024 samples/frame   <- the only one this maths fits
  HE-AAC v1/v2  2048 output samples against a doubled sample_rate
  LD            512
  ELD           480
  Main/SSR/LTP  1024 nominally, unverified here
  xHE-AAC       variable

LD and ELD therefore had their already-correct container duration
overwritten with a value 2x / ~2.13x too large, and an unknown or
missing profile fell through — so an unrecognised HE spelling preserved
the exact truncation the previous commit set out to close.

Now an affirmative match on LC. Skipping the refinement is harmless:
format.duration is already correct before it runs.

Tests: 11 non-LC profiles (including LD, ELD, xHE-AAC, empty and
unrecognised) assert the container duration is kept AND that the second
probe is not launched; LC still refines, with whitespace tolerated. The
pre-existing duration table asserted that an UNPROFILED "aac" stream
refines — the behaviour under review — so it now states LC explicitly
and adds an unprofiled row that must not refine.

Also strengthened the `--` separator test while it was failing: it
compared a flattened count of 3 across three spawns, which one call
emitting three terminators would satisfy. Now asserts the last two argv
entries per call.

Reverting the allowlist fails 8.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 19:52:26 -07:00
Vance IngallsandClaude Opus 5 242a42f6c9 fix(engine): make the AAC duration refinement safe, cancellable and LC-only
The packet-count probe is a refinement — durationSeconds is already
correct from format.duration before it runs — but it was written as if
it were load-bearing.

It could fail the whole call. No try/catch, and `-count_packets` demuxes
the entire container against runFfprobe's fixed 30s deadline, so a long
AAC file on slow or network storage timed out and extractAudioMetadata
rejected. htmlCompiler catches that under the comment "Source file has
no audio stream", returns duration 0, drops the audio element, and the
render ships silent with no warning. Now caught, keeping the container
duration.

It ignored the caller's AbortSignal. Only the first probe received it,
so aborting during the packet probe let the child run to completion and
the call resolved with full metadata after cancellation — while
audioPadTrim's comment claims the wrapper preserves cancellation. The
signal is forwarded, and an abort still propagates rather than being
swallowed as a refinement failure.

It halved HE-AAC durations. ffprobe reports codec_name "aac" for
HE-AAC v1/v2 as well — the marker is in the profile field — and with SBR
each packet carries 2048 output samples against the doubled output
sample_rate, so the 1024 assumption computed exactly half. A 10:00
podcast became 5:00 and htmlCompiler truncated the audio there. Gated on
profile, with `profile` added to FFProbeStream.

Tests: probe failure, junk output, three HE-AAC profile spellings (which
also assert the second probe is not attempted), and that plain AAC-LC is
still refined. Reverting the guards fails 5.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 19:52:25 -07:00
Vance IngallsandClaude Opus 5 4d563fa752 fix(engine): guard the rounded frame rate and strict-parse rationals
Two paths the previous guards still let through.

Rounding could recreate Infinity after the finite check. `raw * 100`
overflows for a finite-but-huge rate — "1e307", "1e307/1" — so `rounded`
became Infinity and passed the positivity check, reaching exactly the
`-r Infinity` failure the finite guard exists to prevent. The rounded
result is now checked too.

The rational operands still used parseFloat. The plain-number path
switched to Number() so trailing garbage fails the whole string, but the
numerator and denominator did not, so "60fps/1", "60/1fps" and
"30garbage/1garbage" returned valid rates while the contract says
malformed frame rates fail closed. Both operands are now parsed strictly,
and an empty operand ("/", "/1", "30/") is rejected rather than coerced.

Tests: 8 malformed inputs and 3 overflow cases in the direct table.
Reverting either fix fails 5.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 19:52:17 -07:00
Vance IngallsandClaude Opus 5 19dc83bc2b fix(engine): reject non-finite, negative and malformed frame rates
parseFrameRate guarded its operands but not its result, so several
inputs produced values that are not usable frame rates — and nothing
downstream catches them, because callers use `meta.fps || 30`, which
only rescues 0 and NaN. Everything below was truthy and flowed into
buildEncoderArgs as `-r <value>` (rejected by ffmpeg mid-render) and
into frameCount arithmetic.

  "1e308/1e-10", "2/1e-320"  -> Infinity  (finite operands, infinite quotient)
  "-30/1", "30/-1", "-60"    -> negative  (sign never checked)
  "30/1/2"                   -> 30        (parts.length !== 2 fell through)
  "60fps"                    -> 60        (parseFloat stops at garbage)

Now: the quotient is checked rather than the operands, non-positive is
rejected, more than two parts is rejected, and the single-part path uses
Number() rather than parseFloat so trailing garbage fails the whole
string.

Separately, 2dp rounding collapsed any rate below 0.005 to exactly 0,
and the caller's 30fps default then re-encoded a 300-second 1/300-fps
timelapse as a ~1/30-second clip with frameCount 9000 for a 1-frame
file. Those floor to 0.01 instead.

parseFrameRate is now exported and tested directly. The previous table
drove it through extractMediaMetadata behind a spawn mock, costing a
vi.resetModules() plus a re-import of core's 238-file barrel per row
(74.9 ms vs 0.094 ms) — and 4 of its 7 rows produced identical values
against the pre-fix implementation, so it could not fail for the bugs it
existed to catch. The replacement fails 9 against that implementation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 19:52:17 -07:00
Vance IngallsandClaude Opus 5 62b96c227e fix(engine): merge colour per field, detect alpha correctly, defer PNG parse
Three defects in how ffprobe output and the PNG fallback are combined.

The cICP fallback was unreachable. `ffprobeColorSpace ?? stillImageMeta
?.colorSpace` discarded the PNG result whenever ffprobe returned ANY
colour field — and ffprobe emits color_space "gbr" for every PNG,
including a plain rgb24 with no colour metadata. So on the build the
parser exists for (reports gbr, does not decode cICP) an HDR PQ PNG
resolved colorTransfer "" , isHdrColorSpace() returned false, and the
still graded SDR. Now merged per field.

hasAlpha's anchor bound to one alternative. In
/(^|[^a-z])yuva|rgba|.../ the `|` is looser than concatenation, so
(^|[^a-z]) guarded `yuva` and nothing else. The list also omitted abgr,
ya8, ya16 and ayuv64, and `gray[a-z0-9]*a` matched only gray8a/gray16a —
names FFmpeg renamed to ya8/ya16 in 2013, so dead against modern builds.
A ya8 grayscale-plus-alpha PNG reported hasAlpha:false, resolveFrameFormat
picked jpg and the overlay flattened to an opaque rectangle. Replaced
with the start-anchored form studio-server already uses, extracted as
exported pixelFormatHasAlpha so the test asserts the shipped predicate
rather than a copy of the pattern.

The PNG parse ran eagerly and was discarded. It sat before the first
await, so readFileSync plus the CRC walk executed for every file before
a single ffprobe was spawned — a caller fanning out over
composition.images with Promise.all serialised entirely: 12 4K PNGs took
2649 ms against 170 ms probe-only, 2.5 s of event-loop stall that also
blocks Puppeteer IPC. On the happy path the value was then thrown away.
Now lazily memoized behind the paths that actually consult it.

Tests: 18 pix_fmt cases against the real predicate. Reverting the regex
fails 4.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 19:51:28 -07:00
Vance IngallsandClaude Opus 5 96a6e8bd95 fix(engine): keep the PNG CRC working on Node 22.0/22.1
zlib.crc32 landed in Node 22.2.0, but engine and cli both declare
`"node": ">=22"` and the runtime gate is major-only, so 22.0 and 22.1
are supported. A NAMED import of a missing export throws at module
EVALUATION — ffprobe.ts would have failed to load at all on those
runtimes, before any PNG was touched, taking every probe with it.

Namespace import plus a capability check, with the previous
bit-at-a-time implementation retained as the fallback. Modern runtimes
keep the 210ms -> 1.3ms win; older ones keep working.

Raising the floor to >=22.2.0 was the alternative, but that is a
user-facing support change and does not belong in a PNG bug fix.

Tests: the same HDR PNG parses identically with the native export
absent, and a corrupt chunk still rejects on the fallback path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 15:54:46 -07:00
Vance IngallsandClaude Opus 5 2af3f4d0ed fix(engine): stop the PNG walk at cICP, anchor IHDR, use native crc32
Three defects in the PNG metadata fallback, all introduced when the
cICP early return became an accumulator.

Corrupt trailing chunk nulls a good result. cICP must precede IDAT, so
continuing past it only visits chunks this parser ignores — while making
whole-file integrity a precondition for returning anything. A truncated
or bad-CRC chunk after cICP in an otherwise-good HDR PNG returned null,
and extractMediaMetadata then re-throws the ffprobe error it had
swallowed instead of using the fallback it just computed: the render
dies on a host without FFmpeg, or grades SDR on a build that does not
decode cICP. Now stops once dimensions and colour are known.

A second IHDR overwrote the dimensions. PNG permits exactly one, first,
but nothing enforced that here — a trailing [IHDR 1x1] replaced a real
3840x2160 and the producer laid out a one-pixel image. Anchored to the
first. The length guard was also `>= 8` against a spec length of 13,
which accepted a truncated header and read height out of the CRC bytes.

crc32 was hand-rolled bit-at-a-time and fed a Buffer.concat per chunk.
Since the walk no longer stops early it CRC'd whole files: 210 ms on a
12 MiB PNG, 647 ms on a 35 MiB 4K one, synchronously on the event loop,
plus ~11 MB of garbage per parse from concatenating a 4-byte type tag
onto every chunk. node:zlib's crc32 is native and takes a running seed,
so type and data hash in sequence with no copy. 210.28 ms -> 1.291 ms.

Tests: 5 regressions — corrupt-after-cICP, truncation after cICP,
second IHDR, short IHDR, and that a corrupt IHDR/cICP still rejects.
Reverting the break or the anchor fails 3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 23:57:17 -07:00
Santhi Prakash f75ca076b7 fix(engine): harden ffprobe parsing and command arguments (#2740)
- parseFrameRate now rejects malformed ratios (e.g. "30/", "30/0") instead of NaN.

- Add "--" before file paths so names starting with "-" are not parsed as options.

- cICP PNG chunk no longer returns before IHDR supplies width and height.

- Add regression tests for option injection, frame rates, and cICP ordering.
2026-07-30 12:54:50 -07:00
James Russo 58869f0878 fix(producer): accept partial color metadata in plan v2 (#2814)
* fix(producer): accept partial color metadata in plan v2

* test(engine): make partial color probe hermetic

* fix(producer): validate plan v2 sentinels in fallback mode
2026-07-26 21:48:51 -04:00
James Russo 8c50770684 fix(engine): preserve bounded ffprobe diagnostics (#2772)
* fix(engine): preserve bounded ffprobe diagnostics

* test(engine): harden ffprobe diagnostic redaction
2026-07-25 17:12:43 -04:00
Miguel Ángel 1895286189 fix(render): normalize local AAC duration before mux (#2472)
* fix(render): normalize local AAC duration before mux

* style(render): apply repository formatter

* fix(render): count AAC packets for duration normalization

Older FFmpeg versions estimate raw ADTS duration from bitrate and can undercount variable-bitrate audio, causing the normalizer to append a false silence tail. Derive the mixed AAC duration from packet count and sample rate instead.

* fix(render): isolate normalized audio temp path

* fix(engine): centralize AAC packet duration

* test(producer): refresh AAC duration golden
2026-07-15 10:07:07 -04:00
Miguel Ángel 5d3a7404fa fix(render): consolidate preflight and local recovery (#2403)
* fix(render): fall back when libx264 is unavailable

* fix(render): recover orphaned browsers before retry

* fix(render): check disk space on write volumes

* test(engine): accept resolved ffmpeg binary paths

* fix(render): harden H.264 capability fallback

* chore(ci): scope inherited Fallow findings

* fix(render): diagnose encoder probe failures
2026-07-14 21:55:39 -04:00
Miguel Ángel b7dcb9e2a3 fix(engine): harden ffmpeg binary resolution 2026-07-05 11:46:38 -07:00
Miguel Ángel cee6fd02d6 fix(cli): verify browser/ffmpeg binaries exist before render starts (#1365)
## Problem

Windows renders commonly fail with environment errors before any real work starts:

- `Browser was not found at the configured executablePath (...chrome-headless-shell.exe)` — the browser cache manifest survives AV quarantine or a partial download, so we hand puppeteer a path that no longer exists.
- `[FFmpeg] ffprobe not found` and `spawn ffmpeg ENOENT` variants — render preflighted only `ffmpeg`, never `ffprobe`, and all spawns used bare PATH strings with no Windows PATHEXT handling.

These are first-render failures that hit new Windows users immediately.

## Fix

- Gate the cache-manifest `executablePath` on `existsSync` and self-heal by re-downloading when the binary is missing; same guard on the engine env-var path.
- New shared environment preflight (`packages/cli/src/browser/preflight.ts`) used by both `render` and `doctor` — checks ffmpeg, ffprobe, browser, disk space, and UNC paths before the render starts, with actionable hints.
- Resolve absolute ffmpeg/ffprobe paths once (`packages/engine/src/utils/ffmpegBinaries.ts`) and pass them to every engine spawn instead of relying on PATH.
- Map opaque Windows ffmpeg exit codes to actionable messages.

## Testing

- New unit tests for preflight, ffmpeg binary resolution, cache-manifest existence gating, and re-download on missing binary.
- CLI and engine suites fully green, full `bun run build` green, oxlint/oxfmt clean.
- Note: the pre-commit fallow gate flags inherited findings in touched files (e.g. `audioExtractor.ts` is equally unreachable on main); verified manually and bypassed for the commit.
2026-06-12 01:36:28 -04:00
JamesandClaude Opus 4.7 6fb782fc09 test(engine): pin ALPHA_MODE uppercase ffprobe tag regression
Locks in the case-insensitive behavior alongside the existing alpha_mode
(lowercase) test. If either path regresses, the producer would silently
extract alpha-having webms as opaque JPGs and the injected <img> overlays
would cover every element below them on the z-stack — a bug that doesn't
surface in the studio preview, only in production renders.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 20:29:33 -07:00
Miguel Ángel 31e8144304 fix: render parity for transparent looped videos (#478)
## Summary
- preserve alpha for render-injected video frames by detecting alpha streams with ffprobe and extracting alpha video frames as PNG
- keep `<video loop>` semantics through static parsing, compiler duration resolution, browser media discovery, and render frame lookup
- fail embedded preview startup before opening a broken browser page when the Studio bundle is missing
- align snapshot frame injection with looped media timing and VP9 alpha extraction

## Why
The Studio preview and rendered MP4 could disagree for timed transparent looped videos. The Comfy funding composition exposed two separate parity bugs: render-injected frames needed alpha-preserving PNG extraction, and the compiler was clamping a looped `data-duration="4"` video down to the 3.125s source duration. After the first source cycle, render lookup treated the video as inactive, hid the native video, and produced the blank polygon/glow the user saw around the rounded `0:03` mark.

`hyperframes lint` and `hyperframes validate` did not catch this because they check syntax/load/console/accessibility, not preview-vs-render visual parity. This PR adds regression coverage for the compiler loop-duration path and frame lookup path.

## Verification
- `bun run --filter @hyperframes/core test -- src/compiler/timingCompiler.test.ts src/compiler/htmlCompiler.test.ts`
- `bun test packages/producer/src/services/htmlCompiler.test.ts`
- `bun run --filter @hyperframes/engine test -- videoFrameExtractor ffprobe`
- `bun run --filter @hyperframes/core typecheck`
- `bun run --filter @hyperframes/engine typecheck`
- `bun run --filter @hyperframes/producer typecheck`
- `bun run --filter @hyperframes/cli typecheck`
- `bun run lint`
- `bun run format:check ...` on touched files
- Comfy project: `node packages/cli/dist/cli.js validate` -> no console errors, 44 text elements pass WCAG AA
- Comfy project patched render from source: `/tmp/comfy-render-compare/fixed6-comfy.mp4`, 1920x1080, 30fps, 21.8s, 654 frames
- 3.00s-3.97s render contact sheet: `/tmp/comfy-render-compare/fixed6-window-contact.png`
- targeted fixed render capture at 3.733s: `/tmp/comfy-render-compare/probe-capture-fixed/captured/frame_000112.jpg`
- agent-browser Studio proof screenshot at 3.7s: `/tmp/comfy-render-compare/agent-browser-studio-3_7-fixed.png`
- agent-browser-driven recording of 3s seek pass: `/tmp/comfy-render-compare/agent-browser-wysiwyg-3s-fixed.webm`

Note: `bun run --filter @hyperframes/cli dev -- validate` is blocked in source mode by the existing `contrast-audit.browser.js` default-export loader issue; packaged `node packages/cli/dist/cli.js validate` passes for this project.
2026-04-24 23:18:20 +02:00
Vance Ingalls 147bb737c9 test(engine): add ffprobe-unavailable fallback regression tests (#379)
## Summary

Mock `node:child_process.spawn` to surface `ENOENT` and verify ffprobe's three callers behave correctly when ffprobe is missing.

## Why

`Chunk 9B` of `plans/hdr-followups.md`. The PNG cICP fallback in `extractMediaMetadata` was added to support environments without ffprobe, but no test pinned the behavior — silently regressing it would break HDR image support on any system without ffprobe installed.

## What changed

`packages/engine/src/utils/ffprobe.test.ts`: mocks `child_process.spawn` to surface `ENOENT` and asserts:

- `extractMediaMetadata` falls back to PNG cICP metadata for image inputs.
- `extractMediaMetadata` rethrows for non-image inputs lacking a still-image fallback.
- `extractAudioMetadata` + `analyzeKeyframeIntervals` propagate the install-hint error verbatim.

## Test plan

- [x] All new tests pass.
- [x] No production code changes — pure regression coverage of existing fallback behavior.

## Stack

Chunk 9B of `plans/hdr-followups.md`. Test-only change, independent of all other chunks.
2026-04-23 11:42:42 -07:00
Vance Ingalls a3d7cc1c95 refactor(producer): extract HDR compositing helpers and rename media metadata (#373)
## Summary

Four behavior-preserving refactors that reduce complexity in `renderOrchestrator.ts` and clarify the engine ffprobe utility surface. Lands after the correctness fixes (Chunks 1–5) so the refactored code is already correct.

## Why

`Chunk 7` of `plans/hdr-followups.md`. The HDR composite block had grown a ~200 LOC inline closure with 14 captured deps, a repeated capture-options spread, a `extractVideoMetadata` name that now also handles still images, and per-frame re-creation of debug helpers.

## What changed

**7A — Hoist `compositeToBuffer` into a module-scoped helper.** Extract the inline HDR closure into a top-level `compositeHdrFrame()` that takes an `HdrCompositeContext` struct. Construct the context once at the top of the HDR render block and pass it through. Removes a deeply-nested closure from the middle of the orchestrator.

**7B — `buildHdrCaptureOptions()` helper.** Factor the repeated `{ ...captureOptions, skipReadinessVideoIds: ... }` spread into a named helper at the call site.

**7C — Rename `extractVideoMetadata` → `extractMediaMetadata`.** Reflects that the helper handles still images (PNG/JPEG/WebP) in addition to video. Update all callers in engine + producer (`videoFrameExtractor`, `htmlCompiler`, regression-harness, producer ffprobe re-export, tests). Re-export the old name as a deprecated alias from `@hyperframes/engine` for backward compatibility, plus the producer re-export shim.

**7D — Hoist debug counters to module scope.** `countNonZeroAlpha` and `countNonZeroRgb48` are now module-scoped so they aren't re-created per frame and so the closure has fewer captures.

Also touches the `hdr-regression` and `hdr-hlg-regression` README + `meta.json` files reviewed during this refactor.

## Test plan

- [x] `bunx tsc --noEmit -p packages/producer && bunx tsc --noEmit -p packages/engine` clean.
- [x] Engine tests: 313 pass, 0 fail (1218 expect calls).
- [x] `bunx oxlint` + `bunx oxfmt --check` clean on 8 changed source files.
- [x] Diff is structural only — no behavioral changes.

## Stack

Chunk 7 of `plans/hdr-followups.md`. Lands after the correctness fixes (Chunks 1–5) per the suggested merge order.
2026-04-22 22:41:30 -07:00
Vance Ingalls d7c1050e44 test(producer): add hdr-regression and hdr-hlg-regression test suites (#365)
## Summary

Replace the trivial `hdr-pq` and `hdr-image-only` tests with two consolidated, time-windowed regression suites that exercise the full HDR pipeline. These goldens are the safety net for every other PR in this stack.

## Why

The pre-existing HDR tests covered only a single full-bleed video or image with a static text label — none of the features that the HDR pipeline has to handle differently from SDR (opacity animation, z-ordered multi-layer compositing, transforms, border-radius clipping, shader transitions, multiple HDR sources, object-fit modes, mixed HDR+SDR layering, HLG transfer). This PR builds the missing safety net first so every subsequent fix can be proven correct.

## What changed

- New `packages/producer/tests/hdr-regression/` (PQ, BT.2020, ~20 s, 1080p, 8 windows A–H):
  - A: static baseline (HDR video + DOM overlay)
  - B: wrapper-opacity fade
  - C: direct-on-`<video>` opacity tween (documents the Chunk 1 bug)
  - D: z-order sandwich (DOM → HDR → DOM)
  - E: two HDR videos side-by-side (pins PR #289)
  - F: rotation + scale + border-radius (documents the Chunk 4 bug)
  - G: `object-fit: contain`
  - H: shader crossfade between HDR video and HDR image
- New `packages/producer/tests/hdr-hlg-regression/` (HLG, ARIB STD-B67, ~5 s, 2 windows A–B) — exercises the separate HLG LUT/OETF code path that previously had **zero** coverage.
- New `scripts/generate-hdr-photo-pq.py` synthesizes `hdr-photo-pq.png` with a cICP chunk for BT.2020/PQ/full.
- Removed `tests/hdr-pq/` and `tests/hdr-image-only/`.
- Updated `.github/workflows/regression.yml` HDR shard to run the new pair sequentially.
- All compositions follow the documented timed-element pattern (`data-start`, `data-duration`, `class="clip"` directly on each timed leaf — no wrapper inheritance).

## Test plan

- [x] Goldens generated with `bun run test:update --sequential`.
- [x] `ffprobe` confirms HEVC/yuv420p10le/bt2020nc/smpte2084 (PQ) and arib-std-b67 (HLG).
- [x] Suite green with `maxFrameFailures` budgets that absorb the documented Chunk 1 / Chunk 4 known-fails — tightened in follow-up PRs in this stack.

## Stack

Foundational PR for the HDR follow-ups stack (Chunk 0 of `plans/hdr-followups.md`). Every subsequent PR builds on this safety net.
2026-04-22 15:43:04 -07:00
Vance Ingalls 00af29c169 fix(cli): forward --hdr through Docker render + HDR docs (#346)
## Summary

This PR ended up covering the full HDR Docker/docs follow-through plus the producer/engine work needed to make HDR still images render and regress correctly in CI.

The branch now does four things:

- forwards `--hdr` through the Docker render path in the CLI
- adds and expands HDR documentation across the docs site
- adds first-class HDR still-image support to the engine/producer pipeline
- adds targeted HDR regression coverage, including a CI-safe fallback for PNG HDR metadata detection when `ffprobe` does not expose PNG color tags

## What changed

### CLI and docs

- `hyperframes render --docker --hdr` now preserves `--hdr` when invoking the in-container CLI
- added a dedicated HDR guide and linked it from CLI, producer, engine, rendering, and common-mistakes docs
- documented HDR constraints and verification flow: HDR source requirements, MP4/H.265 Main10 output, PQ/HLG handling, Docker usage, and common SDR fallback causes

### Engine and producer HDR image support

- added `ImageElement` support to the engine composition model and parsing path
- threaded image elements through producer compilation and orchestration
- probed image sources for HDR color spaces so image-only compositions can trigger HDR output without requiring an HDR video source
- included HDR image start times in stacking queries so the layered compositor can place images correctly in z-order
- integrated HDR image compositing into the layered HDR render loop alongside native HDR video layers and SDR DOM overlays
- forced screenshot mode for HDR layered compositing where required to keep DOM/HDR layer composition deterministic
- skipped readiness waiting for natively extracted HDR videos in the engine path where it was unnecessary and could block layered HDR flows

### HDR metadata robustness

- added a fallback in `extractVideoMetadata()` to read PNG `cICP` metadata directly when `ffprobe` omits color-space fields for PNGs
- this specifically fixes CI/Docker detection for the `hdr-image-only` fixture, where the render was falling back to SDR because the PNG was not being recognized as BT.2020 PQ

### Regression coverage and fixture cleanup

- added `hdr-image-only`, a regression fixture that validates HDR still-image rendering end to end
- added `hdr-pq`, a focused HDR PQ regression fixture for the video path
- updated regression CI to run an `hdr` shard with `--sequential hdr-pq hdr-image-only`
- removed the older larger `hdr-regression/*` fixture set in favor of the smaller targeted regressions used by CI
- added the necessary fixture generation/readme material and checked-in golden outputs for the new HDR tests

## Why

The original PR description only covered the CLI flag forwarding and docs work. Since then, the branch also picked up the missing runtime support needed for HDR still images and the regression coverage to keep that path from breaking.

The practical issue this closes is:

- local host runs could pass while CI failed `hdr-image-only`
- the failure was a full-frame visual mismatch caused by SDR fallback, not unstable rendering
- root cause was PNG HDR metadata not being surfaced by `ffprobe` in the CI Docker environment
- parsing the PNG `cICP` chunk directly makes HDR detection deterministic across environments

## Test plan

### Local targeted checks

```bash
bunx oxlint packages/engine/src/utils/ffprobe.ts packages/engine/src/utils/ffprobe.test.ts
bunx oxfmt packages/engine/src/utils/ffprobe.ts packages/engine/src/utils/ffprobe.test.ts
bun --cwd packages/engine test src/utils/ffprobe.test.ts src/utils/hdr.test.ts
```

### Producer regression runs on host

```bash
bun run --cwd packages/core build:hyperframes-runtime:modular
bun --cwd packages/producer test -- --sequential --exclude-tags slow,render-compat,hdr
bun --cwd packages/producer test -- --sequential hdr-pq hdr-image-only
```

Observed result:
- `fast` shard: 7 passed, 0 failed
- `hdr` shard: 2 passed, 0 failed

### CI-equivalent Docker verification

```bash
docker build -f Dockerfile.test -t hyperframes-producer:test .

docker run --rm \
  --security-opt seccomp=unconfined \
  --shm-size=4g \
  -v "$PWD/packages/producer/tests:/app/packages/producer/tests" \
  hyperframes-producer:test \
  --sequential hdr-pq hdr-image-only
```

Observed result:
- `hdr-image-only`: passed
- `hdr-pq`: passed
- shard summary: 2 passed, 0 failed

### Specific regression fixed

Before the PNG `cICP` fallback, the Docker/CI run failed `hdr-image-only` with:

- missing `"[Render] HDR source detected — output: PQ ..."` log line
- full-frame visual mismatch across all 100 checkpoints
- PSNR ~17 on every frame, indicating a consistent SDR-vs-HDR pipeline mismatch

After the fallback, the same Docker path recognizes the PNG as HDR and the shard passes.
2026-04-20 12:16:24 -07:00