From 127eb193715a79252ac77969a95d431a89fee4fe Mon Sep 17 00:00:00 2001 From: James Russo Date: Mon, 3 Aug 2026 18:42:03 -0700 Subject: [PATCH] fix(producer): correct short VFR frame coverage (#2936) * fix(producer): correct short VFR frame coverage * fix(engine): keep VFR extraction seek-local * fix(engine): match ffmpeg decimal frame boundaries * fix(engine): preserve exact extraction frame rates * fix(engine): key frame cache by exact rate --- packages/engine/src/index.ts | 1 + .../src/services/extractionCache.test.ts | 26 +- .../engine/src/services/extractionCache.ts | 11 +- .../src/services/videoFrameExtractor.test.ts | 265 ++++++++++++++++++ .../src/services/videoFrameExtractor.ts | 124 +++++++- .../render/stages/extractVideosStage.ts | 10 +- .../render/videoFrameCoverage.test.ts | 102 ++++++- .../src/services/render/videoFrameCoverage.ts | 40 ++- .../src/services/renderOrchestrator.ts | 6 +- 9 files changed, 543 insertions(+), 42 deletions(-) diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index d8e554062..7a1630659 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -185,6 +185,7 @@ export { resolveFinalFrameExtractionWindow, resolveVideoExtractionDuration, resolvePlayableVideoDuration, + extractionFrameCountForDuration, resolveProjectRelativeSrc, getFrameAtTime, createFrameLookupTable, diff --git a/packages/engine/src/services/extractionCache.test.ts b/packages/engine/src/services/extractionCache.test.ts index f1f544a4a..0842204c1 100644 --- a/packages/engine/src/services/extractionCache.test.ts +++ b/packages/engine/src/services/extractionCache.test.ts @@ -37,7 +37,7 @@ const keyFor = (videoPath: string, overrides: Partial = {}): Cach size: stat.size, mediaStart: 0, duration: 3, - fps: 30, + fps: "30", format: "jpg", ...overrides, }; @@ -63,8 +63,8 @@ function seedPartialDir(entry: { dir: string; keyHash: string }, frameContent: s } describe("extractionCache constants", () => { - it("exposes the v2 schema prefix", () => { - expect(SCHEMA_PREFIX).toBe("hfcache-v3-"); + it("exposes the v4 schema prefix", () => { + expect(SCHEMA_PREFIX).toBe("hfcache-v4-"); }); it("exposes the frame filename prefix shared with the extractor", () => { @@ -123,10 +123,16 @@ describe("computeCacheKey", () => { it("changes when fps changes (different frame count invalidates key)", () => { const a = computeCacheKey(base(sourceFile)); - const b = computeCacheKey({ ...base(sourceFile), fps: 60 }); + const b = computeCacheKey({ ...base(sourceFile), fps: "60" }); expect(a).not.toBe(b); }); + it("keeps exact rational rates distinct from their JavaScript decimal", () => { + const rational = computeCacheKey({ ...base(sourceFile), fps: "30000/1001" }); + const decimal = computeCacheKey({ ...base(sourceFile), fps: String(30000 / 1001) }); + expect(rational).not.toBe(decimal); + }); + it("changes when format changes", () => { const a = computeCacheKey(base(sourceFile)); const b = computeCacheKey({ ...base(sourceFile), format: "png" }); @@ -200,6 +206,18 @@ describe("lookupCacheEntry / markCacheEntryComplete", () => { const base = (videoPath: string): CacheKeyInput => keyFor(videoPath); + it("does not reuse a complete entry from the v3 numeric-fps namespace", () => { + const input = base(sourceFile); + const keyHash = computeCacheKey(input); + const staleV3Dir = join(tmpRoot, `hfcache-v3-${keyHash.slice(0, 16)}`); + mkdirSync(staleV3Dir, { recursive: true }); + writeFileSync(join(staleV3Dir, COMPLETE_SENTINEL), "", "utf-8"); + + const lookup = lookupCacheEntry(tmpRoot, input); + expect(lookup.hit).toBe(false); + expect(lookup.entry.dir).toBe(join(tmpRoot, `${SCHEMA_PREFIX}${keyHash.slice(0, 16)}`)); + }); + it("misses on an empty cache root", () => { const lookup = lookupCacheEntry(tmpRoot, base(sourceFile)); expect(lookup.hit).toBe(false); diff --git a/packages/engine/src/services/extractionCache.ts b/packages/engine/src/services/extractionCache.ts index a2456087a..ba5dcb70b 100644 --- a/packages/engine/src/services/extractionCache.ts +++ b/packages/engine/src/services/extractionCache.ts @@ -62,8 +62,11 @@ export const GC_MARKER = ".hf-last-gc"; * VFR-to-CFR re-encode, changing frame contents for VFR sources under * identical key tuples. Without the bump, warm v2 entries (two-pass frames) * would keep being served across the deploy boundary. + * v3 -> v4: the target fps identity is the exact FFmpeg argument instead of + * a JavaScript number. This invalidates entries created after rational NTSC + * rates had already been rounded to a decimal. */ -export const SCHEMA_PREFIX = "hfcache-v3-"; +export const SCHEMA_PREFIX = "hfcache-v4-"; /** Truncated hex chars of SHA-256 used for the entry directory name. */ const KEY_HEX_CHARS = 16; @@ -84,8 +87,8 @@ export interface CacheKeyInput { * so callers that pass an unresolved "natural duration" still produce a * stable key across invocations. */ duration: number; - /** Target output frames-per-second. */ - fps: number; + /** Exact target output frame-rate argument (for example `30000/1001`). */ + fps: string; /** Output image format. */ format: CacheFrameFormat; /** Optional source transform applied during extraction. */ @@ -136,7 +139,7 @@ function canonicalKeyBlob(input: CacheKeyInput): string { s: number; ms: number; d: number; - f: number; + f: string; fmt: CacheFrameFormat; t?: string; } = { diff --git a/packages/engine/src/services/videoFrameExtractor.test.ts b/packages/engine/src/services/videoFrameExtractor.test.ts index 437af5ffe..a5470f14a 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, @@ -307,6 +308,51 @@ describe("resolveVideoExtractionDuration", () => { }); }); +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.each([ + [0.33 - 0.03, 30, 9], + [0.29 - 0.04, 24, 6], + [0.35 - 0.05, 60, 18], + [4.03 - 3.53, 30, 15], + [4.03 - 3.78, 24, 6], + ])( + "snaps floating-point integral boundaries before VFR ceil (%s seconds at %i fps)", + (duration, fps, expectedFrames) => { + expect(extractionFrameCountForDuration(duration, fps, true)).toBe(expectedFrames); + }, + ); + + it("still ceils a genuine fractional boundary beyond floating-point noise", () => { + expect(extractionFrameCountForDuration(0.300001, 30, true)).toBe(10); + }); + + it("matches FFmpeg's six-digit duration parsing", () => { + expect(extractionFrameCountForDuration(0.6000009, 30, true)).toBe(18); + expect(extractionFrameCountForDuration(0.600001, 30, true)).toBe(19); + expect(extractionFrameCountForDuration(2.05, 30, false)).toBe(62); + }); + + it("keeps exact NTSC rationals at short CFR and VFR boundaries", () => { + expect(extractionFrameCountForDuration(0.25025, { num: 30000, den: 1001 }, false)).toBe(8); + expect(extractionFrameCountForDuration(0.125125, { num: 24000, den: 1001 }, true)).toBe(3); + expect(extractionFrameCountForDuration(0.5005, { num: 24000, den: 1001 }, true)).toBe(12); + }); + + 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({ @@ -1521,6 +1567,20 @@ describe.skipIf(!HAS_FFMPEG)("extractAllVideoFrames on a VFR source", () => { expect(md.isVFR).toBe(true); }); + it("passes an exact 24000/1001 rate through VFR normalization", async () => { + const outputDir = join(FIXTURE_DIR, "out-vfr-ntsc-boundary"); + mkdirSync(outputDir, { recursive: true }); + + const result = await extractVideoFramesRange(VFR_FIXTURE, "vfr-ntsc-boundary", 0, 0.125125, { + fps: { num: 24000, den: 1001 }, + outputDir, + format: "jpg", + }); + + expect(result.metadata.isVFR).toBe(true); + expect(result.totalFrames).toBe(3); + }, 60_000); + it("produces the expected frame count for a mid-file segment", async () => { const outputDir = join(FIXTURE_DIR, "out-mid-segment"); mkdirSync(outputDir, { recursive: true }); @@ -1735,6 +1795,49 @@ describe.skipIf(!HAS_FFMPEG)("extractAllVideoFrames on a VFR source", () => { rmSync(CACHE_DIR, { recursive: true, force: true }); }, 60_000); + it("does not reuse a decimal-rate VFR cache entry for the exact rational rate", async () => { + const cacheDir = mkdtempSync(join(tmpdir(), "hf-extract-cache-ntsc-rate-test-")); + const decimalOutputDir = join(FIXTURE_DIR, "out-cache-vfr-ntsc-decimal"); + const rationalOutputDir = join(FIXTURE_DIR, "out-cache-vfr-ntsc-rational"); + mkdirSync(decimalOutputDir, { recursive: true }); + mkdirSync(rationalOutputDir, { recursive: true }); + try { + const decimal = await extractAllVideoFrames( + [cfrClipElement("vfr-ntsc-cache", VFR_FIXTURE, 0.125125)], + FIXTURE_DIR, + { fps: 24000 / 1001, outputDir: decimalOutputDir }, + undefined, + { extractCacheDir: cacheDir }, + ); + expect(decimal.errors).toEqual([]); + expect(decimal.phaseBreakdown.cacheMisses).toBe(1); + expect(decimal.extracted[0]?.totalFrames).toBe(3); + // Model the warm v3 entry from before exact-rate extraction: the + // decimal path could persist one extra frame at this boundary. If the + // rational lookup collides, rehydration below will observe all four. + writeFileSync( + join(decimal.extracted[0]!.outputDir, "frame_00004.jpg"), + "stale-decimal-boundary-frame", + "utf-8", + ); + + const rational = await extractAllVideoFrames( + [cfrClipElement("vfr-ntsc-cache", VFR_FIXTURE, 0.125125)], + FIXTURE_DIR, + { fps: { num: 24000, den: 1001 }, outputDir: rationalOutputDir }, + undefined, + { extractCacheDir: cacheDir }, + ); + expect(rational.errors).toEqual([]); + expect(rational.phaseBreakdown.cacheHits).toBe(0); + expect(rational.phaseBreakdown.cacheMisses).toBe(1); + expect(rational.extracted[0]?.totalFrames).toBe(3); + expect(cacheEntryNames(cacheDir)).toHaveLength(2); + } finally { + rmSync(cacheDir, { recursive: true, force: true }); + } + }, 60_000); + it("reuses one-cycle loop extraction across different authored starts", async () => { const cacheDir = mkdtempSync(join(tmpdir(), "hf-extract-loop-phase-cache-test-")); const src = await synthCfrClip("cache-loop-phase-src.mp4", 3); @@ -2067,6 +2170,97 @@ describe.skipIf(!HAS_FFMPEG)("extractAllVideoFrames on a VFR source", () => { expect(supersetDirNames(outputDir)).toEqual([]); }, 60_000); + it.each([ + { label: "decimal underflow half-frame", duration: 2.05, offset: 1, expectedFrames: 62 }, + { + label: "non-half-integer boundary", + duration: 0.616666, + offset: 0.1, + expectedFrames: 18, + }, + ])( + "keeps direct and superset CFR extraction equal at a $label", + async ({ label, duration, offset, expectedFrames }) => { + const fixtureKey = label.replaceAll(" ", "-"); + const src = await synthCfrClip(`superset-cfr-${fixtureKey}.mp4`, 4); + const groupedOutputDir = join(FIXTURE_DIR, `out-superset-cfr-${fixtureKey}`); + const directOutputDir = join(FIXTURE_DIR, `out-direct-cfr-${fixtureKey}`); + mkdirSync(groupedOutputDir, { recursive: true }); + mkdirSync(directOutputDir, { recursive: true }); + + const grouped = await extractAllVideoFrames( + [ + cfrClipElement(`${fixtureKey}-base`, src, duration, 0), + cfrClipElement(`${fixtureKey}-member`, src, duration, offset), + ], + FIXTURE_DIR, + { fps: 30, outputDir: groupedOutputDir }, + ); + const direct = await extractVideoFramesRange(src, `${fixtureKey}-direct`, offset, duration, { + fps: 30, + outputDir: directOutputDir, + format: "jpg", + }); + + expect(grouped.errors).toEqual([]); + expect(direct.totalFrames).toBe(expectedFrames); + expect(extractedFor(grouped, `${fixtureKey}-base`).totalFrames).toBe(expectedFrames); + expect(extractedFor(grouped, `${fixtureKey}-member`).totalFrames).toBe(expectedFrames); + expect(statSync(framePath(grouped, `${fixtureKey}-base`, Math.round(offset * 30))).ino).toBe( + statSync(framePath(grouped, `${fixtureKey}-member`, 0)).ino, + ); + for (let frame = 0; frame < expectedFrames; frame += 1) { + expect( + readFileSync(framePath(grouped, `${fixtureKey}-member`, frame)).equals( + readFileSync(direct.framePaths.get(frame)!), + ), + ).toBe(true); + } + expect(supersetDirNames(groupedOutputDir)).toEqual([]); + }, + 60_000, + ); + + it("keeps direct and superset CFR extraction equal at 30000/1001", async () => { + const src = await synthCfrClip("superset-cfr-ntsc.mp4", 4); + const groupedOutputDir = join(FIXTURE_DIR, "out-superset-cfr-ntsc"); + const directOutputDir = join(FIXTURE_DIR, "out-direct-cfr-ntsc"); + const fps = { num: 30000, den: 1001 }; + const duration = 0.25025; + mkdirSync(groupedOutputDir, { recursive: true }); + mkdirSync(directOutputDir, { recursive: true }); + + const grouped = await extractAllVideoFrames( + [ + cfrClipElement("ntsc-base", src, 0.5005, 0), + cfrClipElement("ntsc-member", src, duration, 0), + ], + FIXTURE_DIR, + { fps, outputDir: groupedOutputDir }, + ); + const direct = await extractVideoFramesRange(src, "ntsc-direct", 0, duration, { + fps, + outputDir: directOutputDir, + format: "jpg", + }); + + expect(grouped.errors).toEqual([]); + expect(direct.totalFrames).toBe(8); + expect(extractedFor(grouped, "ntsc-base").totalFrames).toBe(15); + expect(extractedFor(grouped, "ntsc-member").totalFrames).toBe(8); + expect(statSync(framePath(grouped, "ntsc-base", 0)).ino).toBe( + statSync(framePath(grouped, "ntsc-member", 0)).ino, + ); + for (let frame = 0; frame < 8; frame += 1) { + expect( + readFileSync(framePath(grouped, "ntsc-member", frame)).equals( + readFileSync(direct.framePaths.get(frame)!), + ), + ).toBe(true); + } + expect(supersetDirNames(groupedOutputDir)).toEqual([]); + }, 60_000); + it("does not superset disjoint trims", async () => { const SRC = await synthCfrClip("superset-disjoint-src.mp4", 10); const outputDir = join(FIXTURE_DIR, "out-superset-disjoint"); @@ -2103,6 +2297,77 @@ describe.skipIf(!HAS_FFMPEG)("extractAllVideoFrames on a VFR source", () => { expect(supersetDirNames(outputDir)).toEqual([]); }, 60_000); + it("keeps overlapping VFR trims direct because CFR resampling phase resets per seek", 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).not.toBe( + statSync(framePath(result, "vfr-short-b", 0)).ino, + ); + expect(supersetDirNames(outputDir)).toEqual([]); + }, 60_000); + + it("keeps batched and direct VFR extraction equal at a floating integral boundary", async () => { + const groupedOutputDir = join(FIXTURE_DIR, "out-vfr-superset-integral-boundary"); + const directOutputDir = join(FIXTURE_DIR, "out-vfr-direct-integral-boundary"); + mkdirSync(groupedOutputDir, { recursive: true }); + mkdirSync(directOutputDir, { recursive: true }); + + const first: VideoElement = { + id: "vfr-integral-a", + src: VFR_FIXTURE, + start: 0.03, + end: 0.33, + mediaStart: 0.03, + loop: false, + hasAudio: false, + }; + const second: VideoElement = { + ...first, + id: "vfr-integral-b", + mediaStart: 0.13, + }; + + const direct = await extractAllVideoFrames([{ ...second }], FIXTURE_DIR, { + fps: 30, + outputDir: directOutputDir, + }); + const grouped = await extractAllVideoFrames([{ ...first }, second], FIXTURE_DIR, { + fps: 30, + outputDir: groupedOutputDir, + }); + + expect(direct.errors).toEqual([]); + expect(grouped.errors).toEqual([]); + expect(extractedFor(direct, second.id).totalFrames).toBe(9); + expect(extractedFor(grouped, first.id).totalFrames).toBe(9); + expect(extractedFor(grouped, second.id).totalFrames).toBe(9); + for (let frame = 0; frame < 9; frame += 1) { + expect( + readFileSync(framePath(grouped, second.id, frame)).equals( + readFileSync(framePath(direct, second.id, frame)), + ), + ).toBe(true); + } + expect(statSync(framePath(grouped, first.id, 3)).ino).not.toBe( + statSync(framePath(grouped, second.id, 0)).ino, + ); + expect(supersetDirNames(groupedOutputDir)).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 6b7169d45..501485aa1 100644 --- a/packages/engine/src/services/videoFrameExtractor.ts +++ b/packages/engine/src/services/videoFrameExtractor.ts @@ -9,7 +9,14 @@ import { copyFileSync, existsSync, linkSync, mkdirSync, readdirSync, rmSync } from "fs"; import { isAbsolute, join, posix, resolve, sep } from "path"; import { parseHTML } from "linkedom"; -import { decodeUrlPathVariants, MEDIA_DURATION_CLAMP_EPSILON_SECONDS } from "@hyperframes/core"; +import { + decodeUrlPathVariants, + fpsToFfmpegArg, + fpsToNumber, + MEDIA_DURATION_CLAMP_EPSILON_SECONDS, + toFps, + type FpsInput, +} from "@hyperframes/core"; import { resolveReferencedStart, type RefResolverEl } from "./referenceResolver.js"; import { extractFinalVideoFrameTimestamp, @@ -81,8 +88,83 @@ 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: FpsInput, + isVFR: boolean, +): number { + if (!Number.isFinite(durationSeconds) || durationSeconds <= 0) return 0; + // FFmpeg receives `String(durationSeconds)` and parses at microsecond + // precision. Derive the integer microseconds from that same decimal text: + // multiplying the binary float first is not equivalent (`2.05 * 1e6` is + // 2049999.9999999998 in JS and would incorrectly truncate one microsecond). + const serialized = String(durationSeconds).toLowerCase(); + const decimal = /^(\d+)(?:\.(\d+))?(?:e([+-]?\d+))?$/.exec(serialized); + if (!decimal) return 0; + const whole = decimal[1] ?? "0"; + const fraction = decimal[2] ?? ""; + const exponent = Number.parseInt(decimal[3] ?? "0", 10); + const digits = BigInt(`${whole}${fraction}`); + const microsecondScale = exponent + 6 - fraction.length; + const microseconds = + microsecondScale >= 0 + ? digits * 10n ** BigInt(microsecondScale) + : digits / 10n ** BigInt(-microsecondScale); + + // Keep the frame-boundary calculation rational too. Converting the exact + // microseconds back to a binary float recreates the same problem at .5-frame + // boundaries (`2.05 * 30` is 61.49999999999999 in JS). + let fpsNumerator: bigint; + let fpsDenominator: bigint; + if (typeof fps === "object") { + if ( + !Number.isSafeInteger(fps.num) || + !Number.isSafeInteger(fps.den) || + fps.num <= 0 || + fps.den <= 0 + ) { + return 0; + } + fpsNumerator = BigInt(fps.num); + fpsDenominator = BigInt(fps.den); + } else { + if (!Number.isFinite(fps) || fps <= 0) return 0; + // Number-only callers retain their decimal FFmpeg argument exactly. The + // production render path supplies Fps, so NTSC rates never round-trip + // through `String(30000 / 1001)` here. + const serializedFps = String(fps).toLowerCase(); + const fpsDecimal = /^(\d+)(?:\.(\d+))?(?:e([+-]?\d+))?$/.exec(serializedFps); + if (!fpsDecimal) return 0; + const fpsWhole = fpsDecimal[1] ?? "0"; + const fpsFraction = fpsDecimal[2] ?? ""; + const fpsExponent = Number.parseInt(fpsDecimal[3] ?? "0", 10); + const fpsDigits = BigInt(`${fpsWhole}${fpsFraction}`); + const fpsScale = fpsExponent - fpsFraction.length; + fpsNumerator = fpsScale >= 0 ? fpsDigits * 10n ** BigInt(fpsScale) : fpsDigits; + fpsDenominator = fpsScale >= 0 ? 1n : 10n ** BigInt(-fpsScale); + } + + const frameNumerator = microseconds * fpsNumerator; + const frameDenominator = 1_000_000n * fpsDenominator; + const frameCount = isVFR + ? (frameNumerator + frameDenominator - 1n) / frameDenominator + : (2n * frameNumerator + frameDenominator) / (2n * frameDenominator); + const frames = Number(frameCount); + return Math.max(1, Number.isSafeInteger(frames) ? frames : Number.MAX_SAFE_INTEGER); +} + export interface ExtractionOptions { - fps: number; + /** Exact configured rate. Rational rates are passed to FFmpeg verbatim. */ + fps: FpsInput; outputDir: string; quality?: number; format?: VideoFrameFormat; @@ -547,7 +629,10 @@ export async function extractVideoFramesRange( outputDirOverride?: string, ): Promise { const ffmpegProcessTimeout = config?.ffmpegProcessTimeout ?? DEFAULT_CONFIG.ffmpegProcessTimeout; - const { fps, outputDir, quality = 95 } = options; + const { outputDir, quality = 95 } = options; + const normalizedFps = toFps(options.fps); + const fps = fpsToNumber(normalizedFps); + const ffmpegFps = fpsToFfmpegArg(normalizedFps); const videoOutputDir = outputDirOverride ?? join(outputDir, videoId); if (!existsSync(videoOutputDir)) mkdirSync(videoOutputDir, { recursive: true }); @@ -618,7 +703,7 @@ export async function extractVideoFramesRange( vfFilters.push("format=nv12"); } if (!options.finalFrameOnly && !metadata.isVFR) { - vfFilters.push(`fps=${fps}`); + vfFilters.push(`fps=${ffmpegFps}`); } if (options.sdrToHdrTransfer) { // Ordering intent: fps sampling runs BEFORE the colorspace remap so only @@ -632,7 +717,7 @@ export async function extractVideoFramesRange( } if (vfFilters.length > 0) args.push("-vf", vfFilters.join(",")); if (!options.finalFrameOnly && metadata.isVFR) { - args.push("-fps_mode", "cfr", "-r", String(fps)); + args.push("-fps_mode", "cfr", "-r", ffmpegFps); } args.push("-q:v", format === "jpg" ? String(Math.ceil((100 - quality) / 3)) : "0"); @@ -1102,6 +1187,12 @@ function buildSupersetGroup( ): SupersetGroupPlan | null { if (misses.length < 2) return null; if (misses.some(({ work }) => work.finalFrameOnly)) return null; + // VFR normalization (`-fps_mode cfr -r`) establishes its duplicate/drop + // phase relative to each seek. A union extraction therefore cannot be + // sliced into the same frames as independently sought member ranges, even + // when their offsets land on an integral output-frame boundary. Keep VFR + // ranges direct until the extractor has a proven absolute timestamp phase. + if (misses.some(({ work }) => work.metadata.isVFR)) return null; const baseStart = Math.min(...misses.map(({ work }) => work.video.mediaStart)); if (!misses.every(({ work }) => isIntegralFrameOffset(work.video.mediaStart - baseStart, fps))) { return null; @@ -1181,6 +1272,7 @@ function sliceSupersetMember( superset: ExtractedFrames, outputDir: string, fps: number, + configuredFps: FpsInput, ): ExtractedFrames { const { work } = member.miss; rmSync(outputDir, { recursive: true, force: true }); @@ -1190,7 +1282,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, + configuredFps, + 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) { @@ -1282,6 +1378,9 @@ export async function extractAllVideoFrames( `Video extraction timelineEnd must be finite; got ${String(options.timelineEnd)}`, ); } + const configuredFps = toFps(options.fps); + const fps = fpsToNumber(configuredFps); + const fpsKey = fpsToFfmpegArg(configuredFps); const startTime = Date.now(); const extracted: ExtractedFrames[] = []; const errors: VideoExtractionFailure[] = []; @@ -1563,7 +1662,7 @@ export async function extractAllVideoFrames( const rehydrated = rehydrateCacheEntry(target.entry, { videoId: work.video.id, srcPath: target.srcPath, - fps: options.fps, + fps, format: work.format, metadata: work.metadata, }); @@ -1586,7 +1685,7 @@ export async function extractAllVideoFrames( size: keyInput.size, mediaStart: keyInput.mediaStart, duration: work.videoDuration, - fps: options.fps, + fps: fpsKey, format: work.format, transform, }); @@ -1689,12 +1788,13 @@ export async function extractAllVideoFrames( member, superset, join(options.outputDir, work.video.id), - options.fps, + fps, + configuredFps, ); } const partialDir = partialCacheEntryDir(cacheTarget.entry); - const sliced = sliceSupersetMember(member, superset, partialDir, options.fps); + const sliced = sliceSupersetMember(member, superset, partialDir, fps, configuredFps); const published = publishCacheEntry(cacheTarget.entry, partialDir); if (!published.published) { breakdown.cachePublishFailures += 1; @@ -1801,7 +1901,7 @@ export async function extractAllVideoFrames( const format = resolveFrameFormat(metadata, options.format); const sdrToHdrTransfer = sdrToHdrTransfers[index]; const finalFrameOnly = window.finalFrameOnly === true; - const dedupeKey = `${videoPath}\0${extractionMediaStart}\0${videoDuration}\0${options.fps}\0${format}\0${sdrToHdrTransfer ?? ""}\0${finalFrameOnly ? "final" : "range"}`; + const dedupeKey = `${videoPath}\0${extractionMediaStart}\0${videoDuration}\0${fpsKey}\0${format}\0${sdrToHdrTransfer ?? ""}\0${finalFrameOnly ? "final" : "range"}`; return { work: { @@ -1841,7 +1941,7 @@ export async function extractAllVideoFrames( } } - const supersetPlan = planSupersetGroups(cacheMisses, options.fps); + const supersetPlan = planSupersetGroups(cacheMisses, fps); const directOutcomes = await Promise.all( supersetPlan.direct.map( async (miss) => diff --git a/packages/producer/src/services/render/stages/extractVideosStage.ts b/packages/producer/src/services/render/stages/extractVideosStage.ts index 12db159a9..c0f0965e3 100644 --- a/packages/producer/src/services/render/stages/extractVideosStage.ts +++ b/packages/producer/src/services/render/stages/extractVideosStage.ts @@ -48,7 +48,6 @@ import { resolveProjectRelativeSrc, runVideoExtractionWithRetry, } from "@hyperframes/engine"; -import { fpsToNumber } from "@hyperframes/core"; import { collectVideoMetadataHints, collectVideoReadinessSkipIds, @@ -368,12 +367,11 @@ export async function runExtractVideosStage( extractionResult = await extractAllVideoFrames( composition.videos, projectDir, - // extractAllVideoFrames takes fps as a number (decimal). Frames sampled - // from a video at 29.97 vs 30 differ by ~1 frame in 1000 — not enough - // to break visual parity, and the encoder-side rational keeps the - // output framerate exact. + // Preserve the configured rational through FFmpeg extraction. NTSC + // rates must remain `30000/1001`, not a rounded JavaScript decimal, + // because short boundary counts can differ by one frame. { - fps: fpsToNumber(job.config.fps), + fps: job.config.fps, outputDir: join(compiledDir, "__hyperframes_video_frames"), format: job.config.videoFrameFormat ?? "auto", timelineEnd: composition.duration, diff --git a/packages/producer/src/services/render/videoFrameCoverage.test.ts b/packages/producer/src/services/render/videoFrameCoverage.test.ts index ef3f1a7be..993f72929 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.test.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.test.ts @@ -97,6 +97,12 @@ describe("expectedFramesForClip", () => { expect(expectedFramesForClip(0, 0.633333, 30, "nearest")).toBe(19); }); + it("uses exact NTSC rationals for short CFR and VFR boundaries", () => { + expect(expectedFramesForClip(0, 0.25025, { num: 30000, den: 1001 }, "nearest")).toBe(8); + expect(expectedFramesForClip(0, 0.125125, { num: 24000, den: 1001 })).toBe(3); + expect(expectedFramesForClip(0, 0.5005, { num: 24000, den: 1001 })).toBe(12); + }); + 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); @@ -160,7 +166,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, @@ -172,7 +178,19 @@ describe("computeVideoFrameCoverage", () => { capturedFrames: 18, ratio: 18 / 19, }); - expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError); + expect(() => assertVideoFrameCoverage(reports, 0.95)).not.toThrow(); + }); + + it("does not reject complete 24000/1001 VFR extraction at an exact boundary", () => { + const videos = [makeVideo({ id: "ntsc-vfr", start: 0, end: 0.125125 })]; + const reports = computeVideoFrameCoverage( + videos, + [makeExtracted("ntsc-vfr", 3, { isVFR: true })], + { num: 24000, den: 1001 }, + ); + + expect(reports[0]).toMatchObject({ expectedFrames: 3, capturedFrames: 3, ratio: 1 }); + expect(() => assertVideoFrameCoverage(reports, 0.95)).not.toThrow(); }); it("still fails closed when a positive sub-frame clip captured zero frames", () => { @@ -344,6 +362,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 4e8dd7a71..a7b4a7ac7 100644 --- a/packages/producer/src/services/render/videoFrameCoverage.ts +++ b/packages/producer/src/services/render/videoFrameCoverage.ts @@ -45,12 +45,17 @@ */ import { parseHTML } from "linkedom"; +import { fpsToNumber, toFps, type FpsInput } from "@hyperframes/core"; import { + extractionFrameCountForDuration, resolvePlayableVideoDuration, 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; clipStart: number; @@ -131,22 +136,34 @@ export function resolveVideoCoverageThreshold( export function expectedFramesForClip( start: number, end: number, - fps: number, + fps: FpsInput, rounding: "ceil" | "nearest" = "ceil", ): number { - if (!Number.isFinite(start) || !Number.isFinite(end) || !Number.isFinite(fps)) return 0; - if (fps <= 0) return 0; + const fpsValue = fpsToNumber(toFps(fps)); + if (!Number.isFinite(start) || !Number.isFinite(end) || !Number.isFinite(fpsValue)) return 0; + if (fpsValue <= 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( video: VideoElement, entry: ExtractedFrames | undefined, - fps: number, + fps: FpsInput, ): number { const rounding = entry && !entry.metadata.isVFR ? "nearest" : "ceil"; const slotFrames = expectedFramesForClip(video.start, video.end, fps, rounding); @@ -169,7 +186,7 @@ function expectedFramesForVideo( export function computeVideoFrameCoverage( videos: readonly VideoElement[], extracted: readonly ExtractedFrames[], - fps: number, + fps: FpsInput, ): VideoFrameCoverageReport[] { const byId = new Map(); for (const entry of extracted) byId.set(entry.videoId, entry); @@ -207,7 +224,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. diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index c633a81a9..ff9869e3a 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -2369,11 +2369,7 @@ async function executeRenderPipeline(input: { // Also count authored `[data-start]` clip windows as a coarse proxy // for the ts=1784144554 authored-clip-count-scaled failure shape. const coverageReports: VideoFrameCoverageReport[] = extractionResult - ? computeVideoFrameCoverage( - composition.videos, - extractionResult.extracted, - fpsToNumber(job.config.fps), - ) + ? computeVideoFrameCoverage(composition.videos, extractionResult.extracted, job.config.fps) : []; const coverageThreshold = resolveVideoCoverageThreshold(); const authoredTimedClipCount = countAuthoredTimedClips(compiled.html);