fix(scripts): the capture checker exits 2 when it cannot check

A bad option, a missing binary or file, or an undecodable video used to
read as clean or crash with exit 1, which the caller cannot tell from a
flagged capture. Options are now validated, the frame height is passed
to ffmpeg explicitly so it always matches the size the check assumes,
and the tests pin the thresholds, window boundary, ordering and mean.

CONTRIBUTING.md

@@ -149,7 +149,7 @@ The script wipes `docs/catalog/` before regenerating, so deleted items are autom
 
 ### Checking a capture
 
-A screen recording attached to a PR must not show a region briefly returning to an older picture (a clip that reappears for one frame, a panel that flashes its previous state). Run `node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] <video>...` once per region that matters; it needs `ffmpeg` and `ffprobe`, exits 1 on a flagged video, and names the frames that differ and the two matching frames around them. Continuous motion is not flagged, but a toast or cursor inside the crop can be, so crop it out or check the named frames by eye.
+A screen recording attached to a PR must not show a region briefly returning to an older picture (a clip that reappears for one frame, a panel that flashes its previous state). Run `node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] <video>...` once per region that matters; it needs `ffmpeg` (and `ffprobe` when no `--crop` is given), takes `--window`, `--change` and `--same` to tune the match, exits 0 when clean, 1 when a video is flagged and 2 when it could not check (bad option, missing binary or file), and names the frames that differ and the two matching frames around them. Continuous motion is not flagged, but a toast or cursor inside the crop can be, so crop it out or check the named frames by eye.
 
 ## Packages
 

scripts/check-capture-reversion.mjs

@@ -1,17 +1,23 @@
 #!/usr/bin/env node
 // Flags a capture region that briefly returns to an older picture (A, B, A frames).
 // node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] [--window=N] [--change=D] [--same=D] <video>...
-// Frames are cropped, 96px wide, gray; D is mean abs pixel diff (0-255). Exit 1 if flagged.
+// Frames are cropped, 96px wide, gray; D is mean abs pixel diff (0-255). Exit 0 clean, 1 flagged, 2 could not check.
 
 import { spawn } from "node:child_process";
 import { fileURLToPath } from "node:url";
 
 const WIDTH = 96;
-const args = { crop: undefined, window: 20, change: 1.0, same: 0.25 };
+const USAGE =
+  "usage: node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] [--window=N] [--change=D] [--same=D] <video> ...";
 
-function readFrames(path, crop) {
+/** Output frame height for a source of cw x ch scaled to WIDTH: even, at least 2. */
+export function frameHeight(cw, ch) {
+  return Math.max(2, 2 * Math.round((WIDTH * (ch / cw)) / 2));
+}
+
+function readFrames(path, crop, height) {
   return new Promise((resolve, reject) => {
-    const scale = `scale=${WIDTH}:-2:flags=area,format=gray`;
+    const scale = `scale=${WIDTH}:${height}:flags=area,format=gray`;
     const vf = crop ? `crop=${crop},${scale}` : scale;
     const child = spawn("ffmpeg", ["-v", "error", "-i", path, "-vf", vf, "-f", "rawvideo", "-"]);
     const chunks = [];
@@ -43,9 +49,10 @@ function probeSize(path) {
     let out = "";
     child.stdout.on("data", (c) => (out += c));
     child.on("error", reject);
-    child.on("close", () => {
+    child.on("close", (code) => {
       const [w, h] = out.trim().split(",").map(Number);
-      resolve({ w, h });
+      if (code !== 0 || !(w > 0) || !(h > 0)) return reject(new Error(`ffprobe found no video size in ${path}`));
+      resolve([w, h]);
     });
   });
 }
@@ -88,26 +95,39 @@ export function findReversions(buf, frameSize, { window, change, same }) {
   return [...byPeak.values()].sort((x, y) => x.changed - y.changed);
 }
 
-function parseArgs(argv) {
+/** Options and video paths from argv; throws on a malformed flag so a typo cannot read as "clean". */
+export function parseArgs(argv) {
+  const options = { crop: undefined, window: 20, change: 1.0, same: 0.25 };
   const paths = [];
   for (const arg of argv) {
-    const m = /^--(crop|window|change|same)=(.*)$/.exec(arg);
-    if (m) args[m[1]] = m[1] === "crop" ? m[2] : Number(m[2]);
-    else paths.push(arg);
+    const m = /^--([a-z]+)=(.*)$/.exec(arg);
+    if (!m) {
+      if (arg.startsWith("--")) throw new Error(`unknown option ${arg}`);
+      paths.push(arg);
+    } else if (m[1] === "crop") {
+      if (!/^[1-9]\d*:[1-9]\d*:\d+:\d+$/.test(m[2])) throw new Error(`--crop needs W:H:X:Y, got ${m[2]}`);
+      options.crop = m[2];
+    } else if (m[1] in options) {
+      const n = Number(m[2]);
+      if (m[2] === "" || !Number.isFinite(n) || n < 0) throw new Error(`--${m[1]} needs a number, got ${m[2]}`);
+      options[m[1]] = n;
+    } else {
+      throw new Error(`unknown option ${arg}`);
+    }
   }
-  return paths;
+  return { options, paths };
 }
 
 // fallow-ignore-next-line complexity
-async function checkVideo(path) {
-  const [cw, ch] = args.crop
-    ? args.crop.split(":").map(Number)
-    : Object.values(await probeSize(path));
-  const frameSize = WIDTH * 2 * Math.round((WIDTH * (ch / cw)) / 2);
-  const buf = await readFrames(path, args.crop);
+async function checkVideo(path, options) {
+  const [cw, ch] = options.crop ? options.crop.split(":").map(Number) : await probeSize(path);
+  const height = frameHeight(cw, ch);
+  const frameSize = WIDTH * height;
+  const buf = await readFrames(path, options.crop, height);
   const frames = Math.floor(buf.length / frameSize);
-  const hits = findReversions(buf, frameSize, args);
-  const crop = args.crop ? `  crop=${args.crop}` : "";
+  if (frames === 0) throw new Error(`no frames decoded from ${path}`);
+  const hits = findReversions(buf, frameSize, options);
+  const crop = options.crop ? `  crop=${options.crop}` : "";
   console.log(
     `${hits.length ? "FLAGGED" : "clean"}  ${path}${crop}  (${frames} frames checked, 0-${frames - 1})`,
   );
@@ -121,16 +141,16 @@ async function checkVideo(path) {
 
 // fallow-ignore-next-line complexity
 async function main(argv) {
-  const paths = parseArgs(argv);
-  if (paths.length === 0) {
-    console.error(
-      "usage: node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] [--window=N] [--change=D] [--same=D] <video> ...",
-    );
+  try {
+    const { options, paths } = parseArgs(argv);
+    if (paths.length === 0) throw new Error(USAGE);
+    let flagged = false;
+    for (const path of paths) flagged = (await checkVideo(path, options)) || flagged;
+    process.exit(flagged ? 1 : 0);
+  } catch (error) {
+    console.error(`check-capture-reversion: ${error.message}`);
     process.exit(2);
   }
-  let flagged = false;
-  for (const path of paths) flagged = (await checkVideo(path)) || flagged;
-  process.exit(flagged ? 1 : 0);
 }
 
 if (process.argv[1] === fileURLToPath(import.meta.url)) main(process.argv.slice(2));

scripts/check-capture-reversion.test.mjs

@@ -1,7 +1,7 @@
 import { strict as assert } from "node:assert";
 import { test } from "node:test";
 
-import { findReversions } from "./check-capture-reversion.mjs";
+import { findReversions, frameHeight, parseArgs } from "./check-capture-reversion.mjs";
 
 const OPTS = { window: 20, change: 1, same: 0.25 };
 const SIZE = 4;
@@ -30,3 +30,49 @@ test("a return further away than the window is ignored", () => {
   const far = frames(10, 200, 10);
   assert.deepEqual(findReversions(far, SIZE, { ...OPTS, window: 1 }), []);
 });
+
+test("the window boundary is inclusive", () => {
+  const hits = findReversions(frames(10, 200, 10), SIZE, { ...OPTS, window: 2 });
+  assert.equal(hits.length, 1);
+});
+
+test("a change below the threshold is not flagged, one above it is", () => {
+  assert.deepEqual(findReversions(frames(10, 11, 10), SIZE, { ...OPTS, change: 1 }), []);
+  assert.equal(findReversions(frames(10, 12, 10), SIZE, { ...OPTS, change: 1 }).length, 1);
+});
+
+test("a middle frame that matches only one side is not a reversion", () => {
+  assert.deepEqual(findReversions(frames(10, 10, 10, 200), SIZE, OPTS), []);
+});
+
+test("hits come back ordered by the changed frame", () => {
+  const hits = findReversions(frames(10, 200, 10, 10, 90, 10), SIZE, OPTS);
+  assert.deepEqual(
+    hits.map((h) => h.changed),
+    [1, 4],
+  );
+});
+
+test("the reported delta is the mean difference, not the sum", () => {
+  const [hit] = findReversions(frames(10, 110, 10), SIZE, OPTS);
+  assert.equal(hit.delta, 100);
+});
+
+test("frame height is even and never below 2", () => {
+  assert.equal(frameHeight(1280, 200), 16);
+  assert.equal(frameHeight(1280, 800), 60);
+  assert.equal(frameHeight(1000, 2), 2);
+});
+
+test("a malformed option throws instead of reading as clean", () => {
+  for (const bad of ["--crop=iw:200:0:0", "--window=abc", "--change=", "--same=-1", "--crp=1:1:0:0"]) {
+    assert.throws(() => parseArgs([bad, "v.webm"]), bad);
+  }
+});
+
+test("options and paths are separated", () => {
+  const { options, paths } = parseArgs(["--crop=1280:200:0:600", "--window=5", "a.webm", "b.webm"]);
+  assert.deepEqual(paths, ["a.webm", "b.webm"]);
+  assert.equal(options.crop, "1280:200:0:600");
+  assert.equal(options.window, 5);
+});