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 for audio tracks (#2062)
parseAudioElements read data-start with a bare parseFloat, so a relative reference (data-start="introClip", the documented 'start when that clip ends' pattern) resolved to NaN. The mixer then silently dropped the track, rendering the whole segment as pure digital silence — even though the SAME reference on the sibling <video> placed the visual correctly (#2030 taught parseVideoElements/parseImageElements to resolve refs; audio never learned). Root fix, single source of truth: extract the Node-side reference resolver out of videoFrameExtractor into referenceResolver.ts and use it in parseAudioElements for both <audio> and <video data-has-audio> tracks. Now every media parser resolves relative timing identically, so audio and video cannot drift again. The two near-identical parse loops share one builder; end stays a numeric read (mixer derives real length downstream), NaN-guarded. Verified end-to-end: a composition with <audio data-start="clipId"> now renders an audio stream that is silent before the referenced clip ends and audible after (matches the numeric-start control); previously the output had no audio stream at all. 78 engine media tests pass (4 new).
This commit is contained in:
@@ -31,7 +31,7 @@ vi.mock("../utils/runFfmpeg.js", () => ({
|
||||
runFfmpeg: runFfmpegMock,
|
||||
}));
|
||||
|
||||
import { processCompositionAudio } from "./audioMixer.js";
|
||||
import { parseAudioElements, processCompositionAudio } from "./audioMixer.js";
|
||||
|
||||
describe("processCompositionAudio", () => {
|
||||
const tempDirs: string[] = [];
|
||||
@@ -386,3 +386,50 @@ describe("processCompositionAudio", () => {
|
||||
expect(prepareArgs).toContain(join(baseDir, "assets", filename));
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseAudioElements — relative data-start resolution", () => {
|
||||
const wrap = (body: string) =>
|
||||
`<div id="root" class="composition" data-composition-id="c" data-start="0" data-duration="10">${body}</div>`;
|
||||
|
||||
it("resolves a relative data-start reference to the target clip's end (matches video)", () => {
|
||||
// <audio data-start="v0"> means 'start when clip v0 ends' = v0.start + v0.duration.
|
||||
const html = wrap(
|
||||
`<video id="v0" class="clip" data-start="0" data-duration="3" src="a.mp4" muted></video>` +
|
||||
`<audio id="a0" data-start="v0" data-duration="2" src="a.m4a"></audio>`,
|
||||
);
|
||||
const els = parseAudioElements(html);
|
||||
const a0 = els.find((e) => e.id === "a0");
|
||||
expect(a0).toBeDefined();
|
||||
// Regression guard: the pre-fix parseFloat("v0") produced NaN, and the
|
||||
// mixer silently dropped the track.
|
||||
expect(Number.isNaN(a0!.start)).toBe(false);
|
||||
expect(a0!.start).toBe(3);
|
||||
});
|
||||
|
||||
it("chains references and never emits NaN start (falls back to 0 for an unknown target)", () => {
|
||||
const html = wrap(
|
||||
`<video id="v0" class="clip" data-start="0" data-duration="2" src="a.mp4" muted></video>` +
|
||||
`<video id="v1" class="clip" data-start="v0" data-duration="2" src="b.mp4" muted></video>` +
|
||||
`<audio id="a1" data-start="v1" src="a.m4a"></audio>` +
|
||||
`<audio id="a2" data-start="does-not-exist" src="b.m4a"></audio>`,
|
||||
);
|
||||
const els = parseAudioElements(html);
|
||||
expect(els.find((e) => e.id === "a1")!.start).toBe(4); // v1 ends at 2+2
|
||||
expect(els.find((e) => e.id === "a2")!.start).toBe(0); // unknown ref → 0, not NaN
|
||||
});
|
||||
|
||||
it("still reads a numeric data-start unchanged", () => {
|
||||
const html = wrap(`<audio id="a0" data-start="2.5" data-duration="1" src="a.m4a"></audio>`);
|
||||
expect(parseAudioElements(html).find((e) => e.id === "a0")!.start).toBe(2.5);
|
||||
});
|
||||
|
||||
it("resolves the reference for a data-has-audio video's audio track too", () => {
|
||||
const html = wrap(
|
||||
`<video id="v0" class="clip" data-start="0" data-duration="4" src="a.mp4" muted></video>` +
|
||||
`<video id="v1" class="clip" data-start="v0" data-duration="2" src="b.mp4" data-has-audio="true"></video>`,
|
||||
);
|
||||
const track = parseAudioElements(html).find((e) => e.id === "v1-audio");
|
||||
expect(track).toBeDefined();
|
||||
expect(track!.start).toBe(4);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -13,6 +13,7 @@ import { DEFAULT_CONFIG, type EngineConfig } from "../config.js";
|
||||
import { runFfmpeg } from "../utils/runFfmpeg.js";
|
||||
import { unwrapTemplate } from "../utils/htmlTemplate.js";
|
||||
import { resolveProjectRelativeSrc } from "./videoFrameExtractor.js";
|
||||
import { resolveReferencedStart, type RefResolverEl } from "./referenceResolver.js";
|
||||
import type { AudioElement, AudioTrack, MixResult } from "./audioMixer.types.js";
|
||||
import { applyVolumeEnvelopeToWav } from "./audioVolumeEnvelope.js";
|
||||
|
||||
@@ -173,54 +174,51 @@ export function parseAudioElements(html: string): AudioElement[] {
|
||||
const elements: AudioElement[] = [];
|
||||
const { document } = parseHTML(unwrapTemplate(html));
|
||||
|
||||
// Parse <audio> elements
|
||||
const audioEls = document.querySelectorAll("audio[id][src]");
|
||||
for (const el of audioEls) {
|
||||
const id = el.getAttribute("id");
|
||||
const src = el.getAttribute("src");
|
||||
if (!id || !src) continue;
|
||||
// Shared resolver state so a relative `data-start` ("start when clip X ends")
|
||||
// resolves against every clip in the composition — exactly as
|
||||
// parseVideoElements does. Without this, `parseFloat("clipId")` yields NaN and
|
||||
// the mixer silently drops the track (the segment renders as pure digital
|
||||
// silence), even though the same reference places the *video* correctly.
|
||||
const startCache = new Map<RefResolverEl, number>();
|
||||
const visiting = new Set<RefResolverEl>();
|
||||
const resolveStart = (el: RefResolverEl): number =>
|
||||
el.getAttribute("data-start") ? resolveReferencedStart(document, el, startCache, visiting) : 0;
|
||||
// `end` stays a plain numeric read (the mixer derives the real segment length
|
||||
// from data-duration / natural media downstream); guard NaN so a malformed
|
||||
// value never poisons the mix instead of falling back to 0.
|
||||
const parseEnd = (raw: string | null): number => {
|
||||
const end = raw ? parseFloat(raw) : 0;
|
||||
return Number.isFinite(end) ? end : 0;
|
||||
};
|
||||
|
||||
const startAttr = el.getAttribute("data-start");
|
||||
const endAttr = el.getAttribute("data-end");
|
||||
// <audio> and <video data-has-audio> tracks differ only in the emitted id
|
||||
// and `type`; everything else (timing, layer, volume) is read identically.
|
||||
const build = (el: RefResolverEl, id: string, type: AudioElement["type"]): AudioElement => {
|
||||
const mediaStartAttr = el.getAttribute("data-media-start");
|
||||
const layerAttr = el.getAttribute("data-layer");
|
||||
const volumeAttr = el.getAttribute("data-volume");
|
||||
|
||||
elements.push({
|
||||
return {
|
||||
id,
|
||||
src,
|
||||
start: startAttr ? parseFloat(startAttr) : 0,
|
||||
end: endAttr ? parseFloat(endAttr) : 0,
|
||||
src: el.getAttribute("src") as string,
|
||||
start: resolveStart(el),
|
||||
end: parseEnd(el.getAttribute("data-end")),
|
||||
mediaStart: mediaStartAttr ? parseFloat(mediaStartAttr) : 0,
|
||||
layer: layerAttr ? parseInt(layerAttr) : 0,
|
||||
volume: volumeAttr ? parseFloat(volumeAttr) : 1.0,
|
||||
type: "audio",
|
||||
});
|
||||
type,
|
||||
};
|
||||
};
|
||||
|
||||
for (const el of document.querySelectorAll("audio[id][src]")) {
|
||||
const id = el.getAttribute("id");
|
||||
if (!id || !el.getAttribute("src")) continue;
|
||||
elements.push(build(el, id, "audio"));
|
||||
}
|
||||
|
||||
// Parse <video> elements with data-has-audio="true"
|
||||
const videoEls = document.querySelectorAll('video[id][src][data-has-audio="true"]');
|
||||
for (const el of videoEls) {
|
||||
for (const el of document.querySelectorAll('video[id][src][data-has-audio="true"]')) {
|
||||
const id = el.getAttribute("id");
|
||||
const src = el.getAttribute("src");
|
||||
if (!id || !src) continue;
|
||||
|
||||
const startAttr = el.getAttribute("data-start");
|
||||
const endAttr = el.getAttribute("data-end");
|
||||
const mediaStartAttr = el.getAttribute("data-media-start");
|
||||
const layerAttr = el.getAttribute("data-layer");
|
||||
const volumeAttr = el.getAttribute("data-volume");
|
||||
|
||||
elements.push({
|
||||
id: `${id}-audio`,
|
||||
src,
|
||||
start: startAttr ? parseFloat(startAttr) : 0,
|
||||
end: endAttr ? parseFloat(endAttr) : 0,
|
||||
mediaStart: mediaStartAttr ? parseFloat(mediaStartAttr) : 0,
|
||||
layer: layerAttr ? parseInt(layerAttr) : 0,
|
||||
volume: volumeAttr ? parseFloat(volumeAttr) : 1.0,
|
||||
type: "video",
|
||||
});
|
||||
if (!id || !el.getAttribute("src")) continue;
|
||||
elements.push(build(el, `${id}-audio`, "video"));
|
||||
}
|
||||
|
||||
return elements;
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
/**
|
||||
* Node-side resolver for relative `data-start` timing references, shared by
|
||||
* every parser that reads media timing out of compiled HTML — video frames
|
||||
* (`parseVideoElements`), images (`parseImageElements`), and audio
|
||||
* (`parseAudioElements`). Keeping the resolution in ONE place is load-bearing:
|
||||
* if audio and video disagree on what `data-start="intro"` means, a relative
|
||||
* reference that renders a video at the right time silently drops the audio
|
||||
* track (they used to — audio parsed `parseFloat("intro") = NaN`).
|
||||
*
|
||||
* Mirrors the browser runtime's startResolver so `snapshot`/`render` agree.
|
||||
* DOM access is via a minimal structural shape so it works against linkedom
|
||||
* (Node) without pulling in lib.dom types.
|
||||
*/
|
||||
|
||||
import { parseNumeric, parseStartExpression } from "@hyperframes/core";
|
||||
|
||||
/** Minimal structural DOM shape the reference resolver needs. */
|
||||
export 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 / dropping audio. 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).
|
||||
*/
|
||||
export 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;
|
||||
}
|
||||
@@ -10,13 +10,9 @@ 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,
|
||||
parseNumeric,
|
||||
parseStartExpression,
|
||||
} from "@hyperframes/core";
|
||||
import { decodeUrlPathVariants, MEDIA_DURATION_CLAMP_EPSILON_SECONDS } from "@hyperframes/core";
|
||||
import { trackChildProcess } from "../utils/processTracker.js";
|
||||
import { resolveReferencedStart, type RefResolverEl } from "./referenceResolver.js";
|
||||
import { extractMediaMetadata, type VideoMetadata } from "../utils/ffprobe.js";
|
||||
import {
|
||||
analyzeCompositionHdr,
|
||||
@@ -153,97 +149,6 @@ 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));
|
||||
|
||||
Reference in New Issue
Block a user