Fix video export stall when trim regions cause long decoder gaps#4
Conversation
When a recording has a large trim region (e.g. 400s–828s removed), the decoder must sequentially decode and discard all frames in that region to maintain P/B-frame state. On a 3320x2160 source this can take 40–50 seconds of wall time. During that decode pass the encoder queue drains to empty and lastEncoderOutputAt stops updating. When the next segment's frames arrive and fill the encode queue, the stall detector would compare Date.now() against the stale lastEncoderOutputAt (~50 s ago) and incorrectly throw a stall error, aborting the export. Fix: measure stall timeout from when the queue-full while-loop is entered (stallWaitStartAt), not from the last global encoder output. This gives the encoder a fresh 15 s window to produce output each time the queue fills up, regardless of how long the decoder spent on trimmed frames. Also remove VideoFrame leak-tracker debug code added during diagnosis, and switch latencyMode to "realtime" with a smaller maxEncodeQueue to reduce encoder internal buffering depth.
|
Warning Review limit reached
More reviews will be available in 44 minutes and 59 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces an exported ChangesEncoder Stall Detection Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@m8i-51 I can see the logic of the bug but I was unable to reproduce. From the code path, the fix looks reasonable to me: the stall timeout should measure time spent waiting for encoder queue space, not time spent decoding through a trimmed region. A synthetic regression test for this case would also be helpful since it seems hardware/source dependent. |
|
@EtienneLescot Specs: macOS 26.5.1, MacBook Air (M2, 8-core), 24GB RAM. Tried reproducing on this Mac too (mid-video trim and an early trim, both ~1-2 min) — couldn't trigger it either. Apple Silicon's hardware decoder is fast enough here that the discard phase never got close to the 15s timeout, so this looks hardware/decoder-speed dependent rather than something everyone will see. Matches what you found. I can add a regression test for the timer logic itself (mocking the queue-full wait) if that's useful — let me know. |
It could be useful yes. |
Extracts the queue-full wait loop into waitForEncoderQueueSpace() so the timing logic can be unit tested without real WebCodecs. Covers the original false-positive: a long gap before the call (e.g. decoder discarding frames in a trim region) must not count against the 15s timeout, since the timer starts at call time, not from the encoder's last output.
… fix/export-stall-trim-region
The "long gap before this call" case was mathematically identical to the queue-drain test once now()/sleep() are injected — shifting the fake clock's epoch doesn't change now() - stallWaitStartAt. Replaced with a comment on why the bug can't recur: the function takes no external "last output" timestamp to go stale in the first place.
|
@EtienneLescot Update: I take back my earlier "couldn't reproduce" comment — I reproduced it. It didn't show up with a synthetic test video, but it did with a real, heavily-edited project: a 14:45 screen recording at 3320x2160 with 10 trim regions, including one ~7 minute trim. The error is silently swallowed by the existing hardware→software encoder retry fallback ( GIF of the progress bar silently resetting (79% → 1%) when this happens, for reference:
So: high resolution + a large trim is what it takes — small/synthetic sources decode fast enough on Apple Silicon to never hit the 15s window. Also pushed the regression test you asked for. Extracted the queue-wait loop into |
2 participants
Description
Fixes a false-positive encoder stall that causes video export to abort when a project has trim regions covering a large portion of the source recording.
Motivation
StreamingVideoDecoderreads the source file sequentially — it cannot seek. Frames inside trimmed regions must still be decoded to maintain P/B-frame state, but are discarded. For a recording with a large trim in the middle, this discard phase can take tens of seconds of wall time.During the discard phase the encoder queue is empty and
lastEncoderOutputAtgoes stale. When real frames arrive after the trim and fill the queue, the timeout has already elapsed — a false positive that aborts a healthy export.Fix: reset the timer at the start of each queue-full wait instead of measuring from the last encoder output.
This was originally opened as siddharthvaddem#682, but that repo was archived before it could be reviewed/merged. Re-opening against this fork since the bug is still present in
videoExporter.tshere.Type of Change
Testing
The stall detection path exercises
VideoEncoderdirectly, which is a browser WebCodecs API not available in the Node test environment. I verified the fix manually with a project that previously reproduced the issue consistently.Verified against current
main(post v1.5.0):tsc --noEmit— 0 errorsvitest --run— 225/225 passingChecklist
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests