Skip to content

feat(producer): opt-in video layer passthrough for a lone full-canvas video - #6914

Merged
miguel-heygen merged 9 commits into
mainfrom
feat/video-layer-passthrough-b
Nov 1, 2026
Merged

miguel-heygen merged 9 commits into
mainfrom
feat/video-layer-passthrough-b

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

What

An opt-in render mode (HF_VIDEO_LAYER_PASSTHROUGH=true, off by default) for a composition whose only video fills the whole canvas and the whole timeline. The page is captured without the video, as transparent PNG frames, and ffmpeg composites the source video underneath while encoding. This skips the per-frame round trip of the video through JPEG, the page and a second JPEG (refs #6881).

Anything that does not qualify renders exactly as today, and the log says why (Video layer passthrough off: <reason>).

When it applies

MP4 output, software encode, SDR, streaming encode eligible, exactly one video that starts at 0, covers the whole render, plays once at normal speed, is not colour-graded or hidden, and is a local file. A page check at nine sampled times confirms it fills the canvas, has no opacity, transform, filter, clip or blend on it or its ancestors, and nothing paints under it. HDR sources, variable-frame-rate sources and sources with alpha fall back.

It runs on one worker with screenshot capture. Frame choice and colour use the same filter chain as frame extraction (now one shared builder), so the two cannot drift.

Static-frame dedup and the left-out video

The passthrough capture hides the video, but the dedup eligibility scan still counted any <video> in the page and switched dedup off ("video"). computeStaticFrameSet now ignores videos left out of capture (hiddenElementIds). On a 30 s test film this took capture from 74.8 s to 27.7 s (821 of 900 frames reused) with unchanged SSIM.

Comments moved here

  • The eligibility scan uses totalDuration(), not duration(), because a repeat or yoyo tween animates past one iteration; a repeating timeline is marked opaque over its whole span.
  • Any tl.call() disqualifies the composition. It is a zero-duration tween whose callback is wired as onComplete and onReverseComplete, so both directions run the same forward side effect. A DOM change driven that way is not seek-idempotent: the verifier's own seeking can mutate the page permanently and leak into a later static run. A harmless call cannot be told from a harmful one without running it.
  • The scan still descends into a repeating nested timeline even though its span is already opaque, so a call() inside it is still detected before the verifier seeks through that span.

Checked so far

  • 30 s test film with captions, devbox: same frame count (900 = 900) as today's render, SSIM 0.995 against it, extraction time 0 s.
  • 600 s 1080p30 film, 120 captions, devbox, 4 pinned cores, one run per path: today 1459 s (2 workers, extraction 1m43) -> passthrough 523 s (1 worker, extraction 0 s, 16428 of 18000 frames reused, capture 8m27). 18000 = 18000 frames; PSNR 43.9 dB average (43.3 lowest frame), SSIM 0.995.
  • 600 s film graph-only run: 18000 = 18000 frames, PSNR 43.4 dB average (lowest frame 42.8), SSIM 0.9946.
  • Unit tests for the qualification rule and the encoder graph; typecheck, build and format clean.

Not checked yet

  • CPU use, repeat runs, and a 4K film.
  • Three real films, frame by frame, with eyes on the pairs. The option stays off by default until that is done.
  • Producer stage tests run under bun on the devbox show the same failures on main and on this branch.

Size

About 390 changed lines across engine and producer, one feature.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES · 2 blocker · 1 important · 0 nits · head f306bfb5

This is the first review on the PR, so I reviewed the full diff rather than just the trim since 18cc5dbb.

  • blocker: the required Comments check fails on this head because the comment ratchet rose in three files: engine/src/types.ts (+1 line), extractVideosStage.ts (+1) and probeStage.ts (+1). Cut those comment lines or move the explanation into a name.
  • One more blocker and one important finding are inline. ↓
Scope

Read all 14 files. I also traced initTransparentBackground, which the HDR layered paths share with clearCompositionRoot. CI was partly still running: CI, regression, Windows render verification and CodeQL were in progress. This is a feature PR, so no fails without the fix: line is needed.

— Genesis

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 108 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (2)

  • seqplainkeys-none-center-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true
  • seqplainkeys-none-pct-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: APPROVE — 3 resolved, 0 open, 0 new · f306bfb5 → ee323516. Comments passes on this head. CI, regression, Windows and CodeQL were still running when I approved.

— Genesis

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: APPROVE — 0 resolved, 0 open, 0 new · ee323516 → 59c0f427. Static-frame dedup now ignores only the video that capture leaves out (the ids in hiddenElementIds). Reused frames still go to the encoder once per frame, so ffmpeg's video layer stays in step with the page. The rest of the diff is the evaluate callback's re-indent and shorter comments. Comments passes. CI, regression, Windows and CodeQL were still running when I approved.

— Genesis

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 1, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 9852036 Nov 1, 2026
138 of 139 checks passed
@miguel-heygen
miguel-heygen deleted the feat/video-layer-passthrough-b branch November 1, 2026 05:33

@jrusso1020 jrusso1020 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.

Post-merge advisory review (this PR is already merged into main). Findings flagged for follow-up, not gating.

Reviewed at 59c0f4278900e2aa590efed0de13ea8f772871a4.

Important follow-up — packages/producer/src/services/render/videoLayerPassthroughPlan.ts:61, packages/engine/src/services/videoFrameExtractor.ts:1240: passthrough still accepts video styling that ffmpeg does not reproduce. inspectVideoLayout checks objectFit but discards objectPosition and does not refuse border radii. buildVideoLayerInput always uses a centered crop, and the final graph composites an unrounded source rectangle beneath the hidden video. A full-canvas object-fit:cover; object-position:0% 50% video therefore changes its framing when this opt-in path is enabled. Rounded video corners are also filled back in.

I exercised the exact source layout function in real Chromium with a real 320×90 red/blue VP9 source displayed in a 160×90 canvas. Both centered and left-positioned cases qualify {fit:"cover"}. At pixel (70,45), the left-positioned browser image is red [254,0,0], while the centered ffmpeg crop is blue [0,0,254]; the centered browser control is blue. A border-radius:30px case also qualifies while its browser corner is transparent. The CSS individual-scale control falls back through the bounding-box check. These are targeted function/browser/filter witnesses, not a complete Producer render.

Concrete fix: fail closed on non-centered object-position when cover crops, and on nonzero video border radius / unsupported clipping, or carry the measured positioning/mask through the encoder. Add regression cases for left/right anchoring and rounded corners, with a centered rectangular positive control. Keep the feature off by default until these presentation contracts are covered.

Genesis's previous wrapper-background and nested-composition tint findings are fixed by the current ancestor/subcomposition checks. The changed dedup analysis ignores only supplied hidden video IDs; reuse still feeds each output frame to the encoder. The source frame/color chain is shared, and extraction/audio/readiness and one-worker PNG streaming plumbing are consistent in the inspected paths.

Validation: 155 engine tests (videoLayerPassthrough, streamingEncoder, frameCapture) and 3 producer layout tests pass. The Chromium/ffmpeg controls above expose the missing contract beyond those tests. I did not rerun the author's 30-second/600-second performance or SSIM measurements, 4K films, or the complete render pipeline.

Need: yes, opt-in removal of repeated video capture has a concrete benefit · Size: ok, one engine/producer feature · Reuse: shared extraction color/frame filter builder · Simpler: reject unsupported layout instead of approximating it · Structure: fine
Risk: medium — opt-in rendering optimization can silently alter picture content.

Verdict: COMMENT (post-merge: With fixes)
Reasoning: Opt-in passthrough is already merged, but targeted browser/filter witnesses show a picture mismatch for accepted presentation styles. Refusing unsupported styling is the smallest safe follow-up.

— hf-review

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