Skip to content

feat(engine): wire options.hdr through chunkEncoder + dynamic SDR→HDR transfer#370

Merged
vanceingalls merged 5 commits intomainfrom
vance/chunk-encoder-hdr
Apr 23, 2026
Merged

feat(engine): wire options.hdr through chunkEncoder + dynamic SDR→HDR transfer#370
vanceingalls merged 5 commits intomainfrom
vance/chunk-encoder-hdr

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls commented Apr 21, 2026

Summary

Three independent fixes that share a common thread: HDR config flowing correctly from EngineConfig down through every encoder. The headline fix: disk-based HDR encodes via chunkEncoder were silently producing BT.709-tagged output despite options.hdr being set.

Why

Chunk 3 of plans/hdr-followups.md. The streaming encoder was correct but chunkEncoder.buildEncoderArgs hard-coded BT.709 color tags and the bt709 VUI block in -x265-params, even when callers passed an HDR EncoderOptions. Today this is harmless because renderOrchestrator routes native-HDR content to streamingEncoder and only feeds chunkEncoder sRGB Chrome screenshots — but the contract was a lie, and any future caller that wired HDR through chunkEncoder would silently get SDR output.

What changed

3A — chunkEncoder respects options.hdr (BT.2020 + mastering metadata). When options.hdr is set, the libx265 software path emits bt2020nc plus the matching transfer (smpte2084 for PQ, arib-std-b67 for HLG) at the codec level and embeds master-display + max-cll SEI in -x265-params via getHdrEncoderColorParams. libx264 still tags BT.709 inside -x264-params (libx264 has no HDR support) but the codec-level color flags flip so the container describes pixels truthfully. GPU H.265 (nvenc/videotoolbox/qsv/vaapi) gets the BT.2020 tags but no -x265-params block, so static mastering metadata is omitted — acceptable for previews, not HDR-aware delivery.

3B — convertSdrToHdr accepts a target transfer. videoFrameExtractor.convertSdrToHdr was hard-coded to transfer=arib-std-b67 (HLG) regardless of the surrounding composition's dominant transfer. extractAllVideoFrames now calls analyzeCompositionHdr first, then passes the dominant transfer ("pq" or "hlg") into convertSdrToHdr so an SDR clip mixed into a PQ timeline gets converted with smpte2084, not arib-std-b67.

3C — EngineConfig.hdr type matches its declared shape. The IIFE for the hdr field returned undefined when PRODUCER_HDR_TRANSFER wasn't "hlg" or "pq", but the field is typed as { transfer: HdrTransfer } | false. Returning false matches the type and avoids a downstream undefined check.

Test plan

  • chunkEncoder.test.ts: replaced the previous "HDR options ignored" assertions with 8 new specs covering BT.2020 + transfer tagging, master-display/max-cll embedding, libx264 fallback behavior, GPU H.265 + HDR (tags but no x265-params), and range conversion for both SDR and HDR CPU paths.
  • All 313 engine unit tests pass (5 new HDR specs).
  • ffprobe an HDR composition rendered through the chunk encoder path: shows bt2020nc color matrix, smpte2084 transfer, and mastering display metadata.

Stack

Chunk 3 of plans/hdr-followups.md. Independent of Chunks 1/4 (touches separate code paths).

Copy link
Copy Markdown
Collaborator Author

vanceingalls commented Apr 21, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vanceingalls vanceingalls force-pushed the vance/chunk-encoder-hdr branch from 767b046 to 2e06ff0 Compare April 21, 2026 20:54
@vanceingalls vanceingalls marked this pull request as ready for review April 21, 2026 20:57
@vanceingalls vanceingalls force-pushed the vance/chunk-encoder-hdr branch from 2e06ff0 to db58e11 Compare April 22, 2026 01:16
@vanceingalls vanceingalls force-pushed the vance/chunk-encoder-hdr branch from db58e11 to efd8422 Compare April 22, 2026 02:03
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

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

This is the correctness fix I'm happiest to see in this stack. The prior chunkEncoder was actively lying about color space — hardcoding colorprim=bt709:transfer=bt709:colormatrix=bt709 in -x265-params even when the caller passed an HDR EncoderOptions. Any HDR-aware player would interpret the stream via BT.709 tone-mapping and render it as SDR; HDR TVs would show a washed-out, mis-gamma'd signal. The fact that this didn't fire in production is purely because renderOrchestrator kept HDR content on the streamingEncoder path today, but calling that "defended by routing" is fragile — the contract was wrong.

Three things to call out:

3A — SW vs GPU H.265 split is documented honestly. SW libx265 gets BT.2020 color tags + HDR static mastering metadata (master-display, max-cll SEI). GPU paths (nvenc, videotoolbox, qsv, vaapi) get the color tags but no mastering metadata because ffmpeg doesn't let you push -x265-params through hardware encoders. The PR body calls this out explicitly as "acceptable for previews, not HDR-aware delivery" — that's the right framing. For a production HDR pipeline this is worth a doc note somewhere (README? HDR guide?) so downstream callers don't assume useGpu=true + hdr={transfer: "pq"} produces a fully spec-compliant HDR10 stream.

3B — mixed-transfer composition edge case. convertSdrToHdr now takes dominant transfer from analyzeCompositionHdr. If a composition contains both PQ and HLG HDR sources (rare but legal), "dominant" wins and the non-dominant set gets converted with the wrong transfer. Worth a brief comment at the call site noting that mixed-transfer compositions are caller-error territory, or explicitly validating that analyzeCompositionHdr returns a non-null transfer only when all sources agree.

3C — type safety. Flipping hdr: undefinedhdr: false to match the declared { transfer } | false shape is the right kind of small hygiene fix. Worth verifying there are no if (config.hdr) call sites that were relying on undefined vs false coercion (both falsy, same branch behavior, so should be fine — but it's the kind of thing a static analyzer would flag). Tests pass, so this is confirmed.

libx264 + HDR is a landmine to document. With this PR, calling buildEncoderArgs with codec: "h264" + hdr: { transfer: "pq" } produces a file with BT.2020 codec-level tags but bt709 in the x264-params VUI block. Technically honest (libx264 truly can't encode HDR), but some players get confused by the VUI/container mismatch. Consider making chunkEncoder.ts explicitly warn-and-strip HDR when codec=h264, or throwing upstream so no one accidentally gets a half-HDR artifact.

Test coverage is exactly right — 8 specs across the matrix (HLG/PQ × SW/GPU × h264/h265). Approved.

Rames Jusso

vanceingalls added a commit that referenced this pull request Apr 22, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
@vanceingalls vanceingalls force-pushed the vance/chunk-encoder-hdr branch from efd8422 to 2d844ab Compare April 22, 2026 04:44
vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
@vanceingalls vanceingalls force-pushed the vance/chunk-encoder-hdr branch 2 times, most recently from 6d01ce5 to a31369c Compare April 23, 2026 01:58
@vanceingalls vanceingalls force-pushed the vance/regen-c branch 3 times, most recently from 9b6dc76 to d961acf Compare April 23, 2026 02:52
@vanceingalls vanceingalls force-pushed the vance/chunk-encoder-hdr branch from a31369c to 19e5afc Compare April 23, 2026 02:52
vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
@vanceingalls vanceingalls changed the base branch from vance/regen-c to graphite-base/370 April 23, 2026 03:14
Address jrusso1020's nit on PR #365 (non-blocking review): both READMEs now
explain where the tolerance values come from.

- hdr-regression/README.md: add a budget-breakdown table that derives the 30
  frames from the deltas in PRs #369 (window C fix → 5) and #375 (window F
  fix → 0). The table doubles as a contract: if a future change forces the
  budget back up, exactly one bucket has regressed and the table tells you
  which one to investigate first.
- hdr-hlg-regression/README.md: add a 'Tolerance' section explaining why 0
  is the right floor (HLG is a pure pass-through path, HEVC over rgb48le is
  byte-deterministic on the same fixture, so any drift is a real regression).

The regeneration command for generate-hdr-photo-pq.py was already documented
at README lines 67-71, so no changes needed there.
… transfer

Chunk 3 of HDR follow-ups. Three independent fixes that share a common
thread: HDR config flowing correctly from the EngineConfig down through
the encoders.

3A. chunkEncoder respects options.hdr (BT.2020 + mastering metadata)
  Previously buildEncoderArgs hard-coded BT.709 color tags and the
  bt709 VUI block in -x265-params, even when callers passed an HDR
  EncoderOptions. Today this is harmless because renderOrchestrator
  routes native-HDR content to streamingEncoder and only feeds
  chunkEncoder sRGB Chrome screenshots — but the contract was a lie.

  Now: when options.hdr is set, the libx265 software path emits
  bt2020nc + the matching transfer (smpte2084 for PQ,
  arib-std-b67 for HLG) at the codec level *and* embeds
  master-display + max-cll SEI in -x265-params via
  getHdrEncoderColorParams. libx264 still tags BT.709 inside
  -x264-params (libx264 has no HDR support) but the codec-level color
  flags flip so the container describes pixels truthfully. GPU
  H.265 (nvenc/videotoolbox/qsv/vaapi) gets the BT.2020 tags but no
  -x265-params block, so static mastering metadata is omitted —
  acceptable for previews, not HDR-aware delivery.

3B. convertSdrToHdr accepts a target transfer
  videoFrameExtractor.convertSdrToHdr was hard-coded to
  transfer=arib-std-b67 (HLG) regardless of the surrounding
  composition's dominant transfer. extractAllVideoFrames now calls
  analyzeCompositionHdr first, then passes the dominant transfer
  ("pq" or "hlg") into convertSdrToHdr so an SDR clip mixed into a PQ
  timeline gets converted with smpte2084, not arib-std-b67.

3C. EngineConfig.hdr type matches its declared shape
  The IIFE for the hdr field returned undefined when
  PRODUCER_HDR_TRANSFER wasn't "hlg" or "pq", but the field is typed
  as { transfer: HdrTransfer } | false. Returning false matches the
  type and avoids a downstream undefined check.

Tests
  - chunkEncoder.test.ts: replaced the previous "HDR options ignored"
    assertions with 8 new specs covering BT.2020 + transfer tagging,
    master-display/max-cll embedding, libx264 fallback behavior, GPU
    H.265 + HDR (tags but no x265-params), and range conversion for
    both SDR and HDR CPU paths.
  - All 313 engine unit tests pass (5 new HDR specs).

Follow-ups (separate PRs):
  - Producer regression suite runs in CI; not exercising HDR-tagged
    chunkEncoder yet because no live caller sets options.hdr there.
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
@vanceingalls vanceingalls force-pushed the vance/chunk-encoder-hdr branch from 19e5afc to 2f1942d Compare April 23, 2026 03:14
@graphite-app graphite-app Bot changed the base branch from graphite-base/370 to main April 23, 2026 03:15
… libx264

The hdr field is { transfer: HdrTransfer } | undefined — not boolean.
Setting it to false produced a TS2322 type error. undefined is the correct
sentinel and matches how callers represent SDR throughout the codebase.
@vanceingalls vanceingalls force-pushed the vance/chunk-encoder-hdr branch from 2f1942d to 7b268a9 Compare April 23, 2026 03:15
vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
@vanceingalls vanceingalls merged commit 5256a93 into main Apr 23, 2026
33 checks passed
Copy link
Copy Markdown
Collaborator Author

Merge activity

vanceingalls added a commit that referenced this pull request Apr 23, 2026
…d-transfer caller error

PR #370 review feedback (jrusso1020):

- chunkEncoder: when codec=h264 and hdr is set, log a warning and strip
  hdr instead of emitting a half-HDR file (BT.2020 container tags +
  BT.709 VUI inside the bitstream). libx264 has no HDR support; the only
  honest output is SDR/BT.709. Caller is told to use codec=h265.

- videoFrameExtractor: comment at the convertSdrToHdr call site clarifying
  that dominantTransfer is majority-wins; mixing PQ and HLG sources in a
  single composition is caller-error and the minority transfer's videos
  will be converted with the wrong curve. Render two compositions if you
  need both transfers.

- docs/guides/hdr.mdx: limitations section now documents (a) H.264 + HDR
  is rejected at the encoder layer, and (b) GPU H.265 (nvenc, videotoolbox,
  qsv, vaapi) emits BT.2020 + transfer tags but does NOT embed master-display
  or max-cll SEI, since ffmpeg won't pass x265-params through hardware
  encoders. Acceptable for previews, not for HDR10 delivery.
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