fix(engine): auto-normalize VFR video inputs to CFR before frame extraction#360
Merged
jrusso1020 merged 6 commits intomainfrom Apr 21, 2026
Merged
fix(engine): auto-normalize VFR video inputs to CFR before frame extraction#360jrusso1020 merged 6 commits intomainfrom
jrusso1020 merged 6 commits intomainfrom
Conversation
…action
Screen recordings (macOS ScreenCaptureKit, QuickTime, phone videos) are
commonly variable-frame-rate. When such inputs hit the extractor's
`-ss <start> -i <video> -t <dur> -vf fps=N` pipeline, the fps filter
can emit fewer frames than requested — for a 4-second 30fps segment
starting mid-file, the output was ~90 frames instead of 120.
`FrameLookupTable.getFrameAtTime` returns null for out-of-range indices,
so the compositor held the last valid frame and the user perceived the
video as freezing. This matches the bug report from an X community post
where a user said "all of them freezes" on their screen recording scenes.
The engine already detects VFR via `metadata.isVFR` in ffprobe.ts but
never acted on it — the compiler only logged a warning. This change
mirrors the existing SDR→HDR normalization pattern: when a source is
detected as VFR, re-encode only the used segment with
`-fps_mode cfr -r <fps> -preset fast -crf 18` before extraction.
Scoping the re-encode to `[mediaStart, mediaStart+duration]` means a
30-second clip cut from a 60-minute screen recording pays ~1s of
transcode cost, not 18s. Benchmarked locally:
Baseline (current): 32-39% duplicate frames, 25% frame-count
shortfall on mid-file segments.
Tier 1 (flag changes only): ~same — fps filter issue is not flag-fixable.
Tier 2 (CFR preflight): 1.7-6% duplicate frames, correct frame
count in every scenario tested.
The compiler warning that previously told users to manually re-encode
is downgraded to `console.info` since the engine now handles it.
— Rames Jusso
- Drop the `vfrNormDirCreated` flag; `mkdirSync({recursive:true})` is
idempotent and cheap.
- Don't re-wrap the `VFR→CFR conversion failed` prefix — `convertVfrToCfr`
already throws a message with that label; adding it again in the catch
produced "VFR→CFR conversion failed: VFR→CFR conversion failed (exit 1)".
- Shorten the Phase 2b header comment; the function docstring above
`convertVfrToCfr` already explains the failure modes and rationale.
- Note which frame windows the VFR fixture's select filter drops so the
magic numbers are scannable.
No behavior change; 311/311 engine tests still pass.
— Rames Jusso
miguel-heygen
requested changes
Apr 21, 2026
Collaborator
miguel-heygen
left a comment
There was a problem hiding this comment.
I'd add a regression test just in case
miguel-heygen
approved these changes
Apr 21, 2026
Adds a describe block that synthesizes a VFR fixture via ffmpeg and asserts the extractor produces the expected frame count (no shortfall) and no long runs of duplicate frames — the user-visible "frozen screen recording" symptom. Covers both a mid-file segment and the full-file case. Guarded with describe.skipIf(!HAS_FFMPEG) because the CI Test job on ubuntu-24.04 and the Windows test-windows job don't install ffmpeg. The producer-level regression test in packages/producer/tests/vfr-screen-recording/ runs inside Dockerfile.test (which has ffmpeg) and is the primary CI signal for this bug; these unit tests are supplementary coverage for local and any ffmpeg-equipped CI environment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-to-end CI regression coverage for PR #360 via the existing regression-harness: renders a 3s composition containing a real macOS ScreenCaptureKit clip (r_frame_rate=120, avg≈36fps) seeked to mediaStart=1, then PSNR-compares against a committed output.mp4. Fixture src/clip.mp4 (108 KB) is a 5-second excerpt downscaled to 480×332 with -fps_mode passthrough to preserve the VFR timestamps. Content is the public hyperframes OSS repo root page — see NOTICE.md for provenance. With the fix applied, all 100 PSNR checkpoints pass. With the fix reverted, 66 of 100 fail (PSNR drops from ~43 dB to ~20 dB in the duplicate-frame windows). Tagged "regression,video,vfr" so it runs in the fast shard of .github/workflows/regression.yml automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The committed golden output.mp4 was initially rendered on the host machine; CI runs the renderer inside Dockerfile.test with a different Chrome + ffmpeg build, producing pixel-level drift that failed PSNR at 54/100 checkpoints (~20 dB vs 41 dB in the VFR sparse-content windows). Both renders are valid — the VFR source has inherent sampling ambiguity in static segments, and different Chrome/ffmpeg builds make different valid choices. Regenerated the baseline via `bun run docker:test:update vfr-screen-recording` so it matches the Docker environment CI actually uses. Matches the flow the existing sub-composition-video, hdr-pq, etc. baselines were captured with. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hit this 2026-04-21 with the vfr-screen-recording regression test: host-generated output.mp4 baseline tripped 54/100 PSNR checkpoints in CI because Chrome + ffmpeg drift between the host and Dockerfile.test. Document the `bun run --cwd packages/producer docker:test:update <name>` flow so future contributors don't repeat the mistake. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Auto-normalize variable-frame-rate (VFR) video inputs to constant-frame-rate (CFR) before the frame-extraction stage, eliminating the "frozen screen recording" class of bugs in rendered compositions.
Why
A user reported on X that their screen-recording scenes were freezing in hyperframes renders. Investigation:
packages/engine/src/services/videoFrameExtractor.ts:142runsffmpeg -ss <start> -i <video> -t <dur> -vf fps=Non the input.fpsfilter exhibits two failure modes:FrameLookupTable.getFrameAtTime(line 440) returns null for out-of-range indices, so the compositor holds the last valid frame — the user sees a freeze.The engine already detects VFR via
metadata.isVFR(packages/engine/src/utils/ffprobe.ts:257) but never acted on it — the compiler only logged a warning telling users to manually re-encode.How
Mirrored the existing SDR→HDR normalization pattern in the same file. When
metadata.isVFR === true, re-encode the used segment to CFR before extraction:Scoping to
[mediaStart, mediaStart+duration]rather than full-file means a 30-second clip cut from a 60-minute screen recording pays ~1s of transcode cost, not 18s.Benchmarked locally on a synthesized VFR fixture (10s of testsrc2 with ~40% frames dropped, sparse keyframes every 10s):
-fps_mode cfr,-rvs-vf fps=N, accurate seek)The flag-only tweaks were tested and insufficient — the fps filter's VFR handling is the underlying issue, not the seek mode or output-rate flag. Pre-encoding is the fix experiment-framework already uses internally for the same reason (see
worker/celery/movio/utils/ffmpeg_utils.pyreencode_video_if_potentially_vfr).The compiler warning that used to tell users to manually re-encode VFR videos is downgraded from
console.warntoconsole.infosince the engine now handles it; the message still mentions the pre-encode command for users who want to skip the per-render transcode cost.Test plan
packages/engine/src/services/videoFrameExtractor.test.ts— Rames Jusso