Skip to content

Commit 0f30228

Browse files
authored
fix(studio): unify audio IDs and group state (#3448)
* fix(core): harden audio FX and group identity * fix(core): address audio group review feedback * fix(core): align preview transport with grouped audio * test(core): pin audio group gain ceiling * fix(core): preserve solo bridge through stack * fix(engine): harden grouped audio rendering * docs(engine): explain grouped mix fallback invariant * test(engine): allow grouped mixes to finish on Windows * feat(lint): validate audio group membership and timing * test(lint): pin audio group membership guards * fix(studio): unify audio IDs and group state
1 parent 54091b5 commit 0f30228

16 files changed

Lines changed: 686 additions & 75 deletions

packages/studio/src/player/lib/automationStoreSync.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,30 @@ describe("syncStoredAutomationFromPreview", () => {
8080
syncStoredAutomationFromPreview(null);
8181
expect(usePlayerStore.getState().elements[0]?.automation).toBe(TWO_POINTS);
8282
});
83+
// The reported symptom: automate a parameter on a GROUP from the FX rack and
84+
// the group's row shows no automation. The rack is not group-aware — it
85+
// writes the attribute on the group node through the ordinary element path —
86+
// and the timeline reads a group's lanes from the mirrors its MEMBERS carry,
87+
// which that path used to leave untouched.
88+
it("re-reads what the group carries onto every member that belongs to it", () => {
89+
const chain = '{"version":1,"nodes":[{"type":"gain","id":"n1","params":{"gain":0}}]}';
90+
const groupAutomation =
91+
'{"version":1,"lanes":[{"target":"fx.n1.gain","points":[{"t":0,"v":0}]}]}';
92+
const doc = document.implementation.createHTMLDocument("preview");
93+
const group = doc.createElement("hf-audio-group");
94+
group.id = "voiceover";
95+
group.setAttribute("data-fx-chain", chain);
96+
group.setAttribute("data-automation", groupAutomation);
97+
const audio = doc.createElement("audio");
98+
audio.id = "bgm";
99+
audio.setAttribute("data-audio-group", "voiceover");
100+
doc.body.append(group, audio);
101+
102+
usePlayerStore.setState({ elements: [el({ audioGroup: "voiceover" })] });
103+
syncStoredAutomationFromPreview(doc);
104+
105+
const stored = usePlayerStore.getState().elements[0];
106+
expect(stored?.audioGroupAutomation).toBe(groupAutomation);
107+
expect(stored?.audioGroupFxChain).toBe(chain);
108+
});
83109
});

packages/studio/src/player/lib/automationStoreSync.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import { HF_AUDIO_AUTOMATION_ATTR } from "@hyperframes/core/audio-automation";
1717
import { HF_AUDIO_FX_ATTR } from "@hyperframes/core/audio-fx";
1818
import { usePlayerStore, type TimelineElement } from "../store/playerStore";
19+
import { groupInfoFor } from "./timelineGroupInfo";
1920

2021
/** The preview node an element stands for, by dom id and then by `data-hf-id`. */
2122
function previewNodeFor(doc: Document, element: TimelineElement): Element | null {
@@ -38,18 +39,41 @@ function previewNodeFor(doc: Document, element: TimelineElement): Element | null
3839
* Reads rather than being told: an undo restores whole files, so the attribute it
3940
* reverted is only known by looking.
4041
*/
42+
/**
43+
* What an element's four synced fields SHOULD read, given the preview.
44+
*
45+
* Its own two come off its node; the other two are its copy of what its group
46+
* carries. The timeline derives a group's lanes and chain from these mirrors,
47+
* never from the group element — and the FX rack is not group-aware: selecting
48+
* a group and automating one of its parameters writes `data-automation` on the
49+
* group node through the ordinary element path, which used to refresh an
50+
* element's own two fields and nothing else. So the group's row went on reading
51+
* the value it was born with, and its `∿` never appeared.
52+
*/
53+
function syncedFields(doc: Document, element: TimelineElement, node: Element) {
54+
const group = element.audioGroup ? groupInfoFor(doc, element.audioGroup) : null;
55+
return {
56+
automation: node.getAttribute(HF_AUDIO_AUTOMATION_ATTR) ?? undefined,
57+
fxChain: node.getAttribute(HF_AUDIO_FX_ATTR) ?? undefined,
58+
audioGroupAutomation: group?.automation,
59+
audioGroupFxChain: group?.fxChain,
60+
};
61+
}
62+
4163
export function syncStoredAutomationFromPreview(doc: Document | null | undefined): void {
4264
if (!doc) return;
4365
usePlayerStore.setState((state) => {
4466
let changed = false;
4567
const elements = state.elements.map((element) => {
4668
const node = previewNodeFor(doc, element);
4769
if (!node) return element;
48-
const automation = node.getAttribute(HF_AUDIO_AUTOMATION_ATTR) ?? undefined;
49-
const fxChain = node.getAttribute(HF_AUDIO_FX_ATTR) ?? undefined;
50-
if (automation === element.automation && fxChain === element.fxChain) return element;
70+
const fields = syncedFields(doc, element, node);
71+
// Same array back when nothing moved: `elements` keys memos all over the
72+
// timeline, and a fresh object per sync would re-render every one.
73+
const keys = Object.keys(fields) as (keyof typeof fields)[];
74+
if (keys.every((key) => fields[key] === element[key])) return element;
5175
changed = true;
52-
return { ...element, automation, fxChain };
76+
return { ...element, ...fields };
5377
});
5478
return changed ? { elements } : {};
5579
});
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// @vitest-environment jsdom
2+
3+
/**
4+
* The studio → runtime boundary, which nothing else crosses.
5+
*
6+
* Studio addresses rows by `buildTimelineElementKey`'s composite
7+
* `<sourceFile>#<domId>`; every audio predicate in `@hyperframes/core` keys off
8+
* the live document instead. Both halves have their own passing tests — one
9+
* with composite keys, one with bare ids — and the mismatch between them lived
10+
* in the gap. These parse a real document, take the ids the way the UI does,
11+
* and hand them to the real core predicates.
12+
*/
13+
14+
import { describe, expect, it } from "vitest";
15+
import { resolveAudioGroups } from "@hyperframes/core/audio-groups";
16+
import { parseTimelineFromDOM } from "./timelineDOM";
17+
import { runtimeAudioId } from "./timelineElementHelpers";
18+
19+
function docWith(body: string): Document {
20+
const doc = document.implementation.createHTMLDocument("comp");
21+
doc.body.innerHTML = body;
22+
return doc;
23+
}
24+
25+
const COMPOSITION = `
26+
<div data-composition-id="root" data-duration="30"></div>
27+
<audio id="voice-1" data-start="0" data-duration="10" data-audio-group="voiceover"></audio>
28+
<audio id="voice-2" data-start="10" data-duration="10" data-audio-group="voiceover"></audio>
29+
<audio id="music-bed" data-start="0" data-duration="30"></audio>
30+
<hf-audio-group id="voiceover"></hf-audio-group>
31+
`;
32+
33+
describe("group membership ids cross into the runtime", () => {
34+
it("the ids the timeline hands to onGroupClips are the ids resolveAudioGroups reads back", () => {
35+
const doc = docWith(COMPOSITION);
36+
const trackElements = parseTimelineFromDOM(doc, 30).filter(
37+
(el) => el.tag.toLowerCase() === "audio",
38+
);
39+
const clipIds = trackElements.map(runtimeAudioId).filter((id): id is string => id !== null);
40+
expect(clipIds).toEqual(["voice-1", "voice-2", "music-bed"]);
41+
42+
// Same space membership is read back in — a composite key here produces a
43+
// group whose members nothing can find.
44+
const memberIds = resolveAudioGroups(doc).flatMap((g) => g.memberIds);
45+
expect(memberIds.every((id) => doc.getElementById(id) !== null)).toBe(true);
46+
for (const id of memberIds) expect(clipIds).toContain(id);
47+
});
48+
49+
it("an element with no DOM id is not groupable", () => {
50+
const doc = docWith(`
51+
<div data-composition-id="root" data-duration="10"></div>
52+
<audio data-start="0" data-duration="5"></audio>
53+
`);
54+
const [clip] = parseTimelineFromDOM(doc, 10);
55+
expect(clip).toBeDefined();
56+
expect(runtimeAudioId(clip)).toBeNull();
57+
});
58+
});

packages/studio/src/player/lib/timelineDOM.test.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import {
66
createImplicitTimelineLayersFromDOM,
77
mergeTimelineElementsPreservingDowngrades,
88
} from "./timelineDOM";
9+
import { isTimelineIgnoredElement } from "./timelineElementHelpers";
10+
import { invalidateGroupInfoCache } from "./timelineGroupInfo";
911
import type { TimelineElement } from "../store/playerStore";
1012

1113
function el(id: string, extra: Partial<TimelineElement> = {}): TimelineElement {
@@ -110,6 +112,92 @@ describe("parseTimelineFromDOM — hfId from data-hf-id", () => {
110112
});
111113
});
112114

115+
describe("group info cache", () => {
116+
const parseMember = (doc: Document) =>
117+
createTimelineElementFromManifestClip({
118+
clip: {
119+
id: "voice-1",
120+
label: "voice-1",
121+
kind: "element",
122+
tagName: "audio",
123+
start: 0,
124+
duration: 5,
125+
track: 0,
126+
compositionId: null,
127+
parentCompositionId: null,
128+
compositionSrc: null,
129+
assetUrl: null,
130+
},
131+
fallbackIndex: 0,
132+
doc,
133+
hostEl: doc.getElementById("voice-1"),
134+
});
135+
136+
// Group edits are applied as LIVE patches so the preview iframe never
137+
// reloads, which means the document identity this cache is keyed on never
138+
// changes either. Without an explicit drop, a muted group could never be
139+
// unmuted: the header kept reading the cached `hidden: false` and re-wrote
140+
// `data-hidden` forever.
141+
it("re-reads group state after an invalidation", () => {
142+
const doc = makeDoc(`
143+
<div data-composition-id="root">
144+
<audio id="voice-1" data-start="0" data-duration="5" data-audio-group="voiceover"></audio>
145+
<hf-audio-group id="voiceover" data-label="Voices"></hf-audio-group>
146+
</div>
147+
`);
148+
149+
expect(parseMember(doc).audioGroupHidden).toBe(false);
150+
151+
doc.getElementById("voiceover")?.setAttribute("data-hidden", "");
152+
expect(parseMember(doc).audioGroupHidden).toBe(false); // still the cached scan
153+
154+
invalidateGroupInfoCache(doc);
155+
expect(parseMember(doc).audioGroupHidden).toBe(true);
156+
157+
doc.getElementById("voiceover")?.removeAttribute("data-hidden");
158+
invalidateGroupInfoCache(doc);
159+
expect(parseMember(doc).audioGroupHidden).toBe(false);
160+
});
161+
162+
// The explicit invalidator is a convenience, not the contract. A cache whose
163+
// only defence is "every writer must remember to call this" rots the first
164+
// time a writer does not know it exists — which is precisely what happened
165+
// with the FX rack, whose group writes go through the DOM editor rather than
166+
// the timeline's own writers. The scan carries the DOM revision it was taken
167+
// at, so a forgotten call costs a re-scan rather than a wrong answer.
168+
it("expires itself on a group edit nobody announced", async () => {
169+
const doc = makeDoc(`
170+
<div data-composition-id="root">
171+
<audio id="voice-1" data-start="0" data-duration="5" data-audio-group="voiceover"></audio>
172+
<hf-audio-group id="voiceover" data-label="Voices"></hf-audio-group>
173+
</div>
174+
`);
175+
176+
expect(parseMember(doc).audioGroupHidden).toBe(false);
177+
178+
// No invalidateGroupInfoCache call anywhere in this test.
179+
doc.getElementById("voiceover")?.setAttribute("data-hidden", "");
180+
await new Promise((resolve) => setTimeout(resolve, 0)); // observer microtask
181+
182+
expect(parseMember(doc).audioGroupHidden).toBe(true);
183+
});
184+
185+
it("notices a member joining the group, not just an attribute edit", async () => {
186+
const doc = makeDoc(`
187+
<div data-composition-id="root">
188+
<audio id="voice-1" data-start="0" data-duration="5" data-audio-group="voiceover"></audio>
189+
<hf-audio-group id="voiceover" data-label="Voices"></hf-audio-group>
190+
</div>
191+
`);
192+
expect(parseMember(doc).audioGroupLabel).toBe("Voices");
193+
194+
doc.getElementById("voiceover")?.setAttribute("data-label", "Narration");
195+
await new Promise((resolve) => setTimeout(resolve, 0));
196+
197+
expect(parseMember(doc).audioGroupLabel).toBe("Narration");
198+
});
199+
});
200+
113201
describe("parseTimelineFromDOM — canonical playback rate", () => {
114202
it.each([
115203
["10", 5],
@@ -207,6 +295,33 @@ describe("createTimelineElementFromManifestClip — source-scoped selector ident
207295
});
208296
});
209297

298+
// Caught by looking at the studio, not by reading: a grouped composition drew
299+
// "Voiceover • 0.0s – 12.0s" as a full-duration clip row directly above its own
300+
// group header. `<hf-audio-group>` is a mixer bus — no timing, drawn as a group
301+
// row by the group derivation — but it is still a body child with an id, so the
302+
// implicit-layer fallback happily gave it a track. Draggable and trimmable, and
303+
// writing timing onto a bus means nothing.
304+
describe("<hf-audio-group> is not a timeline layer", () => {
305+
it("gets no implicit row of its own", () => {
306+
const doc = makeDoc(`
307+
<div data-composition-id="root">
308+
<audio id="voice-1" data-start="0" data-duration="6" data-audio-group="voiceover"></audio>
309+
<hf-audio-group id="voiceover" data-label="Voiceover"></hf-audio-group>
310+
</div>
311+
`);
312+
313+
const implicit = createImplicitTimelineLayersFromDOM(doc, 12, []);
314+
315+
expect(implicit.map((el) => el.domId)).not.toContain("voiceover");
316+
});
317+
318+
it("is excluded by the shared ignore predicate", () => {
319+
const doc = makeDoc(`<hf-audio-group id="vo"></hf-audio-group><div id="panel"></div>`);
320+
expect(isTimelineIgnoredElement(doc.getElementById("vo") as Element)).toBe(true);
321+
expect(isTimelineIgnoredElement(doc.getElementById("panel") as Element)).toBe(false);
322+
});
323+
});
324+
210325
describe("createImplicitTimelineLayersFromDOM — hfId from data-hf-id", () => {
211326
it("uses the runtime root paint scope for implicit siblings of manifest clips", () => {
212327
const doc = makeDoc(`

packages/studio/src/player/lib/timelineDOM.ts

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type { TimelineElement } from "../store/playerStore";
1212
import type { ClipManifestClip } from "./playbackTypes";
1313
import { resolveCssStackingContextId } from "@hyperframes/core/runtime/stacking-context";
1414
import { readClipTiming } from "@hyperframes/core/composition-contract";
15-
import { resolveAudioGroups } from "@hyperframes/core/audio-groups";
15+
import { groupInfoFor } from "./timelineGroupInfo";
1616
import {
1717
resolveMediaElement,
1818
applyMediaMetadataFromElement,
@@ -68,37 +68,6 @@ function resolveClipTag(clip: ClipManifestClip): string {
6868
return clip.tagName || clip.kind || "div";
6969
}
7070

71-
// One `<hf-audio-group>` scan per document, not per clip — resolveAudioGroups
72-
// walks the whole tree, and a parse touches every clip in it.
73-
interface GroupInfo {
74-
label: string;
75-
volume: number;
76-
hidden: boolean;
77-
fxChain?: string;
78-
}
79-
80-
const groupInfoCache = new WeakMap<Document, Map<string, GroupInfo>>();
81-
82-
function groupInfoFor(doc: Document | null | undefined, groupId: string): GroupInfo {
83-
if (!doc) return { label: groupId, volume: 1, hidden: false };
84-
let info = groupInfoCache.get(doc);
85-
if (!info) {
86-
info = new Map(
87-
resolveAudioGroups(doc).map((group) => [
88-
group.id,
89-
{
90-
label: group.label,
91-
volume: group.volume,
92-
hidden: group.hidden,
93-
...(group.fxChain ? { fxChain: group.fxChain } : {}),
94-
},
95-
]),
96-
);
97-
groupInfoCache.set(doc, info);
98-
}
99-
return info.get(groupId) ?? { label: groupId, volume: 1, hidden: false };
100-
}
101-
10271
// fallow-ignore-next-line complexity
10372
export function createTimelineElementFromManifestClip(params: {
10473
clip: ClipManifestClip;
@@ -178,6 +147,7 @@ export function createTimelineElementFromManifestClip(params: {
178147
entry.audioGroupVolume = info.volume;
179148
entry.audioGroupHidden = info.hidden;
180149
if (info.fxChain) entry.audioGroupFxChain = info.fxChain;
150+
if (info.automation) entry.audioGroupAutomation = info.automation;
181151
}
182152
const fxChain = hostEl.getAttribute("data-fx-chain");
183153
if (fxChain) entry.fxChain = fxChain;
@@ -405,6 +375,7 @@ export function parseTimelineFromDOM(doc: Document, rootDuration: number): Timel
405375
entry.audioGroupVolume = domGroupInfo.volume;
406376
entry.audioGroupHidden = domGroupInfo.hidden;
407377
if (domGroupInfo.fxChain) entry.audioGroupFxChain = domGroupInfo.fxChain;
378+
if (domGroupInfo.automation) entry.audioGroupAutomation = domGroupInfo.automation;
408379
}
409380

410381
// Sub-compositions

packages/studio/src/player/lib/timelineElementHelpers.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type { TimelineElement } from "../store/playerStore";
1111
import type { ClipManifestClip } from "./playbackTypes";
1212
import { isFinitePositive } from "./playbackAdapter";
1313
import { getSourceScopedSelectorIndex } from "../../utils/sourceScopedSelectorIndex";
14+
import { HF_AUDIO_GROUP_TAG } from "@hyperframes/core/audio-groups";
1415

1516
// ---------------------------------------------------------------------------
1617
// Layer-reveal lift transparency
@@ -81,6 +82,14 @@ function normalizePlaybackRate(raw: number): number {
8182
}
8283

8384
export function isTimelineIgnoredElement(el: Element): boolean {
85+
// An `<hf-audio-group>` is a mixer bus, not a clip: it carries the group's
86+
// label, fader, mute and FX chain, has no timing of its own, and is drawn as
87+
// a GROUP ROW by the group derivation. Left in, the implicit-layer fallback
88+
// also gave it an ordinary full-duration track — so a grouped composition
89+
// showed "Voiceover • 0.0s – 12.0s" as a phantom clip directly above the real
90+
// group header. Harmless-looking, but that row is draggable and trimmable,
91+
// and writing timing onto the bus is meaningless.
92+
if (el.tagName.toLowerCase() === HF_AUDIO_GROUP_TAG) return true;
8493
return Boolean(
8594
el.closest(
8695
[
@@ -346,6 +355,24 @@ export function getTimelineElementIdentity(element: { key?: string | null; id: s
346355
return element.key ?? element.id;
347356
}
348357

358+
/**
359+
* The id space the RUNTIME matches on — a bare DOM id, never a store key.
360+
*
361+
* Studio addresses rows by `buildTimelineElementKey`'s composite
362+
* `<sourceFile>#<domId>`, but everything audio in `@hyperframes/core` keys off
363+
* the live document: `resolveAudioGroups` collects `member.id`,
364+
* `resolveCarveSourceIds` goes through `getElementById`. Anything crossing into
365+
* that space — a group membership list, a carve source — has to be
366+
* converted here first; a composite key silently matches nothing.
367+
*
368+
* `null` for a row with no DOM id at all (selector-addressed elements): such an
369+
* element cannot be grouped, because `resolveAudioGroups` skips
370+
* members without an `id` and would build a group that is half there.
371+
*/
372+
export function runtimeAudioId(element: { domId?: string | null }): string | null {
373+
return element.domId || null;
374+
}
375+
349376
/**
350377
* Timeline store key for a z-reorder entry built OUTSIDE the timeline
351378
* expansion (canvas context menu / LayersPanel), so the reorder commit can

0 commit comments

Comments
 (0)