feat(engine/producer): enable multi-worker streaming encode via interleaved frame distribution

[Claudemeri]

Problem

Parallel renders (--workers N) currently disable streaming encode entirely. shouldUseStreamingEncode gates to workerCount === 1, so any multi-worker render falls back to disk capture: all frames written to disk, then a separate sequential FFmpeg encode.

This means:

• Short comps (<240s) with --workers 2 are ~8× slower than --workers 1 (measured: 1-worker streaming 83s vs 2-worker disk 681s for the same 49s composition).
• Long comps lose the streaming encode benefit entirely even though the capture parallelism would help.

The infrastructure for multi-worker streaming was already present — captureStreamingStage has a full workerCount > 1 branch using FrameReorderBuffer for ordered writes. It was simply unreachable because shouldUseStreamingEncode blocked it.

Root cause

The blocker was the frame-distribution strategy. With contiguous chunk distribution (worker 0 gets frames 0–5000, worker 1 gets 5001–10000), the FrameReorderBuffer blocks worker 1 at its first frame until worker 0 finishes its entire chunk — collapsing N parallel captures to effectively 1 for the streaming path.

Fix

1. distributeFramesInterleaved — interleaved round-robin assignment

Added to packages/engine/src/services/parallelCoordinator.ts. Worker i captures frames i, i+N, i+2N, … where N = workerCount. All workers advance in lockstep, so the FrameReorderBuffer finds the next cursor value ready (or nearly ready) on every write. Implemented via a stride?: number field on WorkerTask and a one-line loop change in captureFrameRange.

2. captureStreamingStage — use interleaved distribution for streaming

The existing workerCount > 1 branch now calls distributeFramesInterleaved instead of distributeFrames. The disk-capture path is unaffected — it keeps contiguous chunks for mergeWorkerFrames compatibility.

3. shouldUseStreamingEncode — lift the workerCount === 1 gate

Removed. Also refined the streamingEncodeMaxDurationSeconds guard: it only applies to workerCount === 1. With interleaved distribution, in-flight frame buffers are bounded by workerCount (not composition length), so the cap is unnecessary for multi-worker renders.

Test

Updated shouldUseStreamingEncode unit tests in renderOrchestrator.test.ts:

• keeps png-sequence on the non-streaming path — unchanged behavior
• enables streaming for multi-worker renders (parallel streaming) — new assertion: workerCount=2,4 → true
• applies the duration cap to single-worker only; multi-worker streaming is uncapped — single-worker cap preserved; multi-worker works for comps >240s

Note on golden baselines: the producer regression baselines need to be regenerated inside Dockerfile.test (host Chrome/FFmpeg drift breaks PSNR). If CI flags a baseline diff, the update procedure is in CLAUDE.md (bun run --cwd packages/producer docker:test:update).

Expected impact

Scenario	Before	After
49s comp, --workers 1 (streaming)	83s wall (17.8 fps)	unchanged
49s comp, --workers 2	681s wall (2.2 fps, disk path)	~42s wall (~35 fps)
548s comp, --workers 2	disk path, encode sequential	streaming, encode overlapped

[jrusso1020]

Reviewed the full pipeline: distributeFramesInterleaved → captureFrameRange (with the stride loop) → onFrameBuffer → FrameReorderBuffer.waitForFrame/advanceTo → StreamingEncoder.writeFrame → ffmpeg.stdin.write. The interleaved distribution is the right answer for the capture side, and the "why don't we do this already" answer is in the diff itself — the multi-worker branch in captureStreamingStage was scaffolded but unreachable because shouldUseStreamingEncode returned false for workerCount > 1. Clean unlock there.

Approve the architecture; want one concrete change before ship.

Important — removing the duration cap for multi-worker is unsafe on long comps

The PR's framing is "in-flight frame buffers are bounded by workerCount" — that's true for the capture-to-reorder-buffer side (each worker holds at most one captured frame, awaits its turn at the reorder buffer, then moves on). It is not true for the reorder-buffer-to-FFmpeg side.

Walk through StreamingEncoder.writeFrame at packages/engine/src/services/streamingEncoder.ts:450-471:

const accepted = ffmpeg.stdin.write(copy);
if (accepted) resetTimer();
return accepted;

writeFrame does not await drain when accepted === false. The caller (onFrameBuffer) doesn't await either. So when FFmpeg is slower than total capture throughput, Node.js's writable-stream internal buffer grows unboundedly — Node respects highWaterMark for the signal (returns false) but doesn't actually refuse the write. The streamingTimeout inactivity timer is the only guardrail, and it only fires after the entire encoder stalls, not on creep.

Concrete numbers for the worst case the PR's test now enables (workerCount=3, durationSeconds=3600 → true):

• 3-worker capture: ~3 × 35 = 105 fps
• FFmpeg h264 medium: ~60 fps
• Net backlog: ~45 fps × ~500 KB/frame × 3600 s ≈ 80 GB of unbounded Node buffer growth

That'll OOM the producer pod well before the 1-hour render finishes. The cap was the only thing preventing this for single-worker; removing it entirely for multi-worker makes the risk worse, not better, because total capture throughput is higher.

Pick one:

1. Keep a (more generous) cap for multi-worker. Halfway between current 240s and removing it. Suggest 1800s — long enough to unblock the real cases (the 548s example in the PR description) but short enough to bound the OOM-territory worst case.
2. Implement real back-pressure in writeFrame. When accepted === false, return a Promise<void> that awaits the drain event before resolving. Then onFrameBuffer actually blocks workers when FFmpeg is behind. Real fix, slightly bigger blast radius (writeFrame signature change).
3. Document the trade-off explicitly + rely on streamingTimeout as the OOM-prevention net. Note in the code comment that uncapped multi-worker streaming assumes encode ≥ total capture throughput, and that violating this risks OOM (mitigated by streamingTimeout SIGTERM). Lowest-effort but leaves the failure mode obscured.

Strong preference for (1) for this PR; (2) is the right long-term fix worth a follow-up.

Minor — sharpen the in-code comment on shouldUseStreamingEncode

Current comment says "in-flight frame buffers are bounded by workerCount (not composition length)". A future reader could read this as the total pipeline being bounded, which it isn't. Suggest:

// Capture-side: workers serialize through FrameReorderBuffer, so at most
// `workerCount` captured frames are in flight at any moment. The encoder-side
// buffer (Node stdin → FFmpeg) is NOT explicitly bounded and relies on FFmpeg
// keeping up with `workerCount × per-worker-fps`. Long comps + slow encode
// can still grow Node's internal write buffer — see streamingEncodeMaxDurationSeconds
// for the single-worker safeguard.

Architectural correctness — verified

• distributeFramesInterleaved math: worker i → frames i, i+N, i+2N, … (where N=workerCount). Loop guard i < workerCount && i < totalFrames correctly handles the workerCount > totalFrames edge case (fewer workers spun up; each remaining worker still produces one frame).
• captureFrameRange stride: for (i = startFrame; i < endFrame; i += stride) with startFrame=i, endFrame=totalFrames, stride=N produces exactly the interleaved sequence.
• FrameReorderBuffer interaction: cursor advances by 1 per write; each worker awaits waitForFrame(i) before its write and calls advanceTo(i+1) after. With interleaved distribution, the next frame's owner (a different worker) is almost-immediately ready — vs contiguous, where worker 1 waits for all of worker 0's chunk before any of its writes can land. That's a real-deal correctness reason for the change.
• outputFrameOffset interaction: streaming path doesn't write per-worker disk files (no fileFrameIdx collision), and disk path keeps distributeFrames (contiguous), so mergeWorkerFrames is unaffected. ✓
• executeWorkerTask separate-browser gating (needsSeparateBrowsers) for multi-worker on Linux + headless-shell is unchanged. ✓

Tests — accurate but should add the edge case

The shouldUseStreamingEncode test changes faithfully describe the new gates. Missing:

• Unit test for distributeFramesInterleaved itself (assert worker i gets [i, i+N, i+2N, …], and the workerCount > totalFrames guard works). Pure function; trivial test; locks in the contract.

Why we don't do this already (your question, James)

The multi-worker streaming branch in captureStreamingStage was already implemented — it has the workerCount > 1 block with FrameReorderBuffer and executeParallelCapture(..., onFrameBuffer, ...) wired up. It was unreachable because:

1. shouldUseStreamingEncode hardcoded return workerCount === 1.
2. The only frame distribution available was distributeFrames (contiguous chunks). If you'd lifted the gate but kept contiguous distribution, the streaming path would collapse to effectively single-worker (worker 1 blocks on the reorder buffer until worker 0 finishes its whole chunk).

So the answer is: someone scaffolded multi-worker streaming, hit the contiguous-chunk pathology, gated the path off, and never wrote the interleaved distribution that would make it work. This PR is exactly the missing piece. Clean architectural win — modulo the encoder-backlog concern above.

Summary

REQUEST_CHANGES for the cap-removal concern (pick option 1 minimum; encoder-side buffer is not bounded by workerCount). All other findings are nits. Ship after.

Review by Jerrai

[Claudemeri]

Thanks @jrusso1020 — the OOM analysis is correct and I should have caught it.

Took option (1): latest commit caps multi-worker streaming at a fixed 1800s (MULTI_WORKER_MAX_DURATION_SECONDS = 1800), independent of the single-worker config value. That's 3× the longest practical composition we're running (~548s), so real workloads all route through the streaming path while compositions >30min fall back to disk capture. Also tightened the shouldUseStreamingEncode comment to explicitly distinguish the capture→reorder bound (workerCount-bounded, as claimed) from the reorder→FFmpeg stdin buffer (unbounded without drain, which is what the cap guards).

For the durable fix (option 2 — real await drain back-pressure in writeFrame), I've filed issue #1353 with a concrete implementation sketch and the caller-chain analysis. Once that lands, the 1800s cap can be relaxed or removed.

[Claudemeri]

Added in the latest commit:

• distributeFramesInterleaved unit tests covering: stride assignment (worker i gets [i, i+N, i+2N, …]), full-coverage invariant across several (total, workers) pairs, the workerCount > totalFrames guard, single-worker degenerate case, and output-dir naming.
• Swapped in your suggested comment verbatim on shouldUseStreamingEncode — captures the capture-side vs encoder-side bound distinction cleanly.

All three pieces from the review are addressed. Let me know if anything else needs adjusting.

[jrusso1020]

All three findings cleanly addressed. Verified the final diff:

• MULTI_WORKER_MAX_DURATION_SECONDS = 1800 lands at renderOrchestrator.ts:1452; maxDuration ternary correctly picks per-worker-count cap.
• Comment now distinguishes capture→reorder bound (workerCount-bounded) from reorder→FFmpeg buffer (unbounded). Stale 720s draft from the first iteration was removed.
• distributeFramesInterleaved unit tests added — stride assignment, full-coverage invariant, workerCount > totalFrames guard, single-worker degenerate, output-dir naming.
• Follow-up issue #1353 filed for the durable await drain fix.

Ship it.

Review by Jerrai

[jrusso1020]

Re-opening — my prior approval missed an interaction with the existing ffmpegStreamingTimeout. Holding the merge.

The concern

ffmpegStreamingTimeout (default 600_000ms = 10 min, at packages/engine/src/services/streamingEncoder.ts:437) is an inactivity timer that resets ONLY on ffmpeg.stdin.write === true (line 470). When FFmpeg falls behind and writes keep returning false (data buffering in Node), the timer doesn't reset → SIGTERM at the 10-min mark. The 1800s cap prevents indefinite buffer growth (OOM) but doesn't prevent the inactivity timer from firing mid-render and failing the encode.

Multi-worker capture has higher net capture-vs-encode delta than single-worker — FFmpeg encodes single-threaded; capture scales with N. So multi-worker is MORE likely to produce sustained false-writes, not less. The cap should arguably be tighter for multi-worker, not more generous.

Ask

Two paths, either works:

1. Test with long renders before merging. A 1500s+ multi-worker render under realistic CPU contention (encode competing with capture) — does the inactivity timer fire? If it doesn't, the 1800s cap is defensible empirically. If it does, the failure mode is real and the cap needs tightening.
2. Tighten the cap conservatively now. Something in the 120-300s range gives the new fast path real-world headroom (covers the cited 548s longest comp? — actually no, 548s > 300s, so this option pushes that workload to disk-capture). If preserving the 548s case matters, do (1).

The durable fix is still the await drain back-pressure (the follow-up issue #1353 you filed). Once that lands, the inactivity timer can't fire from buffer-growth alone because workers actually block. Until then, the cap is the only safety net for both OOM AND timeout.

Happy to take #1 ourselves if it'd unblock you — we have the infrastructure to spin a long multi-worker render locally + monitor accepted=true/false ratio.

Review by Jerrai

[Claudemeri]

Thanks for pulling the approval and sharing the experiment-framework evidence — the inactivity timer interaction is a real concern that the 1800s cap does not address.

We're evaluating two tracks in parallel:

1. Empirical test on your infra: accepting your offer to run the 1500s+ multi-worker test. This gives the most rigorous data on whether the inactivity timer fires in practice at those durations under realistic CPU contention. Please go ahead whenever convenient.
2. Parallel VPS test: we'll run the same workload (our ~548s production composition, 2 workers, chrome-headless-shell) on our own box and report fps + accepted=false rate + timer behaviour from HF stdout/stderr.
3. Implement the feat(engine): real back-pressure in StreamingEncoder.writeFrame (await drain when accepted === false) #1353 back-pressure fix directly in this PR: early estimate is ≤1 day (5 production files + 2 test files, all small changes — writeFrame becomes async, drain awaited on accepted === false, HDR and streaming call sites each get one await). If that estimate holds we'll amend this PR with the durable fix rather than ship the cap as a placeholder.

Will report back within 24h on which path lands. Thanks for the experiment-framework PRs — very useful precedent.

[Claudemeri]

The durable back-pressure fix from #1353 is now in this PR (commit a060e47).

writeFrame is now async and awaits drain when stdin.write returns false. Capture loop throttles to FFmpeg's encode throughput — no unbounded buffer growth, and ffmpegStreamingTimeout resets promptly on the next post-drain write (which returns true). The 1800s cap stays as belt-and-suspenders.

All 5 call sites updated to await. Tests updated (53/53 engine + 4/4 captureStreamingStage). Ready for re-review.

[jrusso1020]

R3 — APPROVE. The new await drain commit (a060e47a) is the right durable fix.

Verified end-to-end:

• writeFrame signature → Promise<boolean>. Both production callers in captureStreamingStage.ts (single-worker line 270, multi-worker line 202) correctly await. Test file updated to match.
• Drain wait listens on drain/finish/error with removeListener cleanup on all three — no listener accumulation.
• if (exitStatus !== "running") return false; after the drain wait handles encoder-death-mid-wait cleanly.
• Inactivity timer behavior: only resets on accepted writes. During a long drain wait the timer ticks normally → SIGTERM still fires if FFmpeg is genuinely hung past ffmpegStreamingTimeout. That's the right shape: drain unblocks fast normally, SIGTERM is the safety net for true hangs.
• 1800s multi-worker cap kept as belt-and-suspenders. With real back-pressure in place, the cap is now redundant for OOM-prevention but still a useful guard against pathological renders. Fine to keep.

One real-world spot check: ran multi-worker streaming on a real composition (hyperframes-launch, 49.77s @ 60fps = 2987 frames, --workers 2, h264). Clean exit (exit=0), 186s wall, 31.3 MB output. Streaming-encode gate engaged correctly. No SIGTERM, no buffer-growth issues.

Ship it.

Review by Jerrai

[miguel-heygen]

Review

Verdict: real fix for a real problem, needs additional test coverage before merge

What it does

Two changes bundled in one PR:

1. Back-pressure in writeFrame (the #1353 fix): Changes writeFrame from sync boolean → async Promise<boolean> and awaits drain when ffmpeg.stdin.write() returns false. This is the correct fix — currently every caller ignores the sync return value (currentEncoder.writeFrame(buffer) with no await or return check), so Node's internal buffer grows without bound when FFmpeg encodes slower than workers capture. The math in #1353 is accurate: 3 workers × ~35fps surplus × ~500KB/frame → OOM on long renders.

2. Interleaved frame distribution: Adds distributeFramesInterleaved where worker i captures frames i, i+N, i+2N, … instead of contiguous chunks. This keeps all workers in lockstep so the FrameReorderBuffer stays nearly uncontended. With contiguous chunks, worker 1 blocks at frame 0 until worker 0 finishes its entire chunk — defeating multi-worker parallelism on the streaming path. Good optimization.

Code quality

• The drain/finish/error listener triple in the back-pressure await is correct — handles all stdin termination cases without leaking listeners
• exitStatus re-check after drain await correctly handles the race where FFmpeg dies while we're waiting
• captureFrameRange stride loop is clean: for (let i = task.startFrame; i < task.endFrame; i += stride)
• All existing callers (captureStreamingStage, captureHdrHybridLoop, captureHdrSequentialLoop) are updated to await the now-async writeFrame

Test gaps

The tests cover interleaved distribution thoroughly (5 tests) and the shouldUseStreamingEncode gate changes. But the core back-pressure behavior has no dedicated test. This is the most critical change in the PR.

Missing tests that should be added:

1. Back-pressure blocks caller until drain — mock ffmpeg.stdin.write() to return false, verify writeFrame() doesn't resolve until drain is emitted, then emit drain and verify it resolves with true.

2. Back-pressure returns false on encoder death — mock write() → false, then set exitStatus = 'error' and emit drain. Verify writeFrame() resolves with false.

3. Back-pressure cleans up listeners on finish/error — verify that if finish fires before drain, the promise resolves and no drain listener leaks.

4. Heartbeat timer resets after drain — verify resetTimer() is called after the drain await, not just on accepted writes. The current code in main only resets on accepted === true, which means back-pressured writes never reset the timer → false timeout kills on slow-but-alive FFmpeg.

These can go in streamingEncoder.test.ts alongside the existing lifecycle tests.

Producer baseline tests

Not needed — the interleaved distribution delivers the same frames in the same order to FFmpeg (the reorder buffer ensures sequential ordering). The back-pressure only throttles write speed, not frame content. Visual output is identical.

Recommendation

The fix is correct and addresses a real OOM risk. Add the 4 back-pressure unit tests above before merging — this is a writeFrame signature change that touches every encode path, and the drain-await behavior is the part most likely to regress.

[Claudemeri]

Thanks @miguel-heygen — four back-pressure tests added in commit 17c7ac6.

On test #4 (timer reset): resetTimer() was already called post-drain in the previous commit — the misleading comment above it ("deliberately don't reset on false") made it look absent. This commit clarifies it with a three-case policy:

1. accepted === true → reset immediately
2. accepted === false (buffer full) → no reset (hung FFmpeg must still SIGTERM)
3. after await drain → reset — slow-but-alive FFmpeg must never trigger spurious SIGTERM

Tests added (streamingEncoder.test.ts, 39/39 passing):

1. blocks until drain — promise stays pending after microtask flush, resolves true after stdin.emit("drain")
2. returns false on encoder death during drain wait — proc.emit("close", 1) sets exitStatus="error", then drain fires → false
3. listener cleanup on finish — all 3 listeners (drain/finish/error) removed, counts back to baseline even when finish fires first
4. timer resets after drain — without post-drain resetTimer() SIGTERM fires at T=1000ms; with it, SIGTERM fires at T=1800ms (1000ms after the drain event)

Ready for re-review.