diff --git a/packages/core/src/audioGroups.ts b/packages/core/src/audioGroups.ts index a83bd3972..c5d14da12 100644 --- a/packages/core/src/audioGroups.ts +++ b/packages/core/src/audioGroups.ts @@ -10,6 +10,7 @@ */ import { HF_AUDIO_FX_ATTR } from "./audioFx.js"; +import { AUDIO_GROUP_RENDER_ID_ATTR, MEDIA_RENDER_ID_ATTR } from "./compiler/mediaRenderIds.js"; import { HF_AUDIO_AUTOMATION_ATTR } from "./audioAutomation.js"; export const HF_AUDIO_GROUP_TAG = "hf-audio-group"; @@ -84,9 +85,18 @@ function buildGroup(id: string, memberIds: string[], el: Element | undefined): H * degrades to a flat sum rather than borrowing a stranger's settings. */ export function resolveGroupElement( - doc: Pick | null | undefined, + doc: + | (Pick & Partial>) + | null + | undefined, groupId: string, ): Element | null { + // A render-stamped key names an INSTANCE, and `getElementById` cannot find it + // — the author id is what is on the element's `id`. Tried first so a compiled + // document resolves the right one of two identically-named buses. + const stamped = + doc?.querySelector?.(`${HF_AUDIO_GROUP_TAG}[${MEDIA_RENDER_ID_ATTR}="${groupId}"]`) ?? null; + if (stamped) return stamped; const el = doc?.getElementById(groupId) ?? null; if (!el) return null; return el.tagName?.toLowerCase() === HF_AUDIO_GROUP_TAG ? el : null; @@ -112,7 +122,15 @@ export function isMemberGroupHidden( export function resolveAudioGroups(root: ParentNode): HfAudioGroup[] { const membersByGroup = new Map(); for (const member of root.querySelectorAll(`audio[${HF_AUDIO_GROUP_ATTR}]`)) { - const groupId = member.getAttribute(HF_AUDIO_GROUP_ATTR); + // The render-stamped instance key when the compiler has been through + // (`assignMediaRenderIds`), else the author id. An author id is unique only + // per composition FILE, so a sub-composition declaring a bus AND its members + // and used twice put both instances' members under one key — one sub-mix for + // two independent buses, instance B's fader and chain over instance A's + // audio, and with only B muted BOTH instances dropped from the export. The + // live preview has no stamps, so it reads exactly as before. + const groupId = + member.getAttribute(AUDIO_GROUP_RENDER_ID_ATTR) ?? member.getAttribute(HF_AUDIO_GROUP_ATTR); if (!groupId || !member.id) continue; const members = membersByGroup.get(groupId); if (members) members.push(member.id); @@ -121,7 +139,10 @@ export function resolveAudioGroups(root: ParentNode): HfAudioGroup[] { const groupElements = new Map(); for (const el of root.querySelectorAll(HF_AUDIO_GROUP_TAG)) { - if (el.id) groupElements.set(el.id, el); + // Keyed the same way, so a stamped document pairs instance for instance and + // an unstamped one keeps id-for-id. + const key = el.getAttribute(MEDIA_RENDER_ID_ATTR) ?? el.id; + if (key) groupElements.set(key, el); } const groups: HfAudioGroup[] = []; diff --git a/packages/core/src/compiler/index.ts b/packages/core/src/compiler/index.ts index 6b2ba12ef..91d01e36b 100644 --- a/packages/core/src/compiler/index.ts +++ b/packages/core/src/compiler/index.ts @@ -90,4 +90,8 @@ export { // Asset-path primitives (shared across core, producer, CLI) export { CSS_URL_RE, PATH_ATTRS, isNonRelativeUrl, isPathInside } from "./assetPaths"; -export { MEDIA_RENDER_ID_ATTR, assignMediaRenderIds } from "./mediaRenderIds"; +export { + AUDIO_GROUP_RENDER_ID_ATTR, + MEDIA_RENDER_ID_ATTR, + assignMediaRenderIds, +} from "./mediaRenderIds"; diff --git a/packages/core/src/compiler/mediaRenderIds.test.ts b/packages/core/src/compiler/mediaRenderIds.test.ts index 465c81604..ddd44e786 100644 --- a/packages/core/src/compiler/mediaRenderIds.test.ts +++ b/packages/core/src/compiler/mediaRenderIds.test.ts @@ -1,6 +1,10 @@ import { describe, it, expect } from "vitest"; import { parseHTML } from "linkedom"; -import { MEDIA_RENDER_ID_ATTR, assignMediaRenderIds } from "./mediaRenderIds"; +import { + AUDIO_GROUP_RENDER_ID_ATTR, + MEDIA_RENDER_ID_ATTR, + assignMediaRenderIds, +} from "./mediaRenderIds"; function stamp(html: string): string[] { const { document } = parseHTML(html); @@ -77,3 +81,53 @@ describe("assignMediaRenderIds", () => { expect(document.querySelector("video")?.hasAttribute(MEDIA_RENDER_ID_ATTR)).toBe(false); }); }); + +describe("audio group render ids", () => { + /** + * A sub-composition declaring a bus AND its members, used twice. The author id + * `bed` is unique per FILE and duplicated once inlined, and the two instances + * are indistinguishable by `data-composition-id` — both carry the file's own — + * so the subtree element is the only thing that separates them. + */ + const doc = (html: string) => parseHTML(`${html}`).document; + const TWICE = ` +
+ + +
+
+ + +
`; + + it("gives each bus instance its own key and pairs each member to its own subtree", () => { + const d = doc(TWICE); + assignMediaRenderIds(d); + + const buses = [...d.querySelectorAll("hf-audio-group")]; + const members = [...d.querySelectorAll("audio")]; + expect(buses.map((b) => b.getAttribute(MEDIA_RENDER_ID_ATTR))).toEqual(["bed", "bed__hf2"]); + // Member N belongs to bus N — the whole point. Cross-paired, one instance's + // fader and chain would land on the other instance's audio. + expect(members.map((m) => m.getAttribute(AUDIO_GROUP_RENDER_ID_ATTR))).toEqual([ + "bed", + "bed__hf2", + ]); + }); + + it("leaves a single-instance bus keyed by its own id", () => { + const d = doc(` + `); + assignMediaRenderIds(d); + expect(d.querySelector("hf-audio-group")?.getAttribute(MEDIA_RENDER_ID_ATTR)).toBe("vo"); + expect(d.querySelector("audio")?.getAttribute(AUDIO_GROUP_RENDER_ID_ATTR)).toBe("vo"); + }); + + it("leaves a member alone when no bus element declares its group", () => { + const d = doc(``); + assignMediaRenderIds(d); + // The element-less group is a supported shape; it just has no instance to + // name, so resolution falls back to the author id as it always did. + expect(d.querySelector("audio")?.hasAttribute(AUDIO_GROUP_RENDER_ID_ATTR)).toBe(false); + }); +}); diff --git a/packages/core/src/compiler/mediaRenderIds.ts b/packages/core/src/compiler/mediaRenderIds.ts index c7e3639a4..41ca3fe28 100644 --- a/packages/core/src/compiler/mediaRenderIds.ts +++ b/packages/core/src/compiler/mediaRenderIds.ts @@ -27,14 +27,36 @@ export const MEDIA_RENDER_ID_ATTR = "data-hf-render-id"; +/** + * The bus a member belongs to, as a DOCUMENT-unique key. + * + * `data-audio-group` names a bus by author id, unique only per composition FILE + * — so a sub-composition that declares a bus AND its members, used twice, put + * both instances' members under one key and let the second bus element + * overwrite the first. The mixer then sub-mixed two independent buses as one, + * applied instance B's fader, chain and label to instance A's audio, and — with + * only B muted — dropped BOTH instances from the export. This attribute is that + * collision resolved at the same boundary the media ids are. + */ +export const AUDIO_GROUP_RENDER_ID_ATTR = "data-hf-group-render-id"; + /** Elements the render pipeline addresses by id. */ const MEDIA_SELECTOR = "video[src], audio[src], 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]"; interface MediaElementLike { getAttribute(name: string): string | null; setAttribute(name: string, value: string): void; } +/** A bus or member, which additionally needs subtree scoping to be paired up. */ +interface ScopedElementLike extends MediaElementLike { + closest?(selector: string): ScopedElementLike | null; + querySelectorAll?(selector: string): Iterable; +} + interface DocumentLike { querySelectorAll(selector: string): Iterable; } @@ -84,4 +106,77 @@ export function assignMediaRenderIds(document: DocumentLike): void { taken.add(renderId); el.setAttribute(MEDIA_RENDER_ID_ATTR, renderId); } + + assignAudioGroupRenderIds(document, taken); +} + +/** + * Give every `` a document-unique render id, and tell each + * member which INSTANCE of its bus it belongs to. + * + * Shares the `taken` set with the media pass, so a bus id and a clip id can + * never resolve to the same key either. + * + * A member is paired to the bus inside its OWN composition subtree. Two + * instances of the same sub-composition are indistinguishable by + * `data-composition-id` — both carry the file's own id — so the subtree + * ELEMENT is the only thing that separates them, which is exactly what + * `closest()` returns. A member whose bus is not in its subtree (a + * hand-authored bus in the root composition, members in a scene) falls back to + * the first bus with that id, which is the pre-existing behaviour and the only + * sensible reading when there is one bus and several scenes referencing it. + */ +function assignAudioGroupRenderIds(document: DocumentLike, taken: Set): void { + const busesById = stampAudioGroupBuses(document, taken); + if (busesById.size === 0) return; + + for (const member of document.querySelectorAll( + "audio[data-audio-group]", + ) as Iterable) { + const groupId = member.getAttribute("data-audio-group"); + const buses = groupId ? busesById.get(groupId) : undefined; + if (!groupId || !buses?.length) continue; + const renderId = busForMember(member, groupId, buses)?.getAttribute(MEDIA_RENDER_ID_ATTR); + if (renderId) member.setAttribute(AUDIO_GROUP_RENDER_ID_ATTR, renderId); + } +} + +/** Every id'd bus, stamped and indexed by author id — several per id when a + * sub-composition that declares one is used more than once. */ +function stampAudioGroupBuses( + document: DocumentLike, + taken: Set, +): Map { + const busesById = new Map(); + for (const bus of document.querySelectorAll( + AUDIO_GROUP_SELECTOR, + ) as Iterable) { + const id = bus.getAttribute("id"); + if (!id) continue; + const renderId = bus.getAttribute(MEDIA_RENDER_ID_ATTR) ?? uniqueRenderId(id, taken); + taken.add(renderId); + bus.setAttribute(MEDIA_RENDER_ID_ATTR, renderId); + const existing = busesById.get(id); + if (existing) existing.push(bus); + else busesById.set(id, [bus]); + } + return busesById; +} + +/** + * Which instance of a bus a member belongs to: the one in its own composition + * subtree. Falls back to the first when the bus is not in the member's subtree + * at all — a hand-authored bus in the root composition with members in scenes — + * which is the pre-existing reading and the only sensible one there. + */ +function busForMember( + member: ScopedElementLike, + groupId: string, + buses: ScopedElementLike[], +): MediaElementLike | undefined { + if (buses.length === 1) return buses[0]; + const scope = member.closest?.("[data-composition-id]"); + if (!scope?.querySelectorAll) return buses[0]; + const inScope = [...(scope.querySelectorAll(AUDIO_GROUP_SELECTOR) as Iterable)]; + return inScope.find((candidate) => candidate.getAttribute("id") === groupId) ?? buses[0]; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 1bcd63c5f..b05ff9ca5 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -144,7 +144,11 @@ export { MEDIA_DURATION_CLAMP_EPSILON_SECONDS, } from "./compiler/timingCompiler"; -export { MEDIA_RENDER_ID_ATTR, assignMediaRenderIds } from "./compiler/mediaRenderIds"; +export { + AUDIO_GROUP_RENDER_ID_ATTR, + MEDIA_RENDER_ID_ATTR, + assignMediaRenderIds, +} from "./compiler/mediaRenderIds"; export { RENDER_FRAME_ID_PREFIX, diff --git a/packages/engine/src/services/audioMixer.grouping.test.ts b/packages/engine/src/services/audioMixer.grouping.test.ts index 91684bd0e..88e844faf 100644 --- a/packages/engine/src/services/audioMixer.grouping.test.ts +++ b/packages/engine/src/services/audioMixer.grouping.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { getFfmpegBinary } from "../utils/ffmpegBinaries.js"; -import { MIXED_AUDIO_FILENAME, processCompositionAudio } from "./audioMixer.js"; +import { MIXED_AUDIO_FILENAME, parseAudioElements, processCompositionAudio } from "./audioMixer.js"; /** * Level arithmetic across the mix graph. @@ -464,3 +464,49 @@ describe.skipIf(!HAS_FFMPEG)("group sub-mix failure contract", () => { expect(readdirSync(parent).sort()).toEqual(["project"]); }); }); + +describe("duplicate bus instances", () => { + /** + * The reviewer's own repro. Two inlined instances of one sub-composition, each + * with its own bus and member, only the SECOND muted. Keyed by author id the + * two collapsed into one bus (last wins), so instance B's `data-hidden` took + * instance A's audio out of the export with it. + */ + it("drops only the muted instance's member", () => { + const html = `
+
+ + +
+
+ + +
+
`; + + const tracks = parseAudioElements(html); + expect(tracks.map((t) => t.groupId)).toEqual(["bed"]); + expect(tracks).toHaveLength(1); + }); + + it("keeps two instances as two separate buses when neither is muted", () => { + const html = `
+ + + + +
`; + + const tracks = parseAudioElements(html); + // Each member keeps its OWN instance's fader — the collapse used to apply + // whichever bus came last to both. + expect(tracks.map((t) => [t.groupId, t.groupVolume])).toEqual([ + ["bed", 0.25], + ["bed__hf2", 0.75], + ]); + }); +}); diff --git a/packages/engine/src/services/audioMixer.ts b/packages/engine/src/services/audioMixer.ts index ba46a812e..b333bfe91 100644 --- a/packages/engine/src/services/audioMixer.ts +++ b/packages/engine/src/services/audioMixer.ts @@ -48,6 +48,7 @@ import { readMediaStart, } from "@hyperframes/core"; import { HF_AUDIO_GROUP_ATTR, resolveAudioGroups } from "@hyperframes/core/audio-groups"; +import { AUDIO_GROUP_RENDER_ID_ATTR } from "@hyperframes/core"; import { applyAudioFxChain, AudioFxRenderError } from "./audioFxRender.js"; import type { AudioVolumeKeyframe } from "./audioMixer.types.js"; @@ -68,6 +69,21 @@ export type { AudioElement, MixResult } from "./audioMixer.types.js"; */ export const MIXED_AUDIO_FILENAME = "audio.m4a"; +/** + * The bus key a member belongs to, as `resolveAudioGroups` keys them. + * + * The compiler's `data-hf-group-render-id` names one INSTANCE of a bus; the + * author's `data-audio-group` names it only within its own composition file. A + * sub-composition declaring a bus and its members, used twice, therefore had + * both instances' members under one key: one sub-mix for two independent buses, + * one instance's fader and chain over the other's audio, and — with only the + * second muted — BOTH instances dropped from the export. Uncompiled documents + * (the live preview) carry no stamp and read exactly as before. + */ +function memberGroupKey(el: RefResolverEl): string | null { + return el.getAttribute(AUDIO_GROUP_RENDER_ID_ATTR) ?? el.getAttribute(HF_AUDIO_GROUP_ATTR); +} + function clampVolume(volume: number): number { return clampAudioGain(volume); } @@ -486,7 +502,7 @@ export function parseAudioElements(html: string): AudioElement[] { resolveAudioGroups(document).map((group) => [group.id, group] as const), ); const memberGroupHidden = (el: AudioMediaElement): boolean => { - const groupId = el.getAttribute(HF_AUDIO_GROUP_ATTR); + const groupId = memberGroupKey(el); return groupId ? (groupsById.get(groupId)?.hidden ?? false) : false; }; @@ -501,7 +517,7 @@ export function parseAudioElements(html: string): AudioElement[] { const automation = el.getAttribute(HF_AUDIO_AUTOMATION_ATTR); // Audio only in v1 (matches resolveAudioGroups, which only scans // `audio[data-audio-group]`) — a stray attribute on a