fix: skip metadata waits for injected video frames#575
Conversation
f6d2a0a to
79d6b41
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
LGTM — well-scoped fix that generalizes the existing HDR-skip path and adds proper layout-hint compensation.
Root cause analysis: the readiness wait at frameCapture.ts:initializeSession was waiting for readyState >= 1 on every <video> element. For videos whose pixels come from out-of-band FFmpeg extraction (HDR + standard), the browser never plays them — the <video> element is kept only for layout. Waiting on those is unnecessary and on Windows with multiple instances of the same MP4 source it actually times out at 45s ("video metadata not ready"). The previous HDR-only skip was the right idea but too narrow.
Fix shape:
- Generalize the skip via
collectVideoReadinessSkipIds— combinesnativeHdrVideoIdswith all extracted videos that have usable dimensions. - Compensate for the lost layout signal via
videoMetadataHints+applyVideoMetadataHints— setswidth/heightHTML attributes and CSSaspect-ratiofrom FFmpeg-probed dimensions, but only when the author hasn't already set them. So author CSS/HTML wins. - Rename
buildHdrCaptureOptions→buildCaptureOptionssince it's no longer HDR-specific.
Cross-platform safety: Linux/macOS used to wait for native metadata then derive layout from it. Now they skip the wait but get explicit width/height/aspect-ratio hints. Equivalent layout outcome, faster path.
CI gate: the new step in windows-render.yml scaffolds the exact issue #574 repro (3× same MP4 source) with PRODUCER_PLAYER_READY_TIMEOUT_MS=15000. If the bug regresses, the timeout fires fast and CI catches it. Solid regression-gate shape.
One nit, non-blocking: CaptureVideoMetadataHint.durationSeconds is collected but not consumed in applyVideoMetadataHints. Probably reserved for future use (programmatic <video>.duration setting?), but worth either using or dropping in a follow-up to avoid dead-data.
Tests: the two new tests on collectVideoMetadataHints and collectVideoReadinessSkipIds cover the right cases (filtering bad dimensions, sort-stability, native HDR id passthrough). Both pass locally. Note: there's an unrelated pre-existing failure in the file ("rejects a maliciously crafted key that tries to escape compileDir" — fails on origin/main too, looks like Linux-runner sensitivity to a Windows-path test).
— Review by Rames Jusso
79d6b41 to
79671dd
Compare
Merge activity
|
Problem
Closes #574.
On Windows with cached headless-shell Chrome, a composition that reuses the same video file in three timeline clips can fail before frame capture starts:
The reported render reaches video frame extraction, then dies at frame-capture initialization with:
The important detail is that by this stage HyperFrames has already extracted video pixels through FFmpeg. Native Chromium video metadata is only being waited on for DOM layout stability, not because Chromium is the source of rendered pixels.
Root Cause
The render pipeline has two separate media responsibilities:
Before this PR, every capture session still waited for every DOM
<video>to reachreadyState >= 1unless the element was a native HDR exception. That made native browser media metadata a hard render prerequisite even when the browser would not decode or provide the final video pixels.That is why the issue fails at
25% Starting frame capture: FFmpeg extraction has already succeeded, but capture initialization blocks on repeated native<video src="1.mp4">metadata loading in cached Windows headless-shell Chrome.There was a second constraint: the readiness wait also prevents first-frame layout bugs. If a skipped
<video>has no native metadata, Chromium can use the default300x150intrinsic video size, which breaks layouts such aswidth: 100%; height: autobefore the first injected frame. The fix therefore must not simply skip all video readiness waits; it must provide dimensions for any skipped videos.What This Fixes
videoMetadataHints.width/heightattributes and an explicitaspect-ratioonly when the element does not already provide one, preserving author styles where present.buildCaptureOptions()helper so calibration, HDR DOM capture, streaming capture, parallel capture, and sequential capture receive the same skip IDs and metadata hints.Reviewer Map
Primary files:
packages/producer/src/services/renderOrchestrator.tscollectVideoReadinessSkipIds()includes native HDR IDs plus extracted videos that have finite positive FFmpeg dimensions.collectVideoMetadataHints()converts extracted FFmpeg metadata into capture hints.buildCaptureOptions()threadsskipReadinessVideoIdsandvideoMetadataHintsinto every capture path.packages/engine/src/services/frameCapture.tsapplyVideoMetadataHints()runs in the page before video readiness polling.readyState >= 1.packages/engine/src/types.tsCaptureVideoMetadataHintand documents that readiness skips should be paired with metadata hints when layout may depend on intrinsic dimensions.packages/producer/src/services/renderOrchestrator.test.ts.github/workflows/windows-render.yml1.mp4.Why This Is Safe
The skip is intentionally gated:
extractAllVideoFrames()succeeded for that video and returned usable dimensions.width,height, and explicitaspect-ratioare not overwritten.window.__hfreadiness keep the existing waits.A first local revision skipped readiness too broadly and caused
overlay-montage-prodfirst-frame layout shrinkage. The current version fixes that by pairing skips with FFmpeg metadata hints;overlay-montage-prodnow passes and is listed in verification below.Verification
Root-Cause Reproduction Before Fix
The reporter did not attach the actual
1.mp4, so the regression uses the exact issue markup and a deterministic generated 12s H.264 file named1.mp4.I reproduced the failure in GitHub Actions by running this branch's new Windows workflow against unpatched
main:That means the workflow contains the new issue #574 regression, but the code under test is
mainwithout this fix.Baseline failure:
ref: main,origin/main, commit8662598a3ac64018a2999d189ffb369e6d46b53a.Browser: cache,staticDuration:12,videoCount:3, then25% Starting frame capture->[FrameCapture] video metadata not ready after 15000ms.This is the same failure class as the issue, on Windows, in cached-browser mode, before the fix.
Fixed Windows Regression
The same regression passes on this PR branch:
79d6b41c9f2ba137cbfb9301678e0815b16c4f5amerged into8662598a3ac64018a2999d189ffb369e6d46b53a.Browser: cache,staticDuration:12,videoCount:3,25% Starting frame capture, captures360/360frames, rendersissue-574.mp4, andffprobeverifies1920x1080 @ 30/1, 12s.Local Checks
bun run build:hyperframes-runtimebunx vitest run packages/producer/src/services/renderOrchestrator.test.tsbun run --filter @hyperframes/producer typecheckbun run --filter @hyperframes/engine typecheckbunx oxlint packages/engine/src/services/frameCapture.ts packages/engine/src/types.ts packages/engine/src/index.ts packages/producer/src/services/renderOrchestrator.ts packages/producer/src/services/renderOrchestrator.test.tsbunx oxfmt --check .github/workflows/windows-render.yml packages/engine/src/services/frameCapture.ts packages/engine/src/types.ts packages/engine/src/index.ts packages/producer/src/services/renderOrchestrator.ts packages/producer/src/services/renderOrchestrator.test.tsgit diff --checkLocal Render Checks
/tmp/hf-issue-574-reprowith the issue shape: three clips using the same1.mp4,data-media-start=0/4/8, 12s total.PRODUCER_PLAYER_READY_TIMEOUT_MS=5000 bun packages/cli/src/cli.ts render /tmp/hf-issue-574-repro --workers 1 --quality draft --fps 30 --output /tmp/hf-issue-574-h264-fixed-v2.mp4-> completed./tmp/hf-issue-574-proreswith the same three-clip shape using one FFmpeg-readable ProRes.mov, which exercises the browser-metadata failure class because Chromium should not be needed to decode the source.PRODUCER_PLAYER_READY_TIMEOUT_MS=3000 bun packages/cli/src/cli.ts render /tmp/hf-issue-574-prores --workers 1 --quality draft --fps 30 --output /tmp/hf-issue-574-prores-fixed-v2.mp4-> completed.bun run --filter @hyperframes/producer test --sequential --keep-temp overlay-montage-prod-> passed; this guards against skipped metadata shrinkingheight:autovideo layout before the first injected frame.ffmpeg -v error -i /tmp/hf-issue-574-prores-fixed-v2.mp4 -f null -ffmpeg -v error -i /tmp/hf-issue-574-h264-fixed-v2.mp4 -f null -ffprobe -v error -show_entries format=duration:stream=codec_name,width,height,r_frame_rate -of json /tmp/hf-issue-574-h264-fixed-v2.mp4-> H.264, 320x180, 30fps, 12.0s.Current PR Checks
overlay-montage-prod. At the time this body was updated, thefastregression shard was still in progress in run https://github.com/heygen-com/hyperframes/actions/runs/25174515546.Browser Verification
agent-browserto openfile:///tmp/hf-issue-574-h264-fixed-v2.mp4and verify the rendered output displays in Chromium..debug/issue-574/h264-output-page.png.debug/issue-574/h264-output-playback.webmNotes / Caveats
1.mp4was not attached to [FrameCapture] video metadata not ready after 45000ms #574. The committed Windows regression uses a generated deterministic H.264 file with the same filename and exact markup from the issue.Browser: cache, matching the reporter's environment..debug/issue-574/and intentionally not committed.