From 5e166ad47ce1c591bed91fdc605d28d85bad11f0 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sat, 8 Aug 2026 11:55:54 -0700 Subject: [PATCH] fix(core): reschedule FX automation when the transport changes rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `attachElementFxChain` gets a frozen `{scheduledAt, elapsed, rate}` and commits every lane to absolute context times from it. `setRate` only assigned `playbackRate.value`, so the audio changed speed and the envelopes did not: a lowpass sweeping over 10 clip-seconds, switched to 2x mid-play, eats 20 s of material in 10 s of wall clock while the sweep keeps its original schedule. The runtime's recovery — `stopAll()` plus a reschedule — only runs when `hasBoundedActiveSources()` is true, and a project-level music bed with no `data-duration` is unbounded, so that case never recovered at all. The timing a chain measures from is now a mutable reference frame that `setRate` rebases: `elapsed` is advanced to the playhead the OLD rate carried it to, then the lanes are replayed from there at the new one. That also fixes the second consequence — `timingNow()` was advancing `elapsed` with the stale rate, so every later chain edit re-aimed the envelope at the wrong clip position for as long as the track played. `attachElementFxChain`'s return type is now the named `ElementFxHandle`, so the transport's `ScheduledSource.fx` carries `setRate` rather than a structural `{ dispose(): void }`. Falsified against four mutations: `setRate` a no-op (envelope stays 8 s long), rescheduled without adopting the new rate (6 s instead of 3), rate swapped without rebasing the frame (booked from t=0 instead of t=2), and the guard dropped (rate 0 books a NaN span). Co-Authored-By: Claude Opus 5 (1M context) --- packages/core/src/runtime/audioFx.test.ts | 118 ++++++++++++++++++ packages/core/src/runtime/audioFx.ts | 43 +++++-- .../src/runtime/webAudioTransport.test.ts | 17 +++ .../core/src/runtime/webAudioTransport.ts | 13 +- 4 files changed, 182 insertions(+), 9 deletions(-) diff --git a/packages/core/src/runtime/audioFx.test.ts b/packages/core/src/runtime/audioFx.test.ts index 9774515810..09374e12e4 100644 --- a/packages/core/src/runtime/audioFx.test.ts +++ b/packages/core/src/runtime/audioFx.test.ts @@ -315,6 +315,124 @@ describe("attachElementFxChain", () => { }); }); + /** + * Lanes are committed to absolute context times, so the schedule is only + * right for the rate it was booked at. Bumping `playbackRate` alone left a + * lowpass sweeping over its original 10 wall-clock seconds while the audio + * underneath ran through 20 clip-seconds of material — and the runtime's + * stopAll()+reschedule recovery never fired for an unbounded source. + */ + describe("a rate change mid-playback", () => { + /** Records what was booked and when, without the browser's overlap rules. */ + class TimedParam { + curves: { time: number; duration: number }[] = []; + ramps: number[] = []; + value = 0; + setValueAtTime(v: number): void { + this.value = v; + } + linearRampToValueAtTime(v: number, t: number): void { + this.ramps.push(t); + this.value = v; + } + setValueCurveAtTime(_v: Float32Array, time: number, duration: number): void { + this.curves.push({ time, duration }); + } + cancelScheduledValues(): void {} + cancelAndHoldAtTime(): void {} + /** The last span booked, however the scheduler chose to express it. */ + last(): { time: number; duration: number } | undefined { + return this.curves.at(-1); + } + } + + const sweep = { + version: 1, + nodes: [{ type: "lowpass", id: "n1", params: { frequency: 300, q: 0.707 } }], + }; + const lane = JSON.stringify({ + version: 1, + lanes: [ + { + target: "fx.n1.frequency", + points: [ + { t: 0, v: 300 }, + { t: 8, v: 3000 }, + ], + }, + ], + }); + + const build = () => { + const clock = { currentTime: 0 }; + const made: { frequency: TimedParam }[] = []; + class TimedNode extends Node { + override frequency = new TimedParam() as unknown as { value: number }; + } + class TimedCtx extends Ctx { + get currentTime(): number { + return clock.currentTime; + } + override createBiquadFilter(): Node { + const n = new TimedNode(); + made.push(n as unknown as { frequency: TimedParam }); + return n; + } + } + const node = document.createElement("audio"); + node.setAttribute("data-fx-chain", JSON.stringify(sweep)); + node.setAttribute("data-automation", lane); + document.body.append(node); + const handle = attachElementFxChain( + new TimedCtx() as unknown as BaseAudioContext, + node, + new Node() as never, + new Node() as never, + { scheduledAt: 0, elapsed: 0, rate: 1 }, + ); + return { clock, node, handle, param: () => made[0]?.frequency as unknown as TimedParam }; + }; + + it("re-aims the envelope so the sweep still ends with the material", () => { + const { clock, handle, param } = build(); + // Booked at 1x: the whole 8 s lane spans 8 s of context time. + expect(param().last()).toEqual({ time: 0, duration: 8 }); + + clock.currentTime = 2; + handle?.setRate(2); + + // 6 clip-seconds are left, and at 2x they take 3 wall-clock seconds. + // Without this the sweep kept its original plan to t=8 while the audio + // ran out at t=5. + expect(param().last()).toEqual({ time: 2, duration: 3 }); + }); + + it("measures later edits from the new rate, not the one it started at", async () => { + // `elapsed` advances at whatever rate the reference frame holds, so a + // frame left at 1x re-aims every subsequent edit at the wrong clip + // position for as long as the track plays. + const { clock, node, handle, param } = build(); + clock.currentTime = 2; + handle?.setRate(2); + + clock.currentTime = 4; + // 2 wall-clock seconds at 2x is 4 clip-seconds, so the playhead is at 6 + // and 2 clip-seconds remain: 1 second of wall clock. + node.setAttribute("data-automation", lane); + await new Promise((r) => setTimeout(r, 0)); + expect(param().last()).toEqual({ time: 4, duration: 1 }); + }); + + it("ignores a rate that is not a rate", () => { + const { clock, handle, param } = build(); + clock.currentTime = 2; + handle?.setRate(0); + handle?.setRate(Number.NaN); + handle?.setRate(1); + expect(param().last()).toEqual({ time: 0, duration: 8 }); + }); + }); + it("tears the chain down on dispose", () => { const src = new Node(); const dst = new Node(); diff --git a/packages/core/src/runtime/audioFx.ts b/packages/core/src/runtime/audioFx.ts index 8bd4c4e3ad..cec709e7a8 100644 --- a/packages/core/src/runtime/audioFx.ts +++ b/packages/core/src/runtime/audioFx.ts @@ -81,15 +81,29 @@ function readChain(el: { getAttribute?(name: string): string | null }): { * first effect is then heard without rescheduling the source. * * With `timing`, the element's automation lanes are scheduled onto the built - * effects as AudioParam ramps, and rescheduled when the attribute is edited. + * effects as AudioParam ramps, and rescheduled when the attribute is edited or + * `setRate` reports the transport changed speed. */ +export interface ElementFxHandle { + dispose(): void; + /** + * Re-aim every booked envelope at a new playback rate. + * + * Lanes are committed to absolute context times, so a param scheduled at 1× + * keeps its original wall-clock plan while the audio underneath runs at the + * new speed: a lowpass sweeping over 10 clip-seconds, switched to 2×, eats + * 20 s of material in 10 s of wall clock with the sweep unchanged. + */ + setRate(rate: number): void; +} + export function attachElementFxChain( ctx: BaseAudioContext, el: { getAttribute?(name: string): string | null }, source: AudioNode, destination: AudioNode, timing?: AutomationTiming, -): { dispose(): void } | null { +): ElementFxHandle | null { const { chain } = readChain(el); // Null means the source runs straight into its gain: an empty chain, or one @@ -166,20 +180,26 @@ export function attachElementFxChain( at && handle ? scheduleChainAutomation(readAutomation(el, next), next, handle.nodes, at) : []; }; + // The reference frame every later reschedule measures from. Mutable because a + // rate change rebases it: `elapsed` has to stop advancing at the old rate the + // instant the new one takes effect, or every subsequent edit re-aims the + // envelope at the wrong clip position. + let frame: AutomationTiming | null = timing ? { ...timing } : null; + attach(chain); - scheduleFor(chain, timing ?? null); + scheduleFor(chain, frame); /** * Re-aim the envelope at the live playhead. An edit lands mid-playback, so * the clip has advanced past the offset the source was scheduled with. */ const timingNow = (): AutomationTiming | null => { - if (!timing) return null; - const now = typeof ctx.currentTime === "number" ? ctx.currentTime : timing.scheduledAt; + if (!frame) return null; + const now = typeof ctx.currentTime === "number" ? ctx.currentTime : frame.scheduledAt; return { scheduledAt: now, - elapsed: timing.elapsed + (now - timing.scheduledAt) * timing.rate, - rate: timing.rate, + elapsed: frame.elapsed + (now - frame.scheduledAt) * frame.rate, + rate: frame.rate, }; }; @@ -239,6 +259,15 @@ export function attachElementFxChain( } return { + setRate: (rate: number) => { + const at = timingNow(); + if (disposed || !at || !Number.isFinite(rate) || rate <= 0 || rate === at.rate) return; + // Rebased at the playhead the OLD rate carried us to, then replayed from + // there at the new one. + frame = { ...at, rate }; + cancelParamLane(automated, at.scheduledAt); + scheduleFor(readChain(el).chain, frame); + }, dispose: () => { disposed = true; observer?.disconnect(); diff --git a/packages/core/src/runtime/webAudioTransport.test.ts b/packages/core/src/runtime/webAudioTransport.test.ts index f1597ccc8b..35f935bc5a 100644 --- a/packages/core/src/runtime/webAudioTransport.test.ts +++ b/packages/core/src/runtime/webAudioTransport.test.ts @@ -289,6 +289,23 @@ describe("WebAudioTransport", () => { expect(mock.sourceNode.playbackRate.value).toBe(2); }); + it("setRate re-aims each source's FX automation, not just its playback rate", async () => { + // The lanes are committed to absolute context times when the source is + // scheduled, so bumping playbackRate alone left every automated parameter + // running its original plan over audio moving at a different speed. + const { transport, mock, gen } = setupTransport(100); + await transport.schedulePlayback(mockEl, mockBuffer, 5, 0, 8, 1, gen, 1); + const active = (transport as unknown as { _activeSources: { fx?: unknown }[] }) + ._activeSources; + const setRate = vi.fn(); + active[0]!.fx = { dispose: vi.fn(), setRate }; + + transport.setRate(2); + + expect(setRate).toHaveBeenCalledWith(2); + expect(mock.sourceNode.playbackRate.value).toBe(2); + }); + it("setRate before any sources are scheduled does not throw", () => { const transport = new WebAudioTransport(); expect(() => transport.setRate(2)).not.toThrow(); diff --git a/packages/core/src/runtime/webAudioTransport.ts b/packages/core/src/runtime/webAudioTransport.ts index 2ce3ea4bba..d77aa7152a 100644 --- a/packages/core/src/runtime/webAudioTransport.ts +++ b/packages/core/src/runtime/webAudioTransport.ts @@ -1,4 +1,4 @@ -import { attachElementFxChain } from "./audioFx.js"; +import { attachElementFxChain, type ElementFxHandle } from "./audioFx.js"; import type { AutomationTiming } from "../audio/audioFxAutomation.js"; import { swallow } from "./diagnostics"; import { getDebugSurface } from "./globals.js"; @@ -63,7 +63,7 @@ export type ScheduledSource = { sourceNode: AudioBufferSourceNode; gainNode: GainNode; /** FX chain spliced between source and gain, when the element carries one. */ - fx?: { dispose(): void } | null; + fx?: ElementFxHandle | null; compositionStart: number; mediaStart: number; scheduledAt: number; @@ -274,6 +274,14 @@ export class WebAudioTransport { * `getTime()` stays continuous across the change. Sources scheduled to * start in the future keep their original wallclock start time — callers * that need rate-correct future starts should `stopAll()` and reschedule. + * + * Each source's FX automation is re-aimed too. Lanes are committed to + * absolute context times when the source is scheduled, so bumping only + * `playbackRate` left every automated parameter running its original plan + * over audio moving at a different speed. The `stopAll()`+reschedule recovery + * in the runtime is no help here: it only fires for bounded sources, and a + * project-level music bed with no `data-duration` is unbounded, so it never + * recovered at all. */ setRate(rate: number): boolean { const safeRate = normalizeRate(rate); @@ -286,6 +294,7 @@ export class WebAudioTransport { for (const source of this._activeSources) { try { source.sourceNode.playbackRate.value = safeRate; + source.fx?.setRate(safeRate); } catch (err) { swallow("webAudioTransport.setRate", err); }