[AUDIO_WORKLET] Added support for MEMORY64 and 2GB heap (including tests) #23508
There are no files selected for viewing
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,14 +29,47 @@ function createWasmAudioWorkletProcessor(audioParams) { | |
|
|
||
| // Capture the Wasm function callback to invoke. | ||
| let opts = args.processorOptions; | ||
| this.callbackFunction = Module['wasmTable'].get({{{ toIndexType("opts['cb']") }}}); | ||
| this.userData = {{{ toIndexType("opts['ud']") }}}; | ||
|
|
||
| // Then the samples per channel to process, fixed for the lifetime of the | ||
| // context that created this processor. Even though this 'render quantum | ||
| // size' is fixed at 128 samples in the 1.0 spec, it will be variable in | ||
| // the 1.1 spec. It's passed in now, just to prove it's settable, but will | ||
| // eventually be a property of the AudioWorkletGlobalScope (globalThis). | ||
| this.samplesPerChannel = opts['sc']; | ||
| this.bytesPerChannel = this.samplesPerChannel * {{{ getNativeTypeSize('float') }}}; | ||
|
|
||
| // Create up-front as many typed views for marshalling the output data as | ||
| // may be required (with an arbitrary maximum of 10, for the case where a | ||
| // multi-MB stack is passed), allocated at the *top* of the worklet's | ||
| // stack (and whose addresses are fixed). The 'minimum alloc' firstly | ||
| // stops STACK_OVERFLOW_CHECK failing (since the stack will be full, and | ||
| // 16 being the minimum allocation size due to alignments) and leaves room | ||
| // for a single AudioSampleFrame as a minumum. | ||
| this.maxBuffers = Math.min(((Module['sz'] - /*minimum alloc*/ 16) / this.bytesPerChannel) | 0, /*sensible limit*/ 10); | ||
| #if ASSERTIONS | ||
| console.assert(this.maxBuffers > 0, `AudioWorklet needs more stack allocating (at least ${this.samplesPerChannel * 4})`); | ||
| #endif | ||
| // These are still alloc'd to take advantage of the overflow checks, etc. | ||
| var oldStackPtr = stackSave(); | ||
| var viewDataIdx = {{{ getHeapOffset('stackAlloc(this.maxBuffers * this.bytesPerChannel)', 'float') }}}; | ||
| #if WEBAUDIO_DEBUG | ||
| console.log(`AudioWorklet creating ${this.maxBuffers} buffer one-time views (for a stack size of ${Module['sz']} at address 0x${(viewDataIdx * 4).toString(16)})`); | ||
| #endif | ||
| this.outputViews = []; | ||
| for (var i = this.maxBuffers; i > 0; i--) { | ||
| // Added in reverse so the lowest indices are closest to the stack top | ||
| this.outputViews.unshift( | ||
| HEAPF32.subarray(viewDataIdx, viewDataIdx += this.samplesPerChannel) | ||
| ); | ||
| } | ||
| stackRestore(oldStackPtr); | ||
|
|
||
| #if ASSERTIONS | ||
| // Explicitly verify this later in process() | ||
| this.ctorOldStackPtr = oldStackPtr; | ||
| #endif | ||
| } | ||
|
|
||
| static get parameterDescriptors() { | ||
|
|
@@ -51,75 +84,122 @@ function createWasmAudioWorkletProcessor(audioParams) { | |
| let numInputs = inputList.length, | ||
| numOutputs = outputList.length, | ||
| numParams = 0, i, j, k, dataPtr, | ||
| outputViewsNeeded = 0, | ||
| stackMemoryNeeded = (numInputs + numOutputs) * {{{ C_STRUCTS.AudioSampleFrame.__size__ }}}, | ||
| oldStackPtr = stackSave(), | ||
| inputsPtr, outputsPtr, paramsPtr, | ||
| didProduceAudio, paramArray; | ||
|
|
||
| // Calculate how much stack space is needed | ||
| for (i of inputList) stackMemoryNeeded += i.length * this.bytesPerChannel; | ||
| for (i of outputList) outputViewsNeeded += i.length; | ||
| stackMemoryNeeded += outputViewsNeeded * this.bytesPerChannel; | ||
| for (i in parameters) stackMemoryNeeded += parameters[i].byteLength + {{{ C_STRUCTS.AudioParamFrame.__size__ }}}, ++numParams; | ||
|
|
||
| #if ASSERTIONS | ||
| console.assert(oldStackPtr == this.ctorOldStackPtr, 'AudioWorklet stack address has unexpectedly moved'); | ||
| console.assert(outputViewsNeeded <= this.outputViews.length, `Too many AudioWorklet outputs (need ${outputViewsNeeded} but have stack space for ${this.outputViews.length})`); | ||
| #endif | ||
|
|
||
| // Allocate the necessary stack space (dataPtr is always in bytes, and | ||
| // advances as space for structs and data is taken, but note the switching | ||
| // between bytes and indices into the various heaps, usually in 'k'). This | ||
| // will be 16-byte aligned (from _emscripten_stack_alloc()), as were the | ||
| // output views, so we round up and advance the required bytes to ensure | ||
| // the addresses all work out at the end. | ||
| i = (stackMemoryNeeded + 15) & ~15; | ||
| dataPtr = stackAlloc(i) + (i - stackMemoryNeeded); | ||
|
|
||
| // Copy input audio descriptor structs and data to Wasm | ||
| // Note: filling the structs was tried with makeSetValue() but it creates | ||
| // minor overhead (adds and shifts) that we can avoid (and no combination | ||
| // of optimisations will fold). | ||
| inputsPtr = dataPtr; | ||
| k = {{{ getHeapOffset('inputsPtr', 'u32') }}}; | ||
| dataPtr += numInputs * {{{ C_STRUCTS.AudioSampleFrame.__size__ }}}; | ||
| for (i of inputList) { | ||
| // Write the AudioSampleFrame struct instance | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.numberOfChannels / getNativeTypeSize('u32') }}}] = i.length; | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.samplesPerChannel / getNativeTypeSize('u32') }}}] = this.samplesPerChannel; | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.data / getNativeTypeSize('u32') }}}] = dataPtr; | ||
| #if MEMORY64 | ||
| // See the note in the constructor for dealing with 64-bit addresses | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.data / getNativeTypeSize('u32') + 1 }}}] = dataPtr / 0x100000000; | ||
| #endif | ||
| k += {{{ C_STRUCTS.AudioSampleFrame.__size__ / getNativeTypeSize('u32') }}}; | ||
| // Marshal the input audio sample data for each audio channel of this input | ||
| for (j of i) { | ||
| HEAPF32.set(j, {{{ getHeapOffset('dataPtr', 'float') }}}); | ||
| dataPtr += this.bytesPerChannel; | ||
| } | ||
| } | ||
|
|
||
| // Copy parameters descriptor structs and data to Wasm | ||
| paramsPtr = dataPtr; | ||
| k = {{{ getHeapOffset('paramsPtr', 'u32') }}}; | ||
| dataPtr += numParams * {{{ C_STRUCTS.AudioParamFrame.__size__ }}}; | ||
| for (i = 0; paramArray = parameters[i++];) { | ||
| // Write the AudioParamFrame struct instance | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioParamFrame.length / getNativeTypeSize('u32') }}}] = paramArray.length; | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioParamFrame.data / getNativeTypeSize('u32') }}}] = dataPtr; | ||
| #if MEMORY64 | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.data / getNativeTypeSize('u32') + 1 }}}] = dataPtr / 0x100000000; | ||
| #endif | ||
| k += {{{ C_STRUCTS.AudioParamFrame.__size__ / getNativeTypeSize('u32') }}}; | ||
| // Marshal the audio parameters array | ||
| HEAPF32.set(paramArray, {{{ getHeapOffset('dataPtr', 'float') }}}); | ||
| dataPtr += paramArray.length * {{{ getNativeTypeSize('float') }}}; | ||
| } | ||
|
|
||
| // Copy output audio descriptor structs to Wasm (note that dataPtr after | ||
| // the struct offsets should now be 16-byte aligned). | ||
| outputsPtr = dataPtr; | ||
| k = {{{ getHeapOffset('outputsPtr', 'u32') }}}; | ||
| dataPtr += numOutputs * {{{ C_STRUCTS.AudioSampleFrame.__size__ }}}; | ||
| for (i of outputList) { | ||
| // Write the AudioSampleFrame struct instance | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.numberOfChannels / getNativeTypeSize('u32') }}}] = i.length; | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.samplesPerChannel / getNativeTypeSize('u32') }}}] = this.samplesPerChannel; | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.data / getNativeTypeSize('u32') }}}] = dataPtr; | ||
| #if MEMORY64 | ||
| HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.data / getNativeTypeSize('u32') + 1 }}}] = dataPtr / 0x100000000; | ||
| #endif | ||
| k += {{{ C_STRUCTS.AudioSampleFrame.__size__ / getNativeTypeSize('u32') }}}; | ||
| // Advance the output pointer to the next output (matching the pre-allocated views) | ||
| dataPtr += this.bytesPerChannel * i.length; | ||
| } | ||
|
|
||
| #if ASSERTIONS | ||
| // If all the maths worked out, we arrived at the original stack address | ||
| console.assert(dataPtr == oldStackPtr, `AudioWorklet stack missmatch (audio data finishes at ${dataPtr} instead of ${oldStackPtr})`); | ||
|
|
||
| // Sanity checks. If these trip the most likely cause, beyond unforeseen | ||
| // stack shenanigans, is that the 'render quantum size' changed. | ||
| if (numOutputs) { | ||
| // First that the output view addresses match the stack positions. | ||
| k = dataPtr - this.bytesPerChannel; | ||
| for (i = 0; i < outputViewsNeeded; i++) { | ||
| console.assert(k == this.outputViews[i].byteOffset, 'AudioWorklet internal error in addresses of the output array views'); | ||
| k -= this.bytesPerChannel; | ||
| } | ||
| // And that the views' size match the passed in output buffers | ||
| for (i of outputList) { | ||
| for (j of i) { | ||
| console.assert(j.byteLength == this.bytesPerChannel, `AudioWorklet unexpected output buffer size (expected ${this.bytesPerChannel} got ${j.byteLength})`); | ||
| } | ||
| } | ||
| } | ||
| #endif | ||
|
|
||
| // Call out to Wasm callback to perform audio processing | ||
| if (didProduceAudio = this.callbackFunction(numInputs, {{{ toIndexType('inputsPtr') }}}, numOutputs, {{{ toIndexType('outputsPtr') }}}, numParams, {{{ toIndexType('paramsPtr') }}}, this.userData)) { | ||
| // Read back the produced audio data to all outputs and their channels. | ||
| // The preallocated 'outputViews' already have the correct offsets and | ||
| // sizes into the stack (recall from the ctor that they run backwards). | ||
| k = outputViewsNeeded - 1; | ||
| for (i of outputList) { | ||
| for (j of i) { | ||
| j.set(this.outputViews[k--]); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -193,14 +273,9 @@ class BootstrapMessages extends AudioWorkletProcessor { | |
| // 'cb' the callback function | ||
| // 'ch' the context handle | ||
| // 'ud' the passed user data | ||
| p.postMessage({'_wsc': {{{ toIndexType("d['cb']") }}}, 'x': [d['ch'], 1/*EM_TRUE*/, {{{ toIndexType("d['ud']") }}}] }); | ||
|
There was a problem hiding this comment. Are the changes on this line necessary? I would hope that There was a problem hiding this comment. Originally the type conversions were in the calling code, here for example: emscripten/src/lib/libwebaudio.js Line 269 in 0c3df92 But you suggested moving them to where they're needed. Without these it results in type errors, e.g.: |
||
| } else if (d['_wsc']) { | ||
| Module['wasmTable'].get({{{ toIndexType("d['_wsc']") }}})(...d['x']); | ||
| }; | ||
| } | ||
| } | ||
|
|
||
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file sseems to have changed a lot more than I would expect just for the memory64 change. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's built on #22753 from last October that was never merged, which contained, besides the performance improvements, the groundwork for the struct offsets which made 2GB and wasm64 support straightforward. The diff between the old PR and this isn't that large, with most of the work going into the tests (which went into their own #23659).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll break this down into a series of smaller PRs on the current main, e.g. move to the
C_STRUCTSoffsets andmakeGetValue, then introduce wasm64, then the performance changes. I couldn't review in its current state what I wrote myself.I don't know when this will be though since we're shipping it and it's done what I set out to do (we have millions of users putting in hour long sessions per day, so a few millis per second saved is a big deal on low-end school hardware when juggling 3D content with audio, and if it didn't work we'd really know about it!).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We appreciate all the work you have done here.
I'm excited to see these changes lands. If you can find time to spit split the PR that would be amazing. If not, we can circle back in a few weeks.