diff --git a/packages/core/src/audioGroups.test.ts b/packages/core/src/audioGroups.test.ts index c315fbc0e..23ceeaa28 100644 --- a/packages/core/src/audioGroups.test.ts +++ b/packages/core/src/audioGroups.test.ts @@ -113,6 +113,18 @@ describe("audioGroupOf", () => { document.body.innerHTML = ``; expect(audioGroupOf(document.getElementById("vo-1") as Element)).toBeNull(); }); + + it("ignores video membership so preview matches the audio-only render", () => { + document.body.innerHTML = ``; + const el = document.getElementById("v-1") as Element; + expect(audioGroupOf(el)).toBeNull(); + expect(resolveAudioGroups(document)).toEqual([]); + }); + + it("normalizes an empty membership attribute to null", () => { + document.body.innerHTML = ``; + expect(audioGroupOf(document.getElementById("vo-1") as Element)).toBeNull(); + }); }); describe("resolveCarveSourceIds", () => { diff --git a/packages/core/src/audioGroups.ts b/packages/core/src/audioGroups.ts index b24b6044d..a95bc3207 100644 --- a/packages/core/src/audioGroups.ts +++ b/packages/core/src/audioGroups.ts @@ -187,17 +187,18 @@ export function resolveCarveSourceIds(doc: Document, ids: readonly string[]): st return out; } -/** The group a member belongs to, or null. Groups do not nest — this ignores - * `data-audio-group` on an `` element itself. +/** The group an audio member belongs to, or null. Membership is audio-only in + * v1, matching `resolveAudioGroups` and the render mixer; video and group-bus + * attributes are inert. * * Tolerant of objects that only partially implement `Element` (test doubles * for `HTMLMediaElement` commonly do) — anything missing `tagName` or * `getAttribute` simply has no group, mirroring `readChain`'s style in * `runtime/audioFx.ts`. */ export function audioGroupOf(el: Element): string | null { - if (typeof el.tagName !== "string") return null; - if (el.tagName.toLowerCase() === HF_AUDIO_GROUP_TAG) return null; - return typeof el.getAttribute === "function" ? el.getAttribute(HF_AUDIO_GROUP_ATTR) : null; + if (typeof el.tagName !== "string" || el.tagName.toLowerCase() !== "audio") return null; + if (typeof el.getAttribute !== "function") return null; + return el.getAttribute(HF_AUDIO_GROUP_ATTR) || null; } /** diff --git a/packages/core/src/compiler/mediaRenderIds.test.ts b/packages/core/src/compiler/mediaRenderIds.test.ts index ddd44e786..3671c4de5 100644 --- a/packages/core/src/compiler/mediaRenderIds.test.ts +++ b/packages/core/src/compiler/mediaRenderIds.test.ts @@ -130,4 +130,23 @@ describe("audio group render ids", () => { // 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); }); + + it("does not bind a root member to a same-named bus in a nested composition", () => { + const d = doc(`
+
+ + +
+ + +
`); + assignMediaRenderIds(d); + + const buses = [...d.querySelectorAll("hf-audio-group")]; + expect(buses.map((bus) => bus.getAttribute(MEDIA_RENDER_ID_ATTR))).toEqual(["bed", "bed__hf2"]); + expect(d.getElementById("child-member")?.getAttribute(AUDIO_GROUP_RENDER_ID_ATTR)).toBe("bed"); + expect(d.getElementById("root-member")?.getAttribute(AUDIO_GROUP_RENDER_ID_ATTR)).toBe( + "bed__hf2", + ); + }); }); diff --git a/packages/core/src/compiler/mediaRenderIds.ts b/packages/core/src/compiler/mediaRenderIds.ts index 41ca3fe28..8f0a517b4 100644 --- a/packages/core/src/compiler/mediaRenderIds.ts +++ b/packages/core/src/compiler/mediaRenderIds.ts @@ -54,7 +54,6 @@ interface MediaElementLike { /** 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 { @@ -176,7 +175,16 @@ function busForMember( ): 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]; + if (!scope) return buses[0]; + // `scope.querySelectorAll()` also sees buses owned by nested compositions. + // Compare each bus's own nearest composition instead, so a root member does + // not bind to a same-named bus inside a child merely because the child comes + // first in document order. + return ( + buses.find( + (candidate) => + candidate.getAttribute("id") === groupId && + candidate.closest?.("[data-composition-id]") === scope, + ) ?? buses[0] + ); } diff --git a/packages/engine/src/services/audioMixer.grouping.test.ts b/packages/engine/src/services/audioMixer.grouping.test.ts index 88e844faf..b4ff5294f 100644 --- a/packages/engine/src/services/audioMixer.grouping.test.ts +++ b/packages/engine/src/services/audioMixer.grouping.test.ts @@ -463,6 +463,40 @@ describe.skipIf(!HAS_FFMPEG)("group sub-mix failure contract", () => { // it: an escaped would survive there. Only the project remains. expect(readdirSync(parent).sort()).toEqual(["project"]); }); + + it("keeps distinct groups isolated when their sanitized ids collide", async () => { + const projectDir = mkdtempSync(join(tmpdir(), "hf-grp-collision-")); + const workDir = mkdtempSync(join(tmpdir(), "hf-grp-collision-work-")); + tempDirs.push(projectDir, workDir); + writeTone(join(projectDir, "a.wav"), 440, 2, 0.4); + writeTone(join(projectDir, "b.wav"), 880, 2, 0.4); + + const groupedOut = join(projectDir, `grouped-${MIXED_AUDIO_FILENAME}`); + const flatOut = join(projectDir, `flat-${MIXED_AUDIO_FILENAME}`); + const grouped = await processCompositionAudio( + [ + { ...track("a", 2), groupId: "bed/a" }, + { ...track("b", 2), groupId: "bed?a" }, + ], + projectDir, + workDir, + groupedOut, + 2, + ); + const flat = await processCompositionAudio( + [track("a", 2), track("b", 2)], + projectDir, + workDir, + flatOut, + 2, + ); + + expect(grouped.success).toBe(true); + expect(flat.success).toBe(true); + // If both ids map to `group-bed_a.wav`, the second submix overwrites the + // first and the outer mix reads the 880 Hz group twice, roughly 3 dB hot. + expect(Math.abs(meanVolumeDb(groupedOut) - meanVolumeDb(flatOut))).toBeLessThan(0.5); + }); }); describe("duplicate bus instances", () => { diff --git a/packages/engine/src/services/audioMixer.ts b/packages/engine/src/services/audioMixer.ts index 36cab19bb..b03a1fb39 100644 --- a/packages/engine/src/services/audioMixer.ts +++ b/packages/engine/src/services/audioMixer.ts @@ -96,13 +96,16 @@ function clampVolume(volume: number): number { * hand-authored or agent-written one is unvalidated. Interpolated raw it could * carry `/` or `..`, and `mkdirSync(recursive)` inside ffmpeg's own path * handling would then write outside `workDir`, where `bail()`'s `rmSync` never - * cleans it up. Everything outside [A-Za-z0-9_-] collapses to `_`, and an id - * that sanitizes to nothing gets a stable positional fallback rather than an - * empty segment that two groups would share. + * cleans it up. Everything outside [A-Za-z0-9_-] collapses to `_`, and every + * result gets a stable positional suffix so distinct ids that sanitize alike + * cannot share one intermediate file. */ function safePathSegment(id: string, fallbackIndex: number): string { const cleaned = id.replace(/[^A-Za-z0-9_-]/g, "_"); - return cleaned.length > 0 ? cleaned : `group-${fallbackIndex}`; + // Sanitisation is many-to-one (`bed/a` and `bed?a` both become `bed_a`). + // The stable position keeps every authored group on a distinct temp path + // even when their readable portions collide. + return `${cleaned || "group"}-${fallbackIndex}`; } function formatFilterNumber(value: number): string { diff --git a/packages/lint/src/rules/media.test.ts b/packages/lint/src/rules/media.test.ts index 7aaaa97c7..e9180a569 100644 --- a/packages/lint/src/rules/media.test.ts +++ b/packages/lint/src/rules/media.test.ts @@ -615,6 +615,16 @@ describe("audio_group_no_members", () => { expect(res.findings.some((f) => f.code === "audio_group_no_members")).toBe(false); }); + it("stays quiet for an unmatched bus when another group has local members", async () => { + const res = await lintHyperframeHtml( + doc(` + + ${BUS} +
`), + ); + expect(res.findings.some((f) => f.code === "audio_group_no_members")).toBe(false); + }); + it("stays quiet for a bus with no id", async () => { const res = await lintHyperframeHtml( doc(``), diff --git a/packages/lint/src/rules/media.ts b/packages/lint/src/rules/media.ts index 7053b965d..2c0d1f621 100644 --- a/packages/lint/src/rules/media.ts +++ b/packages/lint/src/rules/media.ts @@ -811,6 +811,9 @@ function findAudioGroupNoMembersFindings(ctx: LintContext): HyperframeLintFindin // own output as an error, and said "No clip carries `data-audio-group` at all" // about clips it simply could not see. if (memberGroupIds.size === 0) return []; + const mayHaveCrossFileMembers = ctx.tags.some((tag) => + Boolean(readAttr(tag.raw, "data-composition-src")), + ); const findings: HyperframeLintFinding[] = []; for (const tag of ctx.tags) { @@ -820,6 +823,11 @@ function findAudioGroupNoMembersFindings(ctx: LintContext): HyperframeLintFindin const elementId = readAttr(tag.raw, "id"); if (!elementId) continue; if (memberGroupIds.has(elementId)) continue; + // A mixed file is still not closed-world: one bus may have local members + // while another serves clips inside a referenced composition. The linter + // cannot inspect that file here, so an unmatched bus is only provably empty + // when this source has no cross-file composition hosts at all. + if (mayHaveCrossFileMembers) continue; // Naming the near-misses is the whole value: the fix is almost always a // typo on one member, and the author is looking at the bus, not the clip. diff --git a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx index d6e6d2cbf..1705258c6 100644 --- a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx @@ -101,6 +101,37 @@ function mount(dataAttributes: Record, alone = false, voices = 2 return { host, onSetAttributeQuiet, onSetAttributeLive }; } +function mountGroup(memberStart: number) { + const bus = document.createElement("hf-audio-group"); + bus.id = "voiceover"; + document.body.append(bus); + const member = document.createElement("audio"); + member.id = "vo-1"; + member.setAttribute("data-audio-group", "voiceover"); + member.setAttribute("data-start", String(memberStart)); + member.setAttribute("data-duration", "5"); + document.body.append(member); + + const host = document.createElement("div"); + document.body.append(host); + const selection = { + dataAttributes: { "fx-chain": CHAIN }, + id: "voiceover", + element: bus, + tagName: "hf-audio-group", + } as unknown as DomEditSelection; + act(() => { + createRoot(host).render( + , + ); + }); + return host; +} + const rowFor = (host: HTMLElement, label: string): HTMLElement | null => { for (const row of Array.from(host.querySelectorAll(".hf-fx-row"))) { if (row.querySelector(".hf-fx-label")?.textContent === label) return row; @@ -565,6 +596,20 @@ describe("AudioFxGroup dynamic carve", () => { expect(store().playbackRequest?.returnTo).toBe(42); }); + it("seeks a group audition to the next member span", () => { + act(() => + usePlayerStore.setState({ + isPlaying: false, + currentTime: 2, + requestedSeekTime: null, + }), + ); + const host = mountGroup(10); + hoverPreset(host); + expect(store().requestedSeekTime).toBe(10); + leaveShelf(host); + }); + it("leaves a transport the author started alone", () => { // Stopping their playback because they passed over a preset would be the // panel taking a decision nobody offered it. diff --git a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx index d5e6e93fb..df0a0d936 100644 --- a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx +++ b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx @@ -48,9 +48,39 @@ import { useFxChainObserved } from "./useFxChainObserved.js"; import { useFxCarve } from "./useFxCarve.js"; import { audioFxSignalPath } from "./audioFxSignalPath.js"; import type { AuditionSpan } from "./useAuditionTransport.js"; -import { resolveAudioGroups } from "@hyperframes/core/audio-groups"; +import { + HF_AUDIO_GROUP_ATTR, + HF_AUDIO_GROUP_TAG, + resolveAudioGroups, +} from "@hyperframes/core/audio-groups"; import { useFxLevelling } from "./useFxLevelling.js"; +function auditionSpan(startRaw: string | undefined, durationRaw: string | undefined) { + const start = Number.parseFloat(startRaw ?? ""); + const duration = Number.parseFloat(durationRaw ?? ""); + return Number.isFinite(start) && Number.isFinite(duration) && duration > 0 + ? { start, duration } + : null; +} + +/** The selected clip, or every current member when the selected rack is a bus. */ +function auditionSpansFor(element: DomEditSelection): AuditionSpan[] { + const own = auditionSpan(element.dataAttributes?.["start"], element.dataAttributes?.["duration"]); + if (own) return [own]; + if (element.tagName?.toLowerCase() !== HF_AUDIO_GROUP_TAG || !element.id) return []; + const doc = element.element?.ownerDocument; + if (!doc) return []; + return [...doc.querySelectorAll(`audio[${HF_AUDIO_GROUP_ATTR}]`)] + .filter((member) => member.getAttribute(HF_AUDIO_GROUP_ATTR) === element.id) + .flatMap((member) => { + const span = auditionSpan( + member.getAttribute("data-start") ?? undefined, + member.getAttribute("data-duration") ?? undefined, + ); + return span ? [span] : []; + }); +} + /** * Bridges the FX panel to the element/attribute world. Chain and carve are * serialised onto the element the way colour grading carries its config, so @@ -254,18 +284,10 @@ export function AudioFxGroup({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [element, storeElements]); - /** - * The clip this rack belongs to, so hovering a preset auditions where it - * sounds. A group's rack reaches this file too, but a group has no span of - * its own — its members carry the audio, and this panel does not see them, - * so it passes none and the transport plays from the playhead as before. - */ - const auditionSpans = useMemo((): AuditionSpan[] => { - const start = Number.parseFloat(element.dataAttributes?.["start"] ?? ""); - const duration = Number.parseFloat(element.dataAttributes?.["duration"] ?? ""); - if (!Number.isFinite(start) || !Number.isFinite(duration) || duration <= 0) return []; - return [{ start, duration }]; - }, [element]); + // A bus has no span of its own, so resolve the live members. The + // `storeElements` subscription above rerenders this panel when membership + // changes without changing the selection. + const auditionSpans = auditionSpansFor(element); const { carvedAgainstBy, sourceOptions, setCarve } = useFxCarve( element, diff --git a/scripts/check-no-main-deletions.mjs b/scripts/check-no-main-deletions.mjs index d5ac91d14..022345ceb 100644 --- a/scripts/check-no-main-deletions.mjs +++ b/scripts/check-no-main-deletions.mjs @@ -121,6 +121,38 @@ export const ALLOWED_DELETIONS = new Map([ "packages/core/scripts/build-inline-artifact.ts", "a later branch in this stack (wa-20b2-lfo-fixes) independently deduped the same two build scripts a different way — buildInjectedArtifact.ts plus two thin per-target files — before this consolidation and that one had merged; this branch's tree keeps that shape instead, so build-inline-artifact.ts is the one that goes.", ], + [ + "packages/studio/src/hooks/useAudioSoloBridge.ts", + "#3439 deliberately removes track and group solo from the audio workflow", + ], + [ + "packages/studio/src/hooks/useGroupLevel.ts", + "#3439 deliberately removes the group level meter with the group volume strip", + ], + [ + "packages/studio/src/player/components/TimelineGroupBusStrip.test.tsx", + "#3439 deliberately removes the group volume and level-meter strip and its tests", + ], + [ + "packages/studio/src/player/components/TimelineGroupBusStrip.tsx", + "#3439 deliberately removes the group volume and level-meter strip", + ], + [ + "packages/studio/src/player/components/TimelineSoloButton.tsx", + "#3439 deliberately removes track and group solo controls", + ], + [ + "packages/studio/src/player/store/audioSoloSlice.test.ts", + "#3439 deliberately removes session solo state and its tests", + ], + [ + "packages/studio/src/player/store/audioSoloSlice.ts", + "#3439 deliberately removes session solo state", + ], + [ + "packages/studio/src/player/store/groupLevels.ts", + "#3439 deliberately removes group level-meter state", + ], ]); export function parseBase(argv, fallback = "origin/main") {