fix(snapshot): video frames keep the colours render gives the same footage (#6425)

* fix(snapshot): video frames keep the colours render gives the same footage

* fix(snapshot): say so once when video colours cannot be matched to render

* test(snapshot): cut the HEVC fixture's encode time so the flake gate stays inside its timeout

* style(snapshot): format the HEVC fixture line

packages/cli/src/commands/snapshot.test.ts

@@ -446,6 +446,20 @@ describe("containingSourceFrameIndex", () => {
   });
 });
 
+/** The pixel data of a PNG: the colour-tag chunks a retag adds are not part of the picture. */
+function samePixels(actual: Buffer | null | undefined, expected: Buffer): boolean {
+  const idat = (png: Buffer): Buffer => {
+    const chunks: Buffer[] = [];
+    for (let at = 8; at < png.length; at += 12 + png.readUInt32BE(at)) {
+      if (png.toString("latin1", at + 4, at + 8) === "IDAT") {
+        chunks.push(png.subarray(at + 8, at + 8 + png.readUInt32BE(at)));
+      }
+    }
+    return Buffer.concat(chunks);
+  };
+  return !!actual && idat(actual).equals(idat(expected));
+}
+
 describe("extractVideoFrameToBuffer", () => {
   const ffmpeg = findFFmpeg();
   const ffprobe = findFFprobe();
@@ -491,7 +505,7 @@ describe("extractVideoFrameToBuffer", () => {
           expected,
         ]);
         const actual = await extractVideoFrameToBuffer(clip, time);
-        expect(actual?.equals(readFileSync(expected))).toBe(true);
+        expect(samePixels(actual, readFileSync(expected))).toBe(true);
       }
     } finally {
       rmSync(dir, { recursive: true, force: true });
@@ -532,7 +546,7 @@ describe("extractVideoFrameToBuffer", () => {
         "-y",
         expected,
       ]);
-      expect((await extractVideoFrameToBuffer(clip, 0.3))?.equals(readFileSync(expected))).toBe(
+      expect(samePixels(await extractVideoFrameToBuffer(clip, 0.3), readFileSync(expected))).toBe(
         true,
       );
     } finally {
@@ -576,7 +590,7 @@ describe("extractVideoFrameToBuffer", () => {
           ...["-i", clip, "-vf", `select=eq(n\\,${index})`, "-frames:v", "1", "-y", expected],
         ]);
         const actual = await extractVideoFrameToBuffer(clip, (index + 0.5) / 30);
-        expect(actual?.equals(readFileSync(expected)), `frame ${index}`).toBe(true);
+        expect(samePixels(actual, readFileSync(expected)), `frame ${index}`).toBe(true);
       }
     } finally {
       rmSync(dir, { recursive: true, force: true });
@@ -594,7 +608,17 @@ describe("extractVideoFrameToBuffer", () => {
         execFileSync(ffmpeg!, [
           ...quiet,
           ...["-f", "lavfi", "-i", "testsrc2=d=70:r=30:s=160x90", "-pix_fmt", "yuv420p"],
-          ...["-c:v", "libx265", "-x265-params", x265, "-tag:v", "hvc1", source],
+          ...[
+            "-c:v",
+            "libx265",
+            "-preset",
+            "ultrafast",
+            "-x265-params",
+            x265,
+            "-tag:v",
+            "hvc1",
+            source,
+          ],
         ]);
         const clip = join(dir, "cut.mov");
         execFileSync(ffmpeg!, [...quiet, ...["-ss", "31.77", "-i", source, "-c", "copy", clip]]);
@@ -608,7 +632,7 @@ describe("extractVideoFrameToBuffer", () => {
           ...["-i", clip, "-vf", "select=eq(n\\,1056)", "-frames:v", "1", "-y", expected],
         ]);
         const actual = await extractVideoFrameToBuffer(clip, 1056.5 / 30);
-        expect(actual?.equals(readFileSync(expected))).toBe(true);
+        expect(samePixels(actual, readFileSync(expected))).toBe(true);
       } finally {
         rmSync(dir, { recursive: true, force: true });
       }
@@ -716,9 +740,9 @@ describe("extractVideoFrameToBuffer", () => {
           "-y",
           expected,
         ]);
-        expect((await extractVideoFrameToBuffer(clip, time))?.equals(readFileSync(expected))).toBe(
-          true,
-        );
+        expect(
+          samePixels(await extractVideoFrameToBuffer(clip, time), readFileSync(expected)),
+        ).toBe(true);
       } finally {
         rmSync(dir, { recursive: true, force: true });
       }
@@ -760,7 +784,7 @@ describe("extractVideoFrameToBuffer", () => {
             ...["-i", clip, "-vf", `select=eq(n\\,${index})`, "-frames:v", "1", "-y", expected],
           ]);
           const actual = await extractVideoFrameToBuffer(clip, index / 24);
-          expect(actual?.equals(readFileSync(expected)), `frame ${index}`).toBe(true);
+          expect(samePixels(actual, readFileSync(expected)), `frame ${index}`).toBe(true);
         }
       } finally {
         rmSync(dir, { recursive: true, force: true });
@@ -790,7 +814,7 @@ describe("extractVideoFrameToBuffer", () => {
             ...["-i", clip, "-vf", `select=eq(n\\,${index})`, "-frames:v", "1", "-y", expected],
           ]);
           const actual = await extractVideoFrameToBuffer(clip, index / 24);
-          expect(actual?.equals(readFileSync(expected)), `frame ${index}`).toBe(true);
+          expect(samePixels(actual, readFileSync(expected)), `frame ${index}`).toBe(true);
         }
       } finally {
         rmSync(dir, { recursive: true, force: true });
@@ -843,7 +867,7 @@ describe("extractVideoFrameToBuffer", () => {
           ...["-frames:v", "1", "-y", expected],
         ]);
         const actual = await extractVideoFrameToBuffer(clip, time);
-        expect(actual?.equals(readFileSync(expected)), `t=${time}`).toBe(true);
+        expect(samePixels(actual, readFileSync(expected)), `t=${time}`).toBe(true);
       }
     } finally {
       rmSync(dir, { recursive: true, force: true });
@@ -851,6 +875,7 @@ describe("extractVideoFrameToBuffer", () => {
   });
 
   it.skipIf(noProbe)("selects frames in a TS whose timestamps start below zero", async () => {
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
     const dir = mkdtempSync(join(tmpdir(), "hf-snapshot-negative-"));
     try {
       const clip = join(dir, "clip.ts");
@@ -875,7 +900,7 @@ describe("extractVideoFrameToBuffer", () => {
           ...["-i", clip, "-vf", `select=eq(n\\,${index})`, "-frames:v", "1", "-y", expected],
         ]);
         const actual = await extractVideoFrameToBuffer(clip, time);
-        expect(actual?.equals(readFileSync(expected)), `t=${time}`).toBe(true);
+        expect(samePixels(actual, readFileSync(expected)), `t=${time}`).toBe(true);
       };
       for (const [time, index] of [
         [0, 0],
@@ -894,7 +919,10 @@ describe("extractVideoFrameToBuffer", () => {
         [1, 30],
       ] as const)
         await frameAt(time, index);
+      // Colours cannot be matched to render without ffprobe: said once, not per frame.
+      expect(warn.mock.calls.filter(([m]) => /colours may differ/.test(String(m)))).toHaveLength(1);
     } finally {
+      warn.mockRestore();
       vi.unstubAllEnvs();
       rmSync(dir, { recursive: true, force: true });
     }
@@ -924,13 +952,28 @@ describe("extractVideoFrameToBuffer", () => {
           ...["-i", clip, "-vf", `select=eq(n\\,${index})`, "-frames:v", "1", "-y", expected],
         ]);
         const actual = await extractVideoFrameToBuffer(clip, time);
-        expect(actual?.equals(readFileSync(expected)), `t=${time}`).toBe(true);
+        expect(samePixels(actual, readFileSync(expected)), `t=${time}`).toBe(true);
       }
     } finally {
       rmSync(dir, { recursive: true, force: true });
     }
   });
 
+  it.skipIf(noProbe)("retags SDR footage as sRGB, as render does", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "hf-snapshot-retag-"));
+    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", "-colorspace", "bt709", "-color_trc", "bt709", clip],
+      ]);
+      const png = await extractVideoFrameToBuffer(clip, 0.5);
+      expect(png?.includes("sRGB", 0, "latin1")).toBe(true);
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
   it.skipIf(!ffmpeg)("extracts without ffprobe, as before frame selection needed it", async () => {
     const dir = mkdtempSync(join(tmpdir(), "hf-snapshot-no-ffprobe-"));
     vi.stubEnv("HYPERFRAMES_FFPROBE_PATH", join(dir, "missing-ffprobe"));
@@ -945,7 +988,7 @@ describe("extractVideoFrameToBuffer", () => {
         ...["-hide_banner", "-loglevel", "error", "-i", clip, "-vf", "select=eq(n\\,12)"],
         ...["-frames:v", "1", "-y", expected],
       ]);
-      expect((await extractVideoFrameToBuffer(clip, 0.5))?.equals(readFileSync(expected))).toBe(
+      expect(samePixels(await extractVideoFrameToBuffer(clip, 0.5), readFileSync(expected))).toBe(
         true,
       );
     } finally {

packages/cli/src/commands/snapshot.ts

@@ -2,6 +2,8 @@ import {
   DEFAULT_CONFIG,
   buildZeroDurationDiagnostic,
   evaluateHfDiagnostic,
+  extractMediaMetadata,
+  sdrCanvasColourFilters,
 } from "@hyperframes/engine";
 import type { Page } from "puppeteer-core";
 import { reportArtifacts } from "../utils/toolContext.js";
@@ -394,6 +396,16 @@ async function probeSeekLanding(
   }
 }
 
+let warnedColourUnknown = false;
+
+function warnColourUnknownOnce(): void {
+  if (warnedColourUnknown) return;
+  warnedColourUnknown = true;
+  console.warn(
+    `   ${c.warn("⚠")} snapshot colours may differ from render: install ffmpeg/ffprobe so video colours can be matched`,
+  );
+}
+
 class FrameExtractTimeoutError extends Error {}
 
 export async function extractVideoFrameToBuffer(
@@ -419,6 +431,9 @@ export async function extractVideoFrameToBuffer(
     args.push("-i", videoPath, "-map", "0:v:0");
     const filters: string[] = [];
     if (seek.select !== undefined) filters.push(`select=gte(t-start_t\\,${seek.select})`);
+    const metadata = findFFprobe() ? await extractMediaMetadata(videoPath) : null;
+    if (metadata) filters.push(...sdrCanvasColourFilters(metadata));
+    else warnColourUnknownOnce();
     // Shrink so the short side still covers the displayed box at any rotation or fit; never enlarge.
     if (displayedPixels > 0) {
       filters.push(

packages/engine/src/index.ts

@@ -253,6 +253,7 @@ export {
   VIDEO_FRAME_FORMATS,
   isVideoFrameFormat,
   EXTRACT_CACHE_MIN_AGE_MS,
+  sdrCanvasColourFilters,
 } from "./services/videoFrameExtractor.js";
 export { isSelfOrAncestorHidden } from "./services/mediaHidden.js";
 export { directorySizeBytes, gcExtractionCache } from "./services/extractionCache.js";

packages/engine/src/services/videoFrameExtractor.ts

@@ -907,6 +907,13 @@ function chromeGuessForUntaggedMatrix(metadata: VideoMetadata): string[] {
   return [`setparams=colorspace=${hd ? "bt709" : "smpte170m"}`];
 }
 
+export function sdrCanvasColourFilters(metadata: VideoMetadata, toneMappedToSdr = false): string[] {
+  if (!isHdrColorSpaceUtil(metadata.colorSpace)) {
+    return [...chromeGuessForUntaggedMatrix(metadata), SDR_CANVAS_PASSTHROUGH_FILTER];
+  }
+  return toneMappedToSdr ? [SDR_CANVAS_PASSTHROUGH_FILTER] : [];
+}
+
 /**
  * Longest stretch of output one ffmpeg process extracts; longer ranges split into segments
  * that run side by side. Segments sample the same absolute times as a single pass, so their
@@ -1153,10 +1160,7 @@ async function extractVideoFramesRangeUnredacted(
     vfFilters.push(hdrToSdrToneMapFilter(metadata.colorSpace ?? {}, frameColour));
   }
   if (toneMappedToSdr || (!isHdr && !options.sdrToHdrTransfer)) {
-    vfFilters.push(
-      ...(isHdr ? [] : chromeGuessForUntaggedMatrix(metadata)),
-      SDR_CANVAS_PASSTHROUGH_FILTER,
-    );
+    vfFilters.push(...sdrCanvasColourFilters(metadata, toneMappedToSdr));
     if (format === "jpg") {
       if (options.shownBox) vfFilters.push(halveToCoverScaleFilter(options.shownBox));
       vfFilters.push(SDR_JPEG_AS_BT601_FULL_RANGE_FILTER);