Repository navigation
perf(producer): chunks capture frames round-robin so the encoder runs during capture - #6364
Conversation
… during capture The overlapped encoder takes frames in order, but workers captured contiguous ranges, so it sat idle until worker 0 finished and then encoded the other workers' frames after capture. With the encoder on, workers now take frames round-robin and it keeps pace.
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits — head 18ecd937
Interleaved capture keeps output correct for seek-pure compositions. Ordering, frameRangeStart, stride-aware expected counts, merge and encoder backpressure all check out. Two other paths assume each worker captures a contiguous range.
Blocker: after interleaved capture, the missing-frame retry splits into one-frame batches. On the disk path, transient_browser and protocol_timeout don't abort the other workers (isFatalCaptureFailure, captureFailure.ts:212). So when one worker dies, the frames still missing are its stride w, w+N, w+2N, …. findMissingFrameRanges (renderOrchestrator.ts:937) turns each of those into its own range. buildMissingFrameRetryBatches (:968) then gives each range one worker and runs the batches one after another. Example: 1800 frames, 4 workers, worker 2 dies (Target closed) after 10 frames. Before, that left 1 range and 1 batch. Now it leaves ~440 ranges in ~110 sequential executeParallelCapture batches, each opening fresh sessions to capture one frame. A recovery that took seconds now takes minutes, and on a chunk it can run into the timeout. Fix: when the first attempt was interleaved, group the missing frames into one task per worker, e.g. reuse the dead worker's stride or pack the missing frames into contiguous runs.
Important: interleaving turns off static-frame dedup in screenshot mode. captureFrameCore reuses a frame only when lastFrameAbsoluteIndex === absFrameIndex - 1 (frameCapture.ts:4655-4657). That is never true at stride > 1. Every predicted-static frame gets re-seeked and re-screenshotted, while armStaticDedup still pays for its verification seeks. A title-card or slideshow composition could lose more capture time than the overlap saves, and the evidence doesn't include a static-heavy composition. Fix: use the same gap check as the worker path (:4829-4833: reuse when every frame in (last, i] is static), or keep contiguous ranges when dedup is armed.
Scope
I read all 4 changed files plus their consumers: parallelCoordinator distribution/merge/capture loop, static dedup, video injector, BeginFrame ticks, motion blur, drawElement verify (unreachable under overlap), and when the overlap flag is evaluated. The second commit only removes a comment, to fix the Comments ratchet that failed on 34ce93f9. CI at 18ecd937 was still running when I posted.
Edit accuracy: accurate 271 (base branch 271), smooth 140 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (2)
|
…e task and keeps static-frame reuse A worker that dies during round-robin capture leaves every Nth frame missing; the missing-frame retry turned each into its own one-frame batch. It now retries one stride task per dead worker. Screenshot-mode static reuse also required the previous frame to be exactly one back, which never holds at stride above 1; it now reuses when every frame in the gap is static, like the worker path.
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 1 nit — 18ecd937 → 4a722a6e: 2 resolved, 0 open
- ✅ Retry:
buildMissingFrameRetryBatchesnow groups the missing frames into one stride task per residue class, so there are at mostinitialWorkerCounttasks instead of one batch per frame. When I put the old stride of 1 back at the call site, the new retry test fails. - ✅ Dedup:
captureFrameCorereuses a frame across the stride gap through the sharedframesAreStaticcheck. When I put the old=== abs - 1back, the new stride-gap test fails. - Nit:
framesAreStatic(set, last + 1, abs)returns true for an empty range, so if a session ever captured a frame at or below its last one, it would reuse the later buffer. I found no current caller that does this, but an explicitlast < absguard would keep the check as strict as the old one.
All CI on this head is green, including required Test and Windows. Locally: producer renderOrchestrator.test.ts 289/289, engine frameCapture-staticDedupIndex.test.ts 8/8.
Problem
The encoder that runs during capture consumes frames in order (0, 1, 2, ...), but capture workers took contiguous ranges. It could only use worker 0's frames while capture ran, then encoded everything else after capture ended. On a distributed chunk that tail was about a quarter of the chunk's billed time.
Change
When the encoder overlaps capture (distributed chunks and in-process disk capture), workers take frames round-robin, using the existing
distributeFramesInterleavedand stride merge.Evidence