diff --git a/packages/engine/src/services/videoFrameExtractor.test.ts b/packages/engine/src/services/videoFrameExtractor.test.ts index 5089bd01e..7833d5685 100644 --- a/packages/engine/src/services/videoFrameExtractor.test.ts +++ b/packages/engine/src/services/videoFrameExtractor.test.ts @@ -682,6 +682,36 @@ describe.skipIf(!HAS_FFMPEG)("video frame extraction format", () => { rmSync(cacheDir, { recursive: true, force: true }); } }, 60_000); + + it("dedupes identical extractions within one render", async () => { + const outputDir = join(FIXTURE_DIR, "out-dedupe"); + mkdirSync(outputDir, { recursive: true }); + + const videoA: VideoElement = { ...fixtureVideo(), id: "dupe-a" }; + const videoB: VideoElement = { ...fixtureVideo(), id: "dupe-b" }; + + const result = await extractAllVideoFrames([videoA, videoB], FIXTURE_DIR, { + fps: 1, + outputDir, + }); + + expect(result.errors).toEqual([]); + expect(result.extracted).toHaveLength(2); + const first = result.extracted[0]!; + const second = result.extracted[1]!; + expect(first.videoId).toBe("dupe-a"); + expect(second.videoId).toBe("dupe-b"); + expect(second.outputDir).toBe(first.outputDir); + expect(Array.from(second.framePaths.entries())).toEqual(Array.from(first.framePaths.entries())); + + const frameDirs = readdirSync(outputDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name); + expect(frameDirs).toEqual(["dupe-a"]); + expect(readdirSync(first.outputDir).filter((f) => f.endsWith(".jpg"))).toHaveLength( + first.totalFrames, + ); + }, 60_000); }); // Regression test for the VFR (variable frame rate) freeze bug. diff --git a/packages/engine/src/services/videoFrameExtractor.ts b/packages/engine/src/services/videoFrameExtractor.ts index df23f4877..77c70bfbe 100644 --- a/packages/engine/src/services/videoFrameExtractor.ts +++ b/packages/engine/src/services/videoFrameExtractor.ts @@ -746,17 +746,17 @@ export async function extractAllVideoFrames( videoPath: string, videoDuration: number, i: number, + metadata: VideoMetadata, + cacheFormat: CacheFrameFormat, ): Promise { if (!cacheRootDir) return null; const keyInput = cacheKeyInputs[i]; - const probedMeta = videoMetadata[i]; - if (!keyInput || !probedMeta) return null; - const cacheFormat = resolveFrameFormat(probedMeta, options.format); + if (!keyInput) return null; const keyDuration = resolveSegmentDuration( keyInput.end - keyInput.start, keyInput.mediaStart, - probedMeta, + metadata, ); const lookup = lookupCacheEntry(cacheRootDir, { videoPath: keyInput.videoPath, @@ -775,7 +775,7 @@ export async function extractAllVideoFrames( srcPath: keyInput.videoPath, fps: options.fps, format: cacheFormat, - metadata: probedMeta, + metadata, }); return { ...rehydrated, ownedByLookup: true }; } @@ -798,43 +798,118 @@ export async function extractAllVideoFrames( return { ...result, ownedByLookup: true }; } - const results = await Promise.all( - resolvedVideos.map(async ({ video, videoPath }, i) => { + function extractionError(videoId: string, err: unknown): { videoId: string; error: string } { + return { videoId, error: err instanceof Error ? err.message : String(err) }; + } + + type PreparedExtraction = { + video: VideoElement; + videoPath: string; + index: number; + metadata: VideoMetadata; + videoDuration: number; + format: CacheFrameFormat; + dedupeKey: string; + }; + + type PreparedExtractionResult = + | { work: PreparedExtraction } + | { error: { videoId: string; error: string } }; + + const preparedExtractions: PreparedExtractionResult[] = await Promise.all( + resolvedVideos.map(async ({ video, videoPath }, index) => { if (signal?.aborted) { throw new Error("Video frame extraction cancelled"); } try { - const probedMeta = videoMetadata[i] ?? (await extractMediaMetadata(videoPath)); + const metadata = videoMetadata[index] ?? (await extractMediaMetadata(videoPath)); const videoDuration = resolveSegmentDuration( video.end - video.start, video.mediaStart, - probedMeta, + metadata, ); if (video.end - video.start !== videoDuration) { video.end = video.start + videoDuration; } - const cached = await tryCachedExtract(video, videoPath, videoDuration, i); - if (cached) return { result: cached }; + const format = resolveFrameFormat(metadata, options.format); + const dedupeKey = `${videoPath}\0${video.mediaStart}\0${videoDuration}\0${options.fps}\0${format}`; - const result = await extractVideoFramesRange( - videoPath, - video.id, - video.mediaStart, - videoDuration, - { ...options, format: resolveFrameFormat(probedMeta, options.format) }, - signal, - config, - ); - - return { result }; - } catch (err) { return { - error: { - videoId: video.id, - error: err instanceof Error ? err.message : String(err), + work: { + video, + videoPath, + index, + metadata, + videoDuration, + format, + dedupeKey, }, }; + } catch (err) { + return { error: extractionError(video.id, err) }; + } + }), + ); + + // Value carries the leader's videoId so a shared-extraction failure can be + // attributed: N deduped elements otherwise report the same root error under + // N different videoIds, which reads as N independent failures in traces. + const inFlightExtractions = new Map< + string, + { leaderVideoId: string; promise: Promise } + >(); + const results = await Promise.all( + preparedExtractions.map(async (prepared) => { + if ("error" in prepared) return prepared; + const { work } = prepared; + + try { + const existing = inFlightExtractions.get(work.dedupeKey); + if (existing) { + try { + const shared = await existing.promise; + return { result: { ...shared, videoId: work.video.id } }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + return { + error: { + videoId: work.video.id, + error: `[shared extraction, leader ${existing.leaderVideoId}] ${message}`, + }, + }; + } + } + + const extraction = (async () => { + const cached = await tryCachedExtract( + work.video, + work.videoPath, + work.videoDuration, + work.index, + work.metadata, + work.format, + ); + if (cached) return cached; + + return extractVideoFramesRange( + work.videoPath, + work.video.id, + work.video.mediaStart, + work.videoDuration, + { ...options, format: work.format }, + signal, + config, + ); + })(); + + inFlightExtractions.set(work.dedupeKey, { + leaderVideoId: work.video.id, + promise: extraction, + }); + return { result: await extraction }; + } catch (err) { + return { error: extractionError(work.video.id, err) }; } }), );