-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AUDIO_WORKLET] Added API for getting the buffer's quantum size #22681
Changes from all commits
ea8aa11
58422d0
559d097
ab4671a
f0596cf
7ff2a00
24fb2af
6a26421
8ed9588
5249dbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,11 +37,21 @@ let LibraryWebAudio = { | |
// Wasm handle ID. | ||
$emscriptenGetAudioObject: (objectHandle) => EmAudio[objectHandle], | ||
|
||
// emscripten_create_audio_context() does not itself use | ||
// Performs the work of getting the AudioContext's quantum size. | ||
$emscriptenGetContextQuantumSize: (contextHandle) => { | ||
// TODO: in a future release this will be something like: | ||
// return EmAudio[contextHandle].renderQuantumSize || 128; | ||
// It comes two caveats: it needs the hint when generating the context adding to | ||
// emscripten_create_audio_context(), and altering the quantum requires a secure | ||
// context and fallback implementing. Until then we simply use the 1.0 API value: | ||
return 128; | ||
}, | ||
|
||
// emscripten_create_audio_context() does not itself use the | ||
// emscriptenGetAudioObject() function, but mark it as a dependency, because | ||
// the user will not be able to utilize the node unless they call | ||
// emscriptenGetAudioObject() on it on JS side to connect it to the graph, so | ||
// this avoids the user needing to manually do it on the command line. | ||
// this avoids the user needing to manually add the dependency on the command line. | ||
emscripten_create_audio_context__deps: ['$emscriptenRegisterAudioObject', '$emscriptenGetAudioObject'], | ||
emscripten_create_audio_context: (options) => { | ||
let ctx = window.AudioContext || window.webkitAudioContext; | ||
|
@@ -264,6 +274,7 @@ let LibraryWebAudio = { | |
}); | ||
}, | ||
|
||
emscripten_create_wasm_audio_worklet_node__deps: ['$emscriptenGetContextQuantumSize'], | ||
emscripten_create_wasm_audio_worklet_node: (contextHandle, name, options, callback, userData) => { | ||
#if ASSERTIONS | ||
assert(contextHandle, `Called emscripten_create_wasm_audio_worklet_node() with a null Web Audio Context handle!`); | ||
|
@@ -282,7 +293,11 @@ let LibraryWebAudio = { | |
numberOfInputs: HEAP32[options], | ||
numberOfOutputs: HEAP32[options+1], | ||
outputChannelCount: HEAPU32[options+2] ? readChannelCountArray(HEAPU32[options+2]>>2, HEAP32[options+1]) : void 0, | ||
processorOptions: { 'cb': callback, 'ud': userData } | ||
processorOptions: { | ||
'cb': callback, | ||
'ud': userData, | ||
'qs': emscriptenGetContextQuantumSize(contextHandle) | ||
} | ||
} : void 0; | ||
|
||
#if WEBAUDIO_DEBUG | ||
|
@@ -293,6 +308,15 @@ let LibraryWebAudio = { | |
}, | ||
#endif // ~AUDIO_WORKLET | ||
|
||
emscripten_audio_context_quantum_size__deps: ['$emscriptenGetContextQuantumSize'], | ||
emscripten_audio_context_quantum_size: (contextHandle) => { | ||
#if ASSERTIONS | ||
assert(EmAudio[contextHandle], `Called emscripten_audio_context_quantum_size() with an invalid Web Audio Context handle ${contextHandle}`); | ||
assert(EmAudio[contextHandle] instanceof (window.AudioContext || window.webkitAudioContext), `Called emscripten_audio_context_quantum_size() on handle ${contextHandle} that is not an AudioContext, but of type ${EmAudio[contextHandle]}`); | ||
#endif | ||
return emscriptenGetContextQuantumSize(contextHandle); | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would delete the second function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One was to expose it to JS for use in the worklet or in other code, the other native. Originally I didn't pass the value to the worklet, instead reading it from this JS function, but you'd said to demonstrate the value being passed. I can re-look at this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both versions of the function receive the handle ID? So in that case, I think it would be fine to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I can tidy this in another PR. |
||
|
||
emscripten_audio_node_connect: (source, destination, outputIndex, inputIndex) => { | ||
var srcNode = EmAudio[source]; | ||
var dstNode = EmAudio[destination]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
#include <emscripten/webaudio.h> | ||
#include <emscripten/em_math.h> | ||
|
||
#include <stdio.h> | ||
|
||
// This program tests that sharing the WebAssembly Memory works between the | ||
// audio generator thread and the main browser UI thread. Two sliders, | ||
// frequency and volume, can be adjusted on the HTML page, and the audio thread | ||
|
@@ -25,7 +27,7 @@ float currentVolume = 0.3; // [local variable to the audio thread] | |
volatile int audioProcessedCount = 0; | ||
#endif | ||
|
||
// This function will be called for every fixed 128 samples of audio to be processed. | ||
// This function will be called for every fixed-size buffer of audio samples to be processed. | ||
bool ProcessAudio(int numInputs, const AudioSampleFrame *inputs, int numOutputs, AudioSampleFrame *outputs, int numParams, const AudioParamFrame *params, void *userData) { | ||
#ifdef REPORT_RESULT | ||
++audioProcessedCount; | ||
|
@@ -38,12 +40,12 @@ bool ProcessAudio(int numInputs, const AudioSampleFrame *inputs, int numOutputs, | |
|
||
// Produce a sine wave tone of desired frequency to all output channels. | ||
for(int o = 0; o < numOutputs; ++o) | ||
for(int i = 0; i < 128; ++i) | ||
for(int i = 0; i < outputs[o].quantumSize; ++i) | ||
{ | ||
float s = emscripten_math_sin(phase); | ||
phase += phaseIncrement; | ||
for(int ch = 0; ch < outputs[o].numberOfChannels; ++ch) | ||
outputs[o].data[ch*128 + i] = s * currentVolume; | ||
outputs[o].data[ch*outputs[o].quantumSize + i] = s * currentVolume; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that the quantum size is a constant, I wonder if it would be better to have all these examples use that global int quantumSize;
bool ProcessAudio(int numInputs, ...) {
for(int o = 0; o < numOutputs; ++o)
for(int i = 0; i < quantumSize; ++i)
...
}
int main() {
quantumSize = emscripten_audio_context_quantum_size(context);
} This would communicate the invariant that the quantumSize cannot change between inputs and outputs in a single call to ProcessAudio(). That might help someone learning about AudioWorklets. Alternatively, I wonder if the field for(int i = 0; i < outputs[o].length; ++i) {
outputs[o].data[i] = ...; with very idiomatic reading C code. The "quantumSize" name in this context reads fancier than it necessarily needs to be (and doesn't match the Web Audio spec which calls it "renderQuantumSize"). Works either way, leaving it up to you to decide. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like setting it as a global, and I agree, I also don't like the name If I'm going to spend more time with this I'd rather that be spent marking the existing API as deprecated and creating a new callback signature. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha, that makes sense. 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made #22741 since it was easy to do (for me) now this has landed, that calls it |
||
} | ||
|
||
// Range reduce to keep precision around zero. | ||
|
@@ -148,6 +150,12 @@ int main() { | |
|
||
EMSCRIPTEN_WEBAUDIO_T context = emscripten_create_audio_context(&attrs); | ||
|
||
// Get the context's quantum size. Once the audio API allows this to be user | ||
// defined or exposes the hardware's own value, this will be needed to | ||
// determine the worklet stack size. | ||
int quantumSize = emscripten_audio_context_quantum_size(context); | ||
printf("Context quantum size: %d\n", quantumSize); | ||
|
||
// and kick off Audio Worklet scope initialization, which shares the Wasm | ||
// Module and Memory to the AudioWorklet scope and initializes its stack. | ||
emscripten_start_wasm_audio_worklet_thread_async(context, wasmAudioWorkletStack, sizeof(wasmAudioWorkletStack), WebAudioWorkletThreadInitialized, 0); | ||
|
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.
Couldn't this code already today read
That way the code would also be verifying that
contextHandle
is a valid context.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 could, but I was worried about its
[SecureContext]
marking in the spec and what that entails, and also since we can't yet set it then it doesn't make a difference.But I'll happily change it and look at it again if/when it breaks, and when the API allows for the setting.
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.
Ok, works either way.
[SecureContext] just means that the parameter won't be populated if the page is served via an insecure http:// handler. The value won't then be present, and will return
undefined
.(http://localhost URLs get a special case and is always considered secure).
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.
My concern was it'd throw an exception, not just silently return undefined.
(For localhost it still needs
Cross-Origin-Opener-Policy
andCross-Origin-Embedder-Policy
headers for audio demos to run, BTW, sinceSharedArrayBuffer
isn't available and throws a reference error)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.
Ah, right. An exception won't happen, that is guaranteed by the IDL spec of [SecureContext]: https://www.w3.org/TR/secure-contexts/#integration-idl which states multiple times
But let's leave this later for when an implementation comes along.