Skip to content

Commit 0e45804

Browse files
committed
fix(core): unwire an LFO when its effect is disposed
The §4c item here was "four hand-rolled wet/dry shells whose dispose lists have already drifted". Reading them, the drift is one specific thing and it is a leak, not an untidiness: the chorus and the phaser stopped their LFO and left it out of the nodes they disconnect. So every chain rebuild that dropped a modulated effect left a modulator still wired to the delay or the allpass bank it had been driving. Nothing audible came of it — the shell around it was disconnected — but the nodes stayed reachable, and a session of edits to a modulated track piled them up. Same shape as the worklet leak fixed earlier on this stack. `stopLfo` becomes `retireLfo` and does both halves, which is the whole fix. The consolidation the item also asks for is not here: the four shells are wired differently enough (a feedback loop, a modulated delay, six allpass stages between two trims, a convolver) that one factory over them would need a config surface bigger than the four dispose lines it replaces. The leak was the part with a defect behind it. Also folds the two artifact build scripts together — 50 lines each, differing in five names. That one is a genuine copy, and the copy is where a divergence would hide: whichever stopped being edited would go on producing a subtly different artifact with nothing to say so. Output is byte-identical, which `check:position-edits-render` proves by diffing the tracked artifact. Falsified: dropping the `disconnect` fails the new test. core 1726 passing (110 files), engine services 736 passing / 3 skipped.
1 parent 3fcf58b commit 0e45804

5 files changed

Lines changed: 127 additions & 100 deletions

File tree

Lines changed: 10 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,13 @@
11
/** Build the injectable audio-FX runtime artifact from the canonical runtime. */
22

3-
import { mkdirSync, writeFileSync } from "node:fs";
4-
import { dirname, resolve } from "node:path";
5-
import { fileURLToPath } from "node:url";
6-
import { buildSync } from "esbuild";
7-
import { execFileSync } from "node:child_process";
8-
9-
const thisDir = dirname(fileURLToPath(import.meta.url));
10-
const repoRoot = resolve(thisDir, "..");
11-
const entry = resolve(repoRoot, "stubs/audio-fx-runtime-entry.ts");
12-
const generatedDir = resolve(repoRoot, "src/generated");
13-
const outPath = resolve(generatedDir, "audio-fx-runtime-inline.ts");
14-
15-
const result = buildSync({
16-
entryPoints: [entry],
17-
bundle: true,
18-
write: false,
19-
platform: "browser",
20-
format: "iife",
21-
target: ["es2020"],
22-
minify: true,
23-
legalComments: "none",
3+
import { buildInjectedArtifact } from "./buildInjectedArtifact.js";
4+
5+
buildInjectedArtifact({
6+
scriptUrl: import.meta.url,
7+
entry: "stubs/audio-fx-runtime-entry.ts",
8+
out: "audio-fx-runtime-inline.ts",
9+
constName: "AUDIO_FX_RUNTIME_IIFE",
10+
fnName: "getAudioFxRuntimeScript",
11+
what: "audio-FX runtime IIFE",
12+
event: "audio_fx_runtime_generated",
2413
});
25-
const iife = result.outputFiles[0]?.text ?? "";
26-
if (!iife) throw new Error("esbuild produced no output for audio-fx-runtime-entry.ts");
27-
28-
mkdirSync(generatedDir, { recursive: true });
29-
writeFileSync(
30-
outPath,
31-
[
32-
"// AUTO-GENERATED by scripts/build-audio-fx-runtime.ts - do not edit",
33-
`const AUDIO_FX_RUNTIME_IIFE: string = ${JSON.stringify(iife)};`,
34-
"",
35-
"/** Returns the pre-built audio-FX runtime IIFE as a string constant. */",
36-
"export function getAudioFxRuntimeScript(): string {",
37-
" return AUDIO_FX_RUNTIME_IIFE;",
38-
"}",
39-
"",
40-
].join("\n"),
41-
"utf8",
42-
);
43-
44-
try {
45-
execFileSync("bun", ["x", "oxfmt", outPath], { stdio: "ignore" });
46-
} catch {
47-
// Formatting is best effort when the generator runs in a minimal environment.
48-
}
49-
50-
console.log(JSON.stringify({ event: "audio_fx_runtime_generated", outPath, bytes: iife.length }));
Lines changed: 10 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,13 @@
11
/** Build the injectable position-edits render artifact from the canonical runtime. */
22

3-
import { mkdirSync, writeFileSync } from "node:fs";
4-
import { dirname, resolve } from "node:path";
5-
import { fileURLToPath } from "node:url";
6-
import { buildSync } from "esbuild";
7-
import { execFileSync } from "node:child_process";
8-
9-
const thisDir = dirname(fileURLToPath(import.meta.url));
10-
const repoRoot = resolve(thisDir, "..");
11-
const entry = resolve(repoRoot, "stubs/position-edits-render-entry.ts");
12-
const generatedDir = resolve(repoRoot, "src/generated");
13-
const outPath = resolve(generatedDir, "position-edits-render-inline.ts");
14-
15-
const result = buildSync({
16-
entryPoints: [entry],
17-
bundle: true,
18-
write: false,
19-
platform: "browser",
20-
format: "iife",
21-
target: ["es2020"],
22-
minify: true,
23-
legalComments: "none",
3+
import { buildInjectedArtifact } from "./buildInjectedArtifact.js";
4+
5+
buildInjectedArtifact({
6+
scriptUrl: import.meta.url,
7+
entry: "stubs/position-edits-render-entry.ts",
8+
out: "position-edits-render-inline.ts",
9+
constName: "POSITION_EDITS_RENDER_IIFE",
10+
fnName: "getPositionEditsRenderScript",
11+
what: "position-edits render IIFE",
12+
event: "position_edits_render_generated",
2413
});
25-
const iife = result.outputFiles[0]?.text ?? "";
26-
if (!iife) throw new Error("esbuild produced no output for position-edits-render-entry.ts");
27-
28-
mkdirSync(generatedDir, { recursive: true });
29-
writeFileSync(
30-
outPath,
31-
[
32-
"// AUTO-GENERATED by scripts/build-position-edits-render.ts - do not edit",
33-
`const POSITION_EDITS_RENDER_IIFE: string = ${JSON.stringify(iife)};`,
34-
"",
35-
"/** Returns the pre-built position-edits render IIFE as a string constant. */",
36-
"export function getPositionEditsRenderScript(): string {",
37-
" return POSITION_EDITS_RENDER_IIFE;",
38-
"}",
39-
"",
40-
].join("\n"),
41-
"utf8",
42-
);
43-
44-
try {
45-
execFileSync("bun", ["x", "oxfmt", outPath], { stdio: "ignore" });
46-
} catch {
47-
// Formatting is best effort when the generator runs in a minimal environment.
48-
}
49-
50-
console.log(
51-
JSON.stringify({ event: "position_edits_render_generated", outPath, bytes: iife.length }),
52-
);
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* Bundle a stub entry into an injectable IIFE, wrapped as a TypeScript constant.
3+
*
4+
* Two artifacts are built this way — the audio-FX runtime and the position-edits
5+
* render — and the engine injects both into the headless browser as a script tag.
6+
* They were two copies of this file differing in five names, which is a poor
7+
* place for a divergence to hide: whichever copy stopped being edited would go on
8+
* producing a subtly different artifact with nothing to say so.
9+
*/
10+
11+
import { execFileSync } from "node:child_process";
12+
import { mkdirSync, writeFileSync } from "node:fs";
13+
import { dirname, resolve } from "node:path";
14+
import { fileURLToPath } from "node:url";
15+
import { buildSync } from "esbuild";
16+
17+
export interface InjectedArtifact {
18+
/** The build script's own `import.meta.url`, so paths resolve beside it. */
19+
scriptUrl: string;
20+
/** Entry stub, relative to the package root. */
21+
entry: string;
22+
/** Output file name inside `src/generated`. */
23+
out: string;
24+
/** SCREAMING_CASE name for the string constant holding the IIFE. */
25+
constName: string;
26+
/** The accessor the rest of the codebase imports. */
27+
fnName: string;
28+
/** What that accessor returns, for its doc comment: "the pre-built X". */
29+
what: string;
30+
/** Structured log event name. */
31+
event: string;
32+
}
33+
34+
export function buildInjectedArtifact(spec: InjectedArtifact): void {
35+
const scriptDir = dirname(fileURLToPath(spec.scriptUrl));
36+
const scriptName = spec.scriptUrl.split("/").pop() ?? "";
37+
const repoRoot = resolve(scriptDir, "..");
38+
const entry = resolve(repoRoot, spec.entry);
39+
const generatedDir = resolve(repoRoot, "src/generated");
40+
const outPath = resolve(generatedDir, spec.out);
41+
42+
const result = buildSync({
43+
entryPoints: [entry],
44+
bundle: true,
45+
write: false,
46+
platform: "browser",
47+
format: "iife",
48+
target: ["es2020"],
49+
minify: true,
50+
legalComments: "none",
51+
});
52+
const iife = result.outputFiles[0]?.text ?? "";
53+
if (!iife) throw new Error(`esbuild produced no output for ${spec.entry.split("/").pop()}`);
54+
55+
mkdirSync(generatedDir, { recursive: true });
56+
writeFileSync(
57+
outPath,
58+
[
59+
`// AUTO-GENERATED by scripts/${scriptName} - do not edit`,
60+
`const ${spec.constName}: string = ${JSON.stringify(iife)};`,
61+
"",
62+
`/** Returns the pre-built ${spec.what} as a string constant. */`,
63+
`export function ${spec.fnName}(): string {`,
64+
` return ${spec.constName};`,
65+
"}",
66+
"",
67+
].join("\n"),
68+
"utf8",
69+
);
70+
71+
try {
72+
execFileSync("bun", ["x", "oxfmt", outPath], { stdio: "ignore" });
73+
} catch {
74+
// Formatting is best effort when the generator runs in a minimal environment.
75+
}
76+
77+
console.log(JSON.stringify({ event: spec.event, outPath, bytes: iife.length }));
78+
}

packages/core/src/audio/audioFxGraph.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,21 @@ describe("levels and per-channel state", () => {
436436
expect(src?.playbackRate.value).toBeCloseTo(2, 6);
437437
});
438438

439+
/**
440+
* A source node is not retired by disconnecting what it feeds. The chorus and
441+
* phaser stopped their LFO and left it out of the nodes they disconnect, so
442+
* every rebuild that dropped one left a modulator still wired to the delay or
443+
* the allpass bank it had been driving.
444+
*/
445+
it("unwires a modulated effect's LFO when the effect is disposed", () => {
446+
for (const type of ["chorus", "phaser"]) {
447+
const c = ctx();
448+
buildFxNode(asCtx(c), type, defaultAudioFxParams(type)).dispose();
449+
const lfo = c.created.find((node) => node.kind === "bufferSource");
450+
expect(lfo?.disconnected, `${type} left its LFO connected`).toBe(true);
451+
}
452+
});
453+
439454
it("starts the LFO at zero for a render, which always begins at the clip's start", () => {
440455
const c = ctx();
441456
buildFxNode(asCtx(c), "phaser", defaultAudioFxParams("phaser"));

packages/core/src/audio/audioFxGraph.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,13 +144,23 @@ function lfoSource(
144144
return src;
145145
}
146146

147-
/** Stop an LFO that may already have been stopped, on the way to disposal. */
148-
function stopLfo(src: AudioBufferSourceNode): void {
147+
/**
148+
* Retire an LFO: stopped *and* unwired.
149+
*
150+
* Both halves. The old oscillators were stopped and left in their builder's
151+
* dispose list — so every chain rebuild that dropped a chorus or a phaser left a
152+
* modulator still connected to the delay or the allpass bank it had been
153+
* driving. Nothing audible came out of it, because the shell around it was
154+
* disconnected, but the nodes stayed reachable and a session of edits to a
155+
* modulated track accumulated them. Same shape as the worklet leak above.
156+
*/
157+
function retireLfo(src: AudioBufferSourceNode): void {
149158
try {
150159
src.stop();
151160
} catch {
152161
/* already stopped */
153162
}
163+
src.disconnect();
154164
}
155165

156166
/** A wet/dry pair: the dry side is whatever the wet side is not. */
@@ -365,7 +375,7 @@ const chorusLfo: Builder = (ctx, p, elapsed) => {
365375
mix: mixTargets(wet.gain, dry.gain),
366376
},
367377
dispose: () => {
368-
stopLfo(lfo);
378+
retireLfo(lfo);
369379
[input, out, dl, depth, wet, dry].forEach((x) => x.disconnect());
370380
},
371381
};
@@ -438,7 +448,7 @@ const allpassPhaser: Builder = (ctx, p, elapsed) => {
438448
out_gain: [{ param: outTrim.gain }],
439449
},
440450
dispose: () => {
441-
stopLfo(lfo);
451+
retireLfo(lfo);
442452
[input, out, inTrim, outTrim, depth, wet, dry, ...stages].forEach((x) => x.disconnect());
443453
},
444454
};

0 commit comments

Comments
 (0)