Skip to content

Commit 634df5a

Browse files
fix(producer): give inlined media a document-unique render id (#3342)
* fix(producer): give inlined media a document-unique render id Element ids are unique per composition file, but the render document is the inlined union of every file. The producer merged the per-file media lists and deduplicated by id, so clips that shared an id collapsed into a single entry, and every id-keyed stage (extract, inject, visibility, bounds) resolved to whichever element came first in the document. The surviving clip's frames landed on the wrong element and the visible scene rendered without footage. Two shapes hit this, and neither is author error: - Two scenes that each declare `<video id="clip">`. Legal per file, and unavoidable when a scene is duplicated into a copy with inner ids kept, or when one file is mounted twice. - Two scenes that each declare a bare `<video>`. The timing compiler numbers auto-ids per file, so both arrive as `hf-video-0` with no authored id involved at all. Stamp a document-unique `data-hf-render-id` while inlining, and read the media list off the inlined document instead of merging per-file lists. The render id equals the element id whenever that id is already unique, so documents without a collision keep identical pipeline keys. Author `id` attributes are left alone: 158 of the 161 registry blocks reference their own ids from `#id` CSS or getElementById, so renaming would trade broken footage for broken styling. The engine resolves media elements through the render id instead, falling back to getElementById for documents the producer never compiled. Collecting from the inlined document also retires the per-file media extraction in parseSubCompositions along with its offset bookkeeping; host offsets are recovered from the composition hosts the clip sits in. * fix(core): resolve render-frame siblings by render id in the runtime The injector creates each `__render_frame_<id>__` sibling from the media element's render id, but four runtime readers still built that id from the plain `el.id`. On a document where two compositions share a media id, all of them resolved the first collider's frame. colorGrading is the one that changes pixels: findRenderFrameImage returns the image the grading pass samples, with no class check to catch the mismatch, so the second video was graded from the first one's frame. media, mediaProxy and video-texture-compat use it as a render-mode or substitute-source signal, where both colliders happen to agree during render, but none of them should rest on that. Add renderFrameSibling as the single owner of "which frame belongs to this element" and route all four through it. It reads the stamped render id and falls back to the author id, so a collision-free document resolves exactly as before and an uncompiled one (preview, snapshot, check) is unchanged. The engine's in-page bridge keeps its own copy of the rule because code serialized into page.evaluate cannot import; it now names core as the definition, and a test pins the sibling-id format both sides build so they cannot drift apart silently. * refactor(engine): build render-frame sibling ids from core's definition The drift guard named both sides but pinned one. renderFrameSibling.test asserts core's format, while the engine rebuilt the same id from a literal template at six independent sites. Changing the format on either side left the test green and every runtime reader silently unable to find its frame — this PR's own failure mode, one level up. Export the affixes and renderFrameIdForRenderId from core, and take the id from there at all six. Four sites resolve it on the Node side, where the engine can import; the two that iterate the DOM in-page receive the affixes as evaluate arguments, which avoids depending on bridge install order. Also switch two `__hfMediaId?.(el) ?? el.id` reads to `||`. The bridge returns "" for an element with neither id, so `??` kept the empty string and built `__render_frame___`, which no reader looks for. Inert today because the compiler assigns positional ids to id-less timed media, but it made the two sides disagree in the one case they could.
1 parent ec0b23f commit 634df5a

20 files changed

Lines changed: 1079 additions & 446 deletions

‎packages/core/src/compiler/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,5 @@ export {
8989

9090
// Asset-path primitives (shared across core, producer, CLI)
9191
export { CSS_URL_RE, PATH_ATTRS, isNonRelativeUrl, isPathInside } from "./assetPaths";
92+
93+
export { MEDIA_RENDER_ID_ATTR, assignMediaRenderIds } from "./mediaRenderIds";
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { describe, it, expect } from "vitest";
2+
import { parseHTML } from "linkedom";
3+
import { MEDIA_RENDER_ID_ATTR, assignMediaRenderIds } from "./mediaRenderIds";
4+
5+
function stamp(html: string): string[] {
6+
const { document } = parseHTML(html);
7+
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
8+
return Array.from(document.querySelectorAll("video, audio, img")).map(
9+
(el) => el.getAttribute(MEDIA_RENDER_ID_ATTR) ?? "",
10+
);
11+
}
12+
13+
describe("assignMediaRenderIds", () => {
14+
it("keeps the element id when it is already unique", () => {
15+
expect(stamp('<video id="hero" src="a.mp4">')).toEqual(["hero"]);
16+
});
17+
18+
it("disambiguates a media id shared by two inlined compositions", () => {
19+
// Two scenes each authored `<video id="clip">`: legal per file, duplicated
20+
// once both are inlined into one render document.
21+
expect(stamp('<video id="clip" src="a.mp4"><video id="clip" src="a.mp4">')).toEqual([
22+
"clip",
23+
"clip__hf2",
24+
]);
25+
});
26+
27+
it("disambiguates per-file auto-ids, which collide without any authored id", () => {
28+
// The timing compiler numbers unnamed media per file, so two bare <video>s
29+
// in two scenes both arrive as `hf-video-0`.
30+
expect(stamp('<video id="hf-video-0" src="a.mp4"><video id="hf-video-0" src="b.mp4">')).toEqual(
31+
["hf-video-0", "hf-video-0__hf2"],
32+
);
33+
});
34+
35+
it("keeps disambiguating past the first collision", () => {
36+
const html = '<video id="c" src="a.mp4"><video id="c" src="a.mp4"><video id="c" src="a.mp4">';
37+
expect(stamp(html)).toEqual(["c", "c__hf2", "c__hf3"]);
38+
});
39+
40+
it("separates ids across tag types", () => {
41+
expect(
42+
stamp('<video id="m" src="a.mp4"><audio id="m" src="a.mp3"><img id="m" src="a.png">'),
43+
).toEqual(["m", "m__hf2", "m__hf3"]);
44+
});
45+
46+
it("does not renumber elements that already carry a render id", () => {
47+
const { document } = parseHTML(
48+
`<video id="clip" ${MEDIA_RENDER_ID_ATTR}="clip" src="a.mp4">` +
49+
`<video id="clip" ${MEDIA_RENDER_ID_ATTR}="clip__hf2" src="a.mp4">`,
50+
);
51+
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
52+
expect(
53+
Array.from(document.querySelectorAll("video")).map((el) =>
54+
el.getAttribute(MEDIA_RENDER_ID_ATTR),
55+
),
56+
).toEqual(["clip", "clip__hf2"]);
57+
});
58+
59+
it("does not claim an id that a later element already holds as its render id", () => {
60+
// Re-running over a partially stamped document must not hand `clip__hf2`
61+
// to the first element and collide with the element already holding it.
62+
const { document } = parseHTML(
63+
`<video id="clip__hf2" src="a.mp4">` +
64+
`<video id="clip" ${MEDIA_RENDER_ID_ATTR}="clip__hf2" src="a.mp4">`,
65+
);
66+
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
67+
const ids = Array.from(document.querySelectorAll("video")).map((el) =>
68+
el.getAttribute(MEDIA_RENDER_ID_ATTR),
69+
);
70+
expect(new Set(ids).size).toBe(2);
71+
expect(ids[1]).toBe("clip__hf2");
72+
});
73+
74+
it("leaves media without a src alone", () => {
75+
const { document } = parseHTML('<video id="no-src"></video>');
76+
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
77+
expect(document.querySelector("video")?.hasAttribute(MEDIA_RENDER_ID_ATTR)).toBe(false);
78+
});
79+
});
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/**
2+
* Document-unique identity for media elements in a compiled render document.
3+
*
4+
* Element `id`s are only unique within one composition FILE. The render
5+
* document is the inlined union of every file, so ids collide there in two
6+
* ways an author cannot avoid:
7+
*
8+
* 1. Two scenes each declare `<video id="clip">` — legal per file, duplicated
9+
* once inlined.
10+
* 2. Two scenes each declare a bare `<video>` — the timing compiler numbers
11+
* auto-ids per file, so both become `hf-video-0`.
12+
*
13+
* The render pipeline keys media on that id (extract, inject, visibility,
14+
* bounds), so a collision collapses N elements into one entry and every
15+
* lookup resolves to whichever element happens to come first in the document.
16+
* The surviving clip's frames land on the wrong element and the visible scene
17+
* paints without footage.
18+
*
19+
* This module is the single owner of the fix: after inlining, every media
20+
* element gets a document-unique `data-hf-render-id`. It equals the element's
21+
* own id whenever that id is already unique, so uncolliding documents keep
22+
* byte-identical pipeline keys and log output. Author-visible `id` attributes
23+
* are never rewritten — 158 of the 161 registry blocks reference their own ids
24+
* from `#id` CSS or `getElementById`, so renaming would break scene styling to
25+
* fix scene footage.
26+
*/
27+
28+
export const MEDIA_RENDER_ID_ATTR = "data-hf-render-id";
29+
30+
/** Elements the render pipeline addresses by id. */
31+
const MEDIA_SELECTOR = "video[src], audio[src], img[src]";
32+
33+
interface MediaElementLike {
34+
getAttribute(name: string): string | null;
35+
setAttribute(name: string, value: string): void;
36+
}
37+
38+
interface DocumentLike {
39+
querySelectorAll(selector: string): Iterable<MediaElementLike>;
40+
}
41+
42+
/**
43+
* Derive a document-unique render id from an element's own id.
44+
*
45+
* `taken` accumulates every id handed out so far, including the plain ids of
46+
* elements that have not been visited yet is NOT required: a later element
47+
* whose plain id was already claimed simply gets a suffix. Document order
48+
* therefore decides who keeps the plain id, which keeps the first (and, in the
49+
* overwhelmingly common single-occurrence case, only) element stable.
50+
*/
51+
function uniqueRenderId(baseId: string, taken: Set<string>): string {
52+
if (!taken.has(baseId)) return baseId;
53+
let suffix = 2;
54+
while (taken.has(`${baseId}__hf${suffix}`)) suffix += 1;
55+
return `${baseId}__hf${suffix}`;
56+
}
57+
58+
/**
59+
* Stamp `data-hf-render-id` on every media element in a compiled document.
60+
*
61+
* Idempotent: an element that already carries the attribute keeps it, so
62+
* re-compiling a document (the resolved-durations recompile path) does not
63+
* renumber ids out from under an in-flight extraction.
64+
*/
65+
export function assignMediaRenderIds(document: DocumentLike): void {
66+
const taken = new Set<string>();
67+
const pending: MediaElementLike[] = [];
68+
69+
for (const el of document.querySelectorAll(MEDIA_SELECTOR)) {
70+
const existing = el.getAttribute(MEDIA_RENDER_ID_ATTR);
71+
if (existing) {
72+
taken.add(existing);
73+
continue;
74+
}
75+
pending.push(el);
76+
}
77+
78+
for (const el of pending) {
79+
const baseId = el.getAttribute("id");
80+
// An element with no id yet is numbered by the timing compiler before this
81+
// runs. If one slips through, fall back to a positional id rather than
82+
// stamping an empty string that every other id-less element would share.
83+
const renderId = uniqueRenderId(baseId || `hf-media-${taken.size}`, taken);
84+
taken.add(renderId);
85+
el.setAttribute(MEDIA_RENDER_ID_ATTR, renderId);
86+
}
87+
}

‎packages/core/src/index.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,14 @@ export {
144144
MEDIA_DURATION_CLAMP_EPSILON_SECONDS,
145145
} from "./compiler/timingCompiler";
146146

147+
export { MEDIA_RENDER_ID_ATTR, assignMediaRenderIds } from "./compiler/mediaRenderIds";
148+
149+
export {
150+
RENDER_FRAME_ID_PREFIX,
151+
RENDER_FRAME_ID_SUFFIX,
152+
renderFrameIdForRenderId,
153+
} from "./runtime/renderFrameSibling";
154+
147155
// Lint moved to @hyperframes/lint. Import lint APIs from @hyperframes/lint
148156
// directly, or via the back-compat stub at @hyperframes/core/lint. Not
149157
// re-exported here — doing so would cycle core's main entry through the lint

‎packages/core/src/runtime/adapters/video-texture-compat.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@
1313
* sibling), the original `<video>` path is used unchanged.
1414
*/
1515

16+
import { findInjectedRenderFrame } from "../renderFrameSibling.js";
17+
1618
/**
1719
* Resolve the decoded render-frame `<img>` for a source `<video>`, if the
1820
* engine has injected one and it has decoded pixels. Returns null in preview
1921
* mode or before the frame is decoded, so callers fall back to the video.
2022
*
2123
* The injector inserts the `<img>` as the video's immediate next sibling and
22-
* also gives it the id `__render_frame_<videoId>__`; we check the sibling
24+
* also gives it the id `__render_frame_<renderId>__`; we check the sibling
2325
* first (cheap) and fall back to an id lookup in case a node was inserted
2426
* between them.
2527
*/
@@ -33,11 +35,9 @@ function resolveRenderFrameImage(video: HTMLVideoElement): HTMLImageElement | nu
3335
) {
3436
return sibling;
3537
}
36-
if (video.id) {
37-
const byId = document.getElementById(`__render_frame_${video.id}__`);
38-
if (byId instanceof HTMLImageElement && byId.complete && byId.naturalWidth > 0) {
39-
return byId;
40-
}
38+
const byId = findInjectedRenderFrame(video);
39+
if (byId && byId.complete && byId.naturalWidth > 0) {
40+
return byId;
4141
}
4242
return null;
4343
}

‎packages/core/src/runtime/colorGrading.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
import { copyMediaVisualStyles } from "../inline-scripts/parityContract";
4242
import { readVariablesForElement } from "./variableScope";
4343
import { swallow } from "./diagnostics";
44+
import { findInjectedRenderFrame } from "./renderFrameSibling";
4445

4546
type ColorGradingMediaElement = HTMLVideoElement | HTMLImageElement;
4647

@@ -2470,9 +2471,8 @@ function isDrawableSource(source: TexImageSource): boolean {
24702471
}
24712472

24722473
function findRenderFrameImage(video: HTMLVideoElement): HTMLImageElement | null {
2473-
if (!video.id) return null;
2474-
const frame = document.getElementById(`__render_frame_${video.id}__`);
2475-
return frame instanceof HTMLImageElement && isDrawableSource(frame) ? frame : null;
2474+
const frame = findInjectedRenderFrame(video);
2475+
return frame && isDrawableSource(frame) ? frame : null;
24762476
}
24772477

24782478
function hasInjectedRenderFrame(element: ColorGradingMediaElement): boolean {

‎packages/core/src/runtime/media.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { interpolateVolumeGain, type VolumeKeyframe } from "./mediaVolumeEnvelop
33
import { elementVolumeLaneGain } from "./audioAutomationVolume.js";
44
import { readElementPlaybackRate, readMediaStart } from "./playbackRate.js";
55
import { clampAudioGain } from "../audioGain.js";
6+
import { findInjectedRenderFrame } from "./renderFrameSibling.js";
67
export { readElementPlaybackRate, resolveNaturalMediaTimelineDuration } from "./playbackRate.js";
78

89
export function readElementPlaybackStart(el: Element): number {
@@ -410,8 +411,7 @@ export function syncRuntimeMedia(params: {
410411
// effect during render, and the per-tick set just kicks Chrome's
411412
// media pipeline for nothing. Preview is unaffected (the sibling
412413
// only exists during render).
413-
const skipForInjectedVideo =
414-
el.tagName === "VIDEO" && el.id && !!document.getElementById(`__render_frame_${el.id}__`);
414+
const skipForInjectedVideo = el.tagName === "VIDEO" && !!findInjectedRenderFrame(el);
415415
if (!skipForInjectedVideo) {
416416
try {
417417
el.currentTime = relTime;

‎packages/core/src/runtime/mediaProxy.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { postRuntimeMessage } from "./bridge";
22
import { swallow } from "./diagnostics";
33
import { evictMediaSyncState } from "./media";
4+
import { findInjectedRenderFrame } from "./renderFrameSibling";
45
import type { RuntimeJson } from "./types";
56

67
/**
@@ -62,11 +63,7 @@ function currentSrcValue(el: HTMLMediaElement): string {
6263
*/
6364
function isRenderMode(el: HTMLMediaElement): boolean {
6465
if (window.__HF_EXPORT_RENDER_SEEK_CONFIG) return true;
65-
return (
66-
el instanceof HTMLVideoElement &&
67-
!!el.id &&
68-
!!document.getElementById(`__render_frame_${el.id}__`)
69-
);
66+
return el instanceof HTMLVideoElement && !!findInjectedRenderFrame(el);
7067
}
7168

7269
/**
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { describe, it, expect, beforeEach } from "vitest";
2+
import { MEDIA_RENDER_ID_ATTR } from "../compiler/mediaRenderIds";
3+
import {
4+
readMediaRenderId,
5+
renderFrameElementId,
6+
findInjectedRenderFrame,
7+
} from "./renderFrameSibling";
8+
9+
function videoWith(attrs: Record<string, string>): HTMLVideoElement {
10+
const el = document.createElement("video");
11+
for (const [name, value] of Object.entries(attrs)) el.setAttribute(name, value);
12+
return el;
13+
}
14+
15+
describe("readMediaRenderId", () => {
16+
it("prefers the stamped render id over the author id", () => {
17+
expect(readMediaRenderId(videoWith({ id: "clip", [MEDIA_RENDER_ID_ATTR]: "clip__hf2" }))).toBe(
18+
"clip__hf2",
19+
);
20+
});
21+
22+
it("falls back to the author id in an uncompiled document", () => {
23+
expect(readMediaRenderId(videoWith({ id: "clip" }))).toBe("clip");
24+
});
25+
26+
it("returns null when the element has neither", () => {
27+
expect(readMediaRenderId(videoWith({}))).toBeNull();
28+
});
29+
});
30+
31+
describe("renderFrameElementId", () => {
32+
// Pins the id format the engine's in-page bridge mirrors when it CREATES the
33+
// sibling (screenshotService.ensureRenderFrameSiblings). If this format
34+
// changes on one side only, the readers stop finding the frame.
35+
it("wraps the render id in the injector's sibling id format", () => {
36+
expect(renderFrameElementId(videoWith({ id: "hero" }))).toBe("__render_frame_hero__");
37+
expect(renderFrameElementId(videoWith({ id: "c", [MEDIA_RENDER_ID_ATTR]: "c__hf2" }))).toBe(
38+
"__render_frame_c__hf2__",
39+
);
40+
});
41+
});
42+
43+
describe("findInjectedRenderFrame", () => {
44+
beforeEach(() => {
45+
document.body.innerHTML = "";
46+
});
47+
48+
it("resolves each colliding video to its own frame, not the first one's", () => {
49+
// Two scenes sharing `<video id="clip">`. Resolving by author id returned
50+
// scene-a's frame for both, so scene-b read another clip's pixels.
51+
document.body.innerHTML =
52+
`<video id="clip" ${MEDIA_RENDER_ID_ATTR}="clip"></video>` +
53+
`<img id="__render_frame_clip__" class="__render_frame__">` +
54+
`<video id="clip" ${MEDIA_RENDER_ID_ATTR}="clip__hf2"></video>` +
55+
`<img id="__render_frame_clip__hf2__" class="__render_frame__">`;
56+
57+
const [first, second] = Array.from(document.querySelectorAll("video"));
58+
expect(findInjectedRenderFrame(first!)?.id).toBe("__render_frame_clip__");
59+
expect(findInjectedRenderFrame(second!)?.id).toBe("__render_frame_clip__hf2__");
60+
});
61+
62+
it("still resolves by author id when the document was never compiled", () => {
63+
document.body.innerHTML =
64+
'<video id="solo"></video><img id="__render_frame_solo__" class="__render_frame__">';
65+
expect(findInjectedRenderFrame(document.querySelector("video")!)?.id).toBe(
66+
"__render_frame_solo__",
67+
);
68+
});
69+
70+
it("returns null in preview, where no sibling exists", () => {
71+
document.body.innerHTML = '<video id="solo"></video>';
72+
expect(findInjectedRenderFrame(document.querySelector("video")!)).toBeNull();
73+
});
74+
});

0 commit comments

Comments
 (0)