Skip to content

Commit 9b2cd06

Browse files
vanceingallsclaude
andcommitted
fix(core): apply chain edits to the running graph
A structural edit — an effect added, removed, bypassed, or a filter's pole count switched — was dropped. `buildFxChain`'s update reports false when the change is not merely new values, and the attribute observer ignored that, so the edit only took hold when the persisting write reloaded the composition. That reload restarted every playing track, which is what was heard as the audio chopping. The graph is now swapped in place: the old effects are detached, the new ones built and connected between the same source and gain, and any lanes re-scheduled onto the new nodes. The source node is never touched, so playback does not restart. A track with no chain is watched too, rather than wired through and forgotten, so adding its first effect is heard the same way. That means the function always returns a disposer instead of null for the empty case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 88d4e1e commit 9b2cd06

2 files changed

Lines changed: 161 additions & 47 deletions

File tree

packages/core/src/runtime/audioFx.test.ts

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ describe("attachElementFxChain", () => {
7070
it("connects source straight to destination when there is no chain", () => {
7171
const src = new Node();
7272
const dst = new Node();
73-
const handle = attachElementFxChain(ctx(), el(), src as never, dst as never);
74-
expect(handle).toBeNull();
73+
attachElementFxChain(ctx(), el(), src as never, dst as never);
7574
expect(src.connections).toContain(dst);
7675
});
7776

@@ -102,7 +101,7 @@ describe("attachElementFxChain", () => {
102101
src as never,
103102
dst as never,
104103
);
105-
expect(handle).toBeNull();
104+
expect(handle).not.toBeNull();
106105
expect(src.connections).toContain(dst);
107106
});
108107

@@ -115,10 +114,90 @@ describe("attachElementFxChain", () => {
115114
src as never,
116115
dst as never,
117116
);
118-
expect(handle).toBeNull();
117+
expect(handle).not.toBeNull();
119118
expect(src.connections).toContain(dst);
120119
});
121120

121+
/**
122+
* A structural edit — an effect added, removed or bypassed — used to be
123+
* ignored here, so it only took effect when the persisting write reloaded the
124+
* composition. That reload restarted every playing track, which is what was
125+
* heard as the audio chopping. The graph is now swapped in place instead, with
126+
* the source node left alone.
127+
*/
128+
describe("editing the chain while it plays", () => {
129+
const audioEl = (chain?: unknown): HTMLElement => {
130+
const node = document.createElement("audio");
131+
if (chain) node.setAttribute("data-fx-chain", JSON.stringify(chain));
132+
document.body.append(node);
133+
return node;
134+
};
135+
/** Let the MutationObserver's microtask run. */
136+
const settle = () => new Promise((r) => setTimeout(r, 0));
137+
138+
it("routes through an effect added to a track that had none", async () => {
139+
const src = new Node();
140+
const dst = new Node();
141+
const node = audioEl();
142+
attachElementFxChain(ctx(), node, src as never, dst as never);
143+
expect(src.connections).toContain(dst);
144+
145+
node.setAttribute("data-fx-chain", JSON.stringify(CHAIN));
146+
await settle();
147+
// Now feeding the chain, not the gain directly.
148+
expect(src.connections.at(-1)).not.toBe(dst);
149+
});
150+
151+
it("returns to dry when the last effect is removed", async () => {
152+
const src = new Node();
153+
const dst = new Node();
154+
const node = audioEl(CHAIN);
155+
attachElementFxChain(ctx(), node, src as never, dst as never);
156+
const firstTarget = src.connections[0] as Node;
157+
expect(firstTarget).not.toBe(dst);
158+
159+
node.removeAttribute("data-fx-chain");
160+
await settle();
161+
expect(src.connections.at(-1)).toBe(dst);
162+
// The graph that was in the path is torn down, not left running.
163+
expect(firstTarget.disconnected).toBe(true);
164+
});
165+
166+
it("swaps the graph without replacing the source node", async () => {
167+
const src = new Node();
168+
const dst = new Node();
169+
const node = audioEl(CHAIN);
170+
attachElementFxChain(ctx(), node, src as never, dst as never);
171+
const before = src.connections[0] as Node;
172+
173+
node.setAttribute(
174+
"data-fx-chain",
175+
JSON.stringify({
176+
version: 1,
177+
nodes: [
178+
{ type: "peaking", params: { frequency: 1000, gain: -6, q: 1 } },
179+
{ type: "lowpass", params: { frequency: 800, q: 0.7, poles: "2" } },
180+
],
181+
}),
182+
);
183+
await settle();
184+
const after = src.connections.at(-1) as Node;
185+
// A different graph, reached from the same source: the audio never restarts.
186+
expect(after).not.toBe(before);
187+
expect(before.disconnected).toBe(true);
188+
});
189+
190+
it("keeps playing dry when an edit leaves the chain unreadable", async () => {
191+
const src = new Node();
192+
const dst = new Node();
193+
const node = audioEl(CHAIN);
194+
attachElementFxChain(ctx(), node, src as never, dst as never);
195+
node.setAttribute("data-fx-chain", "{not json");
196+
await settle();
197+
expect(src.connections.at(-1)).toBe(dst);
198+
});
199+
});
200+
122201
it("tears the chain down on dispose", () => {
123202
const src = new Node();
124203
const dst = new Node();

packages/core/src/runtime/audioFx.ts

Lines changed: 78 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,6 @@ function readChain(el: { getAttribute?(name: string): string | null }): {
6868
}
6969
}
7070

71-
/**
72-
* An element's automation lanes, bound to whatever chain it carries.
73-
*
74-
* FX lanes need the chain to resolve their target's range, so they are dropped
75-
* for an element with no chain; a volume lane is always readable.
76-
*/
77-
export function readElementAutomation(el: {
78-
getAttribute?(name: string): string | null;
79-
}): HfAutomation {
80-
return readAutomation(el, readChain(el).chain);
81-
}
82-
8371
/**
8472
* Splice an element's FX chain between a decoded source and its gain stage.
8573
*
@@ -88,8 +76,8 @@ export function readElementAutomation(el: {
8876
* point where effects belong — capturing the element would process a stream
8977
* nothing is listening to.
9078
*
91-
* Returns null when the element carries no chain, leaving the original
92-
* source-to-gain connection in place.
79+
* A track with no chain is wired straight through, but still watched: adding its
80+
* first effect is then heard without rescheduling the source.
9381
*
9482
* With `timing`, the element's automation lanes are scheduled onto the built
9583
* effects as AudioParam ramps, and rescheduled when the attribute is edited.
@@ -102,10 +90,6 @@ export function attachElementFxChain(
10290
timing?: AutomationTiming,
10391
): { dispose(): void } | null {
10492
const { chain } = readChain(el);
105-
if (chain.nodes.length === 0) {
106-
source.connect(destination);
107-
return null;
108-
}
10993

11094
// An AudioWorkletNode cannot be constructed before its processor is
11195
// registered — it throws, and the whole chain is lost. So when the chain
@@ -137,21 +121,56 @@ export function attachElementFxChain(
137121
};
138122
}
139123

140-
let handle: FxChainHandle;
141-
try {
142-
handle = buildFxChain(ctx, chain);
143-
} catch {
144-
// A chain we cannot realise plays dry rather than silencing the track.
145-
source.connect(destination);
146-
return null;
147-
}
124+
// Null means the source runs straight into its gain: an empty chain, or one
125+
// that could not be realised. Mutable because a structural edit swaps the
126+
// whole graph rather than re-parameterising it.
127+
let handle: FxChainHandle | null = null;
128+
let automated: FxParamTarget[] = [];
129+
130+
/** Take the current graph out of the path, leaving the source connected dry. */
131+
const detach = (): void => {
132+
try {
133+
if (handle) {
134+
source.disconnect(handle.input);
135+
handle.output.disconnect(destination);
136+
handle.dispose();
137+
} else {
138+
source.disconnect(destination);
139+
}
140+
} catch {
141+
// Already disconnected; nothing to unwind.
142+
}
143+
handle = null;
144+
};
145+
146+
/**
147+
* Put `next` in the signal path.
148+
*
149+
* A chain that cannot be realised — an unregistered worklet, an unknown
150+
* effect — plays dry rather than silencing the track.
151+
*/
152+
const attach = (next: HfAudioFxChain): void => {
153+
if (next.nodes.length === 0) {
154+
source.connect(destination);
155+
return;
156+
}
157+
try {
158+
const built = buildFxChain(ctx, next);
159+
source.connect(built.input);
160+
built.output.connect(destination);
161+
handle = built;
162+
} catch {
163+
source.connect(destination);
164+
}
165+
};
148166

149-
source.connect(handle.input);
150-
handle.output.connect(destination);
167+
const scheduleFor = (next: HfAudioFxChain, at: AutomationTiming | null): void => {
168+
automated =
169+
at && handle ? scheduleChainAutomation(readAutomation(el, next), next, handle.nodes, at) : [];
170+
};
151171

152-
let automated: FxParamTarget[] = timing
153-
? scheduleChainAutomation(readAutomation(el, chain), chain, handle.nodes, timing)
154-
: [];
172+
attach(chain);
173+
scheduleFor(chain, timing ?? null);
155174

156175
/**
157176
* Re-aim the envelope at the live playhead. An edit lands mid-playback, so
@@ -171,14 +190,31 @@ export function attachElementFxChain(
171190
const at = timingNow();
172191
if (!at) return;
173192
cancelParamLane(automated, at.scheduledAt);
174-
automated = scheduleChainAutomation(readAutomation(el, next), next, handle.nodes, at);
193+
scheduleFor(next, at);
194+
};
195+
196+
/**
197+
* Rebuild the graph for a shape change — an effect added, removed, bypassed,
198+
* or a filter's pole count switched — while the source keeps playing.
199+
*
200+
* The source node is untouched, so the audio does not restart; only the
201+
* effects between it and its gain are replaced. Doing this here is what keeps
202+
* a structural edit from needing a composition reload, which is what made the
203+
* audio audibly chop.
204+
*/
205+
const rebuild = (next: HfAudioFxChain): void => {
206+
const at = timingNow();
207+
cancelParamLane(automated, at?.scheduledAt ?? 0);
208+
detach();
209+
attach(next);
210+
scheduleFor(next, at);
175211
};
176212

177-
// Follow the attribute while the source plays, so dragging a knob is heard
178-
// without rescheduling the track. Values-only changes re-parameterise the
179-
// running graph and land on the next 128-sample quantum; a shape change
180-
// (effect added, bypassed, pole count) cannot be patched in place and waits
181-
// for the next schedule rather than cutting the audio mid-play.
213+
// Follow the attribute while the source plays, so editing a chain is heard
214+
// without rescheduling the track. A values-only change re-parameterises the
215+
// running graph and lands on the next 128-sample quantum; anything structural
216+
// swaps the effects between the source and its gain, leaving the source — and
217+
// so the playing audio — alone.
182218
let observer: MutationObserver | null = null;
183219
const target = el as unknown as Node;
184220
if (
@@ -187,11 +223,10 @@ export function attachElementFxChain(
187223
) {
188224
observer = new MutationObserver(() => {
189225
const next = readChain(el);
190-
if (next.chain.nodes.length === 0) return;
191-
handle.update(next.chain);
192-
// Values pushed by `update` would fight a running envelope, so the lanes
193-
// are re-scheduled on top of them from the current playhead.
194-
rescheduleAutomation(next.chain);
226+
// `update` reports false when the change is structural rather than a new
227+
// set of values, which is the signal to swap the graph.
228+
if (!handle || !handle.update(next.chain)) rebuild(next.chain);
229+
else rescheduleAutomation(next.chain);
195230
});
196231
observer.observe(target, {
197232
attributes: true,
@@ -205,7 +240,7 @@ export function attachElementFxChain(
205240
if (automated.length > 0) {
206241
cancelParamLane(automated, typeof ctx.currentTime === "number" ? ctx.currentTime : 0);
207242
}
208-
handle.dispose();
243+
handle?.dispose();
209244
},
210245
};
211246
}

0 commit comments

Comments
 (0)