fix(core,producer): stamp render ids on empty-src media and pair the snapshot by them (#3513)

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.
This commit is contained in:
Val
2026-08-28 00:26:08 +00:00
committed by GitHub
parent e69be30e98
commit 9eb84c91b6
4 changed files with 79 additions and 25 deletions
@@ -75,10 +75,25 @@ describe("assignMediaRenderIds", () => {
expect(ids[1]).toBe("clip__hf2"); expect(ids[1]).toBe("clip__hf2");
}); });
it("leaves media with no source at all alone", () => { it("stamps empty-src media with colliding author ids", () => {
// `src=""` used to skip the stamp. The snapshot then keyed by raw id and
// colliding scenes collapsed. Runtime assignment is why the src is empty,
// not a second path this function sees.
const { document } = parseHTML(
'<video id="clip" src=""></video><video id="clip" src=""></video>',
);
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
expect(
Array.from(document.querySelectorAll("video")).map((el) =>
el.getAttribute(MEDIA_RENDER_ID_ATTR),
),
).toEqual(["clip", "clip__hf2"]);
});
it("stamps a video with no source attribute at all", () => {
const { document } = parseHTML('<video id="no-src"></video>'); const { document } = parseHTML('<video id="no-src"></video>');
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]); assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
expect(document.querySelector("video")?.hasAttribute(MEDIA_RENDER_ID_ATTR)).toBe(false); expect(document.querySelector("video")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("no-src");
}); });
it("stamps media whose source is a <source> child rather than a src attribute", () => { it("stamps media whose source is a <source> child rather than a src attribute", () => {
@@ -103,10 +118,10 @@ describe("assignMediaRenderIds", () => {
expect(document.querySelector("audio")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("bed"); expect(document.querySelector("audio")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("bed");
}); });
it("ignores a <source> child that carries no src", () => { it("stamps a video whose <source> child carries no src", () => {
const { document } = parseHTML('<video id="empty"><source type="video/mp4"></video>'); const { document } = parseHTML('<video id="empty"><source type="video/mp4"></video>');
assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]); assignMediaRenderIds(document as unknown as Parameters<typeof assignMediaRenderIds>[0]);
expect(document.querySelector("video")?.hasAttribute(MEDIA_RENDER_ID_ATTR)).toBe(false); expect(document.querySelector("video")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("empty");
}); });
}); });
+6 -16
View File
@@ -43,11 +43,12 @@ export const AUDIO_GROUP_RENDER_ID_ATTR = "data-hf-group-render-id";
/** /**
* Elements the render pipeline addresses by id. * Elements the render pipeline addresses by id.
* *
* `<video>`/`<audio>` are matched whether the source is a `src` attribute or a * `<video>`/`<audio>` are matched even with an empty `src`. Authors assign the
* `<source>` child. Matching only `[src]` left the `<source>`-child shape * URL from the scene script (`el.src = url`); the static parse then skips them
* unstamped, so two scenes each declaring `<video id="clip"><source …></video>` * and the browser snapshot has to pair the clips. Without a render id on those
* kept colliding ids in the render document, which is exactly the failure this * elements the snapshot keys by raw id and colliding scenes collapse. `<img>`
* module exists to prevent. * still requires a `src` attribute (empty is enough) so we do not stamp every
* decorative image.
*/ */
const MEDIA_SELECTOR = "video, audio, img[src]"; const MEDIA_SELECTOR = "video, audio, img[src]";
@@ -55,20 +56,10 @@ const MEDIA_SELECTOR = "video, audio, img[src]";
* same way. Only an id'd bus can be joined at all. */ * same way. Only an id'd bus can be joined at all. */
const AUDIO_GROUP_SELECTOR = "hf-audio-group[id]"; const AUDIO_GROUP_SELECTOR = "hf-audio-group[id]";
/** A `<source>`-bearing media element is addressable even without its own `src`. */
function hasPlayableSource(el: MediaElementLike): boolean {
if (el.getAttribute("src")) return true;
const sources = el.querySelectorAll?.("source[src]");
if (!sources) return false;
for (const _ of sources) return true;
return false;
}
interface MediaElementLike { interface MediaElementLike {
readonly tagName?: string; readonly tagName?: string;
getAttribute(name: string): string | null; getAttribute(name: string): string | null;
setAttribute(name: string, value: string): void; setAttribute(name: string, value: string): void;
querySelectorAll?(selector: string): Iterable<unknown>;
} }
/** A bus or member, which additionally needs subtree scoping to be paired up. */ /** A bus or member, which additionally needs subtree scoping to be paired up. */
@@ -108,7 +99,6 @@ export function assignMediaRenderIds(document: DocumentLike): void {
const pending: MediaElementLike[] = []; const pending: MediaElementLike[] = [];
for (const el of document.querySelectorAll(MEDIA_SELECTOR)) { for (const el of document.querySelectorAll(MEDIA_SELECTOR)) {
if (!hasPlayableSource(el)) continue;
const existing = el.getAttribute(MEDIA_RENDER_ID_ATTR); const existing = el.getAttribute(MEDIA_RENDER_ID_ATTR);
if (existing) { if (existing) {
taken.add(existing); taken.add(existing);
@@ -107,6 +107,16 @@ describe("discoverMediaFromBrowser", () => {
); );
expect(media[0]).toMatchObject({ start: 0, end: 2, duration: 2, mediaStart: 1 }); expect(media[0]).toMatchObject({ start: 0, end: 2, duration: 2, mediaStart: 1 });
}); });
it("reports colliding empty-src videos by render id, not author id", async () => {
const media = await discover(
`<video id="clip" data-hf-render-id="clip" src="" data-start="0" data-end="4" data-media-start="10"></video>` +
`<video id="clip" data-hf-render-id="clip__hf2" src="" data-start="4" data-end="8" data-media-start="40"></video>`,
{},
);
expect(media.map((entry) => entry.id)).toEqual(["clip", "clip__hf2"]);
expect(media.map((entry) => entry.mediaStart)).toEqual([10, 40]);
});
}); });
function validTestMediaResponse(): Response { function validTestMediaResponse(): Response {
@@ -2811,6 +2821,31 @@ describe("duplicate media ids across nested compositions", () => {
expect(compiled.audios[0]).toMatchObject({ start: 0, end: 3, mediaStart: 5 }); expect(compiled.audios[0]).toMatchObject({ start: 0, end: 3, mediaStart: 5 });
expect(compiled.audios[1]).toMatchObject({ start: 3, end: 6, mediaStart: 50 }); expect(compiled.audios[1]).toMatchObject({ start: 3, end: 6, mediaStart: 50 });
}); });
it("stamps unique render ids on empty-src videos across two scenes", async () => {
const { projectDir, indexPath } = writeTwoSceneProject(
"scene-a.html",
"scene-b.html",
(label, mediaStart) =>
`<div data-composition-id="${label}" data-start="0" data-duration="3"
data-width="640" data-height="360">
<video id="clip" src="" data-start="0" data-duration="3"
data-media-start="${mediaStart}" data-track-index="0"></video>
</div>`,
);
const compiled = await compileForRender(projectDir, indexPath, projectDir);
// Static parse still omits empty src from the media list; the stamp is
// what the snapshot uses to keep the two clips distinct.
expect(compiled.videos).toHaveLength(0);
const { document } = parseHTML(compiled.html);
expect(
Array.from(document.querySelectorAll("video")).map((el) =>
el.getAttribute("data-hf-render-id"),
),
).toEqual(["clip", "clip__hf2"]);
});
}); });
describe("STUDIO-5433 — ffprobe failure includes src URL for attribution", () => { describe("STUDIO-5433 — ffprobe failure includes src URL for attribution", () => {
+19 -5
View File
@@ -2090,7 +2090,9 @@ export async function compileForRender(
* Discover media elements from the browser DOM after JavaScript has run. * Discover media elements from the browser DOM after JavaScript has run.
* This catches videos/audios whose `src` is set dynamically via JS * This catches videos/audios whose `src` is set dynamically via JS
* (e.g. `document.getElementById("pip-video").src = URL`), which the * (e.g. `document.getElementById("pip-video").src = URL`), which the
* static regex parsers miss because the HTML has `src=""`. * static regex parsers miss because the HTML has `src=""`. Clips are keyed
* by `data-hf-render-id` when present — author ids collide across inlined
* scenes, and this snapshot is the only identity those empty-src elements get.
*/ */
export interface BrowserMediaElement { export interface BrowserMediaElement {
id: string; id: string;
@@ -2152,7 +2154,14 @@ export async function discoverMediaFromBrowser(page: Page): Promise<BrowserMedia
: htmlEl.tagName.toLowerCase() === "video" : htmlEl.tagName.toLowerCase() === "video"
? "video" ? "video"
: "audio"; : "audio";
const id = htmlEl.id || (isImage ? autoImageIds.get(htmlEl) : undefined); // Render id is document-unique after inlining; author id is only unique
// per composition file. Empty-src media is skipped by the static parse
// and lives or dies on this snapshot — keying by author id collapses
// colliding scenes onto one clip (residual of #3340).
const id =
htmlEl.getAttribute("data-hf-render-id") ||
htmlEl.id ||
(isImage ? autoImageIds.get(htmlEl) : undefined);
if (!id) return; if (!id) return;
// currentSrc is authoritative for <video>/<audio><source> and responsive images. // currentSrc is authoritative for <video>/<audio><source> and responsive images.
@@ -2213,7 +2222,10 @@ export async function discoverAudioVolumeAutomationFromTimeline(
const sampleStep = 1 / Math.min(60, Math.max(1, sampleFps)); const sampleStep = 1 / Math.min(60, Math.max(1, sampleFps));
const rawWindows = await page.evaluate((ids: string[]) => { const rawWindows = await page.evaluate((ids: string[]) => {
return ids.flatMap((id) => { return ids.flatMap((id) => {
const el = document.getElementById(id) ?? document.getElementById(id.replace(/-audio$/, "")); const el =
window.__hfMediaEl?.(id) ??
document.getElementById(id) ??
document.getElementById(id.replace(/-audio$/, ""));
if (!(el instanceof HTMLAudioElement) && !(el instanceof HTMLVideoElement)) return []; if (!(el instanceof HTMLAudioElement) && !(el instanceof HTMLVideoElement)) return [];
return [ return [
{ {
@@ -2302,7 +2314,9 @@ export async function discoverAudioVolumeAutomationFromTimeline(
for (const { id, start, end } of clips) { for (const { id, start, end } of clips) {
const el = const el =
document.getElementById(id) ?? document.getElementById(id.replace(/-audio$/, "")); window.__hfMediaEl?.(id) ??
document.getElementById(id) ??
document.getElementById(id.replace(/-audio$/, ""));
if (!(el instanceof HTMLAudioElement) && !(el instanceof HTMLVideoElement)) continue; if (!(el instanceof HTMLAudioElement) && !(el instanceof HTMLVideoElement)) continue;
const sampleStart = Math.max(0, start); const sampleStart = Math.max(0, start);
@@ -2419,7 +2433,7 @@ export async function discoverVideoVisibilityFromTimeline(
lastVisible: number | null; lastVisible: number | null;
}[] = []; }[] = [];
for (const videoEl of videos) { for (const videoEl of videos) {
const id = videoEl.id; const id = videoEl.getAttribute?.("data-hf-render-id") || videoEl.id;
if (!id) continue; if (!id) continue;
entries.push({ entries.push({
id, id,