From 4d43a89b318f1a6cc4d94867ef2f4c9d80b6c472 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Wed, 19 Aug 2026 00:58:56 -0400 Subject: [PATCH] fix(core): keep authored gain above unity off el.volume in the sandbox bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raising the authoring ceiling to 12 dB made `data-volume` legal up to ~3.98, but the sandbox runtime's volume bridge assigned `clipVolume * volume` straight to `el.volume`, which the spec pins to [0,1] and which THROWS IndexSizeError outside it. Verified in Chrome and reproduced in the suite: the throw aborts the loop, so every media element after the boosted one keeps the volume it already had while the bridge's own state says otherwise. The element carries the legal part. The boost above unity belongs to Web Audio, which already receives it — the two tests here pin that half, since it is the reason clamping the element is safe rather than lossy: `syncRuntimeMedia` hands the transport the authored gain alongside the element's clamped one, and the per-element gain node keeps it. The fix was written when the ceiling was, and got stranded in a PR whose other work landed in pieces around it. --- packages/core/src/runtime/init.test.ts | 31 +++++++++++++++++++ packages/core/src/runtime/init.ts | 7 ++++- packages/core/src/runtime/media.test.ts | 21 +++++++++++++ .../src/runtime/webAudioTransport.test.ts | 9 ++++++ 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/packages/core/src/runtime/init.test.ts b/packages/core/src/runtime/init.test.ts index ecccece1b0..d6c37f2be0 100644 --- a/packages/core/src/runtime/init.test.ts +++ b/packages/core/src/runtime/init.test.ts @@ -140,6 +140,37 @@ describe("initSandboxRuntimeModular", () => { }, ); + it("keeps a boosted clip legal on the element when the bridge sets volume", () => { + // `data-volume` may hold up to 12 dB of authored gain. `el.volume` is + // spec-pinned to [0,1] and THROWS outside it, so assigning the product raw + // aborted the loop — every element after the boosted one kept its old + // volume, and the bridge's own state said otherwise. + document.body.innerHTML = + `
` + + `` + + `` + + `
`; + window.__timelines = {}; + initSandboxRuntimeModular(); + + const [boosted, quiet] = Array.from(document.querySelectorAll("audio")); + if (!boosted || !quiet) throw new Error("expected both clips"); + // Sentinels, so the assertions cannot be satisfied by what the runtime + // already applied while starting up. + boosted.volume = 0.2; + quiet.volume = 0.1; + + window.dispatchEvent( + new MessageEvent("message", { + data: { source: "hf-parent", type: "control", action: "set-volume", volume: 1 }, + }), + ); + + expect(boosted.volume).toBe(1); + // The clip after the boosted one is what a throw mid-loop strands. + expect(quiet.volume).toBeCloseTo(0.5, 5); + }); + afterEach(() => { window.__hfRuntimeTeardown?.(); document.body.innerHTML = ""; diff --git a/packages/core/src/runtime/init.ts b/packages/core/src/runtime/init.ts index f9260e0c92..c5c291e9fb 100644 --- a/packages/core/src/runtime/init.ts +++ b/packages/core/src/runtime/init.ts @@ -3159,7 +3159,12 @@ export function initSandboxRuntimeModular(): void { if (!(el instanceof HTMLMediaElement)) continue; const parsed = parseFloat(el.dataset.volume ?? ""); const clipVolume = Number.isFinite(parsed) ? parsed : 1; - el.volume = clipVolume * volume; + // `data-volume` carries authored gain, which goes above unity now that + // the ceiling is 12 dB — and `el.volume` is spec-pinned to [0,1], so + // assigning the product raw THROWS IndexSizeError and takes the rest of + // the loop with it. The element carries the legal part; the boost above + // unity belongs to Web Audio, which already has it from `setVolume`. + el.volume = Math.max(0, Math.min(1, clipVolume * volume)); } }, onSetMediaOutputMuted: (muted) => { diff --git a/packages/core/src/runtime/media.test.ts b/packages/core/src/runtime/media.test.ts index 3d10ffe31b..bc348f0eae 100644 --- a/packages/core/src/runtime/media.test.ts +++ b/packages/core/src/runtime/media.test.ts @@ -436,6 +436,27 @@ describe("syncRuntimeMedia", () => { expect(only).toBeCloseTo(0.55, 5); }); + it("sends boosted author gain to Web Audio while keeping the native element legal", () => { + const clip = createMockClip({ start: 0, end: 10, volume: 3.98 }); + Object.defineProperty(clip.el, "readyState", { value: 4, writable: true }); + let transportGain = -1; + + syncRuntimeMedia({ + clips: [clip], + timeSeconds: 1, + playing: true, + playbackRate: 1, + // Third arg is the authored gain, which is the one the transport wants; + // the second is the element's, which the spec pins to [0,1]. + onElementVolume: (_el, _effectiveVolume, authorVolume) => { + transportGain = authorVolume; + }, + }); + + expect(transportGain).toBeCloseTo(3.98, 5); + expect(clip.el.volume).toBe(1); + }); + /** * 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 diff --git a/packages/core/src/runtime/webAudioTransport.test.ts b/packages/core/src/runtime/webAudioTransport.test.ts index 80f3d411bf..92058e1ef3 100644 --- a/packages/core/src/runtime/webAudioTransport.test.ts +++ b/packages/core/src/runtime/webAudioTransport.test.ts @@ -306,6 +306,15 @@ describe("WebAudioTransport", () => { }); describe("schedulePlayback timing", () => { + it("keeps author boost above unity on the per-element gain node", async () => { + const { transport, mock, gen } = setupTransport(100); + + await transport.schedulePlayback(mockEl, mockBuffer, 0, 0, 0, 1, gen); + transport.setElementVolume(mockEl, 3.98); + + expect(mock.gainNode.gain.value).toBeCloseTo(3.98, 5); + }); + it("starts in-progress clips immediately with correct buffer offset", async () => { const { transport, mock, gen } = setupTransport(100);