diff --git a/.fallowrc.jsonc b/.fallowrc.jsonc index 3a29f215b..de8373d2e 100644 --- a/.fallowrc.jsonc +++ b/.fallowrc.jsonc @@ -149,13 +149,6 @@ "file": "packages/studio/src/utils/studioHelpers.ts", "exports": ["resolveDroppedAssetDimensions"], }, - // Audio FX worklets sit near the bottom of the audio stack: the worklet - // source and its test reset are consumed by the runtime and engine PRs - // upstack, so a per-PR audit against the merge base sees them as unused. - { - "file": "packages/core/src/audio/audioFxWorklets.ts", - "exports": ["AUDIO_FX_WORKLET_SOURCE", "__resetAudioFxWorkletsForTests"], - }, { "file": "packages/core/src/audio/audioFxGraph.ts", "exports": ["ensureAudioFxWorklets"], diff --git a/packages/core/src/audio/audioFxGraph.test.ts b/packages/core/src/audio/audioFxGraph.test.ts index 1a8d23519..316deff7a 100644 --- a/packages/core/src/audio/audioFxGraph.test.ts +++ b/packages/core/src/audio/audioFxGraph.test.ts @@ -155,6 +155,16 @@ describe("buildFxNode", () => { expect(workletNodes[0]!.messages[0]).toMatchObject({ threshold: -30 }); }); + it("tells a worklet processor to retire on dispose, not just disconnect it", () => { + // Disconnecting leaves the processor alive — it lives until `process()` + // returns false — so every rebuild that dropped a worklet effect left one + // running on the audio thread for the rest of the session. + workletNodes.length = 0; + const h = buildFxNode(asCtx(ctx()), "compressor", defaultAudioFxParams("compressor")); + h.dispose(); + expect(workletNodes[0]!.messages).toEqual([{ __hfDispose: true }]); + }); + it("rebuilds the saturation curve for the selected shape", () => { const c = ctx(); buildFxNode(asCtx(c), "saturate", { ...defaultAudioFxParams("saturate"), type: "hard" }); diff --git a/packages/core/src/audio/audioFxGraph.ts b/packages/core/src/audio/audioFxGraph.ts index f4bc818dd..f7c7130cb 100644 --- a/packages/core/src/audio/audioFxGraph.ts +++ b/packages/core/src/audio/audioFxGraph.ts @@ -182,7 +182,17 @@ function workletBuilder(processor: string): Builder { input: node, output: node, update: (v) => node.port.postMessage({ ...v }), - dispose: () => node.disconnect(), + dispose: () => { + // Disconnecting is not enough to retire an AudioWorkletProcessor: it + // lives until its `process()` returns false, and these all returned + // true unconditionally. So every chain rebuild that dropped a limiter, + // compressor, gate or bitcrush left it running on the audio thread for + // the rest of the session, and a few edits to a carved bed accumulated + // a stack of them. The processors treat this message as their cue to + // stop. + node.port.postMessage({ __hfDispose: true }); + node.disconnect(); + }, }; }; } diff --git a/packages/core/src/audio/audioFxWorklets.test.ts b/packages/core/src/audio/audioFxWorklets.test.ts new file mode 100644 index 000000000..1e9e06ca6 --- /dev/null +++ b/packages/core/src/audio/audioFxWorklets.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it, vi } from "vitest"; +import { audioFxWorkletsReady, ensureAudioFxWorklets } from "./audioFxWorklets.js"; + +/** Just enough of a BaseAudioContext for the registration cache to key on. */ +const contextWith = (addModule: (url: string) => Promise): BaseAudioContext => + ({ audioWorklet: { addModule } }) as unknown as BaseAudioContext; + +describe("ensureAudioFxWorklets", () => { + it("registers once per context and reuses the result", async () => { + const addModule = vi.fn(async () => undefined); + const ctx = contextWith(addModule); + + await ensureAudioFxWorklets(ctx); + await ensureAudioFxWorklets(ctx); + + expect(addModule).toHaveBeenCalledTimes(1); + expect(audioFxWorkletsReady(ctx)).toBe(true); + }); + + it("retries after a failure instead of replaying it forever", async () => { + // The rejected promise used to stay in the cache, so every later attempt + // got the same rejection back — the limiter, compressor, gate and bitcrush + // were silent for the life of the context after one transient failure. + const addModule = vi + .fn<(url: string) => Promise>() + .mockRejectedValueOnce(new Error("module load failed")) + .mockResolvedValue(undefined); + const ctx = contextWith(addModule); + + await expect(ensureAudioFxWorklets(ctx)).rejects.toThrow("module load failed"); + expect(audioFxWorkletsReady(ctx)).toBe(false); + + await expect(ensureAudioFxWorklets(ctx)).resolves.toBeUndefined(); + expect(addModule).toHaveBeenCalledTimes(2); + expect(audioFxWorkletsReady(ctx)).toBe(true); + }); + + it("refuses a context with no AudioWorklet rather than hanging", async () => { + const ctx = {} as BaseAudioContext; + await expect(ensureAudioFxWorklets(ctx)).rejects.toThrow(/secure context/); + }); +}); + +/** + * A processor lives until its `process()` returns false — disconnecting the + * node does not retire it. These all returned true unconditionally, so every + * chain rebuild that dropped a worklet effect left it running on the audio + * thread for the rest of the session. + * + * The source is taken from the data: URL registration actually hands to + * `addModule`, so this also proves the URL carries what it claims to. + */ +describe("the worklet processors themselves", () => { + /** Evaluate the registered module and hand back the processor classes by name. */ + async function loadProcessors(): Promise Processor>> { + let moduleSource = ""; + await ensureAudioFxWorklets( + contextWith(async (url: string) => { + moduleSource = atob(url.replace("data:text/javascript;base64,", "")); + }), + ); + const made = new Map Processor>(); + class Base { + port = { + onmessage: null as ((e: { data: unknown }) => void) | null, + postMessage: (data: unknown) => this.port.onmessage?.({ data }), + }; + } + new Function("AudioWorkletProcessor", "registerProcessor", "sampleRate", moduleSource)( + Base, + (name: string, cls: new (o: unknown) => Processor) => made.set(name, cls), + 48000, + ); + return made; + } + + interface Processor { + port: { postMessage(data: unknown): void }; + process(inputs: Float32Array[][], outputs: Float32Array[][]): boolean; + } + + const block = (): Float32Array[][] => [[new Float32Array(128)]]; + + it("every processor keeps running until it is told to stop, then retires", async () => { + const processors = await loadProcessors(); + expect([...processors.keys()]).toEqual([ + "hf-compressor", + "hf-limiter", + "hf-gate", + "hf-bitcrush", + ]); + + for (const [name, Cls] of processors) { + const p = new Cls({ processorOptions: {} }); + expect(p.process(block(), block()), `${name} retired before it was disposed`).toBe(true); + p.port.postMessage({ __hfDispose: true }); + expect(p.process(block(), block()), `${name} kept running after dispose`).toBe(false); + // And it stays retired — a later parameter update must not revive it. + p.port.postMessage({ mix: 0.5 }); + expect(p.process(block(), block()), `${name} came back to life`).toBe(false); + } + }); +}); diff --git a/packages/core/src/audio/audioFxWorklets.ts b/packages/core/src/audio/audioFxWorklets.ts index e421d3409..f2261ea8a 100644 --- a/packages/core/src/audio/audioFxWorklets.ts +++ b/packages/core/src/audio/audioFxWorklets.ts @@ -60,11 +60,13 @@ class HfCompressor extends AudioWorkletProcessor { this.p = o.processorOptions || {}; this.env = new EnvBank(this.p.attack ?? 20, this.p.release ?? 250); this.port.onmessage = (e) => { + if (e.data && e.data.__hfDispose) { this.dead = true; return; } this.p = { ...this.p, ...e.data }; this.env.set(this.p.attack ?? 20, this.p.release ?? 250); }; } process(inputs, outputs) { + if (this.dead) return false; const i = inputs[0], o = outputs[0]; if (!i || !i.length) return true; const p = this.p; @@ -105,11 +107,13 @@ class HfLimiter extends AudioWorkletProcessor { this.p = o.processorOptions || {}; this.env = new EnvBank(this.p.attack ?? 5, this.p.release ?? 50); this.port.onmessage = (e) => { + if (e.data && e.data.__hfDispose) { this.dead = true; return; } this.p = { ...this.p, ...e.data }; this.env.set(this.p.attack ?? 5, this.p.release ?? 50); }; } process(inputs, outputs) { + if (this.dead) return false; const i = inputs[0], o = outputs[0]; if (!i || !i.length) return true; const ceiling = dbToLin(this.p.limit ?? -1); @@ -136,11 +140,13 @@ class HfGate extends AudioWorkletProcessor { this.env = new EnvBank(this.p.attack ?? 1, this.p.release ?? 100); this.gains = []; this.port.onmessage = (e) => { + if (e.data && e.data.__hfDispose) { this.dead = true; return; } this.p = { ...this.p, ...e.data }; this.env.set(this.p.attack ?? 1, this.p.release ?? 100); }; } process(inputs, outputs) { + if (this.dead) return false; const i = inputs[0], o = outputs[0]; if (!i || !i.length) return true; const p = this.p; @@ -183,9 +189,13 @@ class HfBitcrush extends AudioWorkletProcessor { this.p = o.processorOptions || {}; this.holds = []; this.held = []; - this.port.onmessage = (e) => { this.p = { ...this.p, ...e.data }; }; + this.port.onmessage = (e) => { + if (e.data && e.data.__hfDispose) { this.dead = true; return; } + this.p = { ...this.p, ...e.data }; + }; } process(inputs, outputs) { + if (this.dead) return false; const i = inputs[0], o = outputs[0]; if (!i || !i.length) return true; const p = this.p; @@ -243,7 +253,16 @@ export function ensureAudioFxWorklets(ctx: BaseAudioContext): Promise { )}`; await ctx.audioWorklet.addModule(url); readyContexts.add(ctx); - })(); + })().catch((err: unknown) => { + // A failed registration must not be remembered. `readyContexts` is only + // written on success, so callers correctly keep asking — and every ask + // replayed this same rejected promise, leaving the limiter, compressor, + // gate and bitcrush silent for the life of the context with no way back. + // One transient failure (a slow module load, a context still warming up) + // permanently disabled half the rack. + registered.delete(ctx); + throw err; + }); registered.set(ctx, modulePromise); } return modulePromise; diff --git a/packages/core/src/audioCarve.test.ts b/packages/core/src/audioCarve.test.ts index c2cb26019..be4dcfb14 100644 --- a/packages/core/src/audioCarve.test.ts +++ b/packages/core/src/audioCarve.test.ts @@ -250,6 +250,48 @@ describe("analyseCarveBands", () => { }); }); +/** + * Both analysis loops reuse one pair of FFT scratch arrays across every window + * rather than allocating a pair per hop — a 5-minute 48 kHz voiceover is ~7000 + * hops, so ~460 MB of transient Float64Array used to churn through the main + * thread for one carve. `re` is fully overwritten each window, but `im` is only + * ever added to, so it has to be cleared; missing that, the imaginary part + * accumulates across windows and every spectrum after the first is wrong by a + * growing amount. + */ +describe("reused FFT scratch across windows", () => { + /** A steady tone on an exact bin centre (48000/4096 x 128), so every window is identical. */ + const steady = (seconds: number): Float32Array => { + const n = Math.floor(SR * seconds); + const out = new Float32Array(n); + for (let i = 0; i < n; i++) out[i] = 0.5 * Math.sin((2 * Math.PI * 1500 * i) / SR); + return out; + }; + + it("measures the same bands however many windows the clip has", () => { + // Every window carries the same spectrum, so the Welch average cannot + // depend on how many were averaged — unless one window is contaminating + // the next. + const short = analyseCarveBands(steady(0.5), SR, PROFILE); + const long = analyseCarveBands(steady(12), SR, PROFILE); + expect(short.length).toBeGreaterThan(0); + expect(long).toEqual(short); + }); + + it("keeps a steady tone's dynamics envelope flat instead of drifting", () => { + const [lane] = analyseCarveDynamics(steady(12), SR, [{ freq: 1600, gainDb: -8, q: 1.4 }]); + const value = (t: number): number => + sampleAutomationLane({ target: "fx.n1.gain", points: lane!.points }, t); + // Past the attack the cut has to sit still, because the signal does. A + // window contaminated by the one before it grows the measured power over + // the clip, and the envelope — which is relative to the band's own peak — + // slides with it. + expect(value(4)).toBeLessThan(-1); + expect(value(8)).toBeCloseTo(value(4), 0); + expect(value(11)).toBeCloseTo(value(4), 0); + }); +}); + describe("carveBandsToChain", () => { it("turns bands into peaking nodes carrying the analysed values", () => { const chain = carveBandsToChain([{ freq: 1000, gainDb: -6, q: 1.4 }]); diff --git a/packages/core/src/audioCarve.ts b/packages/core/src/audioCarve.ts index 080638c4c..265b48352 100644 --- a/packages/core/src/audioCarve.ts +++ b/packages/core/src/audioCarve.ts @@ -332,12 +332,23 @@ function powerSpectrum( const bins = FRAME / 2 + 1; const acc = new Float64Array(bins); + // Reused across hops. These used to be allocated inside the loop: a 5-minute + // 48 kHz voiceover is ~7000 hops, so ~460 MB of transient Float64Array + // churned through the main thread for a single carve. `re` is fully + // overwritten below; only `im` has to be cleared. + const re = new Float64Array(FRAME); + const im = new Float64Array(FRAME); + // + // Every hop is still read. Striding them — Welch's average is supposed to + // settle long before 7000 windows — was measured on a 5-minute voiceover and + // moves the result: at strength 0.9 the chosen band set changed (630 Hz for + // 160 Hz), and it did not converge back to the full read even at 2048 + // windows. 27x faster is not worth silently redrawing the author's carve. let frames = 0; for (let start = 0; start + FRAME <= n; start += HOP) { // Goertzel-free naive DFT would be O(n^2); use a real FFT via recursion on // a copied frame. FRAME is a power of two so the radix-2 split is exact. - const re = new Float64Array(FRAME); - const im = new Float64Array(FRAME); + im.fill(0); for (let i = 0; i < FRAME; i++) re[i] = (padded[start + i] ?? 0) * window[i]!; fft(re, im); for (let k = 0; k < bins; k++) acc[k]! += re[k]! * re[k]! + im[k]! * im[k]!; @@ -601,9 +612,13 @@ export function analyseCarveDynamics( const times: number[] = []; const perBand = bands.map(() => [] as number[]); + // Reused across windows, as in powerSpectrum. `re` is fully overwritten + // below; only `im` has to be cleared. The hop here is already bounded by + // POINT_BUDGET, so there is nothing to stride. + const re = new Float64Array(FRAME); + const im = new Float64Array(FRAME); for (let start = 0; start < voice.length; start += hop) { - const re = new Float64Array(FRAME); - const im = new Float64Array(FRAME); + im.fill(0); for (let i = 0; i < FRAME; i++) re[i] = (voice[start + i] ?? 0) * window[i]!; fft(re, im); const power: number[] = []; diff --git a/packages/core/src/audioFx.test.ts b/packages/core/src/audioFx.test.ts index e59819b95..3caaa2464 100644 --- a/packages/core/src/audioFx.test.ts +++ b/packages/core/src/audioFx.test.ts @@ -69,6 +69,27 @@ describe("normalizeAudioFxParams", () => { expect(v.gain).toBe(0); }); + it("treats a blank or missing value as absent rather than as zero", () => { + // `Number(null)`, `Number("")`, `Number(false)` and `Number([])` are all 0 + // and all finite, so these used to clamp to 0 instead of falling back. Zero + // is a legal setting for most of these knobs, so nothing downstream could + // tell: a compressor whose threshold arrived as null sat at 0 dB and never + // engaged, silently, rather than at its declared -24 dB. + const def = defaultAudioFxParams("compressor").threshold; + expect(def).not.toBe(0); + for (const blank of [null, undefined, "", " ", false, [], {}]) { + expect( + normalizeAudioFxParams("compressor", { threshold: blank as unknown as number }).threshold, + `${JSON.stringify(blank)} was read as a number`, + ).toBe(def); + } + // A string that really does spell a number still counts — that is how the + // panel's inputs arrive. + expect( + normalizeAudioFxParams("compressor", { threshold: "-30" as unknown as number }).threshold, + ).toBe(-30); + }); + it("falls back to the default for an unrecognised enum value", () => { expect(normalizeAudioFxParams("saturate", { type: "sawtooth" }).type).toBe("tanh"); expect(normalizeAudioFxParams("saturate", { type: "atan" }).type).toBe("atan"); diff --git a/packages/core/src/audioFx.ts b/packages/core/src/audioFx.ts index 1e38c4f29..32be62bc6 100644 --- a/packages/core/src/audioFx.ts +++ b/packages/core/src/audioFx.ts @@ -761,7 +761,20 @@ export function normalizeAudioFxParams( out[p.key] = ok ? (raw as string) : p.default; continue; } - const n = typeof raw === "number" ? raw : Number(raw); + // Only a number, or a string that actually spells one. `Number(null)`, + // `Number("")`, `Number(false)` and `Number([])` are all 0 and all pass + // Number.isFinite, so a missing or blanked value used to clamp to 0 rather + // than fall back to the declared default — and 0 is a legal value for most + // of these knobs, so nothing downstream could tell. A compressor whose + // threshold arrived as null sat at 0 dB and never engaged, silently, + // instead of at its -24 dB default. `numberOrNull` in audioAutomation.ts + // already guards exactly this. + const n = + typeof raw === "number" + ? raw + : typeof raw === "string" && raw.trim() !== "" + ? Number(raw) + : Number.NaN; out[p.key] = Number.isFinite(n) ? Math.min(p.max, Math.max(p.min, n)) : p.default; } return out; diff --git a/packages/producer/src/services/render/stages/audioStage.test.ts b/packages/producer/src/services/render/stages/audioStage.test.ts index 7be552b08..ffa0b9715 100644 --- a/packages/producer/src/services/render/stages/audioStage.test.ts +++ b/packages/producer/src/services/render/stages/audioStage.test.ts @@ -122,7 +122,27 @@ describe("runAudioStage", () => { const result = await runAudioStage(makeInput()); expect(result.hasAudio).toBe(false); expect(result.audioError).toMatch(/Audio FX failed for track bgm/); - expect(result.audioFailures).toBeUndefined(); + // And it is classified. This used to come back undefined, so the warning + // policy — which reads owner, retryability, reason and stage off this list + // — described the FATAL failure with strictly less detail than a single + // dropped track gets. + expect(result.audioFailures).toEqual([ + { + stage: "internal", + reason: "internal", + owner: "system", + retryable: false, + detail: "Audio FX failed for track bgm: browser launch failed", + }, + ]); + }); + + it("bounds the synthesised failure's detail", async () => { + // `detail` is contractually bounded diagnostic text; an ffmpeg-flavoured + // message can run to tens of kilobytes. + processCompositionAudioMock.mockRejectedValue(new Error("x".repeat(5_000))); + const result = await runAudioStage(makeInput()); + expect(result.audioFailures?.[0]?.detail.length).toBe(2_000); }); it("lets an abort keep its own shape rather than becoming an audio error", async () => { diff --git a/packages/producer/src/services/render/stages/audioStage.ts b/packages/producer/src/services/render/stages/audioStage.ts index ac243dd82..5e61a4a17 100644 --- a/packages/producer/src/services/render/stages/audioStage.ts +++ b/packages/producer/src/services/render/stages/audioStage.ts @@ -90,12 +90,28 @@ export async function runAudioStage(input: AudioStageInput): Promise