mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
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
This commit is contained in:
Miguel Ángel
@@ -259,6 +259,74 @@ describe("parseVideoElements", () => {
|
||||
loop: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves a relative data-start reference to another clip's end", () => {
|
||||
const videos = parseVideoElements(
|
||||
'<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
|
||||
'<video id="main" src="b.mp4" data-start="intro" data-duration="20"></video>',
|
||||
);
|
||||
const main = videos.find((v) => v.id === "main");
|
||||
// intro ends at 10, so main starts at 10 and ends at 30 — not NaN.
|
||||
expect(main?.start).toBe(10);
|
||||
expect(main?.end).toBe(30);
|
||||
});
|
||||
|
||||
it("applies + and - offsets on a relative reference", () => {
|
||||
const videos = parseVideoElements(
|
||||
'<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
|
||||
'<video id="gap" src="b.mp4" data-start="intro + 2" data-duration="5"></video>' +
|
||||
'<video id="overlap" src="c.mp4" data-start="intro - 0.5" data-duration="5"></video>',
|
||||
);
|
||||
expect(videos.find((v) => v.id === "gap")?.start).toBe(12);
|
||||
expect(videos.find((v) => v.id === "overlap")?.start).toBe(9.5);
|
||||
});
|
||||
|
||||
it("resolves chained references (A -> B -> C)", () => {
|
||||
const videos = parseVideoElements(
|
||||
'<video id="a" src="a.mp4" data-start="0" data-duration="4"></video>' +
|
||||
'<video id="b" src="b.mp4" data-start="a" data-duration="3"></video>' +
|
||||
'<video id="c" src="c.mp4" data-start="b" data-duration="2"></video>',
|
||||
);
|
||||
expect(videos.find((v) => v.id === "b")?.start).toBe(4);
|
||||
expect(videos.find((v) => v.id === "c")?.start).toBe(7); // 4 + 3
|
||||
});
|
||||
|
||||
it("resolves a reference to a non-video timed element (div clip)", () => {
|
||||
const videos = parseVideoElements(
|
||||
'<div id="title" data-start="0" data-duration="6"></div>' +
|
||||
'<video id="clip" src="b.mp4" data-start="title" data-duration="5"></video>',
|
||||
);
|
||||
expect(videos.find((v) => v.id === "clip")?.start).toBe(6);
|
||||
});
|
||||
|
||||
it("derives a referenced clip's duration from data-end when data-duration is absent", () => {
|
||||
const videos = parseVideoElements(
|
||||
'<video id="intro" src="a.mp4" data-start="2" data-end="9"></video>' +
|
||||
'<video id="main" src="b.mp4" data-start="intro" data-duration="5"></video>',
|
||||
);
|
||||
// intro: start 2, end 9 -> duration 7 -> main starts at 9.
|
||||
expect(videos.find((v) => v.id === "main")?.start).toBe(9);
|
||||
});
|
||||
|
||||
it("falls back to 0 (never NaN) for an unknown reference target", () => {
|
||||
const videos = parseVideoElements(
|
||||
'<video id="orphan" src="a.mp4" data-start="does-not-exist" data-duration="5"></video>',
|
||||
);
|
||||
const orphan = videos.find((v) => v.id === "orphan");
|
||||
expect(orphan?.start).toBe(0);
|
||||
expect(Number.isNaN(orphan?.start)).toBe(false);
|
||||
expect(orphan?.end).toBe(5);
|
||||
});
|
||||
|
||||
it("does not hang or NaN on a circular reference", () => {
|
||||
const videos = parseVideoElements(
|
||||
'<video id="a" src="a.mp4" data-start="b" data-duration="4"></video>' +
|
||||
'<video id="b" src="b.mp4" data-start="a" data-duration="3"></video>',
|
||||
);
|
||||
for (const v of videos) {
|
||||
expect(Number.isNaN(v.start)).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("FrameLookupTable", () => {
|
||||
@@ -334,6 +402,21 @@ describe("FrameLookupTable", () => {
|
||||
expect(table.getActiveFramePayloads(1.5).has("hero")).toBe(false);
|
||||
});
|
||||
|
||||
it("places a relative-reference video in its resolved window end-to-end (was blank)", () => {
|
||||
// The reported bug: <video data-start="intro"> gave NaN start/end, so the
|
||||
// active-window checks (start <= t <= end) were always false and the clip
|
||||
// composited blank. With resolution, `main` is active across [10, 30].
|
||||
const videos = parseVideoElements(
|
||||
'<video id="intro" src="a.mp4" data-start="0" data-duration="10"></video>' +
|
||||
'<video id="main" src="b.mp4" data-start="intro" data-duration="20"></video>',
|
||||
);
|
||||
const table = createFrameLookupTable(videos, [{ ...fakeExtracted(600, 30), videoId: "main" }]);
|
||||
expect(table.getActiveFramePayloads(5).has("main")).toBe(false); // before resolved start (10)
|
||||
expect(table.getActiveFramePayloads(15).has("main")).toBe(true); // within [10, 30]
|
||||
expect(table.getActiveFramePayloads(29).has("main")).toBe(true);
|
||||
expect(table.getActiveFramePayloads(31).has("main")).toBe(false); // after resolved end (30)
|
||||
});
|
||||
|
||||
it("holds the last frame at the inclusive clip end (t === end)", () => {
|
||||
// clip [1,3] with exactly 2s of source frames (60 @ 30fps). The frame
|
||||
// landing on t === end used to deactivate one frame early and render blank,
|
||||
|
||||
@@ -10,7 +10,12 @@ import { spawn } from "child_process";
|
||||
import { copyFileSync, existsSync, linkSync, mkdirSync, readdirSync, rmSync } from "fs";
|
||||
import { isAbsolute, join, posix, resolve, sep } from "path";
|
||||
import { parseHTML } from "linkedom";
|
||||
import { decodeUrlPathVariants, MEDIA_DURATION_CLAMP_EPSILON_SECONDS } from "@hyperframes/core";
|
||||
import {
|
||||
decodeUrlPathVariants,
|
||||
MEDIA_DURATION_CLAMP_EPSILON_SECONDS,
|
||||
parseNumeric,
|
||||
parseStartExpression,
|
||||
} from "@hyperframes/core";
|
||||
import { trackChildProcess } from "../utils/processTracker.js";
|
||||
import { extractMediaMetadata, type VideoMetadata } from "../utils/ffprobe.js";
|
||||
import {
|
||||
@@ -148,9 +153,102 @@ export interface ExtractionResult {
|
||||
phaseBreakdown: ExtractionPhaseBreakdown;
|
||||
}
|
||||
|
||||
// Minimal structural DOM shape the reference resolver needs, so it works
|
||||
// against linkedom (Node) without pulling in lib.dom types.
|
||||
interface RefResolverEl {
|
||||
getAttribute(name: string): string | null;
|
||||
}
|
||||
interface RefResolverDoc {
|
||||
getElementById(id: string): RefResolverEl | null;
|
||||
querySelector(selector: string): RefResolverEl | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find the element a relative `data-start` reference points at — by `id`
|
||||
* first, then by `data-composition-id` (a sub-composition can be referenced).
|
||||
* The reference-id grammar (see parseStartExpression) is restricted to
|
||||
* `[A-Za-z0-9_.:-]`, none of which need escaping inside a quoted attribute
|
||||
* selector, so no CSS.escape (absent in linkedom) is required.
|
||||
*/
|
||||
function findReferenceTargetEl(doc: RefResolverDoc, refId: string): RefResolverEl | null {
|
||||
return doc.getElementById(refId) ?? doc.querySelector(`[data-composition-id="${refId}"]`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve an element's absolute start time (seconds) the same way the browser
|
||||
* runtime's startResolver does, so `<video data-start="intro">` (a relative
|
||||
* reference to another clip's end) renders at the right time instead of
|
||||
* producing NaN and compositing blank. Durations come from `data-duration` or
|
||||
* `data-end` here; the natural-media-duration fallback isn't known at parse
|
||||
* time, so — exactly like the runtime — an unknown-duration reference falls
|
||||
* back to the target's start and an unknown target falls back to 0 (never NaN).
|
||||
*/
|
||||
function resolveReferencedStart(
|
||||
doc: RefResolverDoc,
|
||||
el: RefResolverEl,
|
||||
startCache: Map<RefResolverEl, number>,
|
||||
visiting: Set<RefResolverEl>,
|
||||
): number {
|
||||
const cached = startCache.get(el);
|
||||
if (cached !== undefined) return cached;
|
||||
if (visiting.has(el)) return 0; // cycle guard (A -> B -> A)
|
||||
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 = resolveReferencedStart(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);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Duration of a referenced clip, from `data-duration` or `data-end - start`.
|
||||
* Returns null when only the natural media duration would settle it (unknown
|
||||
* at parse time) — the caller then treats the reference as duration-0.
|
||||
*/
|
||||
function resolveReferencedDuration(
|
||||
doc: RefResolverDoc,
|
||||
el: RefResolverEl,
|
||||
startCache: Map<RefResolverEl, number>,
|
||||
visiting: Set<RefResolverEl>,
|
||||
): 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 = resolveReferencedStart(doc, el, startCache, visiting);
|
||||
const delta = endAttr - start;
|
||||
if (Number.isFinite(delta) && delta > 0) return delta;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export function parseVideoElements(html: string): VideoElement[] {
|
||||
const videos: VideoElement[] = [];
|
||||
const { document } = parseHTML(unwrapTemplate(html));
|
||||
const startCache = new Map<RefResolverEl, number>();
|
||||
const visiting = new Set<RefResolverEl>();
|
||||
|
||||
const videoEls = document.querySelectorAll("video[src]");
|
||||
let autoIdCounter = 0;
|
||||
@@ -170,7 +268,12 @@ export function parseVideoElements(html: string): VideoElement[] {
|
||||
const mediaStartAttr = el.getAttribute("data-media-start");
|
||||
const hasAudioAttr = el.getAttribute("data-has-audio");
|
||||
|
||||
const start = startAttr ? parseFloat(startAttr) : 0;
|
||||
// Resolve data-start, including relative references ("intro", "intro + 2")
|
||||
// to another clip's end — the browser runtime resolves these but a raw
|
||||
// parseFloat here would yield NaN, placing the clip at NaN so it composites
|
||||
// blank in the final render. `startAttr` may be a plain number or a
|
||||
// reference; the resolver handles both.
|
||||
const start = startAttr ? resolveReferencedStart(document, el, startCache, visiting) : 0;
|
||||
// Derive end from data-end → data-start+data-duration → Infinity (natural duration).
|
||||
// The caller (htmlCompiler) clamps Infinity to the composition's absoluteEnd.
|
||||
let end = 0;
|
||||
@@ -206,6 +309,8 @@ export interface ImageElement {
|
||||
export function parseImageElements(html: string): ImageElement[] {
|
||||
const images: ImageElement[] = [];
|
||||
const { document } = parseHTML(unwrapTemplate(html));
|
||||
const startCache = new Map<RefResolverEl, number>();
|
||||
const visiting = new Set<RefResolverEl>();
|
||||
|
||||
const imgEls = document.querySelectorAll("img[src]");
|
||||
let autoIdCounter = 0;
|
||||
@@ -222,7 +327,9 @@ export function parseImageElements(html: string): ImageElement[] {
|
||||
const endAttr = el.getAttribute("data-end");
|
||||
const durationAttr = el.getAttribute("data-duration");
|
||||
|
||||
const start = startAttr ? parseFloat(startAttr) : 0;
|
||||
// Resolve relative data-start references (see parseVideoElements) so a
|
||||
// referenced image start doesn't become NaN and drop the image from the render.
|
||||
const start = startAttr ? resolveReferencedStart(document, el, startCache, visiting) : 0;
|
||||
let end = 0;
|
||||
if (endAttr) {
|
||||
end = parseFloat(endAttr);
|
||||
|
||||
Reference in New Issue
Block a user