From 98a4cd70fd2df38f300a2e19c86f75cd8dd93895 Mon Sep 17 00:00:00 2001 From: James Russo Date: Sun, 26 Jul 2026 18:14:53 -0700 Subject: [PATCH] perf(producer): compute regression PSNR in one ffmpeg pass (#2813) * perf(producer): compute regression PSNR in one ffmpeg pass * fix(producer): fail loudly when one PSNR input runs out of frames --- .../producer/scripts/test-classification.mjs | 1 + .../src/regression-harness-psnr.test.ts | 272 ++++++++++++++++++ packages/producer/src/regression-harness.ts | 166 ++++++++--- 3 files changed, 406 insertions(+), 33 deletions(-) create mode 100644 packages/producer/src/regression-harness-psnr.test.ts diff --git a/packages/producer/scripts/test-classification.mjs b/packages/producer/scripts/test-classification.mjs index ec07e0330..25d4d6610 100644 --- a/packages/producer/scripts/test-classification.mjs +++ b/packages/producer/scripts/test-classification.mjs @@ -9,6 +9,7 @@ export const PRODUCER_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ". // or local sockets. Keep the list explicit so a filename-only rename does not // make Git/fallow re-audit thousands of unchanged test lines as new code. const INTEGRATION_TEST_FILES = new Set([ + "src/regression-harness-psnr.test.ts", "src/services/coreRuntimeBrowser.test.ts", "src/services/deterministicFonts-systemCapture.test.ts", "src/services/distributed/assemble.test.ts", diff --git a/packages/producer/src/regression-harness-psnr.test.ts b/packages/producer/src/regression-harness-psnr.test.ts new file mode 100644 index 000000000..4b9572bbe --- /dev/null +++ b/packages/producer/src/regression-harness-psnr.test.ts @@ -0,0 +1,272 @@ +// Equivalence tests for the regression harness's PSNR comparison. +// +// `psnrPerFrame()` replaced a loop that spawned one ffmpeg per checkpoint with +// `select='eq(n,N)'`. That filter has no index, so every spawn re-decoded from +// frame 0 and the comparison phase grew quadratically with checkpoint count. +// These tests pin the replacement to the behaviour of the method it replaced: +// the same frame must yield the same PSNR, within the two-decimal precision +// that ffmpeg's `stats_file` reports. +// +// Lives in the integration lane (see scripts/test-classification.mjs) because +// it shells out to a real ffmpeg. + +import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { frameIndexForCheckpoint, psnrAtCheckpoint, psnrAtFrames } from "./regression-harness.js"; + +const FPS = 30; +const TOTAL_FRAMES = 60; +const SAMPLE_FRAMES = [0, 1, 15, 30, 45, 59]; + +let workDir: string; +let referenceVideo: string; +let degradedVideo: string; +/** Same pixels as `degradedVideo`, but a timeline framesync cannot align. */ +let rebasedVideo: string; +/** Half the frames of `referenceVideo`, for one-sided end-of-stream cases. */ +let halfLengthVideo: string; + +/** + * The pre-existing per-checkpoint implementation, kept here as the oracle. + * Any divergence between this and `psnrPerFrame()` is a behaviour change in + * the regression suite's pass/fail decisions. + */ +function legacyPsnrAtFrame(videoA: string, videoB: string, frameIndex: number): number { + const filter = + `[0:v]select='eq(n\\,${frameIndex})',setpts=PTS-STARTPTS[rv];` + + `[1:v]select='eq(n\\,${frameIndex})',setpts=PTS-STARTPTS[gv];[rv][gv]psnr`; + const result = spawnSync( + "ffmpeg", + [ + "-hide_banner", + "-loglevel", + "info", + "-i", + videoA, + "-i", + videoB, + "-filter_complex", + filter, + "-frames:v", + "1", + "-f", + "null", + "-", + ], + { encoding: "utf-8" }, + ); + const match = result.stderr.match(/average:\s*(\S+)/i); + if (!match) throw new Error(`legacy PSNR parse failed at frame ${frameIndex}`); + const raw = (match[1] ?? "").trim().toLowerCase(); + return raw === "inf" || raw === "infinite" ? Number.POSITIVE_INFINITY : Number(raw); +} + +function ffmpeg(args: string[]): void { + const result = spawnSync("ffmpeg", args, { encoding: "utf-8" }); + if (result.status !== 0) throw new Error(`ffmpeg fixture setup failed: ${result.stderr}`); +} + +beforeAll(() => { + workDir = mkdtempSync(join(tmpdir(), "hf-psnr-test-")); + referenceVideo = join(workDir, "reference.mp4"); + degradedVideo = join(workDir, "degraded.mp4"); + + // testsrc2 is deterministic, so these fixtures are reproducible across hosts. + ffmpeg([ + "-hide_banner", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + `testsrc2=size=320x240:rate=${FPS}:duration=${TOTAL_FRAMES / FPS}`, + "-c:v", + "libx264", + "-crf", + "18", + "-pix_fmt", + "yuv420p", + "-y", + referenceVideo, + ]); + // Re-encode hard enough to land well inside the PSNR range fixtures use. + ffmpeg([ + "-hide_banner", + "-loglevel", + "error", + "-i", + referenceVideo, + "-c:v", + "libx264", + "-crf", + "40", + "-pix_fmt", + "yuv420p", + "-y", + degradedVideo, + ]); + + // Identical frames to degradedVideo, re-timed to a different rate. Rendered + // output does not carry its baseline's timestamps, and comparing the two + // streams directly lets framesync pair frames by PTS instead of by index — + // the bug that made style-3-prod fail 10 checkpoints. Frame N here must + // still compare against frame N of the reference. + rebasedVideo = join(workDir, "rebased.mp4"); + ffmpeg([ + "-hide_banner", + "-loglevel", + "error", + "-i", + degradedVideo, + "-vf", + "setpts=N/25/TB", + "-r", + "25", + "-c:v", + "libx264", + "-crf", + "40", + "-pix_fmt", + "yuv420p", + "-y", + rebasedVideo, + ]); + + // Exactly half the frames, same rate. Used for one-sided EOF: requesting an + // index this video does not have must fail rather than silently pair against + // its repeated last frame. + halfLengthVideo = join(workDir, "half.mp4"); + ffmpeg([ + "-hide_banner", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + `testsrc2=size=320x240:rate=${FPS}:duration=${TOTAL_FRAMES / 2 / FPS}`, + "-c:v", + "libx264", + "-crf", + "18", + "-pix_fmt", + "yuv420p", + "-y", + halfLengthVideo, + ]); +}); + +afterAll(() => { + rmSync(workDir, { recursive: true, force: true }); +}); + +describe("psnrAtFrames()", () => { + it("returns exactly the frames asked for", () => { + const byFrame = psnrAtFrames(referenceVideo, degradedVideo, SAMPLE_FRAMES); + expect([...byFrame.keys()].sort((a, b) => a - b)).toEqual(SAMPLE_FRAMES); + }); + + it("matches the per-checkpoint ffmpeg method it replaced", () => { + const byFrame = psnrAtFrames(referenceVideo, degradedVideo, SAMPLE_FRAMES); + // Sample across the whole range: the old method's cost grew with frame + // index, so late frames are exactly where a regression would hide. + for (const frameIndex of SAMPLE_FRAMES) { + const actual = byFrame.get(frameIndex); + const expected = legacyPsnrAtFrame(referenceVideo, degradedVideo, frameIndex); + expect(actual).toBeDefined(); + // stats_file reports two decimals; the legacy stderr parse was full float. + expect(Math.abs((actual as number) - expected)).toBeLessThanOrEqual(0.01); + } + }); + + it("pairs by frame index even when the two videos carry different timelines", () => { + // The regression this guards: a rendered output whose PTS cadence differs + // from the baseline's must still be compared frame-for-frame. Letting + // framesync align by timestamp silently compares unrelated frames. + const byFrame = psnrAtFrames(referenceVideo, rebasedVideo, SAMPLE_FRAMES); + expect([...byFrame.keys()].sort((a, b) => a - b)).toEqual(SAMPLE_FRAMES); + for (const frameIndex of SAMPLE_FRAMES) { + const expected = legacyPsnrAtFrame(referenceVideo, rebasedVideo, frameIndex); + expect(Math.abs((byFrame.get(frameIndex) as number) - expected)).toBeLessThanOrEqual(0.01); + } + }); + + it("deduplicates repeated frame indices without losing alignment", () => { + // Fixtures shorter than 100 frames map several checkpoints onto one frame. + // `select` emits that frame once, so the row-to-frame mapping has to + // account for it or every later value shifts. + const withRepeats = [0, 0, 15, 15, 15, 59]; + const byFrame = psnrAtFrames(referenceVideo, degradedVideo, withRepeats); + expect([...byFrame.keys()].sort((a, b) => a - b)).toEqual([0, 15, 59]); + for (const frameIndex of [0, 15, 59]) { + const expected = legacyPsnrAtFrame(referenceVideo, degradedVideo, frameIndex); + expect(Math.abs((byFrame.get(frameIndex) as number) - expected)).toBeLessThanOrEqual(0.01); + } + }); + + it("is insensitive to the order frame indices are supplied in", () => { + const ascending = psnrAtFrames(referenceVideo, degradedVideo, [1, 30, 59]); + const shuffled = psnrAtFrames(referenceVideo, degradedVideo, [59, 1, 30]); + expect([...shuffled.entries()].sort()).toEqual([...ascending.entries()].sort()); + }); + + it("reports identical video as infinite PSNR, as the legacy method did", () => { + const byFrame = psnrAtFrames(referenceVideo, referenceVideo, SAMPLE_FRAMES); + expect([...byFrame.values()].every((value) => value === Number.POSITIVE_INFINITY)).toBe(true); + expect(legacyPsnrAtFrame(referenceVideo, referenceVideo, 10)).toBe(Number.POSITIVE_INFINITY); + }); + + it("throws rather than mis-pairing when a frame is past the end of both videos", () => { + expect(() => psnrAtFrames(referenceVideo, degradedVideo, [0, TOTAL_FRAMES + 50])).toThrow( + /differ in frame count|Expected PSNR/, + ); + }); + + it("throws when only one input runs out of frames", () => { + // The asymmetric case, and the one the test above cannot reach: there both + // inputs end together, so framesync ends too. With only one side short, + // framesync's repeatlast default holds that side's last selected frame and + // pads the pairing to full length — ffmpeg then writes one row per + // requested frame and the count guard sees nothing wrong, while the tail + // rows silently compare against a stale frame. + expect(() => psnrAtFrames(referenceVideo, halfLengthVideo, [0, 15, 45])).toThrow( + /differ in frame count|Expected PSNR/, + ); + }); + + it("still compares every requested frame when one input is merely shorter than the last index", () => { + // Guard against over-correcting: truncation must fail, but a shorter input + // that still covers every requested index has to keep working. + const byFrame = psnrAtFrames(referenceVideo, halfLengthVideo, [0, 10, 29]); + expect([...byFrame.keys()].sort((a, b) => a - b)).toEqual([0, 10, 29]); + }); + + it("returns an empty map when asked for nothing", () => { + expect(psnrAtFrames(referenceVideo, degradedVideo, []).size).toBe(0); + }); +}); + +describe("psnrAtCheckpoint()", () => { + it("maps a checkpoint time to the same frame index the legacy code used", () => { + const times = [0, 0.5, 1, 1.9]; + const byFrame = psnrAtFrames( + referenceVideo, + degradedVideo, + times.map((time) => frameIndexForCheckpoint(time, FPS)), + ); + for (const time of times) { + // Math.round(time * fps) is the mapping the per-checkpoint code used. + expect(frameIndexForCheckpoint(time, FPS)).toBe(Math.round(time * FPS)); + expect(psnrAtCheckpoint(byFrame, time, FPS)).toBe( + byFrame.get(Math.round(time * FPS)) as number, + ); + } + }); + + it("throws when the requested frame was not among those compared", () => { + const byFrame = psnrAtFrames(referenceVideo, degradedVideo, [0, 15]); + expect(() => psnrAtCheckpoint(byFrame, 1.5, FPS)).toThrow(/Unable to parse PSNR output/); + }); +}); diff --git a/packages/producer/src/regression-harness.ts b/packages/producer/src/regression-harness.ts index 8bc56cc17..460985c3e 100644 --- a/packages/producer/src/regression-harness.ts +++ b/packages/producer/src/regression-harness.ts @@ -6,6 +6,7 @@ import { readFileSync, writeFileSync, copyFileSync, + mkdtempSync, rmSync, statSync, cpSync, @@ -576,42 +577,135 @@ function extractFrameAsImage( ); } -function psnrAtCheckpoint( +/** The frame a checkpoint time samples. Shared so selection and lookup agree. */ +export function frameIndexForCheckpoint(checkpointSec: number, fps: number): number { + return Math.max(0, Math.round(checkpointSec * fps)); +} + +/** + * PSNR for a set of frame indices, in a single ffmpeg pass. + * + * The original implementation spawned one ffmpeg per checkpoint, each + * selecting its frame with `select='eq(n,N)'`. That filter has no index, so + * ffmpeg decoded from frame 0 every time — checkpoint 99 decoded 99% of both + * videos to read a single frame. Across 100 checkpoints that is roughly 50 + * full decodes of each video, and it dominated regression runtime (27% of + * total suite work; 60-80% on short fixtures). + * + * This keeps the original `select` semantics exactly and only collapses the + * spawns: both inputs are filtered to the same frame indices, chosen **by + * decode index, independently per input**, then compared pairwise. + * + * Selecting by index is load-bearing, not incidental. Handing the streams to + * `psnr` directly (`[0:v][1:v]psnr`) instead makes ffmpeg's framesync align + * them by presentation timestamp, and rendered output does not carry the same + * PTS as its golden baseline. That pairs frames which do not correspond: on + * style-3-prod it moved 80 of 100 checkpoints by more than 2 dB and turned + * three exactly-identical frames into 82/38/51 dB. + * + * `settb=1/1,setpts=N` after each `select` renumbers both selected streams to + * the same synthetic one-tick-per-frame timeline, so framesync pairs the Nth + * selected frame of one input with the Nth of the other. The timebase is + * pinned rather than derived (`setpts=N/FRAME_RATE/TB` is not enough) because + * `FRAME_RATE` is per-input: if the two videos report different rates, that + * form hands framesync two different timelines again and it silently emits a + * different number of rows than frames requested. + * + * Returns a 0-based frame index -> PSNR map. `stats_file` reports `psnr_avg` + * to two decimals where the old stderr parse had full float precision; + * thresholds are integers and fixtures pass with dB of margin, so the 0.005 dB + * rounding is not material. + */ +export function psnrAtFrames( renderedVideo: string, snapshotVideo: string, + frameIndices: number[], +): Map { + // Several checkpoints land on one frame when a fixture has fewer frames than + // checkpoints, and `select` emits such a frame once. Deduplicate so the + // filter output length is predictable and position-addressable. + const wanted = [...new Set(frameIndices)].sort((left, right) => left - right); + if (wanted.length === 0) return new Map(); + + const statsDir = mkdtempSync(join(tmpdir(), "hf-psnr-")); + const statsFile = join(statsDir, "psnr.log"); + try { + // ffmpeg treats `:` and `\` in filter option values as syntax, so a temp + // path containing either would break the filtergraph. mkdtemp under + // tmpdir() does not produce those on POSIX, but escape defensively. + const escaped = statsFile.replace(/\\/g, "\\\\").replace(/:/g, "\\:"); + const selectExpr = wanted.map((frame) => `eq(n\\,${frame})`).join("+"); + const stream = (index: number, label: string) => + `[${index}:v]select='${selectExpr}',settb=1/1,setpts=N[${label}]`; + runFfmpeg( + [ + "-hide_banner", + "-loglevel", + "error", + "-i", + renderedVideo, + "-i", + snapshotVideo, + "-filter_complex", + // shortest=1:repeatlast=0 makes framesync stop at the first stream to + // end instead of holding its last frame. Without them, an input that + // runs out of selected frames has its final frame repeated to pad the + // pairing, so ffmpeg still writes one row per requested frame and the + // count check below cannot tell that the tail rows compare a stale + // frame. Verified: 60-frame vs 30-frame inputs asking for frames + // [0,15,45] emit 3 rows under the defaults (row 3 comparing frame 45 + // against a repeated frame 15, 16.65 dB) and 2 rows with these set. + `${stream(0, "rv")};${stream(1, "gv")};` + + `[rv][gv]psnr=shortest=1:repeatlast=0:stats_file=${escaped}`, + "-f", + "null", + "-", + ], + "Checkpoint PSNR", + ); + + const values: number[] = []; + for (const line of readFileSync(statsFile, "utf-8").split("\n")) { + const psnrMatch = line.match(/(?:^|\s)psnr_avg:(\S+)/); + if (!psnrMatch) continue; + const raw = (psnrMatch[1] ?? "").trim().toLowerCase(); + if (raw === "inf" || raw === "infinite") { + values.push(Number.POSITIVE_INFINITY); + continue; + } + const parsed = Number(raw); + if (!Number.isFinite(parsed)) { + throw new Error(`Invalid PSNR value in ffmpeg stats output: ${psnrMatch[1]}`); + } + values.push(parsed); + } + + // A short count means an input ran out of frames, so every later pairing + // would be silently offset. The per-checkpoint implementation also failed + // loudly here; keep it that way rather than reporting PSNR for frames that + // were never compared. + if (values.length !== wanted.length) { + throw new Error( + `Expected PSNR for ${wanted.length} frames but ffmpeg reported ${values.length}. ` + + "The rendered output and baseline likely differ in frame count.", + ); + } + + return new Map(wanted.map((frame, position) => [frame, values[position] as number])); + } finally { + rmSync(statsDir, { recursive: true, force: true }); + } +} + +export function psnrAtCheckpoint( + psnrByFrame: Map, checkpointSec: number, fps: number, ): number { - const frameIndex = Math.max(0, Math.round(checkpointSec * fps)); - const filter = `[0:v]select='eq(n\\,${frameIndex})',setpts=PTS-STARTPTS[rv];[1:v]select='eq(n\\,${frameIndex})',setpts=PTS-STARTPTS[gv];[rv][gv]psnr`; - const args = [ - "-hide_banner", - "-loglevel", - "info", - "-i", - renderedVideo, - "-i", - snapshotVideo, - "-filter_complex", - filter, - "-frames:v", - "1", - "-f", - "null", - "-", - ]; - const { stderr } = runFfmpeg(args, `Frame PSNR at ${checkpointSec}s`); - const match = stderr.match(/average:\s*([^\s]+)/i); - if (!match) { - throw new Error(`Unable to parse PSNR output at ${checkpointSec}s`); - } - const rawValue = (match[1] ?? "").trim().toLowerCase(); - if (rawValue === "inf" || rawValue === "infinite") { - return Number.POSITIVE_INFINITY; - } - const parsedValue = Number(rawValue); - if (!Number.isFinite(parsedValue)) { - throw new Error(`Invalid PSNR value at ${checkpointSec}s: ${match[1]}`); + const frameIndex = frameIndexForCheckpoint(checkpointSec, fps); + const parsedValue = psnrByFrame.get(frameIndex); + if (parsedValue === undefined) { + throw new Error(`Unable to parse PSNR output at ${checkpointSec}s (frame ${frameIndex})`); } return parsedValue; } @@ -1211,9 +1305,15 @@ async function runTestSuite( const sampleDuration = Math.max(0, videoDuration - 1 / fps); const minPsnrForMode = resolveMinPsnrForMode(options.mode, suite.meta.minPsnr); + const checkpointTimes = Array.from({ length: 100 }, (_, i) => (sampleDuration * i) / 100); + const psnrByFrame = psnrAtFrames( + renderedOutputPath, + snapshotVideoPath, + checkpointTimes.map((time) => frameIndexForCheckpoint(time, fps)), + ); for (let i = 0; i < 100; i++) { - const time = (sampleDuration * i) / 100; - const psnr = psnrAtCheckpoint(renderedOutputPath, snapshotVideoPath, time, fps); + const time = checkpointTimes[i] as number; + const psnr = psnrAtCheckpoint(psnrByFrame, time, fps); visualCheckpoints.push({ time, psnr,