fix(audio): eleven small correctness fixes across engine, core and the panel

packages/core/package.json

@@ -549,7 +549,7 @@
   },
   "scripts": {
     "build": "bun run build:hyperframes-runtime && bun run build:position-edits-render && bun run build:audio-fx-runtime && tsc && tsx scripts/rewrite-esm-extensions.ts",
-    "test": "bun run check:position-edits-render && vitest run",
+    "test": "bun run check:position-edits-render && bun run build:audio-fx-runtime && vitest run",
     "test:watch": "vitest",
     "test:coverage": "vitest run --coverage",
     "test:runtime-coverage": "vitest run --coverage src/runtime",

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

@@ -438,3 +438,54 @@ describe("automatable parameters", () => {
     expect(onePole.automation?.frequency).toBeUndefined();
   });
 });
+
+describe("phaser automation targets", () => {
+  /**
+   * `in_gain` and `out_gain` trim the signal entering and leaving the effect,
+   * which the builder drives through inTrim/outTrim while pinning wet and dry
+   * to 1. The automation map used to aim both lanes at wet/dry — so an envelope
+   * modulated a constant, the trim it was supposed to move stayed frozen, and
+   * "fade the phaser out" left the dry leg playing at full level.
+   *
+   * Asserted by VALUE rather than by node identity: the trims are internal, and
+   * the only honest question is whether the param a lane would drive is the one
+   * the knob sets.
+   */
+  it("drives the trims a lane is named for, not the pinned wet/dry pair", () => {
+    const handle = buildFxNode(ctx() as unknown as BaseAudioContext, "phaser", {
+      ...defaultAudioFxParams("phaser"),
+      in_gain: 0.25,
+      out_gain: 0.5,
+    });
+    expect(handle.automation?.in_gain?.[0]?.param.value).toBeCloseTo(0.25, 6);
+    expect(handle.automation?.out_gain?.[0]?.param.value).toBeCloseTo(0.5, 6);
+  });
+});
+
+describe("chain update keeps ids with their effects", () => {
+  const band = (id: string, frequency: number) => ({
+    type: "peaking",
+    id,
+    enabled: true,
+    params: { ...defaultAudioFxParams("peaking"), frequency },
+  });
+
+  /**
+   * Reordering two effects of the same type leaves the shape string identical,
+   * so the chain updates in place rather than rebuilding — correct for the
+   * audio, since the params move with the position. The ids have to move too:
+   * a lane addresses its effect by id, and an id captured at build time names
+   * whichever effect used to occupy that slot. The scheduler would then drive
+   * `fx.n2.frequency` into the band that is now n1 — the exact swap that
+   * HfAudioFxNode.id documents itself as preventing, and the one the voiceover
+   * carve's all-peaking chains make easy to hit.
+   */
+  it("moves an id with its slot when same-type effects are reordered", () => {
+    const chain: HfAudioFxChain = { version: 1, nodes: [band("n1", 200), band("n2", 4000)] };
+    const handle = buildFxChain(ctx() as unknown as BaseAudioContext, chain);
+
+    const swapped: HfAudioFxChain = { version: 1, nodes: [band("n2", 4000), band("n1", 200)] };
+    expect(handle.update(swapped)).toBe(true);
+    expect(handle.nodes.map((n) => n.id)).toEqual(["n2", "n1"]);
+  });
+});

packages/core/src/audio/audioFxGraph.ts

@@ -360,8 +360,13 @@ const allpassPhaser: Builder = (ctx, p) => {
     // frequency at once — not one knob, one param — so they stay unautomated.
     automation: {
       speed: [{ param: lfo.frequency }],
-      in_gain: [{ param: dry.gain }],
-      out_gain: [{ param: wet.gain }],
+      // The trims, not wet/dry. apply() drives inTrim/outTrim from these knobs
+      // and pins wet and dry to 1 — so a lane aimed at wet/dry modulated a
+      // constant and left the trim frozen, and the next values-only edit slammed
+      // it back over the running envelope. The comment above records that this
+      // wiring was already moved once; the automation map was missed.
+      in_gain: [{ param: inTrim.gain }],
+      out_gain: [{ param: outTrim.gain }],
     },
     dispose: () => {
       try {
@@ -510,7 +515,17 @@ export function buildFxChain(ctx: BaseAudioContext, chain: HfAudioFxChain): FxCh
       if (shapeOf(next) !== shape) return false;
       const active = next.nodes.filter((node) => node.enabled !== false);
       active.forEach((node, i) => {
-        handles[i]?.handle.update(normalizeAudioFxParams(node.type, node.params));
+        const held = handles[i];
+        if (!held) return;
+        held.handle.update(normalizeAudioFxParams(node.type, node.params));
+        // The id follows the position, because the params just did. Reordering
+        // two effects of the same type leaves the shape identical, so the graph
+        // is updated in place — but a lane addresses its effect BY id, and an id
+        // captured at build time then names whichever effect used to be here.
+        // The scheduler would drive `fx.n2.frequency` into the band that is now
+        // n1: exactly what HfAudioFxNode.id documents itself as preventing.
+        if (node.id === undefined) delete held.id;
+        else held.id = node.id;
       });
       shape = shapeOf(next);
       return true;

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

@@ -365,6 +365,37 @@ describe("syncRuntimeMedia", () => {
       expect(only).toBeCloseTo(0.55, 5);
     });
 
+    /**
+     * The render bakes the lane at CLIP-LOCAL time: prepareAudioTrack already
+     * cut the wav with `-ss mediaStart`, so its t=0 is the clip's start, and
+     * normaliseEnvelope subtracts trackStart. Preview used to sample at MEDIA
+     * time — mediaStart included, scaled by playbackRate, wrapped on a loop — so
+     * the same envelope played somewhere else than it rendered.
+     */
+    it("samples the lane at clip-local time, the way the render bakes it", () => {
+      const trimmed = (t: number) => {
+        const clip = createMockClip({ start: 0, end: 10, volume: 0.55, mediaStart: 30 });
+        Object.defineProperty(clip.el, "readyState", { value: 4, writable: true });
+        clip.el.setAttribute("data-automation", DUCK);
+        let seen = -1;
+        syncRuntimeMedia({
+          clips: [clip],
+          timeSeconds: t,
+          playing: true,
+          playbackRate: 1,
+          onElementVolume: (_el, v) => {
+            seen = v;
+          },
+        });
+        return seen;
+      };
+      // `data-media-start="30"` on a clip whose lane holds 0.8 until t=2 then
+      // ducks to 0.1 by t=3. At media time the playhead is already 30 s past the
+      // last point, so preview held 0.1 from the first frame and never ducked.
+      expect(trimmed(1)).toBeCloseTo(0.8, 5);
+      expect(trimmed(5)).toBeCloseTo(0.1, 5);
+    });
+
     it("supersedes keyframes probed from the timeline", () => {
       // Both present: the lane is the explicit one, and `lint` warns about it.
       const clip = createMockClip({ start: 0, end: 10, volume: 0.55 });

packages/core/src/runtime/media.ts

@@ -274,7 +274,15 @@ export function syncRuntimeMedia(params: {
       // An explicit volume lane owns the fader. It is checked before the probed
       // keyframes because the two would otherwise fight, and it is the one the
       // author drew — `lint` warns when a track carries both.
-      const laneGain = elementVolumeLaneGain(el, relTime);
+      // Clip-local, NOT `relTime`. A lane's `t` is "seconds from the start of
+      // the clip" (see HfAutomationPoint), and the render honours that: the wav
+      // is already cut with `-ss mediaStart`, so its t=0 IS the clip's start.
+      // `relTime` is MEDIA time — it carries mediaStart, scales by playbackRate
+      // and wraps on a loop — so feeding it here played the envelope at a
+      // different position than it renders, or ran it off the end entirely on a
+      // trimmed clip. The FX lanes on this same feature use clip-local elapsed;
+      // there is one time base, and this is it.
+      const laneGain = elementVolumeLaneGain(el, params.timeSeconds - clip.start);
       if (laneGain !== null) {
         authorVolume = clampVolume(laneGain);
       } else if (clip.volumeKeyframes && clip.volumeKeyframes.length > 0) {

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

@@ -388,6 +388,19 @@ describe("WebAudioTransport", () => {
       expect(transport.isActive()).toBe(false);
     });
 
+    it("disposes the FX graph when a clip ends naturally", async () => {
+      // stopAll() disposes by walking _activeSources, and the splice above had
+      // already removed this entry — so the handle, its MutationObserver and any
+      // running LFO survived the clip for the rest of the session.
+      const { transport, mock, gen } = setupTransport(100);
+      await transport.schedulePlayback(mockEl, mockBuffer, 0, 0, 0, 1, gen);
+
+      mock.sourceNode._fireEnded();
+
+      expect(mock.sourceNode.disconnect).toHaveBeenCalled();
+      expect(mock.gainNode.disconnect).toHaveBeenCalled();
+    });
+
     it("registers onended listener on the sourceNode", async () => {
       const { transport, mock, gen } = setupTransport(100);
 

packages/core/src/runtime/webAudioTransport.ts

@@ -243,6 +243,21 @@ export class WebAudioTransport {
         if (idx !== -1) {
           this._activeSources.splice(idx, 1);
           el.muted = priorMuted;
+          // The graph goes with it. Splicing alone left the FX handle alive and
+          // then UNREACHABLE — stopAll() disposes by walking this array, which
+          // the splice just emptied of this entry. Every clip that finished
+          // naturally leaked its MutationObserver for the session, and each one
+          // still answered later `data-fx-chain` edits by rebuilding a whole
+          // graph (impulse response, chorus/phaser oscillators started and never
+          // stopped) around a dead source. Not disposed when idx is -1: stopAll()
+          // has already done it, and `stop()` is what fired this event.
+          try {
+            sourceNode.disconnect();
+            fx?.dispose();
+            gainNode.disconnect();
+          } catch {
+            // Already torn down.
+          }
           if (this._activeSources.length === 0) this._paused = true;
         }
       });

packages/core/stubs/audio-fx-runtime-entry.ts

@@ -81,6 +81,11 @@ async function render(
   const chain: HfAudioFxChain = parseAudioFxChain(chainJson);
   const channels = Math.max(1, planes.length);
   const frames = planes[0]?.length ?? 0;
+  // Nothing to process, and `new OfflineAudioContext(ch, 0, rate)` throws — an
+  // error the render treats as fatal. applyAudioFxChain screens empty tracks
+  // out before they reach the browser; this is the same guard at the point the
+  // constructor would actually blow up.
+  if (frames === 0) return planes;
   const parsedAutomation = automationJson
     ? resolveAutomation(parseAutomation(automationJson), chain)
     : null;

packages/engine/src/services/audioFxRender.test.ts

@@ -253,3 +253,25 @@ describe("browser render", () => {
     expect(readWav(outPath).samples.length).toBeGreaterThan(0);
   }, 180_000);
 });
+
+/**
+ * ffmpeg exits 0 and writes a structurally valid but EMPTY wav whenever a
+ * clip's trim starts past the end of its source (`-ss 10 -t 5` on a 2 s file).
+ * That track used to reach `new OfflineAudioContext(ch, 0, rate)`, which throws
+ * — and the error travels past the mixer's per-track failure collector, so one
+ * mis-set `data-media-start` took the whole render down.
+ */
+describe("an empty track", () => {
+  it("is handed back untouched rather than failing the render", async () => {
+    const input = join(dir, "empty.wav");
+    writeWav(input, new Float32Array(0), SR);
+    const output = join(dir, "out.wav");
+
+    // Returns the input path, the same contract as a chain with nothing enabled
+    // — and without paying for a browser to decide it.
+    await expect(
+      applyAudioFxChain(input, chainOf("peaking"), output, { trackId: "t" }),
+    ).resolves.toBe(input);
+    expect(existsSync(output)).toBe(false);
+  });
+});

packages/engine/src/services/audioFxRender.ts

@@ -194,18 +194,30 @@ export async function applyAudioFxChain(
 
   const { samples, sampleRate, channels } = readWav(inputWav);
   const planes = deinterleave(samples, channels);
+  // An empty track has nothing to process — and an OfflineAudioContext of zero
+  // length throws, which is fatal for the WHOLE render rather than this track:
+  // the error travels past the mixer's per-track failure collector. ffmpeg
+  // writes an empty but structurally valid WAV whenever a clip's trim starts
+  // past the end of its source, so one mis-set `data-media-start` used to take
+  // the render down. Guarded here as well as in the runtime so an empty track
+  // never costs a browser.
+  if ((planes[0]?.length ?? 0) === 0) return inputWav;
 
-  // Audio processing needs no GPU or special capture mode; a plain sandboxed
-  // browser is enough, and the lease pool reuses one across tracks.
-  const lease = await acquireBrowser([
-    "--no-sandbox",
-    "--autoplay-policy=no-user-gesture-required",
-  ]);
+  // Both resources are taken INSIDE the try that releases them. The lease used
+  // to be acquired above it, with the mkdtemp between — so a failure there
+  // (a full disk, a read-only tmpdir) leaked a pooled browser, and a pool with
+  // no leases left hangs every later render rather than failing one.
   const hostDir = mkdtempSync(join(tmpdir(), "hf-fx-host-"));
+  let lease: Awaited<ReturnType<typeof acquireBrowser>> | null = null;
   try {
+    // Audio processing needs no GPU or special capture mode; a plain sandboxed
+    // browser is enough, and the lease pool reuses one across tracks.
+    // Checked before the lease, not after: an already-cancelled track has no
+    // reason to take a browser out of the pool just to hand it straight back.
     if (options.signal?.aborted) {
       throw new AudioFxRenderError(`Audio FX cancelled for track ${options.trackId}`);
     }
+    lease = await acquireBrowser(["--no-sandbox", "--autoplay-policy=no-user-gesture-required"]);
     const page = await lease.browser.newPage();
     try {
       // AudioWorklet is only exposed in a secure context, and about:blank is
@@ -291,7 +303,7 @@ export async function applyAudioFxChain(
     );
   } finally {
     rmSync(hostDir, { recursive: true, force: true });
-    await lease.release().catch(() => undefined);
+    await lease?.release().catch(() => undefined);
   }
 }
 

packages/engine/src/services/audioMixer.ts

@@ -765,6 +765,12 @@ export async function processCompositionAudio(
   // be able to abort the in-flight ffmpeg runs before the finally-block removes
   // workDir out from under them. Chained off the caller's signal so external
   // cancellation still behaves as before.
+  //
+  // Every child that can outlive a sibling's failure has to be given THIS
+  // signal, not the caller's: the trim, the video extract and the download all
+  // took `signal`, so `internalController.abort()` cancelled nothing and the
+  // `rmSync(workDir)` on the next line ran while their ffmpeg children were
+  // still writing into it.
   const internalController = new AbortController();
   const effectiveSignal = internalController.signal;
   if (signal) {
@@ -795,7 +801,7 @@ export async function processCompositionAudio(
 
         if (isHttpUrl(srcPath)) {
           try {
-            srcPath = await downloadToTemp(srcPath, workDir);
+            srcPath = await downloadToTemp(srcPath, workDir, undefined, effectiveSignal);
           } catch (err: unknown) {
             failures.push(
               downloadFailure(err instanceof Error ? err.message : String(err), element.id),
@@ -842,7 +848,7 @@ export async function processCompositionAudio(
               startTime: element.mediaStart,
               duration: element.end - element.start,
             },
-            signal,
+            effectiveSignal,
             config,
           );
           if (!extractResult.success) {
@@ -868,7 +874,7 @@ export async function processCompositionAudio(
             trimmedPath,
             element.mediaStart,
             element.end - element.start,
-            signal,
+            effectiveSignal,
             config,
           );
           if (!prepResult.success) {

packages/lint/src/rules/media.test.ts

@@ -457,6 +457,27 @@ describe("audio_volume_double_automation", () => {
     }
   });
 
+  it("does not blame the wrong element in a chained timeline", async () => {
+    // A chain has no semicolon until its very end, so a run that could cross `)`
+    // reached the `volume` in a LATER call and reported the element from an
+    // earlier one. Acting on the fixHint would have deleted #bgm's only real
+    // automation to fix a tween that is on #vo.
+    const res = await lintHyperframeHtml(
+      withScript(
+        LANE,
+        `gsap.timeline().to("#bgm", { duration: 0.6, x: 10 }).to("#vo", { volume: 1 });`,
+      ),
+    );
+    expect(res.findings.some((f) => f.code === "audio_volume_double_automation")).toBe(false);
+  });
+
+  it("still catches a real tween further down the same call", async () => {
+    const res = await lintHyperframeHtml(
+      withScript(LANE, `gsap.timeline().to("#bgm", { duration: 0.6, ease: "none", volume: 0 });`),
+    );
+    expect(res.findings.some((f) => f.code === "audio_volume_double_automation")).toBe(true);
+  });
+
   it("ignores a lane that automates something other than volume", async () => {
     const res = await lintHyperframeHtml(
       withScript(

packages/lint/src/rules/media.ts

@@ -625,7 +625,14 @@ function findVolumeDoubleAutomationFindings(ctx: LintContext): HyperframeLintFin
     // the same call the runtime's own probe would pick up, and the rule only
     // warns, so a miss costs nothing.
     const escaped = escapeRegExp(id);
-    const tweened = new RegExp(`#${escaped}(?![\\w-])[^;]{0,200}?\\bvolume\\s*:`, "s").test(script);
+    // `[^;)]`, not `[^;]`: a chained timeline has no semicolon until the end of
+    // the whole chain, so a run that could cross `)` matched `volume` in a LATER
+    // `.to()` call and named the wrong element — and this rule's fixHint tells
+    // the author to delete their lane. Refusing to cross the closing paren keeps
+    // the match inside the call the selector belongs to. It costs a false
+    // negative when some other value in the same object is a call result, which
+    // is the safe direction for a warning that already only guesses.
+    const tweened = new RegExp(`#${escaped}(?![\\w-])[^;)]{0,200}?\\bvolume\\s*:`).test(script);
     if (!tweened) continue;
     findings.push({
       code: "audio_volume_double_automation",