Skip to content

Commit 874b323

Browse files
fix(core,producer): stamp render ids on empty-src media and pair the snapshot by them
Residual of #3340: runtime-assigned src is skipped by the static parse, so the browser snapshot was still keying clips by author id. Colliding scenes collapsed onto one window.
1 parent ee64c3b commit 874b323

4 files changed

Lines changed: 79 additions & 25 deletions

File tree

packages/core/src/compiler/mediaRenderIds.test.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,25 @@ describe("assignMediaRenderIds", () => {
7575
expect(ids[1]).toBe("clip__hf2");
7676
});
7777

78-
it("leaves media with no source at all alone", () => {
78+
it("stamps empty-src media with colliding author ids", () => {
79+
// `src=""` used to skip the stamp. The snapshot then keyed by raw id and
80+
// colliding scenes collapsed. Runtime assignment is why the src is empty,
81+
// not a second path this function sees.
82+
const { document } = parseHTML(
83+
'<video id="clip" src=""></video><video id="clip" src=""></video>',
84+
);
85+
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
86+
expect(
87+
Array.from(document.querySelectorAll("video")).map((el) =>
88+
el.getAttribute(MEDIA_RENDER_ID_ATTR),
89+
),
90+
).toEqual(["clip", "clip__hf2"]);
91+
});
92+
93+
it("stamps a video with no source attribute at all", () => {
7994
const { document } = parseHTML('<video id="no-src"></video>');
8095
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
81-
expect(document.querySelector("video")?.hasAttribute(MEDIA_RENDER_ID_ATTR)).toBe(false);
96+
expect(document.querySelector("video")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("no-src");
8297
});
8398

8499
it("stamps media whose source is a <source> child rather than a src attribute", () => {
@@ -103,10 +118,10 @@ describe("assignMediaRenderIds", () => {
103118
expect(document.querySelector("audio")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("bed");
104119
});
105120

106-
it("ignores a <source> child that carries no src", () => {
121+
it("stamps a video whose <source> child carries no src", () => {
107122
const { document } = parseHTML('<video id="empty"><source type="video/mp4"></video>');
108123
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
109-
expect(document.querySelector("video")?.hasAttribute(MEDIA_RENDER_ID_ATTR)).toBe(false);
124+
expect(document.querySelector("video")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("empty");
110125
});
111126
});
112127

packages/core/src/compiler/mediaRenderIds.ts

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,32 +43,23 @@ export const AUDIO_GROUP_RENDER_ID_ATTR = "data-hf-group-render-id";
4343
/**
4444
* Elements the render pipeline addresses by id.
4545
*
46-
* `<video>`/`<audio>` are matched whether the source is a `src` attribute or a
47-
* `<source>` child. Matching only `[src]` left the `<source>`-child shape
48-
* unstamped, so two scenes each declaring `<video id="clip"><source …></video>`
49-
* kept colliding ids in the render document, which is exactly the failure this
50-
* module exists to prevent.
46+
* `<video>`/`<audio>` are matched even with an empty `src`. Authors assign the
47+
* URL from the scene script (`el.src = url`); the static parse then skips them
48+
* and the browser snapshot has to pair the clips. Without a render id on those
49+
* elements the snapshot keys by raw id and colliding scenes collapse. `<img>`
50+
* still requires a `src` attribute (empty is enough) so we do not stamp every
51+
* decorative image.
5152
*/
5253
const MEDIA_SELECTOR = "video, audio, img[src]";
5354

5455
/** Buses, which are addressed by id in exactly the same way and collide the
5556
* same way. Only an id'd bus can be joined at all. */
5657
const AUDIO_GROUP_SELECTOR = "hf-audio-group[id]";
5758

58-
/** A `<source>`-bearing media element is addressable even without its own `src`. */
59-
function hasPlayableSource(el: MediaElementLike): boolean {
60-
if (el.getAttribute("src")) return true;
61-
const sources = el.querySelectorAll?.("source[src]");
62-
if (!sources) return false;
63-
for (const _ of sources) return true;
64-
return false;
65-
}
66-
6759
interface MediaElementLike {
6860
readonly tagName?: string;
6961
getAttribute(name: string): string | null;
7062
setAttribute(name: string, value: string): void;
71-
querySelectorAll?(selector: string): Iterable<unknown>;
7263
}
7364

7465
/** A bus or member, which additionally needs subtree scoping to be paired up. */
@@ -108,7 +99,6 @@ export function assignMediaRenderIds(document: DocumentLike): void {
10899
const pending: MediaElementLike[] = [];
109100

110101
for (const el of document.querySelectorAll(MEDIA_SELECTOR)) {
111-
if (!hasPlayableSource(el)) continue;
112102
const existing = el.getAttribute(MEDIA_RENDER_ID_ATTR);
113103
if (existing) {
114104
taken.add(existing);

packages/producer/src/services/htmlCompiler.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,16 @@ describe("discoverMediaFromBrowser", () => {
107107
);
108108
expect(media[0]).toMatchObject({ start: 0, end: 2, duration: 2, mediaStart: 1 });
109109
});
110+
111+
it("reports colliding empty-src videos by render id, not author id", async () => {
112+
const media = await discover(
113+
`<video id="clip" data-hf-render-id="clip" src="" data-start="0" data-end="4" data-media-start="10"></video>` +
114+
`<video id="clip" data-hf-render-id="clip__hf2" src="" data-start="4" data-end="8" data-media-start="40"></video>`,
115+
{},
116+
);
117+
expect(media.map((entry) => entry.id)).toEqual(["clip", "clip__hf2"]);
118+
expect(media.map((entry) => entry.mediaStart)).toEqual([10, 40]);
119+
});
110120
});
111121

112122
function validTestMediaResponse(): Response {
@@ -2811,6 +2821,31 @@ describe("duplicate media ids across nested compositions", () => {
28112821
expect(compiled.audios[0]).toMatchObject({ start: 0, end: 3, mediaStart: 5 });
28122822
expect(compiled.audios[1]).toMatchObject({ start: 3, end: 6, mediaStart: 50 });
28132823
});
2824+
2825+
it("stamps unique render ids on empty-src videos across two scenes", async () => {
2826+
const { projectDir, indexPath } = writeTwoSceneProject(
2827+
"scene-a.html",
2828+
"scene-b.html",
2829+
(label, mediaStart) =>
2830+
`<div data-composition-id="${label}" data-start="0" data-duration="3"
2831+
data-width="640" data-height="360">
2832+
<video id="clip" src="" data-start="0" data-duration="3"
2833+
data-media-start="${mediaStart}" data-track-index="0"></video>
2834+
</div>`,
2835+
);
2836+
2837+
const compiled = await compileForRender(projectDir, indexPath, projectDir);
2838+
2839+
// Static parse still omits empty src from the media list; the stamp is
2840+
// what the snapshot uses to keep the two clips distinct.
2841+
expect(compiled.videos).toHaveLength(0);
2842+
const { document } = parseHTML(compiled.html);
2843+
expect(
2844+
Array.from(document.querySelectorAll("video")).map((el) =>
2845+
el.getAttribute("data-hf-render-id"),
2846+
),
2847+
).toEqual(["clip", "clip__hf2"]);
2848+
});
28142849
});
28152850

28162851
describe("STUDIO-5433 — ffprobe failure includes src URL for attribution", () => {

packages/producer/src/services/htmlCompiler.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,7 +2090,9 @@ export async function compileForRender(
20902090
* Discover media elements from the browser DOM after JavaScript has run.
20912091
* This catches videos/audios whose `src` is set dynamically via JS
20922092
* (e.g. `document.getElementById("pip-video").src = URL`), which the
2093-
* static regex parsers miss because the HTML has `src=""`.
2093+
* static regex parsers miss because the HTML has `src=""`. Clips are keyed
2094+
* by `data-hf-render-id` when present — author ids collide across inlined
2095+
* scenes, and this snapshot is the only identity those empty-src elements get.
20942096
*/
20952097
export interface BrowserMediaElement {
20962098
id: string;
@@ -2152,7 +2154,14 @@ export async function discoverMediaFromBrowser(page: Page): Promise<BrowserMedia
21522154
: htmlEl.tagName.toLowerCase() === "video"
21532155
? "video"
21542156
: "audio";
2155-
const id = htmlEl.id || (isImage ? autoImageIds.get(htmlEl) : undefined);
2157+
// Render id is document-unique after inlining; author id is only unique
2158+
// per composition file. Empty-src media is skipped by the static parse
2159+
// and lives or dies on this snapshot — keying by author id collapses
2160+
// colliding scenes onto one clip (residual of #3340).
2161+
const id =
2162+
htmlEl.getAttribute("data-hf-render-id") ||
2163+
htmlEl.id ||
2164+
(isImage ? autoImageIds.get(htmlEl) : undefined);
21562165
if (!id) return;
21572166

21582167
// currentSrc is authoritative for <video>/<audio><source> and responsive images.
@@ -2213,7 +2222,10 @@ export async function discoverAudioVolumeAutomationFromTimeline(
22132222
const sampleStep = 1 / Math.min(60, Math.max(1, sampleFps));
22142223
const rawWindows = await page.evaluate((ids: string[]) => {
22152224
return ids.flatMap((id) => {
2216-
const el = document.getElementById(id) ?? document.getElementById(id.replace(/-audio$/, ""));
2225+
const el =
2226+
window.__hfMediaEl?.(id) ??
2227+
document.getElementById(id) ??
2228+
document.getElementById(id.replace(/-audio$/, ""));
22172229
if (!(el instanceof HTMLAudioElement) && !(el instanceof HTMLVideoElement)) return [];
22182230
return [
22192231
{
@@ -2302,7 +2314,9 @@ export async function discoverAudioVolumeAutomationFromTimeline(
23022314

23032315
for (const { id, start, end } of clips) {
23042316
const el =
2305-
document.getElementById(id) ?? document.getElementById(id.replace(/-audio$/, ""));
2317+
window.__hfMediaEl?.(id) ??
2318+
document.getElementById(id) ??
2319+
document.getElementById(id.replace(/-audio$/, ""));
23062320
if (!(el instanceof HTMLAudioElement) && !(el instanceof HTMLVideoElement)) continue;
23072321

23082322
const sampleStart = Math.max(0, start);
@@ -2419,7 +2433,7 @@ export async function discoverVideoVisibilityFromTimeline(
24192433
lastVisible: number | null;
24202434
}[] = [];
24212435
for (const videoEl of videos) {
2422-
const id = videoEl.id;
2436+
const id = videoEl.getAttribute?.("data-hf-render-id") || videoEl.id;
24232437
if (!id) continue;
24242438
entries.push({
24252439
id,

0 commit comments

Comments
 (0)