diff --git a/packages/producer/src/services/render/videoFrameCoverage.test.ts b/packages/producer/src/services/render/videoFrameCoverage.test.ts index 9b018d17e9..254607a1dc 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.test.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.test.ts @@ -1,3 +1,5 @@ +// fallow-ignore-file code-duplication + import type { ExtractedFrames, VideoElement, VideoMetadata } from "@hyperframes/engine"; import { describe, expect, it } from "vitest"; import { @@ -35,9 +37,9 @@ function makeVideo(overrides: Partial & { id: string }): VideoElem function makeExtracted( videoId: string, delivered: number, - options: { fps?: number; durationSeconds?: number } = {}, + options: { fps?: number; durationSeconds?: number; isVFR?: boolean } = {}, ): ExtractedFrames { - const { fps = 30, durationSeconds = Number.POSITIVE_INFINITY } = options; + const { fps = 30, durationSeconds = Number.POSITIVE_INFINITY, isVFR = false } = options; const framePaths = new Map(); for (let i = 0; i < delivered; i += 1) framePaths.set(i, `/tmp/${videoId}/${i}.jpg`); const metadata: VideoMetadata = { @@ -48,7 +50,7 @@ function makeExtracted( fps, videoCodec: "h264", hasAudio: false, - isVFR: false, + isVFR, hasAlpha: false, colorSpace: null, }; @@ -72,9 +74,23 @@ describe("expectedFramesForClip", () => { expect(expectedFramesForClip(2, 1, 30)).toBe(0); // negative window collapses to 0 }); - it("ceils fractional-fps windows so a 29.97fps 1s clip demands 30 frames", () => { + it("defaults to fail-closed ceil rounding", () => { expect(expectedFramesForClip(0, 1, 29.97)).toBe(30); expect(expectedFramesForClip(0, 5, 30)).toBe(150); + expect(expectedFramesForClip(0, 0.616666, 30)).toBe(19); + expect(expectedFramesForClip(0, 0.316666, 30)).toBe(10); + }); + + it("matches the CFR fps filter's nearest-boundary rounding when requested", () => { + expect(expectedFramesForClip(0, 0.616666, 30, "nearest")).toBe(18); + expect(expectedFramesForClip(0, 0.316666, 30, "nearest")).toBe(9); + expect(expectedFramesForClip(0, 0.633333, 30, "nearest")).toBe(19); + }); + + it("requires one frame for every positive sub-frame clip", () => { + expect(expectedFramesForClip(0, 0.001, 30)).toBe(1); + expect(expectedFramesForClip(0, 0.001, 30, "nearest")).toBe(1); + expect(expectedFramesForClip(1, 1, 30)).toBe(0); }); }); @@ -123,6 +139,43 @@ describe("computeVideoFrameCoverage", () => { }); }); + it("reports full coverage for a short CFR clip matching FFmpeg boundary rounding", () => { + const videos = [makeVideo({ id: "short", start: 0, end: 0.616666 })]; + const reports = computeVideoFrameCoverage(videos, [makeExtracted("short", 18)], 30); + expect(reports[0]).toMatchObject({ + expectedFrames: 18, + capturedFrames: 18, + ratio: 1, + }); + expect(() => assertVideoFrameCoverage(reports, 0.95)).not.toThrow(); + }); + + it("keeps ceil coverage for the VFR extraction branch", () => { + const videos = [makeVideo({ id: "short-vfr", start: 0, end: 0.616666 })]; + const reports = computeVideoFrameCoverage( + videos, + [makeExtracted("short-vfr", 18, { isVFR: true })], + 30, + ); + expect(reports[0]).toMatchObject({ + expectedFrames: 19, + capturedFrames: 18, + ratio: 18 / 19, + }); + expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); + }); + + it("still fails closed when a positive sub-frame clip captured zero frames", () => { + const videos = [makeVideo({ id: "blank-sub-frame", start: 0, end: 0.001 })]; + const reports = computeVideoFrameCoverage(videos, [makeExtracted("blank-sub-frame", 0)], 30); + expect(reports[0]).toMatchObject({ + expectedFrames: 1, + capturedFrames: 0, + ratio: 0, + }); + expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); + }); + it("reports 0 capturedFrames when a video was never extracted (injection failure)", () => { // Field signal ts=1784139267: later-injected video clips silently drop // out of the extractor under injection-count saturation. diff --git a/packages/producer/src/services/render/videoFrameCoverage.ts b/packages/producer/src/services/render/videoFrameCoverage.ts index 3e8f1896d6..455ccfcf42 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.ts @@ -71,6 +71,8 @@ export interface VideoFrameCoverageErrorDetails { } export class VideoFrameCoverageError extends Error { + // Read structurally by isVideoFrameCoverageError and cross-module callers. + // fallow-ignore-next-line unused-class-member readonly hyperframesVideoFrameCoverageError = true as const; readonly threshold: number; readonly worst: VideoFrameCoverageReport; @@ -117,16 +119,47 @@ export function resolveVideoCoverageThreshold( } /** - * Ceil the clip's authored `[start,end)` window at `fps` — the number of - * captured render frames whose center-time falls inside the window. Kept - * separate so a caller (or a test) can override it if a composition uses - * a non-integer fps whose sampling makes the naive count off by one. + * Count frames in a clip's authored `[start,end)` window. CFR extraction uses + * FFmpeg's `-vf fps=` and rounds to the nearest output-frame boundary; + * VFR extraction uses `-fps_mode cfr -r ` and rounds up. Missing entries + * retain the fail-closed `ceil` default. Positive sub-frame clips emit one. */ -export function expectedFramesForClip(start: number, end: number, fps: number): number { +export function expectedFramesForClip( + start: number, + end: number, + fps: number, + rounding: "ceil" | "nearest" = "ceil", +): number { if (!Number.isFinite(start) || !Number.isFinite(end) || !Number.isFinite(fps)) return 0; if (fps <= 0) return 0; const duration = Math.max(0, end - start); - return Math.ceil(duration * fps); + if (duration === 0) return 0; + const frameCount = + rounding === "nearest" ? Math.round(duration * fps) : Math.ceil(duration * fps); + return Math.max(1, frameCount); +} + +function expectedFramesForVideo( + video: VideoElement, + entry: ExtractedFrames | undefined, + fps: number, +): number { + const rounding = entry && !entry.metadata.isVFR ? "nearest" : "ceil"; + const slotFrames = expectedFramesForClip(video.start, video.end, fps, rounding); + if (!entry) return slotFrames; + + // A short source in a longer slot has a legitimate delivery ceiling of + // the source portion, not the full slot: a non-looping clip holds its + // final decoded frame across the tail (#2516/#2606), and a looping clip + // reuses its full source frame set per repeat (#2665). In both cases the + // full source *has* been delivered — the same 90 unique source frames + // cover the 300-frame slot — so coverage must measure source-source, not + // slot-source. + const sourceDuration = entry.metadata.durationSeconds - video.mediaStart; + if (!Number.isFinite(sourceDuration) || sourceDuration <= 0) return slotFrames; + + const sourceFrames = expectedFramesForClip(0, sourceDuration, fps, rounding); + return Math.min(slotFrames, sourceFrames); } export function computeVideoFrameCoverage( @@ -140,20 +173,7 @@ export function computeVideoFrameCoverage( const reports: VideoFrameCoverageReport[] = []; for (const video of videos) { const entry = byId.get(video.id); - const slotFrames = expectedFramesForClip(video.start, video.end, fps); - // A short source in a longer slot has a legitimate delivery ceiling of - // the source portion, not the full slot: a non-looping clip holds its - // final decoded frame across the tail (#2516/#2606), and a looping clip - // reuses its full source frame set per repeat (#2665). In both cases the - // full source *has* been delivered — the same 90 unique source frames - // cover the 300-frame slot — so coverage must measure source-source, not - // slot-source. A missing extraction (no `entry`) still requires the full - // slot; there's no delivered set to credit. - const sourceDuration = entry ? entry.metadata.durationSeconds - video.mediaStart : NaN; - const hasUsableSourceDuration = Number.isFinite(sourceDuration) && sourceDuration > 0; - const sourceFrames = - entry && hasUsableSourceDuration ? expectedFramesForClip(0, sourceDuration, fps) : slotFrames; - const expectedFrames = entry ? Math.min(slotFrames, sourceFrames) : slotFrames; + const expectedFrames = expectedFramesForVideo(video, entry, fps); // 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