Skip to content

Commit 4a36655

Browse files
fix(engine): resolve relative data-start references in video-frame extraction
* fix(engine): resolve relative data-start references in video-frame extraction <video data-start="intro"> (a relative reference to another clip's end) is resolved by the browser runtime but parseVideoElements/parseImageElements did a raw parseFloat, yielding NaN start/end. The FrameLookupTable active-window checks (start <= t <= end) are then always false, so the clip is never injected and composites BLANK in the final render — while lint/validate/inspect/snapshot and the live preview all look fine. The docs' Relative Timing section teaches exactly this pattern on <video>. Share the pure reference-syntax parser (parseStartExpression) out of the runtime resolver into @hyperframes/core, and resolve references in the extractor against the linkedom document it already holds: a reference resolves to the target clip's resolved start + its duration (data-duration or data-end) + offset, mirroring the runtime. Cycle-guarded; an unknown target or unknown duration falls back to the target's start / 0 (never NaN), matching runtime semantics. Natural-media-duration-only targets aren't known at parse time (same limit as the runtime's fallback). parseImageElements gets the same fix. Runtime resolver behavior is unchanged (its 25-case suite still passes). * chore: re-trigger CI to refresh a stuck CodeQL aggregate check
1 parent 42a2095 commit 4a36655

5 files changed

Lines changed: 246 additions & 39 deletions

File tree

packages/core/src/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,11 @@ export type { FitTextOptions, FitTextResult } from "./text/index.js";
232232

233233
// Runtime helpers (composition-side)
234234
export { getVariables } from "./runtime/getVariables.js";
235+
export {
236+
parseStartExpression,
237+
parseNumeric,
238+
type ReferenceExpression,
239+
} from "./runtime/startExpression.js";
235240

236241
// Variable validation (CLI / tooling-side)
237242
export {
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/**
2+
* Pure parser for the `data-start` timing expression grammar, shared by the
3+
* browser runtime resolver (`createRuntimeStartTimeResolver`) and the Node-side
4+
* video-frame extractor (`parseVideoElements`) so both agree on exactly what a
5+
* relative reference means. No DOM/browser dependencies — safe to import in
6+
* Node.
7+
*
8+
* Grammar (matches the docs' "Relative Timing" section):
9+
* - `"12.5"` -> absolute seconds
10+
* - `"intro"` -> start when clip `intro` ends
11+
* - `"intro + 2"` -> 2s after `intro` ends
12+
* - `"intro - 0.5"` -> 0.5s before `intro` ends (overlap)
13+
*/
14+
15+
export type ReferenceExpression =
16+
| { kind: "absolute"; value: number }
17+
| { kind: "reference"; refId: string; offset: number };
18+
19+
/** Parse a value to a finite number, or `null` if it isn't one. */
20+
export function parseNumeric(value: string | null | undefined): number | null {
21+
if (value == null || value === "") return null;
22+
const parsed = Number(value);
23+
return Number.isFinite(parsed) ? parsed : null;
24+
}
25+
26+
/**
27+
* Parse a raw `data-start` value into an absolute time or a clip reference.
28+
* Returns `null` when the value is empty or not a recognized expression.
29+
*/
30+
export function parseStartExpression(raw: string | null | undefined): ReferenceExpression | null {
31+
const normalized = (raw ?? "").trim();
32+
if (!normalized) return null;
33+
const absolute = parseNumeric(normalized);
34+
if (absolute != null) {
35+
return { kind: "absolute", value: absolute };
36+
}
37+
const referenceMatch = normalized.match(/^([A-Za-z0-9_.:-]+)(?:\s*([+-])\s*([0-9]*\.?[0-9]+))?$/);
38+
if (!referenceMatch) return null;
39+
const refId = (referenceMatch[1] ?? "").trim();
40+
if (!refId) return null;
41+
const sign = referenceMatch[2] ?? "+";
42+
const offsetRaw = referenceMatch[3] ?? "0";
43+
const parsedOffset = Number.parseFloat(offsetRaw);
44+
const offsetMagnitude = Number.isFinite(parsedOffset) ? Math.max(0, parsedOffset) : 0;
45+
const offset = sign === "-" ? -offsetMagnitude : offsetMagnitude;
46+
return { kind: "reference", refId, offset };
47+
}

packages/core/src/runtime/startResolver.ts

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,11 @@
11
import type { RuntimeTimelineLike } from "./types";
22
import { swallow } from "./diagnostics";
33
import { readElementPlaybackRate } from "./media";
4+
import { parseNumeric, parseStartExpression } from "./startExpression";
45

56
const AUTHORED_DURATION_ATTR = "data-hf-authored-duration";
67
const AUTHORED_END_ATTR = "data-hf-authored-end";
78

8-
type ReferenceExpression =
9-
| {
10-
kind: "absolute";
11-
value: number;
12-
}
13-
| {
14-
kind: "reference";
15-
refId: string;
16-
offset: number;
17-
};
18-
19-
function parseNumeric(value: string | null | undefined): number | null {
20-
if (value == null || value === "") return null;
21-
const parsed = Number(value);
22-
return Number.isFinite(parsed) ? parsed : null;
23-
}
24-
259
function parseDurationAttr(element: Element): number | null {
2610
return parseNumeric(element.getAttribute("data-duration"));
2711
}
@@ -38,25 +22,6 @@ function parseAuthoredEndAttr(element: Element): number | null {
3822
return parseNumeric(element.getAttribute(AUTHORED_END_ATTR));
3923
}
4024

41-
function parseStartExpression(raw: string | null | undefined): ReferenceExpression | null {
42-
const normalized = (raw ?? "").trim();
43-
if (!normalized) return null;
44-
const absolute = parseNumeric(normalized);
45-
if (absolute != null) {
46-
return { kind: "absolute", value: absolute };
47-
}
48-
const referenceMatch = normalized.match(/^([A-Za-z0-9_.:-]+)(?:\s*([+-])\s*([0-9]*\.?[0-9]+))?$/);
49-
if (!referenceMatch) return null;
50-
const refId = (referenceMatch[1] ?? "").trim();
51-
if (!refId) return null;
52-
const sign = referenceMatch[2] ?? "+";
53-
const offsetRaw = referenceMatch[3] ?? "0";
54-
const parsedOffset = Number.parseFloat(offsetRaw);
55-
const offsetMagnitude = Number.isFinite(parsedOffset) ? Math.max(0, parsedOffset) : 0;
56-
const offset = sign === "-" ? -offsetMagnitude : offsetMagnitude;
57-
return { kind: "reference", refId, offset };
58-
}
59-
6025
export function createRuntimeStartTimeResolver(params: {
6126
timelineRegistry?: Record<string, RuntimeTimelineLike | undefined>;
6227
includeAuthoredTimingAttrs?: boolean;

packages/engine/src/services/videoFrameExtractor.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,74 @@ describe("parseVideoElements", () => {
259259
loop: true,
260260
});
261261
});
262+
263+
it("resolves a relative data-start reference to another clip's end", () => {
264+
const videos = parseVideoElements(
265+
'<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
266+
'<video id="main" src="b.mp4" data-start="intro" data-duration="20"></video>',
267+
);
268+
const main = videos.find((v) => v.id === "main");
269+
// intro ends at 10, so main starts at 10 and ends at 30 — not NaN.
270+
expect(main?.start).toBe(10);
271+
expect(main?.end).toBe(30);
272+
});
273+
274+
it("applies + and - offsets on a relative reference", () => {
275+
const videos = parseVideoElements(
276+
'<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
277+
'<video id="gap" src="b.mp4" data-start="intro + 2" data-duration="5"></video>' +
278+
'<video id="overlap" src="c.mp4" data-start="intro - 0.5" data-duration="5"></video>',
279+
);
280+
expect(videos.find((v) => v.id === "gap")?.start).toBe(12);
281+
expect(videos.find((v) => v.id === "overlap")?.start).toBe(9.5);
282+
});
283+
284+
it("resolves chained references (A -> B -> C)", () => {
285+
const videos = parseVideoElements(
286+
'<video id="a" src="a.mp4" data-start="0" data-duration="4"></video>' +
287+
'<video id="b" src="b.mp4" data-start="a" data-duration="3"></video>' +
288+
'<video id="c" src="c.mp4" data-start="b" data-duration="2"></video>',
289+
);
290+
expect(videos.find((v) => v.id === "b")?.start).toBe(4);
291+
expect(videos.find((v) => v.id === "c")?.start).toBe(7); // 4 + 3
292+
});
293+
294+
it("resolves a reference to a non-video timed element (div clip)", () => {
295+
const videos = parseVideoElements(
296+
'<div id="title" data-start="0" data-duration="6"></div>' +
297+
'<video id="clip" src="b.mp4" data-start="title" data-duration="5"></video>',
298+
);
299+
expect(videos.find((v) => v.id === "clip")?.start).toBe(6);
300+
});
301+
302+
it("derives a referenced clip's duration from data-end when data-duration is absent", () => {
303+
const videos = parseVideoElements(
304+
'<video id="intro" src="a.mp4" data-start="2" data-end="9"></video>' +
305+
'<video id="main" src="b.mp4" data-start="intro" data-duration="5"></video>',
306+
);
307+
// intro: start 2, end 9 -> duration 7 -> main starts at 9.
308+
expect(videos.find((v) => v.id === "main")?.start).toBe(9);
309+
});
310+
311+
it("falls back to 0 (never NaN) for an unknown reference target", () => {
312+
const videos = parseVideoElements(
313+
'<video id="orphan" src="a.mp4" data-start="does-not-exist" data-duration="5"></video>',
314+
);
315+
const orphan = videos.find((v) => v.id === "orphan");
316+
expect(orphan?.start).toBe(0);
317+
expect(Number.isNaN(orphan?.start)).toBe(false);
318+
expect(orphan?.end).toBe(5);
319+
});
320+
321+
it("does not hang or NaN on a circular reference", () => {
322+
const videos = parseVideoElements(
323+
'<video id="a" src="a.mp4" data-start="b" data-duration="4"></video>' +
324+
'<video id="b" src="b.mp4" data-start="a" data-duration="3"></video>',
325+
);
326+
for (const v of videos) {
327+
expect(Number.isNaN(v.start)).toBe(false);
328+
}
329+
});
262330
});
263331

264332
describe("FrameLookupTable", () => {
@@ -334,6 +402,21 @@ describe("FrameLookupTable", () => {
334402
expect(table.getActiveFramePayloads(1.5).has("hero")).toBe(false);
335403
});
336404

405+
it("places a relative-reference video in its resolved window end-to-end (was blank)", () => {
406+
// The reported bug: <video data-start="intro"> gave NaN start/end, so the
407+
// active-window checks (start <= t <= end) were always false and the clip
408+
// composited blank. With resolution, `main` is active across [10, 30].
409+
const videos = parseVideoElements(
410+
'<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
411+
'<video id="main" src="b.mp4" data-start="intro" data-duration="20"></video>',
412+
);
413+
const table = createFrameLookupTable(videos, [{ ...fakeExtracted(600, 30), videoId: "main" }]);
414+
expect(table.getActiveFramePayloads(5).has("main")).toBe(false); // before resolved start (10)
415+
expect(table.getActiveFramePayloads(15).has("main")).toBe(true); // within [10, 30]
416+
expect(table.getActiveFramePayloads(29).has("main")).toBe(true);
417+
expect(table.getActiveFramePayloads(31).has("main")).toBe(false); // after resolved end (30)
418+
});
419+
337420
it("holds the last frame at the inclusive clip end (t === end)", () => {
338421
// clip [1,3] with exactly 2s of source frames (60 @ 30fps). The frame
339422
// landing on t === end used to deactivate one frame early and render blank,

packages/engine/src/services/videoFrameExtractor.ts

Lines changed: 110 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ import { spawn } from "child_process";
1010
import { copyFileSync, existsSync, linkSync, mkdirSync, readdirSync, rmSync } from "fs";
1111
import { isAbsolute, join, posix, resolve, sep } from "path";
1212
import { parseHTML } from "linkedom";
13-
import { decodeUrlPathVariants, MEDIA_DURATION_CLAMP_EPSILON_SECONDS } from "@hyperframes/core";
13+
import {
14+
decodeUrlPathVariants,
15+
MEDIA_DURATION_CLAMP_EPSILON_SECONDS,
16+
parseNumeric,
17+
parseStartExpression,
18+
} from "@hyperframes/core";
1419
import { trackChildProcess } from "../utils/processTracker.js";
1520
import { extractMediaMetadata, type VideoMetadata } from "../utils/ffprobe.js";
1621
import {
@@ -148,9 +153,102 @@ export interface ExtractionResult {
148153
phaseBreakdown: ExtractionPhaseBreakdown;
149154
}
150155

156+
// Minimal structural DOM shape the reference resolver needs, so it works
157+
// against linkedom (Node) without pulling in lib.dom types.
158+
interface RefResolverEl {
159+
getAttribute(name: string): string | null;
160+
}
161+
interface RefResolverDoc {
162+
getElementById(id: string): RefResolverEl | null;
163+
querySelector(selector: string): RefResolverEl | null;
164+
}
165+
166+
/**
167+
* Find the element a relative `data-start` reference points at — by `id`
168+
* first, then by `data-composition-id` (a sub-composition can be referenced).
169+
* The reference-id grammar (see parseStartExpression) is restricted to
170+
* `[A-Za-z0-9_.:-]`, none of which need escaping inside a quoted attribute
171+
* selector, so no CSS.escape (absent in linkedom) is required.
172+
*/
173+
function findReferenceTargetEl(doc: RefResolverDoc, refId: string): RefResolverEl | null {
174+
return doc.getElementById(refId) ?? doc.querySelector(`[data-composition-id="${refId}"]`);
175+
}
176+
177+
/**
178+
* Resolve an element's absolute start time (seconds) the same way the browser
179+
* runtime's startResolver does, so `<video data-start="intro">` (a relative
180+
* reference to another clip's end) renders at the right time instead of
181+
* producing NaN and compositing blank. Durations come from `data-duration` or
182+
* `data-end` here; the natural-media-duration fallback isn't known at parse
183+
* time, so — exactly like the runtime — an unknown-duration reference falls
184+
* back to the target's start and an unknown target falls back to 0 (never NaN).
185+
*/
186+
function resolveReferencedStart(
187+
doc: RefResolverDoc,
188+
el: RefResolverEl,
189+
startCache: Map<RefResolverEl, number>,
190+
visiting: Set<RefResolverEl>,
191+
): number {
192+
const cached = startCache.get(el);
193+
if (cached !== undefined) return cached;
194+
if (visiting.has(el)) return 0; // cycle guard (A -> B -> A)
195+
visiting.add(el);
196+
try {
197+
const expression = parseStartExpression(el.getAttribute("data-start"));
198+
if (!expression) {
199+
startCache.set(el, 0);
200+
return 0;
201+
}
202+
if (expression.kind === "absolute") {
203+
const value = Math.max(0, expression.value);
204+
startCache.set(el, value);
205+
return value;
206+
}
207+
const target = findReferenceTargetEl(doc, expression.refId);
208+
if (!target) {
209+
startCache.set(el, 0);
210+
return 0;
211+
}
212+
const targetStart = resolveReferencedStart(doc, target, startCache, visiting);
213+
const targetDuration = resolveReferencedDuration(doc, target, startCache, visiting);
214+
const resolved =
215+
targetDuration != null && targetDuration > 0
216+
? Math.max(0, targetStart + targetDuration + expression.offset)
217+
: Math.max(0, targetStart + expression.offset);
218+
startCache.set(el, resolved);
219+
return resolved;
220+
} finally {
221+
visiting.delete(el);
222+
}
223+
}
224+
225+
/**
226+
* Duration of a referenced clip, from `data-duration` or `data-end - start`.
227+
* Returns null when only the natural media duration would settle it (unknown
228+
* at parse time) — the caller then treats the reference as duration-0.
229+
*/
230+
function resolveReferencedDuration(
231+
doc: RefResolverDoc,
232+
el: RefResolverEl,
233+
startCache: Map<RefResolverEl, number>,
234+
visiting: Set<RefResolverEl>,
235+
): number | null {
236+
const durationAttr = parseNumeric(el.getAttribute("data-duration"));
237+
if (durationAttr != null && durationAttr > 0) return durationAttr;
238+
const endAttr = parseNumeric(el.getAttribute("data-end"));
239+
if (endAttr != null) {
240+
const start = resolveReferencedStart(doc, el, startCache, visiting);
241+
const delta = endAttr - start;
242+
if (Number.isFinite(delta) && delta > 0) return delta;
243+
}
244+
return null;
245+
}
246+
151247
export function parseVideoElements(html: string): VideoElement[] {
152248
const videos: VideoElement[] = [];
153249
const { document } = parseHTML(unwrapTemplate(html));
250+
const startCache = new Map<RefResolverEl, number>();
251+
const visiting = new Set<RefResolverEl>();
154252

155253
const videoEls = document.querySelectorAll("video[src]");
156254
let autoIdCounter = 0;
@@ -170,7 +268,12 @@ export function parseVideoElements(html: string): VideoElement[] {
170268
const mediaStartAttr = el.getAttribute("data-media-start");
171269
const hasAudioAttr = el.getAttribute("data-has-audio");
172270

173-
const start = startAttr ? parseFloat(startAttr) : 0;
271+
// Resolve data-start, including relative references ("intro", "intro + 2")
272+
// to another clip's end — the browser runtime resolves these but a raw
273+
// parseFloat here would yield NaN, placing the clip at NaN so it composites
274+
// blank in the final render. `startAttr` may be a plain number or a
275+
// reference; the resolver handles both.
276+
const start = startAttr ? resolveReferencedStart(document, el, startCache, visiting) : 0;
174277
// Derive end from data-end → data-start+data-duration → Infinity (natural duration).
175278
// The caller (htmlCompiler) clamps Infinity to the composition's absoluteEnd.
176279
let end = 0;
@@ -206,6 +309,8 @@ export interface ImageElement {
206309
export function parseImageElements(html: string): ImageElement[] {
207310
const images: ImageElement[] = [];
208311
const { document } = parseHTML(unwrapTemplate(html));
312+
const startCache = new Map<RefResolverEl, number>();
313+
const visiting = new Set<RefResolverEl>();
209314

210315
const imgEls = document.querySelectorAll("img[src]");
211316
let autoIdCounter = 0;
@@ -222,7 +327,9 @@ export function parseImageElements(html: string): ImageElement[] {
222327
const endAttr = el.getAttribute("data-end");
223328
const durationAttr = el.getAttribute("data-duration");
224329

225-
const start = startAttr ? parseFloat(startAttr) : 0;
330+
// Resolve relative data-start references (see parseVideoElements) so a
331+
// referenced image start doesn't become NaN and drop the image from the render.
332+
const start = startAttr ? resolveReferencedStart(document, el, startCache, visiting) : 0;
226333
let end = 0;
227334
if (endAttr) {
228335
end = parseFloat(endAttr);

0 commit comments

Comments
 (0)