fix(engine): hold last frame when a clip's media is shorter than its slot (#1726)

Renders showed the page background (a one-frame black flash) right before a cut
when a video clip's source media was a hair shorter than its data-duration slot
— the common case, since `ffmpeg -t 1.45` emits 43 frames = 1.433s at 30fps.
The frame lookup only held the last frame at the exact clip end, so the
sub-frame remainder rendered blank.

- Hold the last extracted frame for the rest of the slot once the source is
  exhausted, within a tolerance floored at the compiler's 0.05s clamp epsilon so
  the seam is covered at any fps (2 frames alone is < 0.05s above 40fps). Clips
  deliberately much shorter than their slot still blank for the tail (unchanged).
- Warn when the compiler clamps a video's data-duration down to its media length
  (slot longer than source by more than the clamp epsilon): a render-time
  `[compile]` warning in the producer, plus a matching `validate` warning that
  reads each <video>'s live duration in headless Chrome (static HTML lint can't
  see media durations). A shared `analyzeClipMediaFit` keeps both on one
  threshold.

Adds engine unit tests for the hold behavior and the analyzer.
This commit is contained in:
Miguel Ángel
2026-06-25 19:16:27 -04:00
committed by GitHub
parent 764aa02a3a
commit 92385711dc
8 changed files with 195 additions and 11 deletions
+72 -1
View File
@@ -72,6 +72,73 @@ async function seekTo(page: import("puppeteer-core").Page, time: number): Promis
await new Promise((r) => setTimeout(r, SEEK_SETTLE_MS));
}
/**
* Flag `<video>`/`<audio>` clips whose source is meaningfully shorter than their
* `data-duration` slot (the slot gets silently shortened in renders). Runs in
* the live page to read each element's intrinsic `.duration`, which static lint
* can't see.
*/
async function auditClipDurations(
page: import("puppeteer-core").Page,
analyzeClipMediaFit: typeof import("@hyperframes/engine").analyzeClipMediaFit,
): Promise<ConsoleEntry[]> {
const clips = await page.evaluate(() => {
const rows: Array<{
id: string;
kind: string;
slot: number;
mediaStart: number;
duration: number;
loop: boolean;
}> = [];
document.querySelectorAll("video[data-duration], audio[data-duration]").forEach((node) => {
const el = node as HTMLMediaElement;
const slot = parseFloat(el.getAttribute("data-duration") ?? "");
if (!(slot > 0)) return;
rows.push({
id: el.id || el.getAttribute("src") || `(${el.tagName.toLowerCase()})`,
kind: el.tagName === "AUDIO" ? "Audio" : "Video",
slot,
mediaStart: parseFloat(el.getAttribute("data-media-start") ?? "0") || 0,
duration: el.duration,
loop: el.loop || el.getAttribute("data-loop") === "true",
});
});
return rows;
});
const warnings: ConsoleEntry[] = [];
const unreadable: string[] = [];
for (const clip of clips) {
if (!Number.isFinite(clip.duration) || clip.duration <= 0) {
// Metadata never loaded (e.g. slow remote source) — record so the gap in
// coverage isn't silent, rather than dropping it.
unreadable.push(clip.id);
continue;
}
const mediaSeconds = Math.max(0, clip.duration - clip.mediaStart);
const fit = analyzeClipMediaFit({ slotSeconds: clip.slot, mediaSeconds, loop: clip.loop });
if (!fit) continue;
warnings.push({
level: "warning",
text:
`${clip.kind} "${clip.id}" is ${mediaSeconds.toFixed(2)}s but its slot (data-duration) ` +
`is ${clip.slot.toFixed(2)}s — the slot is shortened to the media length when rendered. ` +
`Set data-duration to ~${mediaSeconds.toFixed(2)}s if that isn't intended.`,
});
}
if (unreadable.length > 0) {
warnings.push({
level: "warning",
text:
`Could not read the duration of ${unreadable.length} media element(s) within the ` +
`validate timeout (${unreadable.join(", ")}); their slot vs. source fit was not checked. ` +
`Re-run with a longer --timeout if the source is slow to load.`,
});
}
return warnings;
}
async function runContrastAudit(page: import("puppeteer-core").Page): Promise<ContrastEntry[]> {
const duration = await getCompositionDuration(page);
if (duration <= 0) return [];
@@ -134,7 +201,7 @@ async function validateInBrowser(
try {
const browser = await ensureBrowser();
const puppeteer = await import("puppeteer-core");
const { buildChromeArgs } = await import("@hyperframes/engine");
const { buildChromeArgs, analyzeClipMediaFit } = await import("@hyperframes/engine");
const browserGpuMode =
process.env.PRODUCER_BROWSER_GPU_MODE === "software" ? "software" : "hardware";
const chromeBrowser = await puppeteer.default.launch({
@@ -192,6 +259,10 @@ async function validateInBrowser(
await page.goto(server.url, { waitUntil: "domcontentloaded", timeout: 10000 });
await new Promise((r) => setTimeout(r, opts.timeout ?? 3000));
for (const w of await auditClipDurations(page, analyzeClipMediaFit)) {
warnings.push(w);
}
if (opts.contrast) {
contrast = await runContrastAudit(page);
}
+4 -2
View File
@@ -48,8 +48,10 @@ export interface CompilationResult {
unresolved: UnresolvedElement[];
}
// ffprobe precision can differ slightly across local and CI media stacks.
const MEDIA_DURATION_CLAMP_EPSILON_SECONDS = 0.05;
// ffprobe precision can differ slightly across local and CI media stacks. Also
// the floor for the engine's hold-last-frame tolerance (a slot left unclamped is
// short by at most this), so they must move together.
export const MEDIA_DURATION_CLAMP_EPSILON_SECONDS = 0.05;
export function shouldClampMediaDuration(declaredDuration: number, maxDuration: number): boolean {
return declaredDuration > maxDuration + MEDIA_DURATION_CLAMP_EPSILON_SECONDS;
+1
View File
@@ -135,6 +135,7 @@ export {
extractResolvedMedia,
clampDurations,
shouldClampMediaDuration,
MEDIA_DURATION_CLAMP_EPSILON_SECONDS,
} from "./compiler/timingCompiler";
// Lint
+1
View File
@@ -136,6 +136,7 @@ export {
getFrameAtTime,
createFrameLookupTable,
FrameLookupTable,
analyzeClipMediaFit,
type VideoElement,
type ImageElement,
type ExtractedFrames,
@@ -22,6 +22,7 @@ import {
codecMayHaveAlpha,
decoderForCodec,
getFrameAtTime,
analyzeClipMediaFit,
type VideoElement,
type ExtractedFrames,
} from "./videoFrameExtractor.js";
@@ -375,6 +376,33 @@ describe("FrameLookupTable", () => {
expect(table.getActiveFramePayloads(5.0).get("hero")?.frameIndex).toBe(29);
});
it("holds the last frame when the source is a sub-frame shorter than the slot", () => {
// clip [2, 3.45] declares a 1.45s slot, but `ffmpeg -t 1.45` at 30fps emits
// 43 frames = 1.433s — a half-frame short. The tail between source
// exhaustion (~3.433) and the clip end (3.45) must hold the last frame
// rather than render the page background (a one-frame black flash at the
// cut). The held index is the final extracted frame (42).
const table = createFrameLookupTable(
[
{
id: "hero",
src: "clip.mp4",
start: 2,
end: 3.45,
mediaStart: 0,
loop: false,
hasAudio: false,
},
],
[fakeExtracted(43, 30)],
);
// last real frame
expect(table.getActiveFramePayloads(3.4).get("hero")?.frameIndex).toBe(42);
// source exhausted but within tolerance of the end → hold, don't blank
expect(table.getActiveFramePayloads(3.44).get("hero")?.frameIndex).toBe(42);
expect(table.getActiveFramePayloads(3.45).get("hero")?.frameIndex).toBe(42);
});
it("keeps both clips active at a shared adjacent boundary, matching the runtime", () => {
// clip A ends at 3.0, clip B starts at 3.0. The runtime shows both at the
// shared instant; the active set must too.
@@ -395,6 +423,35 @@ describe("FrameLookupTable", () => {
});
});
describe("analyzeClipMediaFit", () => {
it("returns null for a sub-tolerance shortfall the compiler leaves unclamped", () => {
// 1.433s media in a 1.45s slot — a sub-frame shortfall (<0.05s) the renderer
// freezes seamlessly and the compiler never clamps. Not worth warning about.
expect(analyzeClipMediaFit({ slotSeconds: 1.45, mediaSeconds: 1.433 })).toBeNull();
});
it("returns null when media is longer than or equal to the slot", () => {
expect(analyzeClipMediaFit({ slotSeconds: 2, mediaSeconds: 2 })).toBeNull();
expect(analyzeClipMediaFit({ slotSeconds: 2, mediaSeconds: 5 })).toBeNull();
});
it("reports the shortfall when the slot exceeds media beyond the clamp epsilon", () => {
const fit = analyzeClipMediaFit({ slotSeconds: 5, mediaSeconds: 1 });
expect(fit).not.toBeNull();
expect(fit?.shortfallSeconds).toBeCloseTo(4, 5);
expect(fit?.toleranceSeconds).toBeCloseTo(0.05, 5);
});
it("never flags looping clips (they repeat to fill the slot)", () => {
expect(analyzeClipMediaFit({ slotSeconds: 5, mediaSeconds: 1, loop: true })).toBeNull();
});
it("returns null for unusable inputs (non-finite media, zero slot)", () => {
expect(analyzeClipMediaFit({ slotSeconds: 0, mediaSeconds: 1 })).toBeNull();
expect(analyzeClipMediaFit({ slotSeconds: 5, mediaSeconds: NaN })).toBeNull();
});
});
describe("parseImageElements", () => {
it("parses images with data-start and data-duration", () => {
const images = parseImageElements(
@@ -10,7 +10,7 @@ import { spawn } from "child_process";
import { existsSync, mkdirSync, readdirSync, rmSync } from "fs";
import { isAbsolute, join, posix, resolve, sep } from "path";
import { parseHTML } from "linkedom";
import { decodeUrlPathVariants } from "@hyperframes/core";
import { decodeUrlPathVariants, MEDIA_DURATION_CLAMP_EPSILON_SECONDS } from "@hyperframes/core";
import { trackChildProcess } from "../utils/processTracker.js";
import { extractMediaMetadata, type VideoMetadata } from "../utils/ffprobe.js";
import {
@@ -963,6 +963,32 @@ export function getFrameAtTime(
return extracted.framePaths.get(frameIndex) || null;
}
const HOLD_LAST_FRAME_TOLERANCE_FRAMES = 2;
/**
* Whether a clip's source is shorter than its `data-duration` slot by more than
* the compiler tolerates before clamping the slot to the media
* (MEDIA_DURATION_CLAMP_EPSILON_SECONDS) — the case worth warning about. Shared
* by the render and `validate` warnings. `null` when the media covers the slot,
* the clip loops, or inputs are unusable.
*/
export function analyzeClipMediaFit(params: {
/** Timeline slot length in seconds — `end - start` (a.k.a. data-duration). */
slotSeconds: number;
/** Playable source media after the trim offset — `duration - mediaStart`. */
mediaSeconds: number;
/** Looping clips repeat to fill the slot, so they never fall short. */
loop?: boolean;
}): { shortfallSeconds: number; toleranceSeconds: number } | null {
const { slotSeconds, mediaSeconds, loop } = params;
if (loop) return null;
if (!(slotSeconds > 0) || !Number.isFinite(mediaSeconds) || mediaSeconds < 0) return null;
const toleranceSeconds = MEDIA_DURATION_CLAMP_EPSILON_SECONDS;
const shortfallSeconds = slotSeconds - mediaSeconds;
if (shortfallSeconds <= toleranceSeconds) return null;
return { shortfallSeconds, toleranceSeconds };
}
export class FrameLookupTable {
private videos: Map<
string,
@@ -1079,11 +1105,17 @@ export class FrameLookupTable {
continue;
}
if (frameIndex < 0 || frameIndex >= video.extracted.totalFrames) {
// At the inclusive clip end (globalTime === end), hold the last
// extracted frame so the render matches the runtime, which keeps the
// element visible on its final frame at `t === end`. Mid-clip source
// exhaustion (globalTime < end) stays blank — unchanged.
if (globalTime >= video.end && video.extracted.totalFrames > 0) {
// Source exhausted. Hold the last frame near the clip end so a media that
// falls a hair short of its slot (e.g. `ffmpeg -t 1.45` → 1.433s at 30fps)
// doesn't flash the background for one frame. A clip that's substantially
// shorter than its slot still blanks for the tail. Tolerance floored at
// the clamp epsilon so the seam is covered at any fps (see that const).
const fps = video.extracted.fps;
const holdTolerance = Math.max(
fps > 0 ? HOLD_LAST_FRAME_TOLERANCE_FRAMES / fps : 0,
MEDIA_DURATION_CLAMP_EPSILON_SECONDS,
);
if (globalTime >= video.end - holdTolerance && video.extracted.totalFrames > 0) {
const lastIndex = video.extracted.totalFrames - 1;
const lastPath = video.extracted.framePaths.get(lastIndex);
if (lastPath) frames.set(videoId, { framePath: lastPath, frameIndex: lastIndex });
+21 -2
View File
@@ -41,7 +41,7 @@ import type { Page } from "puppeteer-core";
import { injectDeterministicFontFaces } from "./deterministicFonts.js";
import { prepareAnimatedGifInputs } from "./animatedGifPrep.js";
import { createStudioPositionSeekReapplyScript } from "@hyperframes/core/studio-api/manual-edits-render-script";
import { defaultLogger } from "../logger.js";
import { defaultLogger, type ProducerLogger } from "../logger.js";
export interface CompiledComposition {
html: string;
@@ -209,6 +209,7 @@ async function compileHtmlFile(
html: string,
baseDir: string,
downloadDir: string,
log?: ProducerLogger,
): Promise<{ html: string; unresolvedCompositions: UnresolvedElement[] }> {
const { html: staticCompiled, unresolved } = compileTimingAttrs(html);
@@ -244,13 +245,25 @@ async function compileHtmlFile(
downloadDir,
el.tagName,
);
return { id: el.id, duration: el.duration, maxDuration, src: el.src! };
return { id: el.id, tagName: el.tagName, duration: el.duration, maxDuration, src: el.src! };
}),
);
const clampList: ResolvedDuration[] = [];
for (const r of clampResults) {
if (r.maxDuration > 0 && shouldClampMediaDuration(r.duration, r.maxDuration)) {
clampList.push({ id: r.id, duration: r.maxDuration });
// This clip's `data-duration` is being silently shortened to its source.
// Surface it so the author can confirm the longer slot wasn't intended.
// ponytail: top-level only — sub-composition clips still get clamped (and
// videos still hold the last frame); thread `log` through
// parseSubCompositions to warn for them too.
const kind = r.tagName === "audio" ? "Audio" : "Video";
log?.warn(
`[compile] ${kind} "${r.id}" (${r.src}) is ${r.maxDuration.toFixed(2)}s but its ` +
`data-duration is ${r.duration.toFixed(2)}s — the slot is shortened to the media ` +
`length. Set data-duration to ~${r.maxDuration.toFixed(2)}s, trim data-media-start, ` +
`or use a longer/looping source if that isn't intended.`,
);
}
}
@@ -1295,6 +1308,11 @@ async function embedLocalFontFaces(html: string, projectDir: string): Promise<st
* additive; omitting `options` preserves the in-process renderer's defaults.
*/
export interface CompileForRenderOptions {
/**
* Logger for compile-time diagnostics (e.g. the data-duration vs. media
* mismatch warning). Optional so non-render callers can omit it.
*/
log?: ProducerLogger;
/**
* Threaded through to {@link injectDeterministicFontFaces}. When `true`,
* any external font fetch failure throws `FontFetchError` instead of
@@ -1353,6 +1371,7 @@ export async function compileForRender(
rawHtml,
projectDir,
downloadDir,
options.log,
);
// Parse sub-compositions first (extracts media + compiled HTML for each)
@@ -121,6 +121,7 @@ export async function runCompileStage(input: CompileStageInput): Promise<Compile
const compileStart = Date.now();
const compiled = await compileForRender(projectDir, htmlPath, join(workDir, "downloads"), {
log,
failClosedFontFetch: failClosedFontFetch === true,
allowSystemFontCapture,
animatedGifCacheDir: cfg.extractCacheDir