Skip to content

Commit 88d4e1e

Browse files
vanceingallsclaude
andcommitted
fix(core): make the volume lane audible in preview
The envelope was scheduled onto the transport's gain AudioParam, but the runtime rewrites that gain every tick from `data-volume` and the GSAP-seeked value — so it was erased within a frame. Volume automation was correct in the render and inaudible while previewing. The lane now feeds the per-tick path where the probed volume keyframes already sit, checked ahead of them so the two cannot fight, and the transport no longer schedules volume at all: one mechanism instead of two racing. The cost is honest — in preview the level steps per tick rather than per sample, exactly as the existing keyframe path does. The render still bakes it into the PCM sample-accurately, and FX parameters are still scheduled on their own AudioParams, since nothing rewrites those. Parsed lanes are cached by attribute text: the runtime asks once per tick per track, and parsing there would run the JSON parser 60 times a second for a value that only changes on an edit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2f66e1b commit 88d4e1e

6 files changed

Lines changed: 233 additions & 24 deletions

File tree

.fallowrc.jsonc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,11 @@
655655
// optional fields, but the line-shift fingerprint re-flags the inherited
656656
// complexity.
657657
"packages/studio/src/utils/sourcePatcher.ts",
658+
// runtime/media.ts: refreshRuntimeMediaCache pre-dates this work and is
659+
// untouched by it — this stack only adds a volume-lane branch to
660+
// syncRuntimeMedia's author-volume resolution, but touching the file makes
661+
// fallow report the inherited function.
662+
"packages/core/src/runtime/media.ts",
658663
// timeline.ts: collectRuntimeTimelinePayload (CRITICAL) pre-dates this PR;
659664
// only an import line changed here (slideshow/sceneId → slideshow/index),
660665
// but the line-shift fingerprint makes fallow re-flag inherited complexity.
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { describe, expect, it } from "vitest";
2+
import { elementVolumeLaneGain } from "./audioAutomationVolume.js";
3+
4+
const el = (automation?: string) => ({
5+
getAttribute: (name: string) =>
6+
name === "data-automation" && automation !== undefined ? automation : null,
7+
});
8+
9+
const duck = JSON.stringify({
10+
version: 1,
11+
lanes: [
12+
{
13+
target: "volume",
14+
points: [
15+
{ t: 0, v: 0.55 },
16+
{ t: 1.4, v: 0.55 },
17+
{ t: 1.5, v: 0.15 },
18+
{ t: 9.5, v: 0.15 },
19+
{ t: 11, v: 0.55 },
20+
],
21+
},
22+
],
23+
});
24+
25+
describe("elementVolumeLaneGain", () => {
26+
it("returns the lane's gain at a clip-local time", () => {
27+
expect(elementVolumeLaneGain(el(duck), 0)).toBeCloseTo(0.55, 6);
28+
expect(elementVolumeLaneGain(el(duck), 1.4)).toBeCloseTo(0.55, 6);
29+
expect(elementVolumeLaneGain(el(duck), 5)).toBeCloseTo(0.15, 6);
30+
expect(elementVolumeLaneGain(el(duck), 11)).toBeCloseTo(0.55, 6);
31+
// Holds past the last point rather than falling to zero.
32+
expect(elementVolumeLaneGain(el(duck), 20)).toBeCloseTo(0.55, 6);
33+
});
34+
35+
it("ramps between points", () => {
36+
const mid = elementVolumeLaneGain(el(duck), 1.45) ?? 0;
37+
expect(mid).toBeLessThan(0.55);
38+
expect(mid).toBeGreaterThan(0.15);
39+
});
40+
41+
it("returns null with no automation, so the caller keeps its own rules", () => {
42+
expect(elementVolumeLaneGain(el(), 1)).toBeNull();
43+
expect(elementVolumeLaneGain(el(""), 1)).toBeNull();
44+
});
45+
46+
it("returns null for an element that carries only FX lanes", () => {
47+
const fxOnly = JSON.stringify({
48+
version: 1,
49+
lanes: [{ target: "fx.n1.frequency", points: [{ t: 0, v: 400 }] }],
50+
});
51+
expect(elementVolumeLaneGain(el(fxOnly), 0)).toBeNull();
52+
});
53+
54+
it("plays flat rather than silent when the attribute is unreadable", () => {
55+
expect(elementVolumeLaneGain(el("{not json"), 0)).toBeNull();
56+
});
57+
58+
it("tolerates an object with no getAttribute at all", () => {
59+
expect(elementVolumeLaneGain({}, 0)).toBeNull();
60+
});
61+
62+
it("does not reparse the same attribute on every call", () => {
63+
// The runtime asks once per tick per track; parsing there would run the JSON
64+
// parser 60 times a second for a value that only changes on an edit.
65+
let reads = 0;
66+
const counting = {
67+
getAttribute: (name: string) => {
68+
if (name !== "data-automation") return null;
69+
reads += 1;
70+
return duck;
71+
},
72+
};
73+
for (let i = 0; i < 50; i += 1) elementVolumeLaneGain(counting, i / 10);
74+
expect(reads).toBe(50);
75+
// Same object identity back each time proves the parse was cached.
76+
const a = elementVolumeLaneGain(el(duck), 1);
77+
const b = elementVolumeLaneGain(el(duck), 1);
78+
expect(a).toBe(b);
79+
});
80+
});
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/**
2+
* The volume lane's gain for an element at a moment in its clip.
3+
*
4+
* Volume is not scheduled onto the gain AudioParam the way FX parameters are.
5+
* The runtime rewrites that gain every tick from `data-volume` and the GSAP
6+
* seeked value, which would erase a scheduled envelope within a frame — so the
7+
* lane feeds the same per-tick path the probed `volumeKeyframes` already use,
8+
* and the render still bakes it into the samples.
9+
*/
10+
11+
import {
12+
HF_AUDIO_AUTOMATION_ATTR,
13+
parseAutomation,
14+
sampleAutomationLane,
15+
VOLUME_TARGET,
16+
type HfAutomationLane,
17+
} from "../audioAutomation.js";
18+
19+
/**
20+
* Parsed lanes by the attribute text they came from. Reparsing on every tick
21+
* would run the JSON parser 60 times a second per track for a value that only
22+
* changes when the author edits it.
23+
*/
24+
const cache = new Map<string, HfAutomationLane | null>();
25+
const CACHE_LIMIT = 64;
26+
27+
function laneFromAttr(raw: string): HfAutomationLane | null {
28+
const hit = cache.get(raw);
29+
if (hit !== undefined) return hit;
30+
let lane: HfAutomationLane | null = null;
31+
try {
32+
lane = parseAutomation(raw).lanes.find((l) => l.target === VOLUME_TARGET) ?? null;
33+
} catch {
34+
// Unreadable automation plays the track flat rather than silencing it.
35+
lane = null;
36+
}
37+
if (cache.size > CACHE_LIMIT) cache.clear();
38+
cache.set(raw, lane);
39+
return lane;
40+
}
41+
42+
/**
43+
* Gain the lane asks for at `relTime` clip-local seconds, or null when the
44+
* element has no volume lane and the caller should fall back to its own rules.
45+
*/
46+
export function elementVolumeLaneGain(
47+
el: { getAttribute?(name: string): string | null },
48+
relTime: number,
49+
): number | null {
50+
const raw =
51+
(typeof el.getAttribute === "function" ? el.getAttribute(HF_AUDIO_AUTOMATION_ATTR) : null) ??
52+
"";
53+
if (!raw) return null;
54+
const lane = laneFromAttr(raw);
55+
if (!lane || lane.points.length === 0) return null;
56+
return sampleAutomationLane(lane, relTime);
57+
}

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,87 @@ describe("syncRuntimeMedia", () => {
307307
document.body.innerHTML = "";
308308
});
309309

310+
describe("volume automation lane", () => {
311+
const DUCK = JSON.stringify({
312+
version: 1,
313+
lanes: [
314+
{
315+
target: "volume",
316+
points: [
317+
{ t: 0, v: 0.8 },
318+
{ t: 2, v: 0.8 },
319+
{ t: 3, v: 0.1 },
320+
{ t: 8, v: 0.1 },
321+
],
322+
},
323+
],
324+
});
325+
326+
/**
327+
* The runtime rewrites the transport's gain every tick. Before the lane fed
328+
* this path it was scheduled onto the AudioParam instead and erased within a
329+
* frame, so the envelope was inaudible in preview while being correct in the
330+
* render.
331+
*/
332+
function volumesAt(times: number[], automation?: string, volume = 0.55) {
333+
const clip = createMockClip({ start: 0, end: 10, volume });
334+
Object.defineProperty(clip.el, "readyState", { value: 4, writable: true });
335+
if (automation) clip.el.setAttribute("data-automation", automation);
336+
const seen: number[] = [];
337+
for (const t of times) {
338+
syncRuntimeMedia({
339+
clips: [clip],
340+
timeSeconds: t,
341+
playing: true,
342+
playbackRate: 1,
343+
onElementVolume: (_el, v) => seen.push(v),
344+
});
345+
}
346+
return seen;
347+
}
348+
349+
it("drives the transport gain from the lane, not from data-volume", () => {
350+
const [held, ducked] = volumesAt([1, 5], DUCK);
351+
expect(held).toBeCloseTo(0.8, 5);
352+
expect(ducked).toBeCloseTo(0.1, 5);
353+
});
354+
355+
it("ramps between points across ticks", () => {
356+
const [a, b, c] = volumesAt([2, 2.5, 3], DUCK);
357+
expect(a).toBeCloseTo(0.8, 5);
358+
expect(b).toBeGreaterThan(0.1);
359+
expect(b).toBeLessThan(0.8);
360+
expect(c).toBeCloseTo(0.1, 5);
361+
});
362+
363+
it("falls back to data-volume when there is no lane", () => {
364+
const [only] = volumesAt([5], undefined, 0.55);
365+
expect(only).toBeCloseTo(0.55, 5);
366+
});
367+
368+
it("supersedes keyframes probed from the timeline", () => {
369+
// Both present: the lane is the explicit one, and `lint` warns about it.
370+
const clip = createMockClip({ start: 0, end: 10, volume: 0.55 });
371+
Object.defineProperty(clip.el, "readyState", { value: 4, writable: true });
372+
clip.el.setAttribute("data-automation", DUCK);
373+
clip.volumeKeyframes = [
374+
{ time: 0, volume: 1 },
375+
{ time: 10, volume: 1 },
376+
];
377+
let seen = -1;
378+
syncRuntimeMedia({
379+
clips: [clip],
380+
timeSeconds: 5,
381+
playing: true,
382+
playbackRate: 1,
383+
onElementVolume: (_el, v) => {
384+
seen = v;
385+
},
386+
});
387+
expect(seen).toBeCloseTo(0.1, 5);
388+
});
389+
});
390+
310391
it("plays active clip when playing and buffered", () => {
311392
const clip = createMockClip({ start: 0, end: 10 });
312393
Object.defineProperty(clip.el, "readyState", { value: 4, writable: true });

packages/core/src/runtime/media.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { swallow } from "./diagnostics";
22
import { interpolateVolumeGain, type VolumeKeyframe } from "./mediaVolumeEnvelope.js";
3+
import { elementVolumeLaneGain } from "./audioAutomationVolume.js";
34
import { normalizePlaybackRate } from "./playbackRate.js";
45

56
export function readElementPlaybackRate(el: Element): number {
@@ -270,7 +271,13 @@ export function syncRuntimeMedia(params: {
270271
const currentElementVolume = clampVolume(el.volume);
271272

272273
let authorVolume: number;
273-
if (clip.volumeKeyframes && clip.volumeKeyframes.length > 0) {
274+
// An explicit volume lane owns the fader. It is checked before the probed
275+
// keyframes because the two would otherwise fight, and it is the one the
276+
// author drew — `lint` warns when a track carries both.
277+
const laneGain = elementVolumeLaneGain(el, relTime);
278+
if (laneGain !== null) {
279+
authorVolume = clampVolume(laneGain);
280+
} else if (clip.volumeKeyframes && clip.volumeKeyframes.length > 0) {
274281
// Keyframes probed from the GSAP timeline — same source as the renderer.
275282
// Use the interpolated envelope value directly; no need to track GSAP changes.
276283
authorVolume = clampVolume(interpolateVolumeGain(clip.volumeKeyframes, relTime));

packages/core/src/runtime/webAudioTransport.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
1-
import { attachElementFxChain, readElementAutomation } from "./audioFx.js";
2-
import {
3-
scheduleParamLane,
4-
volumeLane,
5-
type AutomationTiming,
6-
} from "../audio/audioFxAutomation.js";
7-
import { VOLUME_RANGE } from "../audioAutomation.js";
1+
import { attachElementFxChain } from "./audioFx.js";
2+
import type { AutomationTiming } from "../audio/audioFxAutomation.js";
83
import { swallow } from "./diagnostics";
94
import { getDebugSurface } from "./globals.js";
105

@@ -63,20 +58,6 @@ function startBoundedSource(
6358
return true;
6459
}
6560

66-
/**
67-
* The volume lane rides the fader, after the effects — where a DAW puts it,
68-
* and the order the render bakes it in.
69-
*/
70-
function scheduleVolumeLane(
71-
el: HTMLMediaElement,
72-
gainNode: GainNode,
73-
timing: AutomationTiming,
74-
): void {
75-
const lane = volumeLane(readElementAutomation(el));
76-
if (!lane) return;
77-
scheduleParamLane([{ param: gainNode.gain }], lane, VOLUME_RANGE.scale, timing);
78-
}
79-
8061
export type ScheduledSource = {
8162
el: HTMLMediaElement;
8263
sourceNode: AudioBufferSourceNode;
@@ -219,8 +200,6 @@ export class WebAudioTransport {
219200
const fx = attachElementFxChain(this._ctx, el, sourceNode, gainNode, timing);
220201
gainNode.connect(this._masterGain);
221202

222-
scheduleVolumeLane(el, gainNode, timing);
223-
224203
this._rate = safeRate;
225204
this._rateAnchorCtx = scheduledAt;
226205
this._rateAnchorComp = compositionTime;

0 commit comments

Comments
 (0)