mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 23:03:09 +00:00
fix(core): keep authored gain above unity off el.volume in the sandbox bridge (#3349)
Authoring a clip above unity gain throws at runtime today. ## What breaks `MAX_AUDIO_GAIN_DB = 12` makes `data-volume` legal up to ~3.98. The sandbox runtime's volume bridge assigns the product straight to the element: ```ts el.volume = clipVolume * volume; // init.ts, onSetVolume ``` `HTMLMediaElement.volume` is spec-pinned to [0,1] and **throws `IndexSizeError`** outside it — verified in Chrome, and the test DOM agrees: ``` el.volume = 2 → IndexSizeError: Failed to set the 'volume' property... ``` The throw lands inside a `for` loop over every media element, so it takes the rest of the loop with it: every clip after the boosted one keeps whatever volume it already had, while `state.bridgeVolume` says the change was applied. A composition with one boosted clip stops responding to the volume control for every clip authored after it. ## The fix Clamp what the element receives. That is not lossy, because the element was never where the boost lived — the transport gets the authored gain unclamped, and this PR pins that half too: - `syncRuntimeMedia` hands `onElementVolume` both the element's clamped volume **and** the authored gain, so the transport can have the boost the element cannot hold. - `setElementVolume` keeps that gain on the per-element node, clamped only to `MAX_AUDIO_GAIN`. Those two paths already worked; they were untested, and they are the reason clamping the element is the right half to clamp. ## Tests - `init.test.ts` — a boosted clip followed by a quieter one, both seeded with sentinels, then the real `set-volume` control message. Asserts the boosted element lands at 1 **and** that the clip after it still gets its own volume, which is what a throw mid-loop strands. - `media.test.ts` — the transport receives the authored gain while the element stays legal. - `webAudioTransport.test.ts` — the per-element gain node keeps a boost above unity. All three mutation-checked: removing the clamp reds the first, and clamping the gain at either transport seam reds the others. ## Provenance This is the last unlanded piece of #3280. That PR was rebased onto current `main` and collapsed from +3050 to +944, of which everything except these lines is either already merged (#3308, #3309, #3333, #3339) or duplicated by the open #3306 and #3310. Cutting it out separately because the throw is live on `main` now and shouldn't wait behind a PR that is otherwise redundant.
This commit is contained in:
@@ -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 =
|
||||
`<div data-composition-id="main" data-root="true">` +
|
||||
`<audio data-start="0" data-volume="3.98"></audio>` +
|
||||
`<audio data-start="0" data-volume="0.5"></audio>` +
|
||||
`</div>`;
|
||||
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 = "";
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user