Skip to content

fix(engine): long exports with many clips no longer stall at 90% or hide a timeout - #5375

Merged
miguel-heygen merged 5 commits into
mainfrom
fix/engine-bounded-frame-extraction
Oct 10, 2026
Merged

miguel-heygen merged 5 commits into
mainfrom
fix/engine-bounded-frame-extraction

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

What was wrong

Long exports with many video clips failed near the end.

  • Frame extraction ran one ffmpeg per clip, all at once. extractAllVideoFrames started every clip's extraction together (an unbounded Promise.all), each with its own 300 s deadline. With 82 clips the machine was starved: processes stalled and 51 of them hit their deadline. The same export passes when extraction runs four at a time.
  • The final stage ignored the configured ffmpeg timeout. The assemble stage called the audio normalize, mux, faststart and HLS steps without the render's ffmpegProcessTimeout, so each fell back to the fixed 300 s default whatever FFMPEG_PROCESS_TIMEOUT_MS said. When a long 4K mux hit that deadline the error read Audio muxing failed: FFmpeg exited with code 255 ..., with no sign that it had been killed for taking too long. The distributed assemble path had the same gap.

Fix

  • One limit, at the one place extraction ffmpeg is spawned. At most half the CPU count of extraction jobs run at once, across every clip and every long-clip segment, in every render in the process. This is the same formula the segment workers already used. A variable-frame-rate clip's two-process pipeline takes one slot. A queued extraction's deadline starts only when its ffmpeg spawns. The limiter wraps only the spawns, so nothing waits while holding a slot.
  • Cancel is prompt. A queued extraction whose render is cancelled leaves the queue at once, without spawning. runFfmpeg and runFfmpegPipeline start no process when their signal has already aborted.
  • Every assemble step takes the configured timeout. In process, the assemble stage receives ffmpegProcessTimeout and passes it to normalize (including its true-peak probe), mux, faststart and HLS packaging. The distributed assemble reads FFMPEG_PROCESS_TIMEOUT_MS the same way for its remux, concat, CFR re-encode, normalize, mux and faststart.
  • A deadline kill now says so. describeFfmpegFailure leads with FFmpeg timed out after N ms (ffmpegProcessTimeout; long renders can raise FFMPEG_PROCESS_TIMEOUT_MS) and then the usual stderr tail. Other failures read as before.

Behaviour to know

  • Concurrent renders in one producer process now share the extraction queue, so a small render can wait behind a large one's queued clips. Before, they ran side by side and overcommitted the machine.
  • The cap is floor(cpus / 2): 4 on an 8-core machine, which is the value measured above; 6 on 12 cores, 8 on 16. Whether 6 to 8 at once also passes the 82-clip export is unmeasured. It uses os.cpus(), so a CPU-quota container sees host cores, as the rest of the engine does.

Verification

  • videoFrameExtractor.concurrency.test.ts: real extractions through a shim ffmpeg that records how many processes are alive as each starts, with two CPUs (one slot). Four short clips: every start sees one alive. Two 125 s clips, split into segments: every start sees one. Two variable-frame-rate clips through the two-process pipeline: never more than two. Removing the limit at each of the three spawn sites turns its test red. A fourth case holds the only slot with one extraction, queues a second and cancels it: the second fails as cancelled without starting ffmpeg while the first still runs; without the signal reaching the queue it waits for the first and times out. The shim is a shell script, so this file is skipped on Windows.
  • concurrencyLimit.test.ts: never more than the limit at once, waiters start in order, a failing task frees its slot, a cancelled waiter leaves the queue without a slot, an already-cancelled task does not wait. Dropping the cancel handling turns the last two red.
  • runFfmpeg.test.ts: an already-aborted run spawns nothing, alone or as a pipeline. Removing the check turns it red.
  • chunkEncoder.test.ts: a mux and a faststart killed at a configured deadline (fake timers) report FFmpeg timed out after ...; an ordinary failure's message is unchanged.
  • assembleStage.test.ts: the configured timeout reaches normalize, mux, faststart and HLS.
  • distributed/assemble.test.ts (bun): with FFMPEG_PROCESS_TIMEOUT_MS=1, assembly fails with FFmpeg timed out after 1 ms. With the env ignored it passes and the test turns red.
  • Runs on our test box, three each, exit 0: the four engine files 131 passed; the assemble stage 20 passed; assemble.test.ts and audioPadTrim.test.ts 51 passed.

Not covered

  • The 82-clip ProRes alpha export itself was not re-run on this branch. The four-at-a-time measurement came from the same cap applied by hand.
  • Other fan-outs in the same export stay unbounded: the alpha-plane probe (three 8x8 frames per alpha source, 30 s cap, a failure only skips a warning), the HDR first-frame colour read, and the audio mixer's per-element reads. None can stall the export the way extraction did.

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 2061 (base branch 2061), smooth 1486 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)

@miguel-heygen
miguel-heygen force-pushed the fix/engine-bounded-frame-extraction branch from 3730fa9 to 0bbdf1a Compare October 9, 2026 22:09
@miguel-heygen
miguel-heygen marked this pull request as ready for review October 10, 2026 02:15

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

Reviewed 744cc9f8a237aa8c0867308b5464d6304adf3fa3 against base/effective merge-base 1de8b5ae336041e6f0367b577c3870a7b2649f2c. No introduced blocker verified in the audited extraction/assembly paths.

Strengths

  • packages/engine/src/services/videoFrameExtractor.ts:889–891,931–959: one process-wide pool surrounds the actual extraction runners, including direct, segmented and two-child VFR work, rather than holding slots around recursively queued tasks. A slot admits an extraction operation: direct/segment work launches one child, whereas a VFR operation launches two. The cited controls use one slot, not a one-child ceiling. Real-media controls exercise simultaneous render calls, 125-frame long/sparse extraction and VFR frame consumption. Removing only the shared cap raises observed child peaks from 1→3 (direct), 1→2 (segments) and 2→4 (VFR), while the selected consumer assertions still run.
  • packages/engine/src/utils/concurrencyLimit.ts:10–34 and packages/engine/src/utils/runFfmpeg.ts:194–244: cancelled waiters leave promptly; cancellation-aware callbacks return abort without spawning, and slots refill after awaited runners settle. The independent real-process cases cover queued/pre-aborted/handoff abort, active direct/VFR/segment cancellation, spawn failure on either pipeline side, ordinary rejection and queue wait longer than the queued run's execution deadline. Removing both pre-spawn guards starts three pre-aborted children and wrongly starts a cancelled extraction while its holder is active. The head controls wait for child close and successfully consume later extractions.
  • packages/producer/src/services/render/stages/assembleStage.ts:116–155,177–189, packages/producer/src/services/distributed/assemble.ts and packages/engine/src/utils/runFfmpeg.ts:142–151: configured per-process deadlines reach the audited normalize/true-peak correction, mux, faststart, HLS, remux/concat/CFR paths. Fourteen paired controls consume successful real-FFmpeg output or the actual stage/distributed timeout error after the managed shim deadline/SIGTERM path. Deadline wording retains the stderr tail; the exercised abort/inactivity/external-interruption classification and formatting cases retain their non-deadline treatment. Coincident termination-reason races were not exercised.

Audit and evidence
Reviewed the 15-file net change and relevant timeout callers, cancellation/error joins, disposal and scratch cleanup, with separate source and adversarial passes. Thirteen changed files were read end-to-end. Integration-only scope: videoFrameExtractor.ts (2,828 lines) and renderOrchestrator.ts (5,419 lines): changed hunks plus pertinent callers, configuration, error/join and disposal regions, not full-file reads. The sole production runAssembleStage caller and both audio-normalization callers were traced.

Linux, Node 22.22.1, Bun 1.4.2 and actual configured /usr/bin/ffmpeg/ffprobe 5.1.9. Parent checked exact-head source/lock/binary pins, clean restoration in both exclusive worktrees, retained long/sparse/VFR frame hashes, consumed output/error records and absent recorded child PIDs. All 65 recorded children in the final independent control closed; the other retained control/mutant runs were checked too. The normal VFR case explicitly checks that the next operation starts only after both preceding children close, and the queued-cancel case settles while the holder is still running without starting the cancelled extraction. These recorded-process witnesses do not establish absence of unrecorded descendants or universal cleanup across every failure path. Concurrent-render FrameLookupTable assertions cover start/tail byte identity, half-open end and per-clip distinction before cleanup.

Test results and limits

  • Independent real-media harness: log reports 21/21 passed, including both one-sided pipeline-spawn failures. Its enclosing shell continued to a metadata command, so the final Vitest child exit was not separately captured. The four upstream extraction/limiter/runner/encoder files passed 131 tests with child exit 0. These counts overlap other selections and are not summed.
  • Final wider selections: Vitest 173 passed / 2 failed and Bun 50 passed / 1 failed; both test children exited 1. The three failures also occur with all nine changed production files replaced by exact merge-base blobs, keeping head tests/dependencies: HLS audioPts[1] is NaN versus 0.023222 (packageHls.test.ts:471), delivered AAC LUFS delta is 3.1 versus limit 3 (audioPadTrim.integration.test.ts:179), and concat r_frame_rate is 30000/1001 versus 30/1 (distributed/assemble.test.ts:292). These controls support pre-existing local failures, not a claim that the wider suites are green or that a standalone base build was tested.
  • Cap and pre-abort mutants intentionally exited 1 (3 and 2 selected failures). Their exact patches/logs are retained, but there is no test-time mutant source-hash manifest; they used the preceding 19-case harness revision with the same selected cases. Sources were restored and the final controls/upstream selection rerun.
  • The 28 assembly checks use a selected executable shim to stall the targeted path; unstalled calls delegate to actual FFmpeg and produce consumed outputs. They establish the exercised shim deadline/SIGTERM/error propagation and scratch cleanup, not an actual-FFmpeg hang killed in production. SIGTERM-resistant children and stalled pipeline shutdown were not tested.
  • No full 82-clip ProRes-alpha export, end-to-end producer/browser export, throughput/starvation benchmark, Windows/macOS run or proof about the remaining unbounded HDR/alpha/audio fan-outs. FIFO sharing can delay a small render behind a larger one. The cap uses host os.cpus(), not container quota: this host reports 32 CPUs, 16 available/quota CPUs and therefore 16 slots. Those 16 operation slots can admit up to 32 extraction children if all active operations are VFR pipelines. The PR documents these tradeoffs; their operational sizing remains unmeasured here.

Retained local evidence on the review host: /tmp/hf5375-744cc9f8-adversarial-evidence/ contains videoFrameExtractor.review5375.test.ts, independent-control-final.log, upstream-focused-final.log, both mutant-*.diff/log pairs and fixtures/*/{consumed-controls,spawn-settlement,runtime-pins}.json. /tmp/hf5375-744cc9f8-source-evidence/ contains baseline-controls.py, baseline logs/manifest, consumed-timeout-probe.ts and its outcomes/events. Parent checks and current artifact digests are in /tmp/hf5375-744cc9f8-parent-evidence/verify-retained.py and verified-current-artifact-sha256.json. Final independent selector: HF_REVIEW_RUN=control-final vitest run --root <owned-worktree>/packages/engine src/services/videoFrameExtractor.review5375.test.ts --reporter=verbose; cap mutant selects shares one slot|long and sparse|VFR pipeline, pre-abort mutant selects queued, already-aborted|queued real extraction. The retained 19-case mutant patches/logs support selected-case sensitivity, not fully pinned validation of the later 21-case harness.

Verdict: APPROVE
Reasoning: The shared extraction boundary, cancellation-aware pre-spawn guards and assembly deadline/error propagation are supported by source tracing and consumed controls, without a verified introduced blocker in the stated scope. Approval is on code merits, not a CI or full-export performance guarantee, and does not authorize merge.

— tai

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 10, 2026

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

Approve at 744cc9f8. I checked the PR's claims against the code, and they hold.

What the code does

  • The limit can't deadlock. It wraps only the three ffmpeg spawn calls, so nothing that holds a slot ever waits for another one.
  • Waiters start in order. A freed slot goes straight to the next waiter, so a newcomer can't jump ahead.
  • Slots and listeners are cleaned up. The slot is released in finally. The abort listener is removed whether the waiter gets its slot or gives up.
  • Cancel is immediate. A cancelled waiter leaves the queue at once, and its runFfmpeg call returns without spawning anything.
  • Abort and timeout stay distinct. Whichever termination is requested first wins. Mux, faststart and HLS check the signal before formatting an error, so an abort is never reported as a timeout.
  • The timeout reaches every ffmpeg step.
    • In process: normalize (runner and true-peak), mux, faststart and HLS.
    • In the distributed path: remux, concat, CFR, pad/trim, mux and faststart.
  • The default config changes nothing.

Real-ffmpeg probe (8 cores, so 4 slots; I counted only the ffmpeg children the probe spawned)

Case Peak live ffmpeg Result
16 clips, this PR 4 16/16 extracted
16 clips, merge-base extractor 16 16/16 extracted
16 × 60 s clips, aborted after 6 starts 4 no spawns after the abort, returned in about 0.5 s, 0 processes left, 0 unhandled rejections
6 × 130 s clips split into segments, aborted 4 no spawns after the abort, returned in about 0.5 s, nothing left alive

Tests

  • The touched engine files pass 131/131, assembleStage 20/20, and assemble plus audioPadTrim 51/51.
  • The full engine suite passes: 2150 passed, 3 skipped.
  • The producer unit lanes pass, except one loudness test in audioPadTrim.integration. It fails the same way with the merge-base audioPadTrim.ts on my local ffmpeg 8.0.1, so it isn't from this PR.

Mutants: 18 of 30 caught. Each of these turns a test red: removing the limit, unwrapping any spawn call, dropping the queue's abort handling, dropping the already-aborted check in runFfmpeg, breaking the in-order hand-off, skipping the release on failure, and dropping any in-process assemble timeout.

Should-fix: test gaps, not code bugs

  • Cancelling a queued segment or VFR-pipeline extraction is untested. If the segment or pipeline spawn stops passing its signal to the limiter (videoFrameExtractor.ts:952, :1135), every test still passes. This matters when another render holds the slots.
  • Nothing asserts that an abort isn't reported as a timeout. Labelling abort as a timeout in describeFfmpegFailure (runFfmpeg.ts:147) passes every test.
  • Some timeout wiring is untested.
    • Nothing checks that padOrTrimAudioToVideoFrameCount passes timeoutMs on to its runner and true-peak probe.
    • The distributed steps' timeouts aren't tested one by one, because the 1 ms test fails at whichever step runs first.
    • The HLS packager's timeout message has no test.

Nits

  • The normalize step's ffprobe calls keep their fixed 30 s deadline (audioPadTrim.ts:675). "Every assemble step" is accurate for ffmpeg, but not for those probes.
  • The cap is floor(cpus/2) with no setting to change it, so a 2-core host extracts one clip at a time.
  • renderChunk passes no signal to its deferred extraction. That predates this PR, but those calls now can't leave the queue.

All required checks pass at this head. No real HeyGen render or paid API was used: unit tests, mutants and a local ffmpeg probe only.

— Rames

Merged via the queue into main with commit 4b68f73 Oct 10, 2026
88 checks passed
@miguel-heygen
miguel-heygen deleted the fix/engine-bounded-frame-extraction branch October 10, 2026 03:01
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.

3 participants