fix(media): count grid ordinals as skipped when the sample stream ends early

frontend/media/browser/materialize.spec.ts

@@ -221,3 +221,37 @@ test("a sink that rejects fails the import with the sink's own error", async ({
   }, HELPERS);
   expect(message).toBe("the batch is closed");
 });
+
+test("a clip cut mid-GOP skips the frames before its first real key frame", async ({ page }) => {
+  const outcome = await page.evaluate(
+    async ({ helpers, colours }) => {
+      const h = await import(helpers);
+      const { encodedTypes, file } = await h.cutMidGop(colours);
+      const sink = h.collecting();
+      const result = await (
+        await h.materializer()
+      ).materialize(file, { extractionFps: 8, ranges: [], scalePercent: 100 }, sink);
+      const frames = [];
+      for (const frame of sink.frames) {
+        frames.push({ ordinal: frame.ordinal, rgb: (await h.inspectImage(frame.bytes)).rgb });
+      }
+      return { encodedTypes, result, frames };
+    },
+    { helpers: HELPERS, colours: MANIFEST.colours },
+  );
+  // The precondition, so a change in the encoder's key-frame placement fails here
+  // rather than quietly turning this into a clip with nothing to skip.
+  expect(outcome.encodedTypes).toEqual([
+    "key", "delta", "delta", "delta", "key", "delta", "delta", "delta",
+  ]);
+  // The cut starts at source frame 1, so the grid is seven ordinals long and ordinal
+  // k asks for source frame k + 1. Frames 1-3 have no key frame to decode from.
+  expect(outcome.result).toEqual({ materialized: 4, expected: 7, skipped: [0, 1, 2] });
+  expect(outcome.result.materialized + outcome.result.skipped.length).toBe(
+    outcome.result.expected,
+  );
+  expect(outcome.frames.map((frame) => frame.ordinal)).toEqual([3, 4, 5, 6]);
+  outcome.frames.forEach((frame) => {
+    expectColour(frame.rgb as [number, number, number], MANIFEST.colours[frame.ordinal + 1]);
+  });
+});

frontend/media/browser/page-helpers.js

@@ -51,3 +51,59 @@ export async function inspectImage(blob) {
   bitmap.close();
   return { ...size, rgb: [r, g, b], head: hex(0, 2), tail: hex(bytes.length - 2, 2) };
 }
+
+/**
+ * Eight solid-colour VP8 frames at 8 fps with key frames at 0 and 4, cut by dropping the
+ * first packet. Mediabunny's muxer refuses a first packet that is not labelled key, so
+ * the delta frame that now leads is labelled key — what a naive cutter writes. The
+ * decoder sees through the label (`verifyKeyPackets`), so the frames before the next
+ * real key frame have nothing to decode from and come back `null`.
+ */
+export async function cutMidGop(colours) {
+  const mb = await import("/mediabunny.mjs");
+  const canvas = new OffscreenCanvas(64, 64);
+  const context = canvas.getContext("2d");
+  const whole = new mb.Output({ format: new mb.WebMOutputFormat(), target: new mb.BufferTarget() });
+  const source = new mb.CanvasSource(canvas, {
+    codec: "vp8",
+    bitrate: mb.QUALITY_VERY_HIGH,
+    keyFrameInterval: 0.5,
+  });
+  whole.addVideoTrack(source, { frameRate: 8 });
+  await whole.start();
+  for (const [index, colour] of colours.entries()) {
+    context.fillStyle = colour;
+    context.fillRect(0, 0, 64, 64);
+    await source.add(index / 8, 1 / 8);
+  }
+  source.close();
+  await whole.finalize();
+
+  const input = new mb.Input({
+    source: new mb.BufferSource(whole.target.buffer),
+    formats: mb.ALL_FORMATS,
+  });
+  const track = await input.getPrimaryVideoTrack();
+  const decoderConfig = await track.getDecoderConfig();
+  const packets = [];
+  for await (const packet of new mb.EncodedPacketSink(track).packets()) packets.push(packet);
+
+  const cut = new mb.Output({ format: new mb.WebMOutputFormat(), target: new mb.BufferTarget() });
+  const passthrough = new mb.EncodedVideoPacketSource("vp8");
+  cut.addVideoTrack(passthrough, { frameRate: 8 });
+  await cut.start();
+  for (const [index, packet] of packets.slice(1).entries()) {
+    const type = index === 0 ? "key" : packet.type;
+    await passthrough.add(
+      new mb.EncodedPacket(packet.data, type, packet.timestamp, packet.duration),
+      index === 0 ? { decoderConfig } : undefined,
+    );
+  }
+  passthrough.close();
+  await cut.finalize();
+  input.dispose();
+  return {
+    encodedTypes: packets.map((packet) => packet.type),
+    file: new File([cut.target.buffer], "cut-mid-gop.webm"),
+  };
+}

frontend/media/src/grid-walk.test.ts

@@ -0,0 +1,92 @@
+import { describe, expect, it } from "vitest";
+
+import { walkGrid } from "./grid-walk";
+import { gridTimestamps } from "./index";
+
+async function* answering<S>(samples: readonly (S | null)[]): AsyncGenerator<S | null> {
+  yield* samples;
+}
+
+const FPS = 4;
+const GRID = [...gridTimestamps([{ startSeconds: 0.5, endSeconds: 2 }], 10, FPS)];
+
+async function walk(samples: readonly (string | null)[]) {
+  const visited: { sample: string; ordinal: number; requestedTimestamp: number }[] = [];
+  const skipped = await walkGrid(
+    GRID,
+    FPS,
+    answering(samples),
+    async (sample, ordinal, requestedTimestamp) => {
+      visited.push({ sample, ordinal, requestedTimestamp });
+    },
+    () => false,
+  );
+  return { visited, skipped };
+}
+
+describe("walkGrid", () => {
+  it("visits every sample with its grid ordinal and timestamp", async () => {
+    const { visited, skipped } = await walk(["a", "b", "c", "d", "e", "f"]);
+    expect(GRID).toHaveLength(6);
+    expect(visited.map((v) => v.ordinal)).toEqual([2, 3, 4, 5, 6, 7]);
+    expect(visited[0]).toEqual({ sample: "a", ordinal: 2, requestedTimestamp: 0.5 });
+    expect(skipped).toEqual([]);
+  });
+
+  it("skips the ordinal of a null sample and keeps walking", async () => {
+    const { visited, skipped } = await walk(["a", null, "c", null, "e", "f"]);
+    expect(skipped).toEqual([3, 5]);
+    expect(visited.map((v) => v.ordinal)).toEqual([2, 4, 6, 7]);
+  });
+
+  it("skips every remaining ordinal when the samples end before the grid", async () => {
+    const { visited, skipped } = await walk(["a", null, "c"]);
+    expect(visited.map((v) => v.ordinal)).toEqual([2, 4]);
+    expect(skipped).toEqual([3, 5, 6, 7]);
+    expect(visited.length + skipped.length).toBe(GRID.length);
+  });
+
+  it("skips the whole grid when there are no samples at all", async () => {
+    const { visited, skipped } = await walk([]);
+    expect(visited).toEqual([]);
+    expect(skipped).toEqual([2, 3, 4, 5, 6, 7]);
+  });
+
+  it("does not pad skipped when the samples end because of a cancel", async () => {
+    let cancel = false;
+    async function* cancelledMidway(): AsyncGenerator<string | null> {
+      yield "a";
+      yield null;
+      cancel = true;
+    }
+    const visited: number[] = [];
+    const skipped = await walkGrid(
+      GRID,
+      FPS,
+      cancelledMidway(),
+      async (_sample, ordinal) => {
+        visited.push(ordinal);
+      },
+      () => cancel,
+    );
+    expect(visited).toEqual([2]);
+    expect(skipped).toEqual([3]);
+  });
+
+  it("stops without padding when a cancel lands between samples", async () => {
+    let cancel = false;
+    const visited: number[] = [];
+    const skipped = await walkGrid(
+      GRID,
+      FPS,
+      answering(["a", "b", "c"]),
+      async (_sample, ordinal) => {
+        visited.push(ordinal);
+        cancel = true;
+      },
+      () => cancel,
+    );
+    expect(visited).toEqual([2]);
+    expect(skipped).toEqual([]);
+  });
+});

frontend/media/src/grid-walk.ts

@@ -0,0 +1,34 @@
+/**
+ * Pairs each grid timestamp with the sample the decoder answered for it, in order,
+ * and resolves with the grid ordinals that got no sample.
+ *
+ * Every ordinal ends up either visited or skipped — a `null` sample and a sample
+ * stream that ends before the grid does are both "no sample" — so a finished walk
+ * accounts for the whole grid. A cancelled walk does not: what it never reached was
+ * abandoned, not missing.
+ */
+export async function walkGrid<S>(
+  grid: readonly number[],
+  fps: number,
+  samples: AsyncIterator<S | null>,
+  visit: (sample: S, ordinal: number, requestedTimestamp: number) => Promise<void>,
+  cancelled: () => boolean,
+): Promise<number[]> {
+  // `gridTimestamps` yields `i / fps`; the ordinal a frame carries is that `i`.
+  const ordinalAt = (position: number): number => Math.round(grid[position] * fps);
+  const skipped: number[] = [];
+  for (let position = 0; position < grid.length && !cancelled(); position++) {
+    const next = await samples.next();
+    if (next.done === true) {
+      if (cancelled()) break;
+      for (let rest = position; rest < grid.length; rest++) skipped.push(ordinalAt(rest));
+      break;
+    }
+    if (next.value === null) {
+      skipped.push(ordinalAt(position));
+      continue;
+    }
+    await visit(next.value, ordinalAt(position), grid[position]);
+  }
+  return skipped;
+}

frontend/media/src/index.ts

@@ -212,7 +212,11 @@ export interface VideoImportProgress {
 export interface VideoImportResult {
   readonly materialized: number;
   readonly expected: number;
-  /** Grid ordinals with no sample — requested past the last frame, not faked. */
+  /**
+   * Grid ordinals with no sample — requested past the last frame, not faked. A run
+   * that was not cancelled accounts for its whole grid:
+   * `materialized + skipped.length === expected`.
+   */
   readonly skipped: number[];
 }
 

frontend/media/src/mediabunny/worker.ts

@@ -19,6 +19,7 @@ import {
   type VideoInspection,
   type VideoSelection,
 } from "../index.js";
+import { walkGrid } from "../grid-walk.js";
 import { refusedInspection, type FromWorker, type ToWorker } from "./protocol.js";
 
 /**
@@ -122,7 +123,6 @@ async function materialize(file: File, selection: VideoSelection): Promise<numbe
       grid.map((clipRelative) => first + clipRelative),
     ) as AsyncGenerator<{ canvas: OffscreenCanvas; timestamp: number } | null>;
 
-    const skipped: number[] = [];
     let chunk: MaterializedFrame[] = [];
 
     // Never after a cancel: the decode loop can be several awaits deep when one
@@ -134,31 +134,28 @@ async function materialize(file: File, selection: VideoSelection): Promise<numbe
       await awaitAck();
     }
 
-    for (let position = 0; position < grid.length && !cancelled; position++) {
-      const next = await frames.next();
-      if (next.done === true) break;
-      const requestedTimestamp = grid[position];
-      // `gridTimestamps` yields `i / fps`; the ordinal a frame carries is that `i`.
-      const ordinal = Math.round(requestedTimestamp * selection.extractionFps);
-      if (next.value === null) {
-        skipped.push(ordinal);
-        continue;
-      }
-      const bytes = await next.value.canvas.convertToBlob({
-        type: VIDEO_FRAME_MEDIA_TYPE,
-        quality: VIDEO_FRAME_QUALITY,
-      });
-      chunk.push({
-        ordinal,
-        requestedTimestamp,
-        sourceTimestamp: next.value.timestamp - first,
-        width,
-        height,
-        format: "jpeg",
-        bytes,
-      });
-      if (chunk.length === FRAME_CHUNK_SIZE) await flush();
-    }
+    const skipped = await walkGrid(
+      grid,
+      selection.extractionFps,
+      frames,
+      async (sample, ordinal, requestedTimestamp) => {
+        const bytes = await sample.canvas.convertToBlob({
+          type: VIDEO_FRAME_MEDIA_TYPE,
+          quality: VIDEO_FRAME_QUALITY,
+        });
+        chunk.push({
+          ordinal,
+          requestedTimestamp,
+          sourceTimestamp: sample.timestamp - first,
+          width,
+          height,
+          format: "jpeg",
+          bytes,
+        });
+        if (chunk.length === FRAME_CHUNK_SIZE) await flush();
+      },
+      () => cancelled,
+    );
 
     await flush();
     return skipped;