fix(cli): address snapshot PR review feedback

Incorporates four review points on #348:

1. Fix typecheck error — cast the dynamic `@hyperframes/engine` import to a
   typed shape and drop the `as typeof x` cast on a `null`-unioned variable
   that TS rejected. CI `Typecheck` was failing on main because the cast
   conflicted with the declared union.

2. Clear stale injected <img> overlays — always call
   `syncVideoFrameVisibility(page, activeIds)` on every seek (including
   `active.length === 0`), so injected frames from a previous timestamp
   don't leak into later snapshots. The runtime's visibility toggles act
   on the native <video> but not on its injected sibling, which Copilot
   correctly flagged as a leakage source.

3. URL-decode the served video path before resolving to a filesystem path.
   Files whose names contain spaces or other URL-encoded characters were
   skipped because `new URL().pathname` preserves `%20` while the file
   server decodes inbound requests and the file on disk lives at the
   decoded name.

4. Mirror `packages/core/src/runtime/media.ts` media math so clips with a
   non-1 `defaultPlaybackRate` get the correct active window and relTime.
   Specifically: clamp `defaultPlaybackRate` to [0.1, 5], apply
   `(t - start) * rate + mediaStart`, and adjust the duration-fallback
   branch by `/ rate` when only source duration is known.

5. Kill FFmpeg on a 30s timeout so a pathological clip cannot wedge
   snapshot indefinitely. Matches the default in
   `@hyperframes/engine`'s `runFfmpeg`.

Re-verified against the same 4-timestamp A/B on launch-video-2:
MD5s match the ffmpeg-from-render ground truth
  12.5s → ef9684e36fea53a0db7adf7cfcaacad3  (Stripe)
  16.0s → 487494ca16344d55d7181408dc439a56  (Framer)
  20.5s → 8835c34ad2a45755a1c98a7e079427a1  (HeyGen 3D)
  32.5s → 34dc9450f2bd661c12039d7aa82a30b0  (GitHub finale)

No-video projects (basecamp-tour, linear-brand, commissioned/github)
still produce identical output to the pre-fix path. Latency unchanged.

Made-with: Cursor
This commit is contained in:
ukimsanov
2026-04-20 10:30:29 -04:00
parent a3e7c6176f
commit 0730f88dc7
+118 -72
View File
@@ -11,6 +11,12 @@ import type { Example } from "./_examples.js";
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
/** Maximum time a single-frame FFmpeg extract is allowed to run. Mirrors the
* default applied by `@hyperframes/engine`'s `runFfmpeg` so a pathological
* clip (corrupt media, stalled network mount, codec edge case) cannot wedge
* `hyperframes snapshot` indefinitely. */
const FFMPEG_EXTRACT_TIMEOUT_MS = 30_000;
/**
* Extract a single frame from a video file at `timeSeconds` via FFmpeg.
* Used to work around Chrome-headless's inability to reliably seek
@@ -23,32 +29,45 @@ async function extractVideoFrameToBuffer(
const tmp = mkdtempSync(join(tmpdir(), "hf-snapshot-frame-"));
const outPath = join(tmp, "frame.png");
try {
const result = await new Promise<{ code: number | null; stderr: string }>((resolvePromise) => {
// `-ss` before `-i` performs a fast keyframe seek; adequate for snapshot accuracy
// (±1 frame) and orders of magnitude faster than the decode-and-scan alternative.
const ff = spawn("ffmpeg", [
"-hide_banner",
"-loglevel",
"error",
"-ss",
String(Math.max(0, timeSeconds)),
"-i",
videoPath,
"-frames:v",
"1",
"-q:v",
"2",
"-y",
outPath,
]);
let stderr = "";
ff.stderr.on("data", (d: Buffer) => {
stderr += d.toString();
});
ff.on("close", (code) => resolvePromise({ code, stderr }));
ff.on("error", () => resolvePromise({ code: null, stderr: "ffmpeg spawn failed" }));
});
if (result.code !== 0 || !existsSync(outPath)) return null;
const result = await new Promise<{ code: number | null; stderr: string; timedOut: boolean }>(
(resolvePromise) => {
// `-ss` before `-i` performs a fast keyframe seek; adequate for snapshot accuracy
// (±1 frame) and orders of magnitude faster than the decode-and-scan alternative.
const ff = spawn("ffmpeg", [
"-hide_banner",
"-loglevel",
"error",
"-ss",
String(Math.max(0, timeSeconds)),
"-i",
videoPath,
"-frames:v",
"1",
"-q:v",
"2",
"-y",
outPath,
]);
let stderr = "";
let timedOut = false;
const timer = setTimeout(() => {
timedOut = true;
ff.kill("SIGTERM");
}, FFMPEG_EXTRACT_TIMEOUT_MS);
ff.stderr.on("data", (d: Buffer) => {
stderr += d.toString();
});
ff.on("close", (code) => {
clearTimeout(timer);
resolvePromise({ code, stderr, timedOut });
});
ff.on("error", () => {
clearTimeout(timer);
resolvePromise({ code: null, stderr: "ffmpeg spawn failed", timedOut });
});
},
);
if (result.code !== 0 || result.timedOut || !existsSync(outPath)) return null;
return readFileSync(outPath);
} finally {
try {
@@ -228,12 +247,20 @@ async function captureSnapshots(
// already extracts each frame via FFmpeg and injects it as an <img> sibling
// over the <video>. We reuse that same primitive here so `snapshot` and
// `render` behave identically for timed <video data-start> elements.
let injectVideoFramesBatch:
| ((page: any, updates: Array<{ videoId: string; dataUri: string }>) => Promise<void>)
| null = null;
type InjectFn = (
page: unknown,
updates: Array<{ videoId: string; dataUri: string }>,
) => Promise<void>;
type SyncVisibilityFn = (page: unknown, activeVideoIds: string[]) => Promise<void>;
let injectVideoFramesBatch: InjectFn | null = null;
let syncVideoFrameVisibility: SyncVisibilityFn | null = null;
try {
const engine = await import("@hyperframes/engine");
injectVideoFramesBatch = engine.injectVideoFramesBatch as typeof injectVideoFramesBatch;
const engine = (await import("@hyperframes/engine")) as {
injectVideoFramesBatch: InjectFn;
syncVideoFrameVisibility: SyncVisibilityFn;
};
injectVideoFramesBatch = engine.injectVideoFramesBatch;
syncVideoFrameVisibility = engine.syncVideoFrameVisibility;
} catch {
// Engine unavailable in this install — snapshot will still run, and
// compositions without <video data-start> get exactly the old behaviour.
@@ -271,70 +298,89 @@ async function captureSnapshots(
// Without this, Chrome-headless renders them blank/first-frame because
// it silently drops programmatic `currentTime` writes during capture.
// No-op when the composition has no timed videos (basecamp, linear, etc.)
if (injectVideoFramesBatch) {
if (injectVideoFramesBatch && syncVideoFrameVisibility) {
// Mirror the runtime's media math in packages/core/src/runtime/media.ts
// so clips with non-1 `defaultPlaybackRate` get the right active
// window and the right `relTime`:
// playbackRate = clamp(defaultPlaybackRate, 0.1, 5) — default 1
// duration fallback = (sourceDuration - mediaStart) / playbackRate
// relTime = (t - start) * playbackRate + mediaStart
// active = t >= start && t < start+duration && relTime >= 0
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
? srcDur
? Math.max(0, (srcDur - mediaStart) / playbackRate)
: Number.POSITIVE_INFINITY;
const mediaStart =
parseFloat(v.dataset.playbackStart ?? v.dataset.mediaStart ?? "0") || 0;
const relTime = (t - start) * playbackRate + mediaStart;
const activeNow = t >= start && t < start + duration && relTime >= 0 && !!v.id;
return {
id: v.id,
src: v.currentSrc || v.src,
start,
duration,
mediaStart,
relTime,
active: activeNow,
};
})
.filter(
(entry) =>
entry.id && entry.src && t >= entry.start && t < entry.start + entry.duration,
);
.filter((entry) => entry.active && entry.src);
}, time);
if (active.length > 0) {
const updates: Array<{ videoId: string; dataUri: string }> = [];
for (const v of active) {
// The page-served URL (http://127.0.0.1:PORT/relative/path.mp4)
// maps 1:1 to <projectDir>/relative/path.mp4. Reconstruct the
// filesystem path from the URL pathname.
let filePath: string | null = null;
try {
const url = new URL(v.src);
const candidate = resolve(projectDir, url.pathname.replace(/^\//, ""));
const rel = relative(projectDir, candidate);
if (!rel.startsWith("..") && !isAbsolute(rel) && existsSync(candidate)) {
filePath = candidate;
}
} catch {
/* unresolvable src (e.g. blob:, data:) — skip */
const updates: Array<{ videoId: string; dataUri: string }> = [];
for (const v of active) {
// The page-served URL (http://127.0.0.1:PORT/relative/path.mp4)
// maps 1:1 to <projectDir>/relative/path.mp4. decodeURIComponent
// the pathname — the file server decodes inbound requests, so a
// file with spaces in its path lives at the decoded name on disk
// while `new URL().pathname` preserves the %-encoding.
let filePath: string | null = null;
try {
const url = new URL(v.src);
const decodedPath = decodeURIComponent(url.pathname).replace(/^\//, "");
const candidate = resolve(projectDir, decodedPath);
const rel = relative(projectDir, candidate);
if (!rel.startsWith("..") && !isAbsolute(rel) && existsSync(candidate)) {
filePath = candidate;
}
if (!filePath) continue;
const relTime = Math.max(0, time - v.start + v.mediaStart);
const png = await extractVideoFrameToBuffer(filePath, relTime);
if (!png) continue;
updates.push({
videoId: v.id,
dataUri: `data:image/png;base64,${png.toString("base64")}`,
});
} catch {
/* unresolvable src (e.g. blob:, data:) — skip */
}
if (!filePath) continue;
const png = await extractVideoFrameToBuffer(filePath, Math.max(0, v.relTime));
if (!png) continue;
updates.push({
videoId: v.id,
dataUri: `data:image/png;base64,${png.toString("base64")}`,
});
}
// Always run the visibility sync — even when `active` is empty and
// no new updates were injected. Without this, stale __render_frame__
// <img> overlays left by a previous seek (where different clips were
// active) remain visible in later snapshots, because the runtime's
// visibility toggles act on the <video> element but not its injected
// <img> sibling.
try {
if (updates.length > 0) {
try {
await injectVideoFramesBatch(page, updates);
} catch {
// If injection fails, fall through to the plain screenshot — no worse
// than pre-fix behaviour.
}
await injectVideoFramesBatch(page, updates);
}
await syncVideoFrameVisibility(
page,
active.map((a) => a.id),
);
} catch {
// If either step fails, fall through to the plain screenshot —
// no worse than the pre-fix behaviour.
}
}