diff --git a/packages/engine/src/services/extractedFrameIndex.test.ts b/packages/engine/src/services/extractedFrameIndex.test.ts new file mode 100644 index 0000000000..c3738e85e2 --- /dev/null +++ b/packages/engine/src/services/extractedFrameIndex.test.ts @@ -0,0 +1,81 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; + +import { + ExtractedFrameSequenceError, + extractedFrameIndex, + framePathsFromDirectory, +} from "./extractedFrameIndex.js"; + +const roots: string[] = []; + +function frameDir(): string { + const root = mkdtempSync(join(tmpdir(), "hf-extracted-frame-index-")); + roots.push(root); + return root; +} + +function seed(root: string, ...files: string[]): void { + for (const file of files) writeFileSync(join(root, file), file); +} + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +describe("extractedFrameIndex", () => { + it("derives frame identity across the five-to-six-digit boundary", () => { + expect(extractedFrameIndex("frame_99999.jpg", "jpg")).toBe(99_998); + expect(extractedFrameIndex("frame_100000.jpg", "jpg")).toBe(99_999); + }); + + it("refuses malformed, zero, and wrong-format frame candidates", () => { + expect(() => extractedFrameIndex("frame_bad.jpg", "jpg")).toThrow(ExtractedFrameSequenceError); + expect(() => extractedFrameIndex("frame_00000.jpg", "jpg")).toThrow( + ExtractedFrameSequenceError, + ); + expect(() => extractedFrameIndex("frame_00001.png", "jpg")).toThrow( + ExtractedFrameSequenceError, + ); + }); +}); + +describe("framePathsFromDirectory", () => { + it("maps by the filename ordinal instead of directory or lexical position", () => { + const root = frameDir(); + seed( + root, + ...Array.from({ length: 10 }, (_, index) => `frame_${index + 1}.jpg`).reverse(), + "notes.txt", + ); + + const paths = framePathsFromDirectory(root, "jpg"); + + expect(paths.size).toBe(10); + expect(basename(paths.get(8)!)).toBe("frame_9.jpg"); + expect(basename(paths.get(9)!)).toBe("frame_10.jpg"); + }); + + it("fails loudly when two filenames claim the same numeric frame", () => { + const root = frameDir(); + seed(root, "frame_1.jpg", "frame_00001.jpg"); + + expect(() => framePathsFromDirectory(root, "jpg")).toThrow(/duplicate.*frame index 0/i); + }); + + it("fails loudly instead of shifting later frames across a gap", () => { + const root = frameDir(); + seed(root, "frame_00001.jpg", "frame_00003.jpg"); + + expect(() => framePathsFromDirectory(root, "jpg")).toThrow(/missing.*frame index 1/i); + }); + + it("fails on frame-prefixed malformed candidates but ignores unrelated files", () => { + const root = frameDir(); + seed(root, "frame_00001.jpg", "frame_bad.jpg", "notes.txt"); + + expect(() => framePathsFromDirectory(root, "jpg")).toThrow(/invalid.*frame filename/i); + }); +}); diff --git a/packages/engine/src/services/extractedFrameIndex.ts b/packages/engine/src/services/extractedFrameIndex.ts new file mode 100644 index 0000000000..af6e1bde54 --- /dev/null +++ b/packages/engine/src/services/extractedFrameIndex.ts @@ -0,0 +1,55 @@ +import { readdirSync } from "node:fs"; +import { join } from "node:path"; + +export type ExtractedFrameFormat = "jpg" | "png"; + +export const FRAME_FILENAME_PREFIX = "frame_"; + +export class ExtractedFrameSequenceError extends Error { + constructor(message: string) { + super(message); + this.name = "ExtractedFrameSequenceError"; + } +} + +export function extractedFrameIndex(file: string, format: ExtractedFrameFormat): number { + const match = new RegExp(`^${FRAME_FILENAME_PREFIX}(\\d+)\\.${format}$`).exec(file); + if (!match) { + throw new ExtractedFrameSequenceError(`Invalid extracted frame filename: ${file}`); + } + const ordinal = Number.parseInt(match[1]!, 10); + if (!Number.isSafeInteger(ordinal) || ordinal < 1) { + throw new ExtractedFrameSequenceError(`Invalid extracted frame ordinal: ${file}`); + } + return ordinal - 1; +} + +export function framePathsFromDirectory( + outputDir: string, + format: ExtractedFrameFormat, +): Map { + const suffix = `.${format}`; + const indexed = new Map(); + for (const file of readdirSync(outputDir)) { + if (!file.startsWith(FRAME_FILENAME_PREFIX) || !file.endsWith(suffix)) continue; + const index = extractedFrameIndex(file, format); + if (indexed.has(index)) { + throw new ExtractedFrameSequenceError( + `Duplicate extracted frame index ${index}: ${indexed.get(index)} and ${file}`, + ); + } + indexed.set(index, join(outputDir, file)); + } + + const ordered = new Map(); + for (let index = 0; index < indexed.size; index += 1) { + const path = indexed.get(index); + if (!path) { + throw new ExtractedFrameSequenceError( + `Missing extracted frame index ${index} in ${outputDir}`, + ); + } + ordered.set(index, path); + } + return ordered; +} diff --git a/packages/engine/src/services/extractionCache.test.ts b/packages/engine/src/services/extractionCache.test.ts index ee80038ff6..860f3df2e9 100644 --- a/packages/engine/src/services/extractionCache.test.ts +++ b/packages/engine/src/services/extractionCache.test.ts @@ -25,6 +25,7 @@ import { partialCacheEntryDir, publishCacheEntry, readKeyStat, + rehydrateCacheEntry, type CacheKeyInput, } from "./extractionCache.js"; @@ -76,6 +77,42 @@ describe("extractionCache constants", () => { }); }); +describe("rehydrateCacheEntry frame identity", () => { + it("fails loudly when a complete cache has a frame-number gap", () => { + const { tmpRoot } = makeCacheRoot(); + try { + writeFileSync(join(tmpRoot, "frame_00001.jpg"), "one"); + writeFileSync(join(tmpRoot, "frame_00003.jpg"), "three"); + + expect(() => + rehydrateCacheEntry( + { dir: tmpRoot, keyHash: "a".repeat(64) }, + { + videoId: "video-gap", + srcPath: "/video.mp4", + fps: 30, + format: "jpg", + metadata: { + durationSeconds: 1, + videoStreamDurationSeconds: 1, + width: 1920, + height: 1080, + fps: 30, + videoCodec: "h264", + hasAudio: false, + isVFR: false, + hasAlpha: false, + colorSpace: null, + }, + }, + ), + ).toThrow(/missing.*frame index 1/i); + } finally { + removeCacheRoot(tmpRoot); + } + }); +}); + describe("computeCacheKey", () => { let tmpRoot: string; let sourceFile: string; diff --git a/packages/engine/src/services/extractionCache.ts b/packages/engine/src/services/extractionCache.ts index ae045a37ee..fac2406153 100644 --- a/packages/engine/src/services/extractionCache.ts +++ b/packages/engine/src/services/extractionCache.ts @@ -46,9 +46,10 @@ import { } from "node:fs"; import { join } from "node:path"; import type { VideoMetadata } from "../utils/ffprobe.js"; +import { FRAME_FILENAME_PREFIX, framePathsFromDirectory } from "./extractedFrameIndex.js"; /** Filename prefix for extracted frames. Shared with the extractor. */ -export const FRAME_FILENAME_PREFIX = "frame_"; +export { FRAME_FILENAME_PREFIX } from "./extractedFrameIndex.js"; /** Sentinel filename written after a cache entry is fully populated. */ export const COMPLETE_SENTINEL = ".hf-complete"; @@ -508,14 +509,7 @@ export function rehydrateCacheEntry( options: RehydrateOptions, ): RehydratedFrames { const framePattern = `${FRAME_FILENAME_PREFIX}%05d.${options.format}`; - const framePaths = new Map(); - const suffix = `.${options.format}`; - const files = readdirSync(entry.dir) - .filter((f) => f.startsWith(FRAME_FILENAME_PREFIX) && f.endsWith(suffix)) - .sort(); - files.forEach((file, idx) => { - framePaths.set(idx, join(entry.dir, file)); - }); + const framePaths = framePathsFromDirectory(entry.dir, options.format); return { videoId: options.videoId, srcPath: options.srcPath, diff --git a/packages/engine/src/services/videoFrameExtractor.ts b/packages/engine/src/services/videoFrameExtractor.ts index d5a9e5f4f8..dc90bf06aa 100644 --- a/packages/engine/src/services/videoFrameExtractor.ts +++ b/packages/engine/src/services/videoFrameExtractor.ts @@ -6,7 +6,7 @@ * Videos are replaced with elements during capture. */ -import { copyFileSync, existsSync, linkSync, mkdirSync, readdirSync, rmSync } from "fs"; +import { copyFileSync, existsSync, linkSync, mkdirSync, rmSync } from "fs"; import { isAbsolute, join, posix, resolve, sep } from "path"; import { parseHTML } from "linkedom"; import { @@ -55,6 +55,7 @@ import { type CacheEntry, type CacheFrameFormat, } from "./extractionCache.js"; +import { framePathsFromDirectory } from "./extractedFrameIndex.js"; export interface VideoElement { id: string; @@ -802,13 +803,7 @@ export async function extractVideoFramesRange( ); } - const framePaths = new Map(); - const files = readdirSync(videoOutputDir) - .filter((f) => f.startsWith(FRAME_FILENAME_PREFIX) && f.endsWith(`.${format}`)) - .sort(); - files.forEach((file, index) => { - framePaths.set(index, join(videoOutputDir, file)); - }); + const framePaths = framePathsFromDirectory(videoOutputDir, format); if (framePaths.size === 0 && duration > 0) { throw new VideoSourceExtractionError( "zero_output", @@ -1180,13 +1175,6 @@ type SupersetGroupPlan = { members: SupersetMemberPlan[]; }; -function extractedFrameFileNames(outputDir: string, format: CacheFrameFormat): string[] { - const suffix = `.${format}`; - return readdirSync(outputDir) - .filter((file) => file.startsWith(FRAME_FILENAME_PREFIX) && file.endsWith(suffix)) - .sort(); -} - function extractedFramesFromDirectory( work: PreparedExtraction, outputDir: string, @@ -1194,10 +1182,7 @@ function extractedFramesFromDirectory( fps: number, ): ExtractedFrames { const framePattern = `${FRAME_FILENAME_PREFIX}%05d.${work.format}`; - const framePaths = new Map(); - extractedFrameFileNames(outputDir, work.format).forEach((file, index) => { - framePaths.set(index, join(outputDir, file)); - }); + const framePaths = framePathsFromDirectory(outputDir, work.format); return { videoId: work.video.id, srcPath, diff --git a/packages/producer/src/services/distributed/rebuildExtractedFrames.test.ts b/packages/producer/src/services/distributed/rebuildExtractedFrames.test.ts index 6ff11548fa..119fe43ff7 100644 --- a/packages/producer/src/services/distributed/rebuildExtractedFrames.test.ts +++ b/packages/producer/src/services/distributed/rebuildExtractedFrames.test.ts @@ -178,6 +178,34 @@ describe("rebuildExtractedFramesFromPlanDir", () => { } }); + it("orders mixed-width dense-v1 filenames by numeric ordinal", () => { + const planDir = mkdtempSync(join(tmpdir(), "hf-rebuild-frames-v1-mixed-width-")); + try { + const frameNames = Array.from({ length: 10 }, (_, index) => `frame_${index + 1}.jpg`); + makeFramesDir(planDir, "vid-v1-mixed-width", frameNames.toReversed()); + + const [extracted] = rebuildExtractedFramesFromPlanDir(planDir, [ + { + videoId: "vid-v1-mixed-width", + srcPath: "/v1-mixed-width.mp4", + framePattern: "frame_%05d.jpg", + fps: 30, + totalFrames: frameNames.length, + metadata: VIDEO_METADATA_STUB, + }, + ]); + + expect(extracted!.framePaths.get(8)).toBe( + join(planDir, "video-frames", "vid-v1-mixed-width", "frame_9.jpg"), + ); + expect(extracted!.framePaths.get(9)).toBe( + join(planDir, "video-frames", "vid-v1-mixed-width", "frame_10.jpg"), + ); + } finally { + rmSync(planDir, { recursive: true, force: true }); + } + }); + it("preserves original indexes for a sparse v2 chunk materialization", () => { const planDir = mkdtempSync(join(tmpdir(), "hf-rebuild-frames-sparse-")); try { diff --git a/packages/producer/src/services/distributed/renderChunk.ts b/packages/producer/src/services/distributed/renderChunk.ts index fe99e98b85..7b1ac20d51 100644 --- a/packages/producer/src/services/distributed/renderChunk.ts +++ b/packages/producer/src/services/distributed/renderChunk.ts @@ -324,6 +324,13 @@ export async function beginFrameSessionNeedsScreenshotFallback( return !(await probe(session.page, timeoutMs, probeTick, session.beginFrameIntervalMs)); } +function frameNumberFromFileName(name: string): number | null { + const match = /(\d+)(?=\.[^.]+$)/.exec(name); + if (!match) return null; + const frameNumber = Number(match[1]); + return Number.isSafeInteger(frameNumber) ? frameNumber : null; +} + /** * Rebuild the engine's in-memory `ExtractedFrames[]` from the on-disk * planDir layout. `/video-frames//` holds the numbered @@ -355,13 +362,18 @@ export function rebuildExtractedFramesFromPlanDir( ); } // framePattern looks like `frame_%05d.jpg`; sprintf isn't available at - // runtime so list-and-sort the directory. Sorted-by-name matches - // sorted-by-frame-index because the extractor writes zero-padded - // monotonic indices. + // runtime so list the directory and order numeric names by their ordinal. + // Width changes once FFmpeg passes the padding minimum, so lexical order + // would interleave frame_100000 before frame_10001. const ext = (extname(v.framePattern) || ".jpg").toLowerCase(); const frames = readdirSync(outputDir) .filter((name) => name.toLowerCase().endsWith(ext)) - .sort(); + .sort((left, right) => { + const leftNumber = frameNumberFromFileName(left); + const rightNumber = frameNumberFromFileName(right); + if (leftNumber === null || rightNumber === null) return left.localeCompare(right); + return leftNumber - rightNumber || left.localeCompare(right); + }); const framePaths = new Map(); for (let i = 0; i < frames.length; i++) { const frameName = frames[i]; @@ -369,8 +381,8 @@ export function rebuildExtractedFramesFromPlanDir( // V1 plans preserve the historical sorted-position behavior even for // unusual zero-based filenames. V2 materialization is sparse, so only // that mode derives the original index from ffmpeg's 1-based filename. - const numbered = indexMode === "sparse-v2" ? /(\d+)(?=\.[^.]+$)/.exec(frameName) : null; - const frameIndex = numbered ? Number(numbered[1]) - 1 : i; + const frameNumber = indexMode === "sparse-v2" ? frameNumberFromFileName(frameName) : null; + const frameIndex = frameNumber === null ? i : frameNumber - 1; framePaths.set(frameIndex, join(outputDir, frameName)); } result.push({