From 8de1f50f9003ab0786a9978921ee8cca934d814c Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:39:15 -0700 Subject: [PATCH 1/2] fix(media): count grid ordinals as skipped when the sample stream ends early (#850) The worker's materialize() loop broke out when the decoder's sample generator finished before the grid did, so every remaining ordinal was in neither the materialized frames nor `skipped`. The loop now lives in a pure core helper, walkGrid(), which records each remaining ordinal as skipped unless the run was cancelled, and computes the ordinal in one place for visited, null and padded positions. walkGrid has vitest coverage for null samples, an early end, an empty stream and both cancel paths. The doc on VideoImportResult.skipped now states materialized + skipped.length === expected for an uncancelled run. --- frontend/media/src/grid-walk.test.ts | 92 +++++++++++++++++++++++++ frontend/media/src/grid-walk.ts | 34 +++++++++ frontend/media/src/index.ts | 6 +- frontend/media/src/mediabunny/worker.ts | 49 +++++++------ 4 files changed, 154 insertions(+), 27 deletions(-) create mode 100644 frontend/media/src/grid-walk.test.ts create mode 100644 frontend/media/src/grid-walk.ts diff --git a/frontend/media/src/grid-walk.test.ts b/frontend/media/src/grid-walk.test.ts new file mode 100644 index 00000000..ddfdbd15 --- /dev/null +++ b/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(samples: readonly (S | null)[]): AsyncGenerator { + 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 { + 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([]); + }); +}); diff --git a/frontend/media/src/grid-walk.ts b/frontend/media/src/grid-walk.ts new file mode 100644 index 00000000..fd8154c0 --- /dev/null +++ b/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( + grid: readonly number[], + fps: number, + samples: AsyncIterator, + visit: (sample: S, ordinal: number, requestedTimestamp: number) => Promise, + cancelled: () => boolean, +): Promise { + // `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; +} diff --git a/frontend/media/src/index.ts b/frontend/media/src/index.ts index 9bbae3a8..5ddc0d4e 100644 --- a/frontend/media/src/index.ts +++ b/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[]; } diff --git a/frontend/media/src/mediabunny/worker.ts b/frontend/media/src/mediabunny/worker.ts index 384ec859..984320e7 100644 --- a/frontend/media/src/mediabunny/worker.ts +++ b/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 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 { + 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; From a712dda8c9da7169c1e9e848478a63b95d5c47d5 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:09:09 -0700 Subject: [PATCH 2/2] test(media): drive real null samples through materialize() in Chromium (#850) Every browser case asserted an empty `skipped`, so the null-sample path was covered only by the walkGrid unit tests. A VP8 clip cut mid-GOP, built in the page with mediabunny, leaves three grid ordinals with no key frame to decode from; the spec asserts they land in `skipped` and that materialized plus skipped equals expected. --- frontend/media/browser/materialize.spec.ts | 34 +++++++++++++ frontend/media/browser/page-helpers.js | 56 ++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/frontend/media/browser/materialize.spec.ts b/frontend/media/browser/materialize.spec.ts index 67139d47..69d292ad 100644 --- a/frontend/media/browser/materialize.spec.ts +++ b/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]); + }); +}); diff --git a/frontend/media/browser/page-helpers.js b/frontend/media/browser/page-helpers.js index 3a64d688..20e32084 100644 --- a/frontend/media/browser/page-helpers.js +++ b/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"), + }; +}