fix(render): consolidate duration and timing correctness (#2405)

* fix(producer): pass variables to duration probe

* fix(producer): tolerate rounded frame-boundary durations

* fix(cli): resolve relative data-start references in composition duration

`compositions --json` computed each timed child's start with a bare
parseFloat(data-start ?? "0") in parseCompositions (host duration) and
parseSubComposition (sub-comp duration). A relative reference like
data-start="s1" ("start when clip s1 ends") is not numeric, so parseFloat
returned NaN and that clip's contribution to the max-end was silently
dropped — a host with two 3s clips (2nd data-start="s1") reported duration 3
instead of 6, breaking compositions/inspect/snapshot for composition-clip
relative timing.

Resolve relative references the same way the extractor does (parseStartExpression
from @hyperframes/core + a findReferenceTargetEl/resolveReferencedStart port,
since the engine's referenceResolver isn't a public export across the package
boundary). Verified: host duration now 6; 3 tests pass.
(Implemented via Codex; verified independently.)
This commit is contained in:
Miguel Ángel
2026-07-14 17:12:36 -04:00
committed by GitHub
parent 6ac18fd68d
commit 0b3dfb3f84
4 changed files with 167 additions and 6 deletions
+41 -1
View File
@@ -1,6 +1,46 @@
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { describe, expect, it, beforeEach } from "vitest";
import { ensureDOMParser } from "../utils/dom.js";
import { parseSubComposition } from "./compositions.js";
import { parseCompositions, parseSubComposition } from "./compositions.js";
describe("parseCompositions", () => {
beforeEach(() => {
ensureDOMParser();
});
it("resolves relative sub-composition starts when computing host duration", () => {
const baseDir = mkdtempSync(join(tmpdir(), "hyperframes-compositions-"));
try {
const compositionsDir = join(baseDir, "compositions");
mkdirSync(compositionsDir);
const subCompositionHtml = `
<template id="scene-template">
<div data-composition-id="scene" data-width="1920" data-height="1080" data-duration="3">
<div class="clip" data-start="0" data-duration="3"></div>
</div>
</template>`;
writeFileSync(join(compositionsDir, "scene.html"), subCompositionHtml);
const html = `
<div data-composition-id="host" data-width="1920" data-height="1080">
<div id="s1" data-composition-id="s1" data-composition-src="compositions/scene.html" data-start="0" data-duration="3" data-track="main"></div>
<div data-composition-id="s2" data-composition-src="compositions/scene.html" data-start="s1" data-duration="3" data-track="main"></div>
</div>`;
const host = parseCompositions(html, baseDir).find(
(composition) => composition.id === "host",
);
expect(host?.duration).toBe(6);
} finally {
rmSync(baseDir, { recursive: true, force: true });
}
});
});
describe("parseSubComposition", () => {
beforeEach(() => {
+72 -3
View File
@@ -1,4 +1,5 @@
import { defineCommand } from "citty";
import { parseNumeric, parseStartExpression } from "@hyperframes/core";
import type { Example } from "./_examples.js";
import { existsSync, readFileSync } from "node:fs";
import { resolve, dirname } from "node:path";
@@ -45,12 +46,78 @@ function estimateDurationFromScripts(root: ParentNode): number {
return duration;
}
function parseCompositions(html: string, baseDir: string): CompositionInfo[] {
function findReferenceTargetEl(doc: Document, refId: string): Element | null {
return doc.getElementById(refId) ?? doc.querySelector(`[data-composition-id="${refId}"]`);
}
function resolveStart(
doc: Document,
el: Element,
startCache: Map<Element, number>,
visiting: Set<Element>,
): number {
const cached = startCache.get(el);
if (cached !== undefined) return cached;
if (visiting.has(el)) return 0;
visiting.add(el);
try {
const expression = parseStartExpression(el.getAttribute("data-start"));
if (!expression) {
startCache.set(el, 0);
return 0;
}
if (expression.kind === "absolute") {
const value = Math.max(0, expression.value);
startCache.set(el, value);
return value;
}
const target = findReferenceTargetEl(doc, expression.refId);
if (!target) {
startCache.set(el, 0);
return 0;
}
const targetStart = resolveStart(doc, target, startCache, visiting);
const targetDuration = resolveReferencedDuration(doc, target, startCache, visiting);
const resolved =
targetDuration != null && targetDuration > 0
? Math.max(0, targetStart + targetDuration + expression.offset)
: Math.max(0, targetStart + expression.offset);
startCache.set(el, resolved);
return resolved;
} finally {
visiting.delete(el);
}
}
function resolveReferencedDuration(
doc: Document,
el: Element,
startCache: Map<Element, number>,
visiting: Set<Element>,
): number | null {
const durationAttr = parseNumeric(el.getAttribute("data-duration"));
if (durationAttr != null && durationAttr > 0) return durationAttr;
const endAttr = parseNumeric(el.getAttribute("data-end"));
if (endAttr != null) {
const start = resolveStart(doc, el, startCache, visiting);
const delta = endAttr - start;
if (Number.isFinite(delta) && delta > 0) return delta;
}
return null;
}
export function parseCompositions(html: string, baseDir: string): CompositionInfo[] {
const parser = new DOMParser();
const doc = parser.parseFromString(html, "text/html");
const compositionDivs = doc.querySelectorAll("[data-composition-id]");
const compositions: CompositionInfo[] = [];
const startCache = new Map<Element, number>();
const visiting = new Set<Element>();
compositionDivs.forEach((div) => {
const id = div.getAttribute("data-composition-id") ?? "unknown";
@@ -75,7 +142,7 @@ function parseCompositions(html: string, baseDir: string): CompositionInfo[] {
timedChildren.forEach((el) => {
elementCount++;
const start = parseFloat(el.getAttribute("data-start") ?? "0");
const start = resolveStart(doc, el, startCache, visiting);
const endAttr = el.getAttribute("data-end");
const durationAttr = el.getAttribute("data-duration");
@@ -141,9 +208,11 @@ export function parseSubComposition(
// Also check timed children for max end time
if (compDiv) {
const timedEls = compDiv.querySelectorAll("[data-start]");
const startCache = new Map<Element, number>();
const visiting = new Set<Element>();
timedEls.forEach((el) => {
elementCount = Math.max(elementCount, timedEls.length);
const start = parseFloat(el.getAttribute("data-start") ?? "0");
const start = resolveStart(doc, el, startCache, visiting);
const endAttr = el.getAttribute("data-end");
const durAttr = el.getAttribute("data-duration");
@@ -10,6 +10,7 @@ import {
// the correct forceScreenshot value (regression for #1236 — probe was launched
// in beginframe mode even when lowMemoryMode demanded screenshot capture).
const capturedCfgs: unknown[] = [];
const capturedOptions: unknown[] = [];
const mockPage = {
evaluate: async () => ({
@@ -43,12 +44,13 @@ mock.module("@hyperframes/engine", () => ({
createCaptureSession: async (
_url: string,
_dir: string,
_opts: unknown,
opts: unknown,
_nullArg: unknown,
cfg: unknown,
) => {
createSessionCallCount++;
capturedCfgs.push(cfg);
capturedOptions.push(opts);
if (createSessionError && createSessionCallCount <= createSessionFailUntilAttempt) {
throw createSessionError;
}
@@ -312,6 +314,45 @@ describe("runProbeStage — forceScreenshot threading", () => {
});
});
describe("runProbeStage — render variable threading", () => {
it("passes render variables to the duration-discovery capture session", async () => {
capturedOptions.length = 0;
const { runProbeStage } = await import("./probeStage.js");
const input = makeProbeInput({ stageForceScreenshot: false });
input.job.config.variables = { short: true, sceneCount: 2 };
await runProbeStage(input);
expect(capturedOptions[0]).toMatchObject({
variables: { short: true, sceneCount: 2 },
});
});
});
describe("runProbeStage — decimal duration frame count", () => {
it("does not add a frame for a six-decimal duration rounded from an exact frame boundary", async () => {
const { runProbeStage } = await import("./probeStage.js");
const input = makeProbeInput({});
input.composition.duration = 32.866667;
input.compiled.staticDuration = 32.866667;
const result = await runProbeStage(input);
expect(result.totalFrames).toBe(986);
});
it("still ceilings a duration that genuinely extends into the next frame", async () => {
const { runProbeStage } = await import("./probeStage.js");
const input = makeProbeInput({});
input.composition.duration = 32.867;
input.compiled.staticDuration = 32.867;
const result = await runProbeStage(input);
expect(result.totalFrames).toBe(987);
});
});
describe("runProbeStage — transient browser error retry (#1687)", () => {
it("retries once on a transient 'Navigating frame was detached' error and succeeds", async () => {
resetRetryMocks();
@@ -83,6 +83,16 @@ export interface ProbeStageInput {
deviceScaleFactor: number;
}
const FRAME_BOUNDARY_EPSILON = 1e-3;
function durationToFrameCount(duration: number, fps: number): number {
const rawFrameCount = duration * fps;
const nearestFrame = Math.round(rawFrameCount);
return Math.abs(rawFrameCount - nearestFrame) <= FRAME_BOUNDARY_EPSILON
? nearestFrame
: Math.ceil(rawFrameCount);
}
export interface ProbeStageResult {
/** May be reassigned from `recompileWithResolutions`. */
compiled: CompiledComposition;
@@ -221,6 +231,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
fps: job.config.fps,
format: needsAlpha ? "png" : "jpeg",
quality: needsAlpha ? undefined : 80,
variables: job.config.variables,
deviceScaleFactor,
};
@@ -558,7 +569,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
const browserProbeMs = Date.now() - probeStart;
const duration = composition.duration;
const totalFrames = Math.ceil(duration * fpsToNumber(job.config.fps));
const totalFrames = durationToFrameCount(duration, fpsToNumber(job.config.fps));
if (duration <= 0) {
// Gather diagnostics to help users understand why the render would produce a black video.