fix: measure a torso against the frame it was actually cropped from

apps/cv-worker/src/appearance.test.ts

@@ -9,6 +9,7 @@ import {
   toHsv,
   torsoRect,
 } from './appearance.js';
+import { decodePlanFor } from './signatures.js';
 
 /**
  * The safety property under all of this: a signature that cannot tell two teams
@@ -95,6 +96,39 @@ describe('the torso crop', () => {
   });
 });
 
+describe('deciding what to decode, and at what scale', () => {
+  it('scales boxes from their own space, not from the file being read', () => {
+    /**
+     * The bug this exists for: tracks are in source-video pixels (1920x1080)
+     * while the file read is the 540p proxy. Deriving the space from the proxy
+     * scaled every crop by 1, put every torso off the right edge of the frame,
+     * and returned a confident zero matches on footage with eight to find.
+     */
+    const plan = decodePlanFor(1920, 1080, 960);
+    expect(plan.decodeWidth).toBe(960);
+    expect(plan.decodeHeight).toBe(540);
+    expect(plan.scale).toBe(0.5);
+  });
+
+  it('never upscales past the footage it was given', () => {
+    const plan = decodePlanFor(640, 360, 960);
+    expect(plan.decodeWidth).toBe(640);
+    expect(plan.scale).toBe(1);
+  });
+
+  it('keeps the aspect ratio, so a box’s y scales like its x', () => {
+    const plan = decodePlanFor(1440, 1080, 720);
+    expect(plan.scale).toBe(0.5);
+    expect(plan.decodeHeight).toBe(540);
+  });
+
+  it('survives a video with no readable width instead of dividing by zero', () => {
+    const plan = decodePlanFor(0, 0, 960);
+    expect(Number.isFinite(plan.scale)).toBe(true);
+    expect(plan.decodeHeight).toBeGreaterThan(0);
+  });
+});
+
 describe('comparing two players', () => {
   it('matches a shirt against itself', () => {
     const red = signatureOf(solidFrame(40, 80, [30, 30, 200]), 40, 80);

apps/cv-worker/src/index.ts

@@ -215,9 +215,15 @@ const appearance = async (flags: Record<string, string>): Promise<void> => {
     return;
   }
 
-  let request: { boxes?: SignatureBox[] };
+  interface Request {
+    boxes?: SignatureBox[];
+    /** The pixel space the boxes are in — see below. */
+    sourceWidth?: number;
+    sourceHeight?: number;
+  }
+  let request: Request;
   try {
-    request = JSON.parse(await readStdin()) as { boxes?: SignatureBox[] };
+    request = JSON.parse(await readStdin()) as Request;
   } catch (cause) {
     emit({ error: `stdin was not the JSON box list this expects: ${String(cause)}` });
     return;
@@ -229,8 +235,19 @@ const appearance = async (flags: Record<string, string>): Promise<void> => {
   }
 
   const media = await probe(input);
-  const width = media.video?.width ?? 0;
-  const height = media.video?.height ?? 0;
+  /**
+   * The boxes' coordinate space travels with the boxes, and is not the same
+   * thing as the size of the file being decoded.
+   *
+   * Tracks are stored in source-video pixels, but this reads the 540p proxy
+   * because a shirt's colour survives that and decodes in a fraction of the
+   * time. Taking the space from the decoded file measured every torso against
+   * the wrong scale — crops landed off the edge of the frame, signatures came
+   * back empty or meaningless, and the result was a confident zero matches on
+   * footage where eight were there to be found.
+   */
+  const width = request.sourceWidth ?? media.video?.width ?? 0;
+  const height = request.sourceHeight ?? media.video?.height ?? 0;
   if (width <= 0 || height <= 0) {
     emit({ error: `${input} has no readable video stream.` });
     return;

apps/cv-worker/src/signatures.integration.test.ts

@@ -0,0 +1,133 @@
+import { spawnSync } from 'node:child_process';
+import { mkdtempSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+
+import { similarity } from './appearance.js';
+import { computeSignatures } from './signatures.js';
+
+/**
+ * The seam a unit test cannot reach: boxes in one coordinate space, frames
+ * decoded in another.
+ *
+ * Tracks are stored in source-video pixels while this reads the 540p proxy, and
+ * taking the scale from the decoded file put every torso crop off the edge of
+ * the frame. The unit tests all passed; the feature returned a confident zero
+ * matches on footage with eight to find. Only measuring real pixels catches it,
+ * so this builds a video whose colours are known and checks that a box given in
+ * a *larger* space still lands on the right one.
+ *
+ * Skipped where ffmpeg is absent; the container image has it.
+ */
+
+const ffmpeg = spawnSync('ffmpeg', ['-version']);
+const available = ffmpeg.status === 0;
+
+let dir: string;
+let video: string;
+
+beforeAll(() => {
+  if (!available) return;
+  dir = mkdtempSync(path.join(tmpdir(), 'reeleel-signatures-'));
+  video = path.join(dir, 'bands.mp4');
+
+  /**
+   * Two seconds of a 640x360 clip: a red left half and a blue right half. The
+   * "source" space the boxes will be quoted in is twice that, so a correct
+   * implementation has to halve them before cropping.
+   */
+  const result = spawnSync(
+    'ffmpeg',
+    [
+      '-hide_banner',
+      '-loglevel',
+      'error',
+      '-f',
+      'lavfi',
+      '-i',
+      'color=c=red:s=320x360:d=2:r=10',
+      '-f',
+      'lavfi',
+      '-i',
+      'color=c=blue:s=320x360:d=2:r=10',
+      '-filter_complex',
+      '[0:v][1:v]hstack=inputs=2[v]',
+      '-map',
+      '[v]',
+      '-pix_fmt',
+      'yuv420p',
+      video,
+    ],
+    { encoding: 'utf8' },
+  );
+  if (result.status !== 0) throw new Error(`could not build the fixture: ${result.stderr}`);
+});
+
+afterAll(() => {
+  if (dir !== undefined) rmSync(dir, { recursive: true, force: true });
+});
+
+describe.skipIf(!available)('signatures from real pixels', () => {
+  /** A box in 1280x720 space, over a 640x360 video. */
+  const box = (x: number) => ({ ts: 1, x, y: 100, w: 200, h: 400 });
+
+  it('scales boxes out of their own space and onto the decoded frame', async () => {
+    const result = await computeSignatures({
+      input: video,
+      ffmpegPath: 'ffmpeg',
+      // Twice the video's real size: this is the bug's shape.
+      sourceWidth: 1280,
+      sourceHeight: 720,
+      fps: 10,
+      boxes: [
+        { track: 'left', ...box(100) },
+        { track: 'right', ...box(900) },
+      ],
+    });
+
+    // Both crops must have landed on actual pixels.
+    expect(result.pixels['left']).toBeGreaterThan(0);
+    expect(result.pixels['right']).toBeGreaterThan(0);
+
+    const left = result.signatures['left'] ?? [];
+    const right = result.signatures['right'] ?? [];
+    expect(left.length).toBeGreaterThan(0);
+    expect(right.length).toBeGreaterThan(0);
+
+    // The whole point: one landed on red, the other on blue. Get the scaling
+    // wrong and both land on the same place, or on nothing.
+    expect(similarity(left, right)).toBeLessThan(0.2);
+    expect(similarity(left, left)).toBeCloseTo(1);
+  });
+
+  it('reads the same shirt as the same shirt from two moments', async () => {
+    const result = await computeSignatures({
+      input: video,
+      ffmpegPath: 'ffmpeg',
+      sourceWidth: 1280,
+      sourceHeight: 720,
+      fps: 10,
+      boxes: [
+        { track: 'early', ...box(100), ts: 0.5 },
+        { track: 'late', ...box(100), ts: 1.5 },
+      ],
+    });
+    expect(
+      similarity(result.signatures['early'] ?? [], result.signatures['late'] ?? []),
+    ).toBeGreaterThan(0.9);
+  });
+
+  it('measures nothing for a box that is off the frame', async () => {
+    const result = await computeSignatures({
+      input: video,
+      ffmpegPath: 'ffmpeg',
+      sourceWidth: 1280,
+      sourceHeight: 720,
+      fps: 10,
+      boxes: [{ track: 'gone', ts: 1, x: -4000, y: 100, w: 200, h: 400 }],
+    });
+    expect(result.signatures['gone']).toBeUndefined();
+  });
+});

apps/cv-worker/src/signatures.ts

@@ -50,6 +50,30 @@ export interface SignatureResult {
 export const frameIndexFor = (ts: number, fps: number, stride: number): number =>
   Math.max(0, Math.round((ts * fps) / stride) * stride);
 
+/**
+ * How to decode, and what to multiply a box by once decoded.
+ *
+ * `sourceWidth`/`sourceHeight` are the space the *boxes* are in, which is not
+ * necessarily the size of the file being read: tracks are stored in
+ * source-video pixels while this usually reads the 540p proxy. Taking the space
+ * from the decoded file instead scaled every crop by 1 and put every torso rect
+ * somewhere off the right-hand edge, which produced empty signatures and a
+ * confident zero matches.
+ */
+export const decodePlanFor = (
+  sourceWidth: number,
+  sourceHeight: number,
+  requestedWidth: number,
+): { decodeWidth: number; decodeHeight: number; scale: number } => {
+  const decodeWidth = Math.min(requestedWidth, Math.max(1, sourceWidth));
+  const scale = sourceWidth > 0 ? decodeWidth / sourceWidth : 1;
+  return {
+    decodeWidth,
+    decodeHeight: Math.max(2, Math.round(sourceHeight * scale)),
+    scale,
+  };
+};
+
 /**
  * Colour signatures for a set of tracks, from one pass over the video.
  *
@@ -64,9 +88,11 @@ export const computeSignatures = async (
   const perSecond = request.samplesPerSecond ?? 2;
   const stride = Math.max(1, Math.round(fps / perSecond));
 
-  const decodeWidth = Math.min(request.decodeWidth ?? 960, request.sourceWidth);
-  const scale = request.sourceWidth > 0 ? decodeWidth / request.sourceWidth : 1;
-  const decodeHeight = Math.max(2, Math.round(request.sourceHeight * scale));
+  const { decodeWidth, decodeHeight, scale } = decodePlanFor(
+    request.sourceWidth,
+    request.sourceHeight,
+    request.decodeWidth ?? 960,
+  );
 
   // Boxes bucketed by the frame they will be measured on, so each decoded frame
   // is a single lookup rather than a scan of every box.