feat(engine): preflight psnr filter availability and force-fallback to screenshot on missing (#3418)

## What Adds a one-shot ffmpeg-psnr filter probe at drawElement session bootstrap. When the resident ffmpeg is missing or lacks libpostproc (no `psnr` filter), the capture-session router now force-fallbacks to the screenshot capture path and emits a `de_gate_reason = "ffmpeg_no_psnr_filter"` telemetry signal via the existing `render_complete` breakdown. Also tightens `psnrForDiskSample`'s catch: infrastructure-class ffmpeg failures (ENOENT, "No such filter") no longer silently skip the sample — they abort the render so the safety net cannot fail-open post-preflight. ## Why The drawElement self-verify safety net (parallelCoordinator's `psnrForDiskSample` → `psnrDb`) shells to `ffmpeg -lavfi psnr`. If ffmpeg is missing, or was compiled without libpostproc (so the `psnr` filter is absent), every per-sample compare throws. The existing catch swallows the error and returns `null` — callers treat that as "skip this sample" and the render completes with the safety net inoperative. Field signal (⭐ 9/10 CLI feedback, Slack ts=1787380767.210079, hyperframes 0.8.7, darwin/arm64, tid=93ff9910-2207-45c2-bc1f-54c0b347d4fe): > "host ffmpeg lacked psnr filter used by drawElement self-verification, > but render completed." The user's frames happened to be byte-identical so no visual damage shipped — but the safety net silently wasn't running. Any future compositor-damage bug on that host would have shipped straight through. ## How Two-part fix, both in `packages/engine`: 1. New `utils/psnrFilterAvailability.ts` — cached probe that runs `ffmpeg -hide_banner -filters` once per process and word-boundary- matches `psnr` in the output. Any failure (ENOENT, non-zero exit, timeout, unparseable output) returns `false`; never rejects. 2. Wired into `services/frameCapture.ts` `initDrawElementOrTransparentBackground` right after the Chrome capability probe: when useDrawElement resolves true and the preflight returns false, set `session.deGateReason = "ffmpeg_no_psnr_filter"` (same low-cardinality bucket every other DE gate uses; flows through `getCapturePerfSummary` → `render_complete.de_gate_reason` in PostHog), emit a stderr warning naming what's missing, and call `routeToFallback()` — the same fail-graceful shape as the SwiftShader / CSS-effect / at-risk-timeline gates. Skipped under `HF_FORCE_DRAWELEMENT=1` (matches the diagnostic knob's policy of bypassing every other gate). Belt-and-braces: `psnrForDiskSample` now discriminates infrastructure- class failures (ENOENT / "No such filter" / "Unknown filter") from per-sample noise (readFile races, transient EPERM). Only the former re-throw — per-sample noise still returns `null` (skipped sample). The preflight normally catches this at bootstrap; the re-throw covers ffmpeg-swapped-mid-render. ## Test plan - [x] Unit tests added: `packages/engine/src/utils/psnrFilterAvailability.test.ts` — mocked `execFile` covers: `psnr` present → true; `psnr` absent → false; ENOENT → false; non-zero exit → false; result memoized + reset works; substring-not-word-boundary → false. - [x] Unit tests added: `isFfmpegInfrastructureFailure` in `packages/engine/src/services/parallelCoordinator.test.ts` covers ENOENT, "No such filter", "Unknown filter", per-sample EACCES, parse errors, null/non-object. - [x] `bun run test` — `packages/engine/src/utils/psnrFilterAvailability.test.ts` (6 tests) + `packages/engine/src/services/parallelCoordinator.test.ts` (50 tests) + `frameCapture.test.ts` (26 tests) all pass. Pre-existing ffprobe test failures (4) on the base commit are unrelated (missing PNG fixture bytes — the file is 129 B on disk, likely LFS-stored). - [x] `bunx tsc --noEmit -p packages/engine/tsconfig.json` — clean. - [x] `bunx oxlint <files>` — 0 warnings, 0 errors. - [x] `bunx oxfmt --check <files>` — clean. Not covered here: an integration test that boots `initDrawElementOrTransparentBackground` end-to-end. That path is Puppeteer-driven and has no unit-scale bootstrap harness in the repository — the pure preflight + pure discriminator coverage above are what this PR can prove at the vitest layer. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-09-07 18:26:17 +00:00 · 2026-08-22 04:23:34 -07:00
parent 718bf5ef32
commit 073b098e21
5 changed files with 351 additions and 3 deletions
@@ -0,0 +1,172 @@
+import { promisify } from "node:util";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+interface ExecFileCall {
+  file: string;
+  args: readonly string[];
+}
+
+type ExecFileOutcome =
+  | { kind: "ok"; stdout: string; stderr?: string }
+  | { kind: "exit_nonzero"; code: number; stdout?: string; stderr?: string }
+  | { kind: "enoent" };
+
+// Node's built-in `child_process.execFile` carries a `util.promisify.custom`
+// implementation that resolves to `{stdout, stderr}`. A plain-callback mock
+// without that Symbol would be promisified as a single-result function, so
+// `{stdout} = await execFileP(...)` would silently destructure to `undefined`
+// — the exact hazard psnr.ts documents. Stamp the custom impl on the mock so
+// promisify keeps the `{stdout, stderr}` shape.
+function createExecFileSpy(outcome: ExecFileOutcome): {
+  execFile: (
+    file: string,
+    args: readonly string[],
+    options: unknown,
+    callback: (err: Error | null, stdout?: string, stderr?: string) => void,
+  ) => void;
+  calls: ExecFileCall[];
+} {
+  const calls: ExecFileCall[] = [];
+
+  async function run(
+    file: string,
+    args: readonly string[],
+  ): Promise<{ stdout: string; stderr: string }> {
+    calls.push({ file, args });
+    if (outcome.kind === "enoent") {
+      const err = new Error("spawn ffmpeg ENOENT") as NodeJS.ErrnoException;
+      err.code = "ENOENT";
+      throw err;
+    }
+    if (outcome.kind === "exit_nonzero") {
+      const err = new Error(`Command failed: ffmpeg (exit ${outcome.code})`) as Error & {
+        code: number;
+        stdout?: string;
+        stderr?: string;
+      };
+      err.code = outcome.code;
+      err.stdout = outcome.stdout ?? "";
+      err.stderr = outcome.stderr ?? "";
+      throw err;
+    }
+    return { stdout: outcome.stdout, stderr: outcome.stderr ?? "" };
+  }
+
+  const execFile = ((
+    file: string,
+    args: readonly string[],
+    _options: unknown,
+    callback: (err: Error | null, stdout?: string, stderr?: string) => void,
+  ) => {
+    run(file, args).then(
+      ({ stdout, stderr }) => process.nextTick(() => callback(null, stdout, stderr)),
+      (err: Error) => process.nextTick(() => callback(err)),
+    );
+  }) as ((
+    file: string,
+    args: readonly string[],
+    options: unknown,
+    callback: (err: Error | null, stdout?: string, stderr?: string) => void,
+  ) => void) & { [key: symbol]: unknown };
+  (execFile as { [k: symbol]: unknown })[promisify.custom] = (
+    file: string,
+    args: readonly string[],
+  ) => run(file, args);
+
+  return { execFile, calls };
+}
+
+beforeEach(() => {
+  vi.resetModules();
+});
+
+afterEach(() => {
+  vi.doUnmock("node:child_process");
+});
+
+describe("isPsnrFilterAvailable", () => {
+  it("returns true when `ffmpeg -filters` output lists the psnr filter", async () => {
+    const { execFile } = createExecFileSpy({
+      kind: "ok",
+      stdout: [
+        "Filters:",
+        "  T.. overlay          VV->V      Overlay a video source on top of the input.",
+        "  T.. psnr             VV->V      Calculate the PSNR between two video streams.",
+        "  ... yadif            V->V       Deinterlace the input image.",
+      ].join("\n"),
+    });
+    vi.doMock("node:child_process", () => ({ execFile }));
+
+    const { isPsnrFilterAvailable } = await import("./psnrFilterAvailability.js");
+    await expect(isPsnrFilterAvailable()).resolves.toBe(true);
+  });
+
+  it("returns false when `ffmpeg -filters` output omits the psnr filter", async () => {
+    const { execFile } = createExecFileSpy({
+      kind: "ok",
+      stdout: [
+        "Filters:",
+        "  T.. overlay          VV->V      Overlay a video source on top of the input.",
+        "  ... yadif            V->V       Deinterlace the input image.",
+      ].join("\n"),
+    });
+    vi.doMock("node:child_process", () => ({ execFile }));
+
+    const { isPsnrFilterAvailable } = await import("./psnrFilterAvailability.js");
+    await expect(isPsnrFilterAvailable()).resolves.toBe(false);
+  });
+
+  it("returns false when the ffmpeg binary is missing (ENOENT from execFile)", async () => {
+    const { execFile } = createExecFileSpy({ kind: "enoent" });
+    vi.doMock("node:child_process", () => ({ execFile }));
+
+    const { isPsnrFilterAvailable } = await import("./psnrFilterAvailability.js");
+    await expect(isPsnrFilterAvailable()).resolves.toBe(false);
+  });
+
+  it("returns false on a non-zero exit from `ffmpeg -filters`", async () => {
+    const { execFile } = createExecFileSpy({
+      kind: "exit_nonzero",
+      code: 1,
+      stderr: "Unrecognized option '-filters'.",
+    });
+    vi.doMock("node:child_process", () => ({ execFile }));
+
+    const { isPsnrFilterAvailable } = await import("./psnrFilterAvailability.js");
+    await expect(isPsnrFilterAvailable()).resolves.toBe(false);
+  });
+
+  it("memoizes the probe across calls and re-probes after resetPsnrFilterAvailabilityCache", async () => {
+    const { execFile, calls } = createExecFileSpy({
+      kind: "ok",
+      stdout: "  T.. psnr             VV->V      Calculate the PSNR",
+    });
+    vi.doMock("node:child_process", () => ({ execFile }));
+
+    const { isPsnrFilterAvailable, resetPsnrFilterAvailabilityCache } =
+      await import("./psnrFilterAvailability.js");
+
+    await isPsnrFilterAvailable();
+    await isPsnrFilterAvailable();
+    await isPsnrFilterAvailable();
+    expect(calls.length).toBe(1);
+
+    resetPsnrFilterAvailabilityCache();
+    await isPsnrFilterAvailable();
+    expect(calls.length).toBe(2);
+  });
+
+  it("does not treat a whole-string 'psnr' inside another word as the filter", async () => {
+    const { execFile } = createExecFileSpy({
+      kind: "ok",
+      stdout: [
+        "Filters:",
+        "  T.. bpsnrx            V->V       (hypothetical extended filter, not the real psnr)",
+      ].join("\n"),
+    });
+    vi.doMock("node:child_process", () => ({ execFile }));
+
+    const { isPsnrFilterAvailable } = await import("./psnrFilterAvailability.js");
+    await expect(isPsnrFilterAvailable()).resolves.toBe(false);
+  });
+});
@@ -0,0 +1,58 @@
+import { execFile } from "node:child_process";
+import { promisify } from "node:util";
+import { getFfmpegBinary } from "./ffmpegBinaries.js";
+
+/**
+ * Preflight for the ffmpeg `psnr` filter used by drawElement self-verify
+ * (see `psnr.ts`). Some host ffmpeg builds ship without `libpostproc` and
+ * silently omit the filter — every downstream `psnrDb()` call then throws
+ * mid-render and the disk-sample verifier swallows it (fail-open safety net).
+ * A cached one-shot probe surfaces the shape once, at bootstrap, so the
+ * capture-session router can force-fallback to the reliable screenshot path
+ * instead of arming a drawElement render whose safety net cannot run.
+ *
+ * Cache lifetime is the current process: an operator's ffmpeg install does
+ * not change across renders within the same CLI invocation, and re-probing
+ * per session would burn ~50-100ms of subprocess spawn per capture worker.
+ */
+let cached: Promise<boolean> | null = null;
+
+/**
+ * Returns true when the resident ffmpeg exposes the `psnr` filter. False on
+ * any probe failure — missing binary (ENOENT), non-zero exit, timeout,
+ * unparseable output — because in every case the drawElement self-verify
+ * path cannot function. Never rejects.
+ *
+ * Result is memoized per process; call {@link resetPsnrFilterAvailabilityCache}
+ * from tests that need to re-probe.
+ */
+export function isPsnrFilterAvailable(): Promise<boolean> {
+  if (cached === null) cached = probe();
+  return cached;
+}
+
+/** Test-only: drop the memoized probe result. */
+export function resetPsnrFilterAvailabilityCache(): void {
+  cached = null;
+}
+
+async function probe(): Promise<boolean> {
+  // Match `psnr.ts`: promisify lazily so a partial `node:child_process` mock
+  // (test that omits `execFile`) doesn't crash at module load — it fails at
+  // call time instead, and the try/catch below converts that to `false`.
+  const execFileP = promisify(execFile);
+  try {
+    const { stdout } = await execFileP(getFfmpegBinary(), ["-hide_banner", "-filters"], {
+      maxBuffer: 4 * 1024 * 1024,
+      timeout: 5_000,
+    });
+    // ffmpeg's `-filters` output lists one filter per line, e.g.
+    //   " T.. psnr             VV->V      Calculate the PSNR between two video streams."
+    // A whole-word match keeps `multi-psnr` (hypothetical) from masquerading
+    // as the real filter, and dodges the banner text that mentions PSNR in
+    // prose on some builds.
+    return /(^|\s)psnr(\s|$)/m.test(stdout);
+  } catch {
+    return false;
+  }
+}