From ca69b6b0809019b1d835b7abb14f5eed99e96634 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 15 Jul 2026 08:13:20 -0400 Subject: [PATCH] fix(cli): preserve media at snapshot end boundary (#2475) * fix(cli): preserve media at snapshot end boundary * test(cli): cover snapshot media boundary branches --- packages/cli/src/commands/snapshot.test.ts | 101 +++++++++++++++++++++ packages/cli/src/commands/snapshot.ts | 99 +++++++++++++------- 2 files changed, 169 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/commands/snapshot.test.ts b/packages/cli/src/commands/snapshot.test.ts index 73397d845..24e4a1544 100644 --- a/packages/cli/src/commands/snapshot.test.ts +++ b/packages/cli/src/commands/snapshot.test.ts @@ -4,6 +4,7 @@ import { computeSnapshotTimes, parseZoomScale, requireSnapshotFfmpeg, + resolveSnapshotVideoFrameTime, tailFrameTime, } from "./snapshot.js"; @@ -36,6 +37,106 @@ describe("transparent snapshot capture", () => { }); }); +describe("resolveSnapshotVideoFrameTime", () => { + it("keeps media active at the inclusive clip end and samples its last decodable frame", () => { + expect( + resolveSnapshotVideoFrameTime({ + globalTime: 15, + clipStart: 0, + clipDuration: 15, + relativeTime: 15, + sourceDuration: 15, + }), + ).toBeCloseTo(15 - 1 / 30, 6); + }); + + it("keeps ordinary in-window media timestamps unchanged", () => { + expect( + resolveSnapshotVideoFrameTime({ + globalTime: 7.5, + clipStart: 0, + clipDuration: 15, + relativeTime: 7.5, + sourceDuration: 15, + }), + ).toBe(7.5); + }); + + it("does not activate media after the clip end", () => { + expect( + resolveSnapshotVideoFrameTime({ + globalTime: 15.001, + clipStart: 0, + clipDuration: 15, + relativeTime: 15.001, + sourceDuration: 15, + }), + ).toBeNull(); + }); + + it.each([ + { + name: "before clip start", + input: { + globalTime: 4.9, + clipStart: 5, + clipDuration: 10, + relativeTime: 0, + sourceDuration: 10, + }, + expected: null, + }, + { + name: "negative relative time", + input: { + globalTime: 5, + clipStart: 5, + clipDuration: 10, + relativeTime: -0.1, + sourceDuration: 10, + }, + expected: null, + }, + { + name: "unknown source duration", + input: { + globalTime: 15, + clipStart: 5, + clipDuration: 10, + relativeTime: 10, + sourceDuration: 0, + }, + expected: 10 - 1 / 30, + }, + { + name: "offset clip inclusive end", + input: { + globalTime: 15, + clipStart: 5, + clipDuration: 10, + relativeTime: 10, + sourceDuration: 10, + }, + expected: 10 - 1 / 30, + }, + { + name: "clip end within floating-point tolerance", + input: { + globalTime: 15 + 5e-10, + clipStart: 5, + clipDuration: 10, + relativeTime: 10, + sourceDuration: 10, + }, + expected: 10 - 1 / 30, + }, + ])("handles $name", ({ input, expected }) => { + const result = resolveSnapshotVideoFrameTime(input); + if (expected === null) expect(result).toBeNull(); + else expect(result).toBeCloseTo(expected, 6); + }); +}); + describe("computeSnapshotTimes (FINDING [7]: tail is always captured)", () => { it("default frames: last point is the readable tail, never exact duration", () => { const { times, appendedTail } = computeSnapshotTimes(8, { frames: 5 }); diff --git a/packages/cli/src/commands/snapshot.ts b/packages/cli/src/commands/snapshot.ts index bbdbd6f91..a3502e9de 100644 --- a/packages/cli/src/commands/snapshot.ts +++ b/packages/cli/src/commands/snapshot.ts @@ -60,6 +60,33 @@ function orbitStageSource(): string { * `hyperframes snapshot` indefinitely. */ const FFMPEG_EXTRACT_TIMEOUT_MS = 30_000; +/** Keep an exact clip-end snapshot aligned with the renderer's inclusive media + * window. This intentionally differs from the live player's exclusive-end + * visibility so an explicit end-boundary review does not become blank. FFmpeg + * cannot decode at a source's exclusive duration, so sample one nominal 30fps + * frame inside the source. This also clamps clips whose configured media window + * extends beyond the source. An infinite clip duration intentionally never + * enters the end-boundary branch. */ +export function resolveSnapshotVideoFrameTime(input: { + globalTime: number; + clipStart: number; + clipDuration: number; + relativeTime: number; + sourceDuration: number; +}): number | null { + const { globalTime, clipStart, clipDuration, relativeTime, sourceDuration } = input; + const clipEnd = clipStart + clipDuration; + const clipEndTolerance = 1e-9; + if (globalTime < clipStart || globalTime > clipEnd + clipEndTolerance || relativeTime < 0) + return null; + + const atClipEnd = Math.abs(globalTime - clipEnd) <= clipEndTolerance; + if (!atClipEnd) return relativeTime; + + const sourceEnd = sourceDuration > 0 ? sourceDuration : relativeTime; + return Math.max(0, Math.min(relativeTime, sourceEnd - 1 / 30)); +} + export function requireSnapshotFfmpeg(ffmpegPath: string | undefined): string { if (ffmpegPath) return ffmpegPath; throw new Error( @@ -365,38 +392,48 @@ async function captureSnapshots( if (cameraExpr) await page.evaluate(cameraExpr); if (injectVideoFramesBatch && syncVideoFrameVisibility) { - const active = await page.evaluate((t: number) => { - return Array.from(document.querySelectorAll("video[data-start]")) - .map((el) => { - const v = el as HTMLVideoElement; - const start = parseFloat(v.dataset.start ?? "0") || 0; - const rawRate = v.defaultPlaybackRate; - const playbackRate = - Number.isFinite(rawRate) && rawRate > 0 ? Math.max(0.1, Math.min(5, rawRate)) : 1; - const mediaStart = - parseFloat(v.dataset.playbackStart ?? v.dataset.mediaStart ?? "0") || 0; - const rawDuration = parseFloat(v.dataset.duration ?? ""); - const srcDur = Number.isFinite(v.duration) && v.duration > 0 ? v.duration : 0; - const duration = - Number.isFinite(rawDuration) && rawDuration > 0 - ? rawDuration - : srcDur > 0 - ? Math.max(0, (srcDur - mediaStart) / playbackRate) - : Number.POSITIVE_INFINITY; - let relTime = (t - start) * playbackRate + mediaStart; - if (v.loop && srcDur > mediaStart && relTime >= srcDur) { - relTime = mediaStart + ((relTime - mediaStart) % (srcDur - mediaStart)); - } - const activeNow = t >= start && t < start + duration && relTime >= 0 && !!v.id; - return { - id: v.id, - src: v.currentSrc || v.src, - relTime, - active: activeNow, - }; - }) - .filter((entry) => entry.active && entry.src); + const candidates = await page.evaluate((t: number) => { + return Array.from(document.querySelectorAll("video[data-start]")).map((el) => { + const v = el as HTMLVideoElement; + const start = parseFloat(v.dataset.start ?? "0") || 0; + const rawRate = v.defaultPlaybackRate; + const playbackRate = + Number.isFinite(rawRate) && rawRate > 0 ? Math.max(0.1, Math.min(5, rawRate)) : 1; + const mediaStart = + parseFloat(v.dataset.playbackStart ?? v.dataset.mediaStart ?? "0") || 0; + const rawDuration = parseFloat(v.dataset.duration ?? ""); + const srcDur = Number.isFinite(v.duration) && v.duration > 0 ? v.duration : 0; + const duration = + Number.isFinite(rawDuration) && rawDuration > 0 + ? rawDuration + : srcDur > 0 + ? Math.max(0, (srcDur - mediaStart) / playbackRate) + : Number.POSITIVE_INFINITY; + let relTime = (t - start) * playbackRate + mediaStart; + if (v.loop && srcDur > mediaStart && relTime >= srcDur) { + relTime = mediaStart + ((relTime - mediaStart) % (srcDur - mediaStart)); + } + return { + id: v.id, + src: v.currentSrc || v.src, + start, + duration, + srcDuration: srcDur, + relTime, + }; + }); }, time); + const active = candidates.flatMap((candidate) => { + if (!candidate.id || !candidate.src) return []; + const frameTime = resolveSnapshotVideoFrameTime({ + globalTime: time, + clipStart: candidate.start, + clipDuration: candidate.duration, + relativeTime: candidate.relTime, + sourceDuration: candidate.srcDuration, + }); + return frameTime === null ? [] : [{ ...candidate, relTime: frameTime }]; + }); const updates: Array<{ videoId: string; dataUri: string }> = []; for (const v of active) {