Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 56 additions & 3 deletions packages/producer/src/services/render/videoFrameCoverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,27 @@ function makeVideo(overrides: Partial<VideoElement> & { id: string }): VideoElem
};
}

function makeExtracted(videoId: string, delivered: number, fps = 30): ExtractedFrames {
// `durationSeconds` defaults to Infinity, not `delivered / fps` — the two are
// unrelated in production (source duration comes from ffprobe; `delivered`
// is how many frames the extractor happened to deliver) and coupling them
// here made any test modeling a delivery shortfall silently report full
// coverage unless it remembered to override durationSeconds afterward. An
// unbounded default instead falls into computeVideoFrameCoverage's "no
// usable source duration" branch, which requires the full authored slot —
// the same behavior every test had before source-duration crediting
// existed. Tests that care about a specific source duration (the held-tail
// cases below) pass it explicitly.
function makeExtracted(
videoId: string,
delivered: number,
options: { fps?: number; durationSeconds?: number } = {},
): ExtractedFrames {
const { fps = 30, durationSeconds = Number.POSITIVE_INFINITY } = options;
const framePaths = new Map<number, string>();
for (let i = 0; i < delivered; i += 1) framePaths.set(i, `/tmp/${videoId}/${i}.jpg`);
const metadata: VideoMetadata = {
durationSeconds: delivered / fps,
videoStreamDurationSeconds: delivered / fps,
durationSeconds,
videoStreamDurationSeconds: durationSeconds,
width: 1280,
height: 720,
fps,
Expand Down Expand Up @@ -132,6 +147,44 @@ describe("computeVideoFrameCoverage", () => {
expect(reports[0]!.capturedFrames).toBe(5);
expect(reports[0]!.ratio).toBeCloseTo(5 / 30, 5);
});

it("credits a non-looping held tail against the source portion only", () => {
const videos = [makeVideo({ id: "held", start: 0, end: 10 })];
const extracted = [makeExtracted("held", 90, { durationSeconds: 3 })];
const reports = computeVideoFrameCoverage(videos, extracted, 30);
expect(reports[0]).toMatchObject({ expectedFrames: 90, capturedFrames: 90, ratio: 1 });
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [nit] No test exercises mediaStart > 0. The formula on line 152 (durationSeconds - video.mediaStart) is only exercised at mediaStart = 0 in all three new tests (the default from makeVideo). A sibling test with, say, a 5s source, mediaStart=1, 10s non-looping slot would confirm sourceFrames = ceil((5 - 1) * 30) = 120 (not 150) — that path is otherwise dark.

— Rames D Jusso


it("still requires the full authored slot for looping clips", () => {
const videos = [makeVideo({ id: "loop", start: 0, end: 10, loop: true })];
const extracted = [makeExtracted("loop", 90, { durationSeconds: 3 })];
const reports = computeVideoFrameCoverage(videos, extracted, 30);
expect(reports[0]).toMatchObject({ expectedFrames: 300, capturedFrames: 90 });
expect(reports[0]!.ratio).toBeCloseTo(0.3, 5);
});

it("still fails when extraction is truncated before the held-tail source", () => {
const videos = [makeVideo({ id: "truncated", start: 0, end: 10 })];
const extracted = [makeExtracted("truncated", 60, { durationSeconds: 3 })];
const reports = computeVideoFrameCoverage(videos, extracted, 30);
expect(reports[0]).toMatchObject({ expectedFrames: 90, capturedFrames: 60 });
expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError);
});

it("fails closed for invalid or exhausted source durations", () => {
const videos = [
makeVideo({ id: "zero", start: 0, end: 10 }),
makeVideo({ id: "trimmed-away", start: 0, end: 10, mediaStart: 4 }),
];
const extracted = [
makeExtracted("zero", 0, { durationSeconds: 0 }),
makeExtracted("trimmed-away", 30, { durationSeconds: 3 }),
];
const reports = computeVideoFrameCoverage(videos, extracted, 30);
expect(reports[0]).toMatchObject({ expectedFrames: 300, capturedFrames: 0 });
expect(reports[1]).toMatchObject({ expectedFrames: 300, capturedFrames: 30 });
expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError);
});
});

describe("assertVideoFrameCoverage", () => {
Expand Down
13 changes: 12 additions & 1 deletion packages/producer/src/services/render/videoFrameCoverage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,18 @@ export function computeVideoFrameCoverage(
const reports: VideoFrameCoverageReport[] = [];
for (const video of videos) {
const entry = byId.get(video.id);
const expectedFrames = expectedFramesForClip(video.start, video.end, fps);
const slotFrames = expectedFramesForClip(video.start, video.end, fps);
// Non-looping clips intentionally hold their final decoded frame when the
// authored slot outlasts the source (#2516). Coverage must therefore
// measure the source portion, while still requiring the full slot for
// looping clips and for missing extractions (where no hold is possible).
const sourceDuration = entry ? entry.metadata.durationSeconds - video.mediaStart : NaN;
const hasUsableSourceDuration = Number.isFinite(sourceDuration) && sourceDuration > 0;
const sourceFrames =
entry && !video.loop && hasUsableSourceDuration
? expectedFramesForClip(0, sourceDuration, fps)
: slotFrames;
const expectedFrames = entry && !video.loop ? Math.min(slotFrames, sourceFrames) : slotFrames;
// framePaths is a Map — `size` is the number of distinct captured frames
// delivered to the runtime injector, which is the load-bearing count
// (some extractors report a total that includes cache-hit-skipped frames
Expand Down
Loading