From c98adcfb75fcba1b5261d6d05eb378b5571a0fe5 Mon Sep 17 00:00:00 2001 From: Carl Woffenden Date: Mon, 28 Oct 2024 18:17:45 +0100 Subject: [PATCH] Correct stack offsets and verified code Tested with various stack sizes, output sizes, and generators. --- src/audio_worklet.js | 57 ++++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/audio_worklet.js b/src/audio_worklet.js index 322daeb6d138b..6d937951fda00 100644 --- a/src/audio_worklet.js +++ b/src/audio_worklet.js @@ -40,10 +40,16 @@ function createWasmAudioWorkletProcessor(audioParams) { this.samplesPerChannel = opts['sc']; // Create up-front as many typed views for marshalling the output data as - // may be required (with an arbitrary maximum of 16, for the case where a + // 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). - this.maxBuffers = Math.min(((Module['sz'] - /*stack guards?*/ 16) / (this.samplesPerChannel * 4)) | 0, /*sensible limit*/ 16); + // 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.samplesPerChannel * 4)) | 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 = stackAlloc(this.maxBuffers * this.samplesPerChannel * 4) >> 2; @@ -77,7 +83,6 @@ function createWasmAudioWorkletProcessor(audioParams) { stackMemoryNeeded = (numInputs + numOutputs) * {{{ C_STRUCTS.AudioSampleFrame.__size__ }}}, oldStackPtr = stackSave(), inputsPtr, outputsPtr, paramsPtr, - outputDataPtr, didProduceAudio, paramArray; // Calculate how much stack space is needed @@ -91,14 +96,18 @@ function createWasmAudioWorkletProcessor(audioParams) { #endif // Allocate the necessary stack space (dataPtr is always in bytes, and - // advances as space for structs as data is taken, but note the switching - // between bytes and indices into the various heaps, usually in 'k'). - dataPtr = stackAlloc(stackMemoryNeeded); + // 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 inputsPtr = dataPtr; - k = inputsPtr >> 2; dataPtr += numInputs * {{{ C_STRUCTS.AudioSampleFrame.__size__ }}}; + k = inputsPtr >> 2; for (i of inputList) { // Write the AudioSampleFrame struct instance HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.numberOfChannels / 4 }}}] = i.length; @@ -114,8 +123,8 @@ function createWasmAudioWorkletProcessor(audioParams) { // Copy parameters descriptor structs and data to Wasm paramsPtr = dataPtr; - k = paramsPtr >> 2; dataPtr += numParams * {{{ C_STRUCTS.AudioParamFrame.__size__ }}}; + k = paramsPtr >> 2; for (i = 0; paramArray = parameters[i++];) { // Write the AudioParamFrame struct instance HEAPU32[k + {{{ C_STRUCTS.AudioParamFrame.length / 4 }}}] = paramArray.length; @@ -126,13 +135,11 @@ function createWasmAudioWorkletProcessor(audioParams) { dataPtr += paramArray.length*4; } - // Copy output audio descriptor structs to Wasm + // Copy output audio descriptor structs to Wasm (not that dataPtr after + // the struct offsets should now be 16-byte aligned). outputsPtr = dataPtr; - k = outputsPtr >> 2; dataPtr += numOutputs * {{{ C_STRUCTS.AudioSampleFrame.__size__ }}}; -#if ASSERTIONS - outputDataPtr = dataPtr; -#endif + k = outputsPtr >> 2; for (i of outputList) { // Write the AudioSampleFrame struct instance HEAPU32[k + {{{ C_STRUCTS.AudioSampleFrame.numberOfChannels / 4 }}}] = i.length; @@ -144,23 +151,27 @@ function createWasmAudioWorkletProcessor(audioParams) { } #if ASSERTIONS - // TODO: addresses are now wrong - k = outputDataPtr; - for (i = outputViewsNeeded - 1; i >= 0; i--) { - console.assert(this.outputViews[i].byteOffset == k, 'Internal error in addresses of the output array views'); - k += bytesPerChannel; + // 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 check the output view addresses + if (numOutputs) { + k = dataPtr - 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 -= bytesPerChannel; + } } #endif // Call out to Wasm callback to perform audio processing if (didProduceAudio = this.callbackFunction(numInputs, inputsPtr, numOutputs, outputsPtr, numParams, paramsPtr, this.userData)) { // Read back the produced audio data to all outputs and their channels. - // The 'outputViews' are subarray views into the heap, each with the - // correct offset and size to be copied directly into the output. - k = 0; + // 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++]); + j.set(this.outputViews[k--]); } } }