[AUDIO_WORKET] Add support for setting the render quantum size #25747
Conversation
src/lib/libwebaudio.js
Outdated
| // AUDIO_CONTEXT_RENDER_SIZE_DEFAULT and AUDIO_CONTEXT_RENDER_SIZE_HARDWARE | ||
| // into their AudioContextRenderSizeCategory enum, or take the positive int. | ||
| function readRenderSizeHint(val) { | ||
| return (val == 0) ? "default" : ((val < 0) ? "hardware" : val); |
There was a problem hiding this comment.
I think it is better to be clearer and have a couple of extra lines here. Nested ternary expressions are hard to read.
| return (val == 0) ? "default" : ((val < 0) ? "hardware" : val); | |
| if (val == 0) return "default"; | |
| if (val == -1) return "hardware"; | |
| return val; |
There was a problem hiding this comment.
I agree, it's terrible to read, but @juj would've had something to say about the extra bytes!
There was a problem hiding this comment.
(Replaced with something less hideous)
| "latencyHint", | ||
| "sampleRate" | ||
| "sampleRate", | ||
| "renderSizeHint" |
There was a problem hiding this comment.
Why not
quantumSizeHint?
I copied what the JS API now calls it:
emscripten/src/lib/libwebaudio.js
Line 106 in e455f22
There was a problem hiding this comment.
I wonder if there are any users of this API yet, could we rename quantum -> render as well? E.g. in
$emscriptenGetContextQuantumSize -> $emscriptenGetContextRenderSize
emscripten_audio_context_quantum_size -> emscripten_audio_context_render_size
?
There was a problem hiding this comment.
The spec mixes the two, even for the renderSizeHint it says "This allows users to ask for a particular render quantum size".
Currently there's no reason to use this call, since it's always been 128. It's handy before the callback for calculating the stack size, and I should write an example of this.
src/lib/libwebaudio.js
Outdated
| #endif | ||
|
|
||
| // AUDIO_CONTEXT_RENDER_SIZE_DEFAULT and AUDIO_CONTEXT_RENDER_SIZE_HARDWARE | ||
| // into their AudioContextRenderSizeCategory enum, or take the positive int. |
There was a problem hiding this comment.
Wonder if a verb got cut off from this comment, or maybe this was intentionally brief?
There was a problem hiding this comment.
It made sense when I typed it... I'll clarify it.
| { | ||
| const char *latencyHint; // Specify one of "balanced", "interactive" or "playback" | ||
| uint32_t sampleRate; // E.g. 44100 or 48000 | ||
| int32_t renderSizeHint; // AUDIO_CONTEXT_RENDER_SIZE_* or number of samples |
There was a problem hiding this comment.
I presume since this is a hint, the context creation is allowed to not honor it?
There was a problem hiding this comment.
I presume since this is a hint, the context creation is allowed to not honor it?
From the spec:
https://webaudio.github.io/web-audio-api/#dom-audiocontextoptions-rendersizehint
"It is a hint that might not be honored."
juj
left a comment
juj
left a comment
There was a problem hiding this comment.
Looks great - I'll wait for that one comment before merging.
4 participants
The
renderSizeHintis now available in Chrome Canary (--enable-features=WebAudioConfigurableRenderQuantum) so this can finally be tested. Most of the work was already done in preparation, so this required few changes.Test with:
Which has been extended to request a large
2048sample audio frame (which in an older browser will fallback to the default 128 samples).Note for me: look at why and since how long we can bust out of the stack without asserting. The stackAlloc() no longer appears to trip when requesting more bytes than
wwParams.stackSize.