From 81055167c8beeb55a53679f2303cac8db055e072 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 31 Jul 2026 22:34:27 +0000 Subject: [PATCH] fix(producer): correct short VFR frame coverage --- packages/engine/src/index.ts | 1 + .../src/services/videoFrameExtractor.test.ts | 40 +++++++++ .../src/services/videoFrameExtractor.ts | 26 +++++- .../render/videoFrameCoverage.test.ts | 84 ++++++++++++++++++- .../src/services/render/videoFrameCoverage.ts | 33 ++++++-- 5 files changed, 176 insertions(+), 8 deletions(-) diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index b479920dc8..30455caf7f 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -179,6 +179,7 @@ export { parseImageElements, extractVideoFramesRange, extractAllVideoFrames, + extractionFrameCountForDuration, resolveProjectRelativeSrc, getFrameAtTime, createFrameLookupTable, diff --git a/packages/engine/src/services/videoFrameExtractor.test.ts b/packages/engine/src/services/videoFrameExtractor.test.ts index d494565db9..d1a7926700 100644 --- a/packages/engine/src/services/videoFrameExtractor.test.ts +++ b/packages/engine/src/services/videoFrameExtractor.test.ts @@ -19,6 +19,7 @@ import { parseImageElements, extractAllVideoFrames, extractVideoFramesRange, + extractionFrameCountForDuration, createFrameLookupTable, resolveProjectRelativeSrc, resolveFrameFormat, @@ -45,6 +46,22 @@ import { COMPLETE_SENTINEL, GC_MARKER, SCHEMA_PREFIX } from "./extractionCache.j // synthesized VFR fixture. const HAS_FFMPEG = spawnSync("ffmpeg", ["-version"]).status === 0; +describe("extractionFrameCountForDuration", () => { + it("uses the same VFR ceil and CFR nearest-boundary rules as FFmpeg", () => { + expect(extractionFrameCountForDuration(0.466666, 30, true)).toBe(14); + expect(extractionFrameCountForDuration(0.466666, 30, false)).toBe(14); + expect(extractionFrameCountForDuration(0.616666, 30, true)).toBe(19); + expect(extractionFrameCountForDuration(0.616666, 30, false)).toBe(18); + }); + + it("fails closed for invalid durations and emits one frame for positive sub-frame work", () => { + expect(extractionFrameCountForDuration(Number.NaN, 30, true)).toBe(0); + expect(extractionFrameCountForDuration(1, 0, true)).toBe(0); + expect(extractionFrameCountForDuration(0, 30, true)).toBe(0); + expect(extractionFrameCountForDuration(0.001, 30, false)).toBe(1); + }); +}); + describe("video extraction failure taxonomy and bounded retry", () => { it("classifies missing and transient HTTP sources without exposing retry ambiguity", () => { expect(classifyVideoExtractionError(new Error("HTTP 404: Not Found"))).toMatchObject({ @@ -1558,6 +1575,29 @@ describe.skipIf(!HAS_FFMPEG)("extractAllVideoFrames on a VFR source", () => { expect(supersetDirNames(outputDir)).toEqual([]); }, 60_000); + it("uses VFR ceil frame counts when slicing overlapping short trims from a superset", async () => { + const outputDir = join(FIXTURE_DIR, "out-vfr-superset-short"); + mkdirSync(outputDir, { recursive: true }); + + const result = await extractAllVideoFrames( + [ + cfrClipElement("vfr-short-a", VFR_FIXTURE, 0.616666, 0), + cfrClipElement("vfr-short-b", VFR_FIXTURE, 0.616666, 0.1), + ], + FIXTURE_DIR, + { fps: 30, outputDir }, + ); + + expect(result.errors).toEqual([]); + expect(extractedFor(result, "vfr-short-a").metadata.isVFR).toBe(true); + expect(extractedFor(result, "vfr-short-a").totalFrames).toBe(19); + expect(extractedFor(result, "vfr-short-b").totalFrames).toBe(19); + expect(statSync(framePath(result, "vfr-short-a", 3)).ino).toBe( + statSync(framePath(result, "vfr-short-b", 0)).ino, + ); + expect(supersetDirNames(outputDir)).toEqual([]); + }, 60_000); + it("publishes overlapping superset slices to cache entries and hits them on the next render", async () => { const CACHE_DIR = mkdtempSync(join(tmpdir(), "hf-extract-superset-cache-test-")); const SRC = await synthCfrClip("superset-cache-src.mp4", 10); diff --git a/packages/engine/src/services/videoFrameExtractor.ts b/packages/engine/src/services/videoFrameExtractor.ts index a3d7df8699..5f08f87f89 100644 --- a/packages/engine/src/services/videoFrameExtractor.ts +++ b/packages/engine/src/services/videoFrameExtractor.ts @@ -77,6 +77,26 @@ export function isVideoFrameFormat(value: unknown): value is VideoFrameFormat { return typeof value === "string" && (VIDEO_FRAME_FORMATS as readonly string[]).includes(value); } +/** + * Resolve the frame count produced for a requested extraction duration. + * + * CFR extraction uses FFmpeg's fps filter, whose end boundary rounds to the + * nearest frame. The VFR path normalizes with `-fps_mode cfr -r`, whose end + * boundary rounds up. Keep this calculation shared by superset slicing and + * producer coverage accounting so a complete VFR extraction cannot be + * rejected because the two paths disagree by one frame. + */ +export function extractionFrameCountForDuration( + durationSeconds: number, + fps: number, + isVFR: boolean, +): number { + if (!Number.isFinite(durationSeconds) || !Number.isFinite(fps)) return 0; + if (durationSeconds <= 0 || fps <= 0) return 0; + const frames = isVFR ? Math.ceil(durationSeconds * fps) : Math.round(durationSeconds * fps); + return Math.max(1, frames); +} + export interface ExtractionOptions { fps: number; outputDir: string; @@ -942,7 +962,11 @@ function sliceSupersetMember( // offset_i + k, so its source time is // baseStart + (offset_i + k) / fps = mediaStart_i + k / fps. // The frame-alignment precondition is what makes offset_i integral. - const requestedFrames = Math.round(work.videoDuration * fps); + const requestedFrames = extractionFrameCountForDuration( + work.videoDuration, + fps, + work.metadata.isVFR, + ); const availableFrames = Math.max(0, superset.totalFrames - member.offsetFrames); const frameCount = Math.min(requestedFrames, availableFrames); for (let i = 0; i < frameCount; i += 1) { diff --git a/packages/producer/src/services/render/videoFrameCoverage.test.ts b/packages/producer/src/services/render/videoFrameCoverage.test.ts index 254607a1dc..89fc08af99 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.test.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.test.ts @@ -150,7 +150,7 @@ describe("computeVideoFrameCoverage", () => { expect(() => assertVideoFrameCoverage(reports, 0.95)).not.toThrow(); }); - it("keeps ceil coverage for the VFR extraction branch", () => { + it("tolerates a single FFmpeg boundary frame on a short 18/19 VFR extraction", () => { const videos = [makeVideo({ id: "short-vfr", start: 0, end: 0.616666 })]; const reports = computeVideoFrameCoverage( videos, @@ -162,7 +162,7 @@ describe("computeVideoFrameCoverage", () => { capturedFrames: 18, ratio: 18 / 19, }); - expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); + expect(() => assertVideoFrameCoverage(reports, 0.95)).not.toThrow(); }); it("still fails closed when a positive sub-frame clip captured zero frames", () => { @@ -312,6 +312,86 @@ describe("assertVideoFrameCoverage", () => { expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); }); + it.each([ + [13, 14], + [18, 19], + ])( + "tolerates exactly one nonzero boundary frame for a short clip (%i/%i)", + (capturedFrames, expectedFrames) => { + const reports = [ + { + videoId: "short-boundary", + clipStart: 0, + clipEnd: expectedFrames / 30, + expectedFrames, + capturedFrames, + ratio: capturedFrames / expectedFrames, + }, + ]; + expect(() => assertVideoFrameCoverage(reports, 0.95)).not.toThrow(); + }, + ); + + it("does not treat a one-frame deficit as tolerance when it represents major loss", () => { + const reports = [ + { + videoId: "major-loss", + clipStart: 0, + clipEnd: 2 / 30, + expectedFrames: 2, + capturedFrames: 1, + ratio: 0.5, + }, + ]; + expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); + }); + + it("does not tolerate two missing frames, zero captured frames, or an exact threshold", () => { + const report = { + videoId: "short-incomplete", + clipStart: 0, + clipEnd: 19 / 30, + expectedFrames: 19, + capturedFrames: 17, + ratio: 17 / 19, + }; + expect(() => assertVideoFrameCoverage([report], 0.95)).toThrow(VideoFrameCoverageError); + expect(() => + assertVideoFrameCoverage([{ ...report, capturedFrames: 0, ratio: 0 }], 0.95), + ).toThrow(VideoFrameCoverageError); + expect(() => + assertVideoFrameCoverage([{ ...report, capturedFrames: 18, ratio: 18 / 19 }], 1), + ).toThrow(VideoFrameCoverageError); + }); + + it("does not apply the one-frame tolerance to longer clips", () => { + const reports = [ + { + videoId: "long-boundary", + clipStart: 0, + clipEnd: 21 / 30, + expectedFrames: 21, + capturedFrames: 20, + ratio: 20 / 21, + }, + ]; + expect(() => assertVideoFrameCoverage(reports, 0.99)).toThrow(VideoFrameCoverageError); + }); + + it("still rejects a material long-clip shortfall even when it is five frames", () => { + const reports = [ + { + videoId: "long-partial", + clipStart: 0, + clipEnd: 89 / 30, + expectedFrames: 89, + capturedFrames: 84, + ratio: 84 / 89, + }, + ]; + expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); + }); + it("respects a threshold override — 0.5 passes 60% coverage", () => { const reports = [ { diff --git a/packages/producer/src/services/render/videoFrameCoverage.ts b/packages/producer/src/services/render/videoFrameCoverage.ts index 455ccfcf42..3952b758bd 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.ts @@ -45,7 +45,14 @@ */ import { parseHTML } from "linkedom"; -import type { ExtractedFrames, VideoElement } from "@hyperframes/engine"; +import { + extractionFrameCountForDuration, + type ExtractedFrames, + type VideoElement, +} from "@hyperframes/engine"; + +const SHORT_CLIP_ONE_FRAME_TOLERANCE_MIN_EXPECTED_FRAMES = 14; +const SHORT_CLIP_ONE_FRAME_TOLERANCE_MAX_EXPECTED_FRAMES = 20; export interface VideoFrameCoverageReport { videoId: string; @@ -134,9 +141,20 @@ export function expectedFramesForClip( if (fps <= 0) return 0; const duration = Math.max(0, end - start); if (duration === 0) return 0; - const frameCount = - rounding === "nearest" ? Math.round(duration * fps) : Math.ceil(duration * fps); - return Math.max(1, frameCount); + return extractionFrameCountForDuration(duration, fps, rounding === "ceil"); +} + +function isToleratedShortClipBoundaryMiss( + report: VideoFrameCoverageReport, + threshold: number, +): boolean { + return ( + threshold < 1 && + report.capturedFrames > 0 && + report.expectedFrames >= SHORT_CLIP_ONE_FRAME_TOLERANCE_MIN_EXPECTED_FRAMES && + report.expectedFrames <= SHORT_CLIP_ONE_FRAME_TOLERANCE_MAX_EXPECTED_FRAMES && + report.expectedFrames - report.capturedFrames === 1 + ); } function expectedFramesForVideo( @@ -203,7 +221,12 @@ export function assertVideoFrameCoverage( threshold: number | null, ): void { if (threshold === null) return; - const failed = reports.filter((report) => report.expectedFrames > 0 && report.ratio < threshold); + const failed = reports.filter( + (report) => + report.expectedFrames > 0 && + report.ratio < threshold && + !isToleratedShortClipBoundaryMiss(report, threshold), + ); if (failed.length === 0) return; // Sort ascending by ratio so the "worst" is first — that's what we cite // in the message and pin on the error details for telemetry.