Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions electron/frames/extractor.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import assert from "node:assert/strict";
import { execFile } from "node:child_process";
import { existsSync } from "node:fs";
import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
import { createRequire } from "node:module";
import { tmpdir } from "node:os";
import path from "node:path";
import test from "node:test";
import { promisify } from "node:util";
import sharp from "sharp";

import type { CapturedVideoFrame } from "../../common/frames";
Expand Down Expand Up @@ -171,6 +173,24 @@ test("FrameExtractor deduplicates identical source JPEGs without deleting retain
assert.equal(existsSync(path.join(framesDir, reloaded.manifest[0].file)), true);
});

test("a hung legacy ffmpeg process is killed and rejects within its timeout instead of hanging forever", async () => {
// extractLegacyWindow/extractLegacySingle bound their execFileAsync calls with
// LEGACY_FFMPEG_TIMEOUT_MS so a stalled ffmpeg (corrupt input, codec edge case,
// stuck pipe) can't hang the extraction indefinitely. Those methods are private
// and resolve their ffmpeg binary through a module-level, cached `which`/`where`
// lookup that isn't test-injectable without expanding this fix's scope, so this
// exercises the same execFileAsync-with-timeout mechanism directly: a child
// process that never exits on its own must still be killed and the call must
// still reject well within the timeout, not hang.
const execFileAsync = promisify(execFile);
const start = Date.now();
await assert.rejects(
execFileAsync(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { timeout: 200 }),
);
const elapsedMs = Date.now() - start;
assert.ok(elapsedMs < 5000, `expected the timeout to bound the hang, took ${elapsedMs}ms`);
});

test("sharp loads through the app's require path as a callable factory (guards sharp 0.35 export shape)", async () => {
// The frame extractor loads sharp with createRequire(import.meta.url)("sharp"), which
// resolves sharp's CommonJS "require" export condition. sharp 0.35 split its import and
Expand Down
9 changes: 7 additions & 2 deletions electron/frames/extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ export interface CapturedFrameSample<T extends CapturedVideoFrame = CapturedVide
const DEFAULTS = { dedupeThreshold: 8, maxFrames: 300, frameGridSec: 0.5 };
const DEFAULT_WINDOW_FPS = 1;
const DEFAULT_WINDOW_MAX_FRAMES = 24;
/** Bounds how long a legacy system-ffmpeg extraction can run before it's treated as a failure. */
const LEGACY_FFMPEG_TIMEOUT_MS = 60_000;

export function nearestCapturedFrame<T extends CapturedVideoFrame>(
frames: readonly T[],
Expand Down Expand Up @@ -355,7 +357,10 @@ export class FrameExtractor {
"-q:v", "3",
pattern,
],
{ maxBuffer: 32 * 1024 * 1024 },
// A stalled ffmpeg (corrupt input, codec edge case, stuck pipe) would
// otherwise hang this promise forever; the timeout bounds that and
// routes into the same failure handling as any other ffmpeg error.
{ maxBuffer: 32 * 1024 * 1024, timeout: LEGACY_FFMPEG_TIMEOUT_MS },
);
} catch (err) {
log.warn("legacy probe window failed:", message(err));
Expand Down Expand Up @@ -402,7 +407,7 @@ export class FrameExtractor {
"-q:v", "3",
"-y", file,
],
{ maxBuffer: 16 * 1024 * 1024 },
{ maxBuffer: 16 * 1024 * 1024, timeout: LEGACY_FFMPEG_TIMEOUT_MS },
);
} catch (err) {
log.warn(`legacy frame at ${offsetSec.toFixed(2)}s failed:`, message(err));
Expand Down