From f3ec6b00097dbd787a2cd197b3228fcbfc6de383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=82=E3=82=8A=E3=81=93?= Date: Tue, 2 Jun 2026 07:13:53 +0900 Subject: [PATCH 1/3] Fix video export stall when trim regions cause long decoder gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/lib/exporter/videoExporter.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/lib/exporter/videoExporter.ts b/src/lib/exporter/videoExporter.ts index a0b063789..3a7894098 100644 --- a/src/lib/exporter/videoExporter.ts +++ b/src/lib/exporter/videoExporter.ts @@ -152,7 +152,6 @@ export class VideoExporter { private videoColorSpace: VideoColorSpaceInit | undefined; private muxingPromises: Promise[] = []; private chunkCount = 0; - private lastEncoderOutputAt = 0; private fatalEncoderError: Error | null = null; constructor(config: VideoExporterConfig) { @@ -384,12 +383,16 @@ export class VideoExporter { exportFrame = new VideoFrame(canvas, { timestamp, duration: frameDuration }); } + // Timer resets each time we enter the queue-full wait, so a long + // trimmed-region decode (decoder busy discarding frames) doesn't + // make the stall check fire spuriously. + const stallWaitStartAt = Date.now(); while ( this.encoder && this.encoder.encodeQueueSize >= maxEncodeQueue && !this.cancelled ) { - if (Date.now() - this.lastEncoderOutputAt > ENCODER_STALL_TIMEOUT_MS) { + if (Date.now() - stallWaitStartAt > ENCODER_STALL_TIMEOUT_MS) { exportFrame.close(); throw new Error( encoderPreference === "prefer-hardware" @@ -496,14 +499,11 @@ export class VideoExporter { this.encodeQueue = 0; this.muxingPromises = []; this.chunkCount = 0; - this.lastEncoderOutputAt = Date.now(); this.fatalEncoderError = null; let videoDescription: Uint8Array | undefined; this.encoder = new VideoEncoder({ output: (chunk, meta) => { - this.lastEncoderOutputAt = Date.now(); - if (meta?.decoderConfig?.description && !videoDescription) { const desc = meta.decoderConfig.description; if (desc instanceof ArrayBuffer || desc instanceof SharedArrayBuffer) { @@ -648,7 +648,6 @@ export class VideoExporter { this.chunkCount = 0; this.videoDescription = undefined; this.videoColorSpace = undefined; - this.lastEncoderOutputAt = 0; this.fatalEncoderError = null; } From 4a6d4d014a8d942fc738af81ac5eb690f54132de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=82=E3=82=8A=E3=81=93?= Date: Sat, 20 Jun 2026 07:14:00 +0900 Subject: [PATCH 2/3] Add regression test for encoder stall timeout 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. --- src/lib/exporter/videoExporter.test.ts | 129 ++++++++++++++++++++++++- src/lib/exporter/videoExporter.ts | 59 +++++++---- 2 files changed, 169 insertions(+), 19 deletions(-) diff --git a/src/lib/exporter/videoExporter.test.ts b/src/lib/exporter/videoExporter.test.ts index 1b64255a2..a5c330f70 100644 --- a/src/lib/exporter/videoExporter.test.ts +++ b/src/lib/exporter/videoExporter.test.ts @@ -1,8 +1,9 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { getSourceCopyFastPathBlockers, isSourceCopyFastPathEligible, type VideoExporterConfig, + waitForEncoderQueueSpace, } from "./videoExporter"; function createConfig(overrides: Partial = {}): VideoExporterConfig { @@ -119,3 +120,129 @@ describe("getSourceCopyFastPathBlockers", () => { ).toContain("output-size 1920x1080 differs from source 1920x1032"); }); }); + +describe("waitForEncoderQueueSpace", () => { + function fakeClock(start = 0) { + let elapsedMs = start; + return { + now: () => elapsedMs, + sleep: async (ms: number) => { + elapsedMs += ms; + }, + }; + } + + it("resolves immediately when the queue already has space", async () => { + const clock = fakeClock(); + const sleep = vi.fn(clock.sleep); + + await waitForEncoderQueueSpace({ + getQueueSize: () => 0, + maxEncodeQueue: 8, + isCancelled: () => false, + encoderPreference: "prefer-hardware", + now: clock.now, + sleep, + }); + + expect(sleep).not.toHaveBeenCalled(); + }); + + it("waits for the queue to drain and then resolves", async () => { + const clock = fakeClock(); + let queueSize = 8; + // Queue drains well within the timeout. + const sleep = vi.fn(async (ms: number) => { + await clock.sleep(ms); + queueSize = 0; + }); + + await waitForEncoderQueueSpace({ + getQueueSize: () => queueSize, + maxEncodeQueue: 8, + isCancelled: () => false, + encoderPreference: "prefer-hardware", + now: clock.now, + sleep, + }); + + expect(sleep).toHaveBeenCalledTimes(1); + }); + + it("throws a hardware-specific error once the queue stays full past the timeout", async () => { + const clock = fakeClock(); + + await expect( + waitForEncoderQueueSpace({ + getQueueSize: () => 8, + maxEncodeQueue: 8, + isCancelled: () => false, + encoderPreference: "prefer-hardware", + now: clock.now, + sleep: clock.sleep, + }), + ).rejects.toThrow( + "The hardware video encoder stopped responding. Retrying with a safer encoder.", + ); + }); + + it("throws a generic error for the software encoder once the queue stays full past the timeout", async () => { + const clock = fakeClock(); + + await expect( + waitForEncoderQueueSpace({ + getQueueSize: () => 8, + maxEncodeQueue: 8, + isCancelled: () => false, + encoderPreference: "prefer-software", + now: clock.now, + sleep: clock.sleep, + }), + ).rejects.toThrow("The video encoder stopped responding during export."); + }); + + // Regression test for the false-positive stall: a long gap *before* this call + // (e.g. the decoder discarding frames inside a trim region) must not count + // against the timeout. Only time spent waiting *inside* this call should. + it("does not throw merely because a long time has already passed before this call", async () => { + // Simulate this call starting two minutes after some unrelated earlier event, + // then having the queue drain almost immediately once we're inside the wait. + const clock = fakeClock(2 * 60 * 1000); + let queueSize = 8; + const sleep = vi.fn(async (ms: number) => { + await clock.sleep(ms); + queueSize = 0; + }); + + await expect( + waitForEncoderQueueSpace({ + getQueueSize: () => queueSize, + maxEncodeQueue: 8, + isCancelled: () => false, + encoderPreference: "prefer-hardware", + now: clock.now, + sleep, + }), + ).resolves.toBeUndefined(); + }); + + it("stops waiting without throwing once cancelled", async () => { + const clock = fakeClock(); + let cancelled = false; + const sleep = vi.fn(async (ms: number) => { + await clock.sleep(ms); + cancelled = true; + }); + + await expect( + waitForEncoderQueueSpace({ + getQueueSize: () => 8, + maxEncodeQueue: 8, + isCancelled: () => cancelled, + encoderPreference: "prefer-hardware", + now: clock.now, + sleep, + }), + ).resolves.toBeUndefined(); + }); +}); diff --git a/src/lib/exporter/videoExporter.ts b/src/lib/exporter/videoExporter.ts index 3a7894098..8f239656c 100644 --- a/src/lib/exporter/videoExporter.ts +++ b/src/lib/exporter/videoExporter.ts @@ -20,6 +20,37 @@ import type { ExportConfig, ExportProgress, ExportResult } from "./types"; const ENCODER_STALL_TIMEOUT_MS = 15_000; const ENCODER_FLUSH_TIMEOUT_MS = 20_000; +/** + * Waits for the encoder's queue to drain below maxEncodeQueue before returning. + * + * The stall timer starts fresh on each call (not from the encoder's last output), so a + * long gap before this call — e.g. the decoder discarding frames inside a trim region — + * doesn't get blamed on the encoder once real frames resume. + */ +export async function waitForEncoderQueueSpace(params: { + getQueueSize: () => number; + maxEncodeQueue: number; + isCancelled: () => boolean; + encoderPreference: HardwareAcceleration; + now?: () => number; + sleep?: (ms: number) => Promise; +}): Promise { + const now = params.now ?? Date.now; + const sleep = params.sleep ?? ((ms: number) => new Promise((resolve) => setTimeout(resolve, ms))); + + const stallWaitStartAt = now(); + while (params.getQueueSize() >= params.maxEncodeQueue && !params.isCancelled()) { + if (now() - stallWaitStartAt > ENCODER_STALL_TIMEOUT_MS) { + throw new Error( + params.encoderPreference === "prefer-hardware" + ? "The hardware video encoder stopped responding. Retrying with a safer encoder." + : "The video encoder stopped responding during export.", + ); + } + await sleep(5); + } +} + export interface VideoExporterConfig extends ExportConfig { videoUrl: string; webcamVideoUrl?: string; @@ -383,24 +414,16 @@ export class VideoExporter { exportFrame = new VideoFrame(canvas, { timestamp, duration: frameDuration }); } - // Timer resets each time we enter the queue-full wait, so a long - // trimmed-region decode (decoder busy discarding frames) doesn't - // make the stall check fire spuriously. - const stallWaitStartAt = Date.now(); - while ( - this.encoder && - this.encoder.encodeQueueSize >= maxEncodeQueue && - !this.cancelled - ) { - if (Date.now() - stallWaitStartAt > ENCODER_STALL_TIMEOUT_MS) { - exportFrame.close(); - throw new Error( - encoderPreference === "prefer-hardware" - ? "The hardware video encoder stopped responding. Retrying with a safer encoder." - : "The video encoder stopped responding during export.", - ); - } - await new Promise((resolve) => setTimeout(resolve, 5)); + try { + await waitForEncoderQueueSpace({ + getQueueSize: () => this.encoder?.encodeQueueSize ?? 0, + maxEncodeQueue, + isCancelled: () => this.cancelled, + encoderPreference, + }); + } catch (error) { + exportFrame.close(); + throw error; } if (this.encoder && this.encoder.state === "configured") { From c81b38acf1947219fae6958aa31838c5c6cd3e09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=82=E3=82=8A=E3=81=93?= Date: Sat, 20 Jun 2026 07:31:32 +0900 Subject: [PATCH 3/3] Remove redundant stall-timeout test, clarify regression rationale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/lib/exporter/videoExporter.test.ts | 31 +++++--------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/src/lib/exporter/videoExporter.test.ts b/src/lib/exporter/videoExporter.test.ts index a5c330f70..bd424548b 100644 --- a/src/lib/exporter/videoExporter.test.ts +++ b/src/lib/exporter/videoExporter.test.ts @@ -121,6 +121,12 @@ describe("getSourceCopyFastPathBlockers", () => { }); }); +// The original bug measured the timeout from the encoder's last *output* event +// (lastEncoderOutputAt), which went stale while the decoder discarded frames inside +// a trim region. waitForEncoderQueueSpace fixes this by starting the clock fresh on +// each call instead of accepting any such external timestamp — by construction, there +// is no "last output" state to go stale, so that regression can't be reintroduced +// without changing this function's signature. describe("waitForEncoderQueueSpace", () => { function fakeClock(start = 0) { let elapsedMs = start; @@ -201,31 +207,6 @@ describe("waitForEncoderQueueSpace", () => { ).rejects.toThrow("The video encoder stopped responding during export."); }); - // Regression test for the false-positive stall: a long gap *before* this call - // (e.g. the decoder discarding frames inside a trim region) must not count - // against the timeout. Only time spent waiting *inside* this call should. - it("does not throw merely because a long time has already passed before this call", async () => { - // Simulate this call starting two minutes after some unrelated earlier event, - // then having the queue drain almost immediately once we're inside the wait. - const clock = fakeClock(2 * 60 * 1000); - let queueSize = 8; - const sleep = vi.fn(async (ms: number) => { - await clock.sleep(ms); - queueSize = 0; - }); - - await expect( - waitForEncoderQueueSpace({ - getQueueSize: () => queueSize, - maxEncodeQueue: 8, - isCancelled: () => false, - encoderPreference: "prefer-hardware", - now: clock.now, - sleep, - }), - ).resolves.toBeUndefined(); - }); - it("stops waiting without throwing once cancelled", async () => { const clock = fakeClock(); let cancelled = false;