mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(core,engine,producer): handle id-less media in sub-composition renders (#96)
## What Move the id-less media fix into the shared timing compiler so producer can resolve durations for sub-composition videos before inlining, then carry the merged result through engine parsing, regression coverage, and the regression Docker image used in CI. This PR now does five concrete things: - assigns stable ids to id-less media in core `compileTimingAttrs()` so unresolved duration injection can target them - keeps the engine-side `parseVideoElements()` support for `video[src]` plus the newer `data-duration` / natural-duration fallback from `main` - makes producer prefer sub-composition media metadata over the later inlined-document parse when the same media id appears in both places - makes `sub-composition-video` a runnable regression test by fixing its metadata and checking in the missing `output/compiled.html` snapshot - removes the stale `pnpm-workspace.yaml` copy step from `Dockerfile.test`, so regression CI builds the Bun-based test image from the current workspace layout ## Why - media without an explicit `id` could not participate in unresolved-duration resolution early enough - producer could lose the resolved sub-composition timing by overwriting it with the later inlined parse - the regression fixture intended to cover this case was not actually running in CI because its `meta.json` was incomplete and the required compiled snapshot was missing - the regression image definition still expected a deleted `pnpm-workspace.yaml`, so GitHub Actions failed before the test shard could start Putting the id-generation step in core makes the behavior reusable instead of relying on producer-only HTML patching. ## How ### Shared compiler - core `compileTimingAttrs()` now auto-assigns stable ids to id-less `video` / `audio` tags - those generated ids are returned in `unresolved`, so `injectDurations()` can add `data-duration` and `data-end` to the same media element later in the pipeline - added core tests that cover auto-id assignment and duration injection for generated ids ### Producer - when producer combines `subVideos` / `subAudios` with the media re-parsed from the final inlined HTML, it now lets the sub-composition metadata win - this preserves the resolved/clamped timing already computed for nested media instead of overwriting it with the later parse - `sub-composition-video` now has valid regression metadata and a checked-in `output/compiled.html` snapshot so CI actually executes it ### Engine - resolved the merge conflict in `videoFrameExtractor` by keeping the broader `video[src]` parsing from this branch and the `data-duration` / natural-duration fallback that landed on `main` - added a focused engine unit test for videos without ids ### CI image - `Dockerfile.test` now copies only `package.json` and `bun.lock` at the workspace root before `bun install --frozen-lockfile` - this matches the current monorepo layout and removes the obsolete pnpm-era dependency on `pnpm-workspace.yaml` ## Test plan - [x] `bun run --filter @hyperframes/core test` - [x] `bun run --filter @hyperframes/engine test` - [x] `bun run --filter @hyperframes/producer test --update --sequential sub-composition-video` - [x] `bun run --filter @hyperframes/producer test --sequential sub-composition-video` - [x] Browser check with `agent-browser` against the compiled fixture page (`http://127.0.0.1:8123/compiled.html`) - [x] Clean tracked-only Docker build of `Dockerfile.test` with the PR version of the file applied ## Notes - Latest regression workflow is green on `main`, but before this PR the `sub-composition-video` fixture was being skipped by the harness rather than exercised end to end. - The CI Docker fix was validated from a tracked-only export to avoid local untracked worktree artifacts affecting the result.
This commit is contained in:
+1
-1
@@ -65,7 +65,7 @@ RUN curl -fsSL https://bun.sh/install | bash
|
||||
ENV PATH="/root/.bun/bin:$PATH"
|
||||
|
||||
# Install dependencies (full, including devDependencies for tsx + test harness)
|
||||
COPY package.json bun.lock pnpm-workspace.yaml ./
|
||||
COPY package.json bun.lock ./
|
||||
COPY packages/core/package.json packages/core/package.json
|
||||
COPY packages/engine/package.json packages/engine/package.json
|
||||
COPY packages/producer/package.json packages/producer/package.json
|
||||
|
||||
@@ -35,6 +35,18 @@ describe("compileTimingAttrs", () => {
|
||||
expect(unresolved[0].start).toBe(1);
|
||||
});
|
||||
|
||||
it("auto-assigns ids to id-less videos so unresolved duration resolution can target them", () => {
|
||||
const html = '<video src="a.mp4" data-start="1">';
|
||||
const { html: compiled, unresolved } = compileTimingAttrs(html);
|
||||
|
||||
expect(compiled).toContain('id="hf-video-0"');
|
||||
expect(compiled).toContain('data-has-audio="true"');
|
||||
expect(unresolved).toHaveLength(1);
|
||||
expect(unresolved[0].id).toBe("hf-video-0");
|
||||
expect(unresolved[0].tagName).toBe("video");
|
||||
expect(unresolved[0].start).toBe(1);
|
||||
});
|
||||
|
||||
it("compiles audio tags the same as video (minus data-has-audio)", () => {
|
||||
const html = '<audio id="a1" src="music.mp3" data-start="0" data-duration="10">';
|
||||
const { html: compiled } = compileTimingAttrs(html);
|
||||
@@ -70,6 +82,15 @@ describe("injectDurations", () => {
|
||||
expect(result).toContain('data-end="6"');
|
||||
});
|
||||
|
||||
it("injects durations for auto-assigned media ids", () => {
|
||||
const { html, unresolved } = compileTimingAttrs('<video src="a.mp4" data-start="1">');
|
||||
const result = injectDurations(html, [{ id: unresolved[0]!.id, duration: 4 }]);
|
||||
|
||||
expect(result).toContain('id="hf-video-0"');
|
||||
expect(result).toContain('data-duration="4"');
|
||||
expect(result).toContain('data-end="5"');
|
||||
});
|
||||
|
||||
it("does not overwrite existing data-duration", () => {
|
||||
const html = '<video id="v1" src="a.mp4" data-start="0" data-duration="3">';
|
||||
const result = injectDurations(html, [{ id: "v1", duration: 10 }]);
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
* Works in both Node.js and browser (no dependencies, regex-based).
|
||||
*
|
||||
* Guarantees every timed element gets:
|
||||
* - id on media elements when missing
|
||||
* - data-end (computed from data-start + data-duration when possible)
|
||||
* - data-has-audio="true" on <video> elements
|
||||
*
|
||||
@@ -66,11 +67,16 @@ function injectAttr(tag: string, attr: string, value: string): string {
|
||||
function compileTag(
|
||||
tag: string,
|
||||
isVideo: boolean,
|
||||
generateId: () => number,
|
||||
): { tag: string; unresolved: UnresolvedElement | null } {
|
||||
let result = tag;
|
||||
let unresolved: UnresolvedElement | null = null;
|
||||
|
||||
const id = getAttr(result, "id");
|
||||
let id = getAttr(result, "id");
|
||||
if (!id) {
|
||||
id = `${isVideo ? "hf-video" : "hf-audio"}-${generateId()}`;
|
||||
result = injectAttr(result, "id", id);
|
||||
}
|
||||
const startStr = getAttr(result, "data-start");
|
||||
const start = startStr !== null ? parseFloat(startStr) : 0;
|
||||
const mediaStartStr = getAttr(result, "data-media-start");
|
||||
@@ -114,17 +120,19 @@ function compileTag(
|
||||
*/
|
||||
export function compileTimingAttrs(html: string): CompilationResult {
|
||||
const unresolved: UnresolvedElement[] = [];
|
||||
let nextVideoId = 0;
|
||||
let nextAudioId = 0;
|
||||
|
||||
// Process <video ...> tags
|
||||
html = html.replace(/<video[^>]*>/gi, (match) => {
|
||||
const { tag, unresolved: u } = compileTag(match, true);
|
||||
const { tag, unresolved: u } = compileTag(match, true, () => nextVideoId++);
|
||||
if (u) unresolved.push(u);
|
||||
return tag;
|
||||
});
|
||||
|
||||
// Process <audio ...> tags
|
||||
html = html.replace(/<audio[^>]*>/gi, (match) => {
|
||||
const { tag, unresolved: u } = compileTag(match, false);
|
||||
const { tag, unresolved: u } = compileTag(match, false, () => nextAudioId++);
|
||||
if (u) unresolved.push(u);
|
||||
return tag;
|
||||
});
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { parseVideoElements } from "./videoFrameExtractor.js";
|
||||
|
||||
describe("parseVideoElements", () => {
|
||||
it("parses videos without an id or data-start attribute", () => {
|
||||
const videos = parseVideoElements('<video src="clip.mp4"></video>');
|
||||
|
||||
expect(videos).toHaveLength(1);
|
||||
expect(videos[0]).toMatchObject({
|
||||
id: "hf-video-0",
|
||||
src: "clip.mp4",
|
||||
start: 0,
|
||||
end: Infinity,
|
||||
mediaStart: 0,
|
||||
hasAudio: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves explicit ids and derives end from data-duration", () => {
|
||||
const videos = parseVideoElements(
|
||||
'<video id="hero" src="clip.mp4" data-start="2" data-duration="5" data-media-start="1.5" data-has-audio="true"></video>',
|
||||
);
|
||||
|
||||
expect(videos).toHaveLength(1);
|
||||
expect(videos[0]).toEqual({
|
||||
id: "hero",
|
||||
src: "clip.mp4",
|
||||
start: 2,
|
||||
end: 7,
|
||||
mediaStart: 1.5,
|
||||
hasAudio: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -52,21 +52,17 @@ export function parseVideoElements(html: string): VideoElement[] {
|
||||
const videos: VideoElement[] = [];
|
||||
const { document } = parseHTML(html);
|
||||
|
||||
// Union: original "video[id][src]" (backward compat) + "video[src][data-start]"
|
||||
// (sub-composition videos that have timing but no explicit id).
|
||||
const videoEls = Array.from(
|
||||
new Set([
|
||||
...Array.from(document.querySelectorAll("video[id][src]")),
|
||||
...Array.from(document.querySelectorAll("video[src][data-start]")),
|
||||
]),
|
||||
);
|
||||
videoEls.forEach((el, i) => {
|
||||
if (!el.id) el.id = `hf-video-${i}`;
|
||||
});
|
||||
const videoEls = document.querySelectorAll("video[src]");
|
||||
let autoIdCounter = 0;
|
||||
for (const el of videoEls) {
|
||||
const id = el.getAttribute("id");
|
||||
const src = el.getAttribute("src");
|
||||
if (!id || !src) continue;
|
||||
if (!src) continue;
|
||||
// Generate a stable ID for videos without one — the producer needs IDs
|
||||
// to track extracted frames and composite them during encoding.
|
||||
const id = el.getAttribute("id") || `hf-video-${autoIdCounter++}`;
|
||||
if (!el.getAttribute("id")) {
|
||||
el.setAttribute("id", id);
|
||||
}
|
||||
|
||||
const startAttr = el.getAttribute("data-start");
|
||||
const endAttr = el.getAttribute("data-end");
|
||||
|
||||
@@ -746,8 +746,8 @@ export async function compileForRender(
|
||||
const mainVideos = parseVideoElements(html);
|
||||
const mainAudios = parseAudioElements(html);
|
||||
|
||||
const videos = dedupeElementsById([...subVideos, ...mainVideos]);
|
||||
const audios = dedupeElementsById([...subAudios, ...mainAudios]);
|
||||
const videos = dedupeElementsById([...mainVideos, ...subVideos]);
|
||||
const audios = dedupeElementsById([...mainAudios, ...subAudios]);
|
||||
|
||||
// Advisory video checks (sparse keyframes, VFR). Fire-and-forget — these spawn
|
||||
// ffprobe subprocesses and should not block compilation since they only produce warnings.
|
||||
@@ -773,28 +773,6 @@ export async function compileForRender(
|
||||
.catch(() => {});
|
||||
}
|
||||
|
||||
// Persist auto-assigned IDs back into the HTML so the compiled file served
|
||||
// to Puppeteer has matching element IDs. parseVideoElements uses parseHTML
|
||||
// internally and sets el.id = "hf-video-N" on the JSDOM node, but that does
|
||||
// not mutate the html string. We do one more DOM pass here to write those IDs
|
||||
// into the document and re-serialize — only if there are any id-less videos.
|
||||
const autoIdVideos = videos.filter((v) => v.id.startsWith("hf-video-"));
|
||||
let htmlWithIds = html;
|
||||
if (autoIdVideos.length > 0) {
|
||||
const { document: idDoc } = parseHTML(html);
|
||||
let changed = false;
|
||||
for (const v of autoIdVideos) {
|
||||
const el = idDoc.querySelector(`video[src="${v.src}"]:not([id])`);
|
||||
if (el) {
|
||||
el.id = v.id;
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
if (changed) {
|
||||
htmlWithIds = idDoc.documentElement?.outerHTML ?? html;
|
||||
}
|
||||
}
|
||||
|
||||
// Read dimensions from root composition element using DOM parser
|
||||
const { document } = parseHTML(html);
|
||||
const rootEl = document.querySelector("[data-composition-id]");
|
||||
@@ -812,7 +790,7 @@ export async function compileForRender(
|
||||
: 0;
|
||||
|
||||
return {
|
||||
html: htmlWithIds,
|
||||
html,
|
||||
subCompositions,
|
||||
videos,
|
||||
audios,
|
||||
@@ -970,8 +948,8 @@ export async function recompileWithResolutions(
|
||||
const mainVideos = parseVideoElements(html);
|
||||
const mainAudios = parseAudioElements(html);
|
||||
|
||||
const videos = dedupeElementsById([...subVideos, ...mainVideos]);
|
||||
const audios = dedupeElementsById([...subAudios, ...mainAudios]);
|
||||
const videos = dedupeElementsById([...mainVideos, ...subVideos]);
|
||||
const audios = dedupeElementsById([...mainAudios, ...subAudios]);
|
||||
|
||||
const remaining = compiled.unresolvedCompositions.filter(
|
||||
(c) => !resolutions.some((r) => r.id === c.id),
|
||||
|
||||
@@ -4,6 +4,8 @@
|
||||
"tags": ["video", "sub-composition", "regression"],
|
||||
"minPsnr": 25,
|
||||
"maxFrameFailures": 5,
|
||||
"minAudioCorrelation": 0.0,
|
||||
"maxAudioLagWindows": 120,
|
||||
"renderConfig": {
|
||||
"fps": 30
|
||||
}
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -1,3 +1,3 @@
|
||||
version https://git-lfs.github.com/spec/v1
|
||||
oid sha256:c08dcb89ed447e5bbff18214576ea7fff9ed6b5129cf12795e97199dff3d8e21
|
||||
size 3830455
|
||||
oid sha256:4a1ac6cb0517b8224364833754981228f1138848e4f70715065f64f939ab4253
|
||||
size 7277596
|
||||
|
||||
Reference in New Issue
Block a user