Skip to content

fix(engine): extraction waits out the decode up to a deep seek in a long-GOP video - #6583

Merged
miguel-heygen merged 4 commits into
mainfrom
fix/extract-timeout-keyframe-gap
Oct 27, 2026
Merged

miguel-heygen merged 4 commits into
mainfrom
fix/extract-timeout-keyframe-gap

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What changed

Video frame extraction sizes each ffmpeg process's no-output timeout on the work in front of its first frame. A seek into a video lands on the nearest keyframe before it, and ffmpeg decodes and discards every frame from there to the seek point without writing anything. The stall clock (ffmpegProcessTimeout, 300 s) counted that as a hang. The allowance is now timeout + (seconds from the keyframe to the seek point) x source fps / 30, a floor of 30 decoded frames per second. A new keyframeGapSeconds probe reads the keyframe ffmpeg will land on with a one-packet ffprobe seek (20-40 ms). The final-frame path, which seeks on the output side and decodes from the start, gets the whole start time as its gap.

Why

Trimming deep into a long video with few keyframes (a 4K60 screen recording or camera file) failed in prepare with VIDEO_EXTRACTION_FAILED / ffmpeg_timeout, every time. Each 120 s segment seeks separately, so each one pays the silent decode again.

Evidence

Measured on a Linux box, 27-min 3840x2160 60 fps H.264 source with a single keyframe, 60 s window starting 1500 s in:

timeout result
main 60 s fails at 60.3 s, ffmpeg_timeout
main 120 s fails at 120.4 s
main 300 s (default) succeeds in 183 s here (2 Mbps source decodes fast); a high-bitrate source takes longer to reach frame one
this PR 60 s succeeds in 132 s
this PR, 130 s window (2 segments) 60 s succeeds in 298 s

The same trim on a source with a keyframe every 2 s costs 65-71 s at 10 s, 900 s and 1500 s in: seek depth alone is not the cost, keyframe distance is.

The new test builds a one-keyframe and a one-keyframe-per-second clip and asserts the timeout passed to ffmpeg for a seek 8 s in: expected 2000 to be 10000 on main, passes on this branch 3 of 3 runs; the whole videoFrameExtractor.test.ts suite passes (217 tests).

Why under 100 lines

One probe, one pure timeout formula and two call sites (single pass and per segment); the rest is the test.

Not changed

A genuine hang during that silent decode now takes the longer allowance to be noticed. The 30 fps floor is a conservative software-decode rate; a slower machine would still time out.

@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 · 1 blocker · 0 important · 0 nits

The required Test check is red on this head. Test (runtime) fails videoFrameExtractor.concurrency.test.ts > lets a cancelled extraction leave the queue while another holds the slot, and it fails 3/3 locally on this head while main passes 3/3. Details inline. (Test reachability failed separately: a checkout timeout, which is infra.)

— claude

Comment thread packages/engine/src/services/videoFrameExtractor.ts Outdated
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 114 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 (1)

  • seqplainkeys-none-px-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, 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: REQUEST CHANGES — 1 resolved, 0 open, 1 new · 7d16d45 → 1d45a9f

Comment thread packages/engine/src/services/videoFrameExtractor.ts Outdated

@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 — 1 resolved, 0 open, 0 new · 1d45a9f → f0c1f62

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 3b2058a Oct 27, 2026
78 checks passed
@miguel-heygen
miguel-heygen deleted the fix/extract-timeout-keyframe-gap branch October 27, 2026 20:32
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.

1 participant