fix(cli): a snapshot clip whose frame fails is named with ffmpeg's reason (#6564)

This commit is contained in:
Miguel Ángel
2026-10-27 14:48:54 +00:00
committed by GitHub
parent 5561640809
commit 7a0c75bb69
2 changed files with 69 additions and 42 deletions
+46 -31
View File
@@ -17,6 +17,7 @@ const snapshotState = vi.hoisted(() => ({
settle: vi.fn(async (_page: unknown, _options?: unknown) => ({ pending: [], failed: [] })),
closeServer: vi.fn(async () => undefined),
ffmpegTimesOut: false,
ffmpegFails: false,
noFfmpeg: false,
}));
@@ -38,7 +39,13 @@ vi.mock("../capture/captureCompositionFrame.js", async (importOriginal) => {
runFfmpegOnce: (...args: Parameters<typeof actual.runFfmpegOnce>) =>
snapshotState.ffmpegTimesOut
? Promise.resolve({ code: null, timedOut: true, stderr: "" })
: actual.runFfmpegOnce(...args),
: snapshotState.ffmpegFails
? Promise.resolve({
code: 1,
timedOut: false,
stderr: "\nError while decoding stream #0:0\n",
})
: actual.runFfmpegOnce(...args),
};
});
@@ -126,7 +133,7 @@ describe("transparent snapshot capture", () => {
it("pairs every frame with the frame-exact reference frame under --against", () => {
const source = readFileSync(new URL("./snapshot.ts", import.meta.url), "utf8");
expect(source).toContain("against: {");
expect(source).toContain("extractVideoFrameToBuffer(opts.against, time)");
expect(source).toContain("extractVideoFrameOrWarn(warnings, opts.against, time)");
expect(source).toContain('labels: ["render", "reference"]');
});
@@ -650,22 +657,37 @@ describe("extractVideoFrameToBuffer", () => {
},
);
const oneSecondClip = (dir: string) => {
const clip = join(dir, "clip.mp4");
execFileSync(ffmpeg!, [
...["-hide_banner", "-loglevel", "error", "-f", "lavfi", "-i", "testsrc=d=1:r=24:s=160x90"],
...["-pix_fmt", "yuv420p", clip],
]);
return clip;
};
it.skipIf(noProbe)(
"records the clip, time and FFmpeg's reason when the extract fails",
async () => {
const dir = mkdtempSync(join(tmpdir(), "hf-snapshot-fails-"));
snapshotState.ffmpegFails = true;
try {
const warnings: string[] = [];
expect(await extractVideoFrameOrWarn(warnings, oneSecondClip(dir), 0.5)).toBeNull();
expect(warnings).toEqual([
expect.stringMatching(/clip\.mp4 at 0\.50s: Error while decoding stream #0:0$/),
]);
} finally {
snapshotState.ffmpegFails = false;
rmSync(dir, { recursive: true, force: true });
}
},
);
it.skipIf(noProbe)("names the clip and time when FFmpeg times out", async () => {
const dir = mkdtempSync(join(tmpdir(), "hf-snapshot-timeout-"));
try {
const clip = join(dir, "clip.mp4");
execFileSync(ffmpeg!, [
"-hide_banner",
"-loglevel",
"error",
"-f",
"lavfi",
"-i",
"testsrc=d=1:r=24:s=160x90",
"-pix_fmt",
"yuv420p",
clip,
]);
const clip = oneSecondClip(dir);
snapshotState.ffmpegTimesOut = true;
await expect(extractVideoFrameToBuffer(clip, 0.5)).rejects.toThrow(/clip\.mp4 at 0\.50s/);
} finally {
@@ -679,19 +701,7 @@ describe("extractVideoFrameToBuffer", () => {
async () => {
const dir = mkdtempSync(join(tmpdir(), "hf-snapshot-finding-"));
try {
const clip = join(dir, "clip.mp4");
execFileSync(ffmpeg!, [
"-hide_banner",
"-loglevel",
"error",
"-f",
"lavfi",
"-i",
"testsrc=d=1:r=24:s=160x90",
"-pix_fmt",
"yuv420p",
clip,
]);
const clip = oneSecondClip(dir);
snapshotState.ffmpegTimesOut = true;
const warnings: string[] = [];
expect(await extractVideoFrameOrWarn(warnings, clip, 0.5)).toBeNull();
@@ -761,14 +771,19 @@ describe("extractVideoFrameToBuffer", () => {
);
it.skipIf(noProbe)(
"leaves an unreadable video blank instead of failing the snapshot",
"leaves an unreadable video blank, naming it and why, instead of failing the snapshot",
async () => {
const dir = mkdtempSync(join(tmpdir(), "hf-snapshot-unreadable-"));
try {
const garbage = join(dir, "garbage.mp4");
writeFileSync(garbage, "not a video");
expect(await extractVideoFrameToBuffer(join(dir, "missing.mp4"), 1)).toBeNull();
expect(await extractVideoFrameToBuffer(garbage, 1)).toBeNull();
const warnings: string[] = [];
expect(await extractVideoFrameOrWarn(warnings, join(dir, "missing.mp4"), 1)).toBeNull();
expect(await extractVideoFrameOrWarn(warnings, garbage, 1)).toBeNull();
expect(warnings).toEqual([
expect.stringMatching(/FFprobe could not read .*missing\.mp4: .*No such file/),
expect.stringMatching(/FFprobe could not read .*garbage\.mp4: .*Invalid data found/),
]);
} finally {
rmSync(dir, { recursive: true, force: true });
}
+23 -11
View File
@@ -319,6 +319,14 @@ export function containingSourceFrameIndex(timestamps: readonly number[], time:
return index;
}
class FrameExtractError extends Error {}
const lastLine = (text: string) => text.trim().split("\n").at(-1) ?? "";
const processStderr = (error: unknown) =>
error instanceof Error && "stderr" in error && typeof error.stderr === "string"
? error.stderr
: "";
/** Keyframe seek plus the decoded-time threshold that selects the frame containing `time`, or null. */
export async function probeContainingSourceFrameSeek(
videoPath: string,
@@ -346,9 +354,10 @@ export async function probeContainingSourceFrameSeek(
return typeof parsed === "object" && parsed !== null
? (parsed as Record<string, unknown>)
: null;
} catch {
// An unreadable source blanks this one video, as a failed ffmpeg extraction does.
return null;
} catch (error) {
throw new FrameExtractError(
`FFprobe could not read ${videoPath}: ${lastLine(processStderr(error) || String(error))}`,
);
}
};
const data = await probe("%+#1", "stream=time_base,start_pts:format=start_time,duration");
@@ -473,8 +482,6 @@ function warnNoHdrFiltersOnce(): void {
);
}
class FrameExtractTimeoutError extends Error {}
export async function extractVideoFrameToBuffer(
videoPath: string,
timeSeconds: number,
@@ -536,12 +543,17 @@ export async function extractVideoFrameToBuffer(
warnNoHdrFiltersOnce();
result = await runFfmpegOnce(ffmpegPath, frameArgs(null), FFMPEG_EXTRACT_TIMEOUT_MS);
}
const frameOf = `the frame of ${videoPath} at ${timeSeconds.toFixed(2)}s`;
if (result.timedOut) {
throw new FrameExtractTimeoutError(
`FFmpeg timed out after ${FFMPEG_EXTRACT_TIMEOUT_MS / 1000}s extracting the frame of ${videoPath} at ${timeSeconds.toFixed(2)}s`,
throw new FrameExtractError(
`FFmpeg timed out after ${FFMPEG_EXTRACT_TIMEOUT_MS / 1000}s extracting ${frameOf}`,
);
}
if (result.code !== 0 || !existsSync(outPath)) {
throw new FrameExtractError(
`FFmpeg could not extract ${frameOf}: ${lastLine(result.stderr) || `exit code ${result.code}`}`,
);
}
if (result.code !== 0 || !existsSync(outPath)) return null;
return readFileSync(outPath);
} finally {
try {
@@ -552,7 +564,7 @@ export async function extractVideoFrameToBuffer(
}
}
/** A timed-out clip becomes a finding naming it, so the other frames of the snapshot still land. */
/** A clip whose frame fails becomes a finding naming it and why, so the other frames of the snapshot still land. */
export async function extractVideoFrameOrWarn(
warnings: string[],
...args: Parameters<typeof extractVideoFrameToBuffer>
@@ -560,7 +572,7 @@ export async function extractVideoFrameOrWarn(
try {
return await extractVideoFrameToBuffer(...args);
} catch (error) {
if (!(error instanceof FrameExtractTimeoutError)) throw error;
if (!(error instanceof FrameExtractError)) throw error;
warnings.push(error.message);
console.warn(` ${c.warn("⚠")} ${error.message}`);
return null;
@@ -1133,7 +1145,7 @@ async function captureSnapshots(
if (opts.against) {
// Frame-exact reference frame beside the render, so a rebuild can be
// checked against its footage without hand-rolled ffmpeg + montage.
const refPng = await extractVideoFrameToBuffer(opts.against, time);
const refPng = await extractVideoFrameOrWarn(warnings, opts.against, time);
if (!refPng) {
console.error(
` ${c.warn("⚠")} --against has no frame at ${timeLabel} — reference pair skipped`,