fix(core): keep authored gain above unity off el.volume in the sandbox bridge - #3349
Open
miguel-heygen wants to merge 1 commit into
Open
fix(core): keep authored gain above unity off el.volume in the sandbox bridge#3349miguel-heygen wants to merge 1 commit into
miguel-heygen wants to merge 1 commit into
Conversation
…x bridge 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Authoring a clip above unity gain throws at runtime today.
What breaks
MAX_AUDIO_GAIN_DB = 12makesdata-volumelegal up to ~3.98. The sandbox runtime's volume bridge assigns the product straight to the element:HTMLMediaElement.volumeis spec-pinned to [0,1] and throwsIndexSizeErroroutside it — verified in Chrome, and the test DOM agrees:The throw lands inside a
forloop 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, whilestate.bridgeVolumesays 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:
syncRuntimeMediahandsonElementVolumeboth the element's clamped volume and the authored gain, so the transport can have the boost the element cannot hold.setElementVolumekeeps that gain on the per-element node, clamped only toMAX_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 realset-volumecontrol 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
mainand 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 onmainnow and shouldn't wait behind a PR that is otherwise redundant.