diff --git a/packages/engine/src/services/videoFrameExtractor.test.ts b/packages/engine/src/services/videoFrameExtractor.test.ts index 84f59cff..c63e0e93 100644 --- a/packages/engine/src/services/videoFrameExtractor.test.ts +++ b/packages/engine/src/services/videoFrameExtractor.test.ts @@ -196,6 +196,87 @@ describe.skipIf(!HAS_FFMPEG)("extractAllVideoFrames on a VFR source", () => { expect(result.phaseBreakdown.vfrPreflightMs).toBeGreaterThan(0); }, 60_000); + // Regression test for the segment-scope HDR preflight fix: pre-fix, + // convertSdrToHdr re-encoded the entire source, so a 30-minute SDR source + // contributing a 2-second clip took ~200× longer than needed. Post-fix the + // converted file's duration matches the used segment. + it("bounds the SDR→HDR preflight re-encode to the used segment", async () => { + const SDR_LONG = join(FIXTURE_DIR, "sdr-long.mp4"); + const HDR_SHORT = join(FIXTURE_DIR, "hdr-short.mp4"); + + const sdrResult = await runFfmpeg([ + "-y", + "-hide_banner", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + "testsrc2=s=320x180:d=10:rate=30", + "-c:v", + "libx264", + "-preset", + "ultrafast", + "-pix_fmt", + "yuv420p", + SDR_LONG, + ]); + if (!sdrResult.success) { + throw new Error(`SDR fixture synthesis failed: ${sdrResult.stderr.slice(-400)}`); + } + + // Tag as bt2020nc / smpte2084 so the preflight path considers the timeline mixed-HDR. + const hdrResult = await runFfmpeg([ + "-y", + "-hide_banner", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + "testsrc2=s=320x180:d=2:rate=30", + "-c:v", + "libx264", + "-preset", + "ultrafast", + "-pix_fmt", + "yuv420p", + "-color_primaries", + "bt2020", + "-color_trc", + "smpte2084", + "-colorspace", + "bt2020nc", + HDR_SHORT, + ]); + if (!hdrResult.success) { + throw new Error(`HDR fixture synthesis failed: ${hdrResult.stderr.slice(-400)}`); + } + + const outputDir = join(FIXTURE_DIR, "out-hdr-segment"); + mkdirSync(outputDir, { recursive: true }); + + const videos: VideoElement[] = [ + { id: "sdr", src: SDR_LONG, start: 0, end: 2, mediaStart: 0, hasAudio: false }, + { id: "hdr", src: HDR_SHORT, start: 2, end: 4, mediaStart: 0, hasAudio: false }, + ]; + + const result = await extractAllVideoFrames(videos, FIXTURE_DIR, { + fps: 30, + outputDir, + }); + expect(result.errors).toEqual([]); + expect(result.phaseBreakdown.hdrPreflightCount).toBe(1); + + const convertedPath = join(outputDir, "_hdr_normalized", "sdr_hdr.mp4"); + expect(existsSync(convertedPath)).toBe(true); + const convertedMeta = await extractVideoMetadata(convertedPath); + // Pre-fix duration matched the 10s source; post-fix it matches the 2s segment + // (±0.2s for encoder keyframe/seek alignment). + expect(convertedMeta.durationSeconds).toBeGreaterThan(1.8); + expect(convertedMeta.durationSeconds).toBeLessThan(2.5); + }, 60_000); + // Asserts both frame-count correctness and that we don't emit long runs of // byte-identical "duplicate" frames — the user-visible "frozen screen // recording" symptom. Pre-fix duplicate rate on this fixture is ~38% diff --git a/packages/engine/src/services/videoFrameExtractor.ts b/packages/engine/src/services/videoFrameExtractor.ts index 406d45cf..c4ff8253 100644 --- a/packages/engine/src/services/videoFrameExtractor.ts +++ b/packages/engine/src/services/videoFrameExtractor.ts @@ -293,22 +293,38 @@ export async function extractVideoFramesRange( * function (PQ for HDR10, HLG for broadcast HDR). The output transfer must * match the dominant transfer of the surrounding HDR content; otherwise the * downstream encoder will tag the final video with the wrong curve. + * + * `startTime` and `duration` bound the re-encode to the segment the composition + * actually uses. Without them a 30-minute screen recording that contributes a + * 2-second clip was transcoded in full — a >100× waste for long sources. + * Mirrors the segment-scope fix already applied to the VFR→CFR preflight. */ async function convertSdrToHdr( inputPath: string, outputPath: string, + startTime: number, + duration: number, targetTransfer: HdrTransfer, signal?: AbortSignal, config?: Partial>, ): Promise { + // Positive duration is required — FFmpeg's `-t 0` silently produces a 0-byte + // output that the downstream extractor then treats as a valid (empty) file. + if (duration <= 0) { + throw new Error(`convertSdrToHdr: duration must be positive (got ${duration})`); + } const timeout = config?.ffmpegProcessTimeout ?? DEFAULT_CONFIG.ffmpegProcessTimeout; // smpte2084 = PQ (HDR10), arib-std-b67 = HLG. const colorTrc = targetTransfer === "pq" ? "smpte2084" : "arib-std-b67"; const args = [ + "-ss", + String(startTime), "-i", inputPath, + "-t", + String(duration), "-vf", "colorspace=all=bt2020:iall=bt709:range=tv", "-color_primaries", @@ -455,16 +471,21 @@ export async function extractAllVideoFrames( // Phase 2: Probe color spaces and normalize if mixed HDR/SDR const phase2ProbeStart = Date.now(); - const videoColorSpaces = await Promise.all( - resolvedVideos.map(async ({ videoPath }) => { - const metadata = await extractMediaMetadata(videoPath); - return metadata.colorSpace; - }), + const videoMetadata = await Promise.all( + resolvedVideos.map(({ videoPath }) => extractMediaMetadata(videoPath)), ); + const videoColorSpaces = videoMetadata.map((m) => m.colorSpace); breakdown.hdrProbeMs = Date.now() - phase2ProbeStart; const hdrPreflightStart = Date.now(); const hdrInfo = analyzeCompositionHdr(videoColorSpaces); + // Track entries the HDR preflight validated as non-extractable so they can + // be removed from every parallel array before Phase 2b and Phase 3 see them. + // Without this, `errors.push({...}); continue;` only short-circuits the + // normalization step — the invalid entry stays in `resolvedVideos` and + // Phase 3 still calls `extractVideoFramesRange` on the same past-EOF + // mediaStart, surfacing a second raw FFmpeg error for the same clip. + const hdrSkippedIndices = new Set(); if (hdrInfo.hasHdr && hdrInfo.dominantTransfer) { // dominantTransfer is "majority wins" — if a composition mixes PQ and HLG // sources (rare but legal), the minority transfer's videos get converted @@ -483,11 +504,46 @@ export async function extractAllVideoFrames( // SDR video in a mixed timeline — convert to the dominant HDR transfer // so the encoder tags the final video correctly (PQ vs HLG). const entry = resolvedVideos[i]; - if (!entry) continue; + const metadata = videoMetadata[i]; + if (!entry || !metadata) continue; + + // Guard against mediaStart past EOF — FFmpeg's `-ss` silently produces + // a 0-byte file when seeking beyond the source duration, and the + // downstream extractor then points at a broken input. + if (entry.video.mediaStart >= metadata.durationSeconds) { + errors.push({ + videoId: entry.video.id, + error: `SDR→HDR conversion skipped: mediaStart (${entry.video.mediaStart}s) ≥ source duration (${metadata.durationSeconds}s)`, + }); + hdrSkippedIndices.add(i); + continue; + } + + // Scope the re-encode to the segment the composition actually uses. + // Long sources (e.g. 30-minute screen recordings) contributing short + // clips were transcoded in full pre-fix — a >100× waste. + let segDuration = entry.video.end - entry.video.start; + if (!Number.isFinite(segDuration) || segDuration <= 0) { + const sourceRemaining = metadata.durationSeconds - entry.video.mediaStart; + segDuration = sourceRemaining > 0 ? sourceRemaining : metadata.durationSeconds; + } + const convertedPath = join(convertDir, `${entry.video.id}_hdr.mp4`); try { - await convertSdrToHdr(entry.videoPath, convertedPath, targetTransfer, signal, config); + await convertSdrToHdr( + entry.videoPath, + convertedPath, + entry.video.mediaStart, + segDuration, + targetTransfer, + signal, + config, + ); entry.videoPath = convertedPath; + // Segment-scoped re-encode starts the new file at t=0, so downstream + // extraction must seek from 0, not the original mediaStart. Shallow-copy + // to avoid mutating the caller's VideoElement (mirrors the VFR fix). + entry.video = { ...entry.video, mediaStart: 0 }; breakdown.hdrPreflightCount += 1; } catch (err) { errors.push({ @@ -500,6 +556,19 @@ export async function extractAllVideoFrames( } breakdown.hdrPreflightMs = Date.now() - hdrPreflightStart; + // Remove HDR-preflight-skipped entries from every parallel array so Phase 2b + // (VFR) and Phase 3 (extract) don't re-process them. Iterate backwards to + // keep indices stable while splicing. + if (hdrSkippedIndices.size > 0) { + for (let i = resolvedVideos.length - 1; i >= 0; i--) { + if (hdrSkippedIndices.has(i)) { + resolvedVideos.splice(i, 1); + videoMetadata.splice(i, 1); + videoColorSpaces.splice(i, 1); + } + } + } + // Phase 2b: Re-encode VFR inputs to CFR so the fps filter in Phase 3 produces // the expected frame count. Only the used segment is transcoded. const vfrPreflightStart = Date.now();