Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions packages/core/src/compiler/mediaRenderIds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,25 @@ describe("assignMediaRenderIds", () => {
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>');
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", () => {
Expand All @@ -103,10 +118,10 @@ describe("assignMediaRenderIds", () => {
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>');
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");
});
});

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

/** Buses, which are addressed by id in exactly the same way and collide the
* same way. Only an id'd bus can be joined at all. */
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 {
readonly tagName?: string;
getAttribute(name: string): string | null;
setAttribute(name: string, value: string): void;
querySelectorAll?(selector: string): Iterable<unknown>;
}

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

for (const el of document.querySelectorAll(MEDIA_SELECTOR)) {
if (!hasPlayableSource(el)) continue;
const existing = el.getAttribute(MEDIA_RENDER_ID_ATTR);
if (existing) {
taken.add(existing);
Expand Down
35 changes: 35 additions & 0 deletions packages/producer/src/services/htmlCompiler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,16 @@ describe("discoverMediaFromBrowser", () => {
);
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 {
Expand Down Expand Up @@ -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[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", () => {
Expand Down
24 changes: 19 additions & 5 deletions packages/producer/src/services/htmlCompiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2090,7 +2090,9 @@ export async function compileForRender(
* Discover media elements from the browser DOM after JavaScript has run.
* This catches videos/audios whose `src` is set dynamically via JS
* (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 {
id: string;
Expand Down Expand Up @@ -2152,7 +2154,14 @@ export async function discoverMediaFromBrowser(page: Page): Promise<BrowserMedia
: htmlEl.tagName.toLowerCase() === "video"
? "video"
: "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;

// currentSrc is authoritative for <video>/<audio><source> and responsive images.
Expand Down Expand Up @@ -2213,7 +2222,10 @@ export async function discoverAudioVolumeAutomationFromTimeline(
const sampleStep = 1 / Math.min(60, Math.max(1, sampleFps));
const rawWindows = await page.evaluate((ids: string[]) => {
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 [];
return [
{
Expand Down Expand Up @@ -2302,7 +2314,9 @@ export async function discoverAudioVolumeAutomationFromTimeline(

for (const { id, start, end } of clips) {
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;

const sampleStart = Math.max(0, start);
Expand Down Expand Up @@ -2419,7 +2433,7 @@ export async function discoverVideoVisibilityFromTimeline(
lastVisible: number | null;
}[] = [];
for (const videoEl of videos) {
const id = videoEl.id;
const id = videoEl.getAttribute?.("data-hf-render-id") || videoEl.id;
if (!id) continue;
entries.push({
id,
Expand Down
Loading