Repository navigation
fix(producer): stop HDR video windows after the first from repeating one frame - #6575
Conversation
…one frame A raw HDR window seeks to a whole second, then trims its lead-in frames. trim keeps their timestamps, and the raw muxer writes none, so when the input has an audio track FFmpeg's default CFR sync pads from pts 0 with copies of the window's first frame and -frames:v cuts the real ones off. Phone footage always has audio, so HLG clips rendered at ~half their frames. Pass frames through as the filter graph emits them. Fixes #6556 Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
jerrai-bot-heygen
left a comment
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
The duplicate-window fix is promising, but removing the initial timestamp padding also changes the clock mapping for sources with delayed video. Please preserve the offset and add a delayed-start parity regression before merging. Separately, the Comments check currently fails because the production file's comment share rose. I did not run tests or compare Chrome playback locally.
— Jerrai
| ...["-vf", filters.join(","), "-frames:v", String(a.frames)], | ||
| // The raw muxer has no timestamps, so FFmpeg's default CFR sync would pad from pts 0 up to the | ||
| // trimmed window's first frame with copies of it. Write exactly the frames the graph emits. | ||
| ...["-fps_mode", "passthrough"], |
There was a problem hiding this comment.
Could we preserve the initial media-time offset while avoiding padding introduced by the per-window trim? For a 30 fps source whose video starts 0.5 s after audio, probeFirstTick is 15 and passthrough puts the first decoded frame in raw slot 0, but resolveHdrVideoFrameIndex still indexes slot 15 at composition time 0.5 s. That advances the moving video relative to audio, rather than only making the first frame visible early. The new uniqueness test starts video at zero and cannot detect this. Please cover a delayed-start source with frame-by-frame or playback parity assertions across window boundaries. I haven't visually reproduced the Chrome comparison.
Window slots counted from the first decoded frame, so a video stream that starts after its audio played early by its lead. Slots now count from the extraction start and show the first frame until the stream begins, as they did before windowed extraction (#6304) and as Chrome does. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
The frame-loss fix and delayed-start correction check out at 7959d47e4ba243b4427cac5908b25e2a87819fcd.
captureHdrResources.ts:586preserves the filter-emitted raw frames; I reproduced 22/120 distinct frames without passthrough and 120/120 with it.captureHdrResources.ts:590-599restores the initial clock offset while delegating lease release and close to the existing window owner. I independently reproduced the timing problem Jerrai raised on the earlier head: with audio at zero and video starting at 0.5 s, the old proposal selected source frame 15 at 0.5 s; Chrome selected frame 0. The new mapping selects frame 0. Chrome seek/pixel comparisons at 0.5, 1 and 2 seconds identify source frames 0, 15 and 45; the no-offset control identifies 15, 30 and 60 as expected.
Local verification: 37 tests pass (36 current HDR resource tests plus an independent real-extraction offset witness), using FFmpeg 7.0.2. A separate synthetic 10-bit HEVC/HLG probe preserved all 300 frame buffers byte-for-byte against the decoded source. Its 64x64 source contains one duplicate, so I do not independently claim the description's exact distinct-frame counts for its different fixture. The negative control without passthrough fails the added regression.
Limits: no reporter iPhone footage, VFR, macOS or complete composited HDR render by me. Chrome comparison used a synthetic H.264 timing fixture; the HEVC/HLG check separately exercised raw frame extraction. Existing generated bootstrap artifacts were reused only to collect the tests; no dependency installation. Current CI has no reported failures: producer unit tests, typecheck, build and Comments pass. Required Windows render and several other checks are still running, so this is code approval, not a claim that all CI is green. The earlier review's offset finding is addressed by this successor; its review state remains with that reviewer.
— Magi (desktop-review)
Verdict: APPROVE
Reasoning: the frame-loss regression is reproduced and fixed, and the successor repairs the delayed-video clock mapping with independent browser and extraction evidence. No remaining code blocker found in this bounded review.
…xtraction windows Neither HDR fixture had an audio track, which is how #6556 shipped. Window A checks frames after the first extraction window are not repeats; window B checks a video stream that starts 0.5 s after its audio holds its first frame until the picture starts. The baseline was generated in Docker and the fixture fails against main. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
jerrai-bot-heygen
left a comment
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
The first-tick lease mapping addresses my delayed-start clock finding, and the successor compares the delayed sequence against the undelayed source in order while adding an HLG-with-audio golden fixture across windows. I reviewed the fixture definitions and LFS pointers, not the binary baseline itself; I did not run the tests or reproduce Chrome playback. Hosted checks are still running.
— Jerrai
Edit accuracy: accurate 271 (base branch 271), smooth 134 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
What
HDR video frame windows after the first now hold their own source frames instead of one frame repeated.
Why
Since #6304, each raw HDR window seeks to a whole second, then drops its lead-in frames with
trim=start_frame.trimkeeps those frames' timestamps, and the raw muxer writes none, so when the input has an audio track FFmpeg's default constant-frame-rate sync pads from pts 0 with copies of the window's first frame, and-frames:vcuts the real frames off the end. Phone footage always carries audio, so HLG clips rendered at about half their frames (the reporter saw 155/300 distinct, 64/302 on iPhone footage). Silent sources and window 0 were unaffected, which is why existing tests passed.Related work
Fixes #6556. Regressed in #6304 (first released in v0.8.199).
How
Add
-fps_mode passthroughto the per-window FFmpeg args inhdrWindowFfmpegArgs, so FFmpeg writes exactly the frames the filter graph emits.-fps_modeis already used indistributed/assemble.ts, so no new FFmpeg requirement. Chosen oversetpts=PTS-STARTPTSafter the trim because it treats every window the same way.Since #6304, window slots also counted from the first decoded frame, so a video stream that starts after its audio (an edit-list delay) played early by that lead. A wrapper now shifts leases by the probed first tick: slots count from the extraction start and show the first frame until the stream begins. Before #6304, CFR extraction did exactly this, and Chrome also shows the upcoming first frame during the lead.
Test plan
hdr-hlg-audio-windows(1080p HLG clips with an audio track, spanning several extraction windows; one clip delays its video 0.5 s behind its audio). Baseline generated in Docker. It fails against main at the visual stage and passes with this PR.ibyte for byte. SDR control 300/300. A 59.94 fps HLG source in a 30 fps comp also gives 300/300.Not tested: the reporter's real iPhone footage, VFR sources, macOS.
🤖 Generated with Claude Code