emscripten icon indicating copy to clipboard operation
emscripten copied to clipboard

AudioWorklet: move the oft called context/worklet connect to native code

Open cwoffenden opened this issue 1 year ago • 3 comments

In all the examples and in the docs, connecting the context and worklet is done with the following type of code:

EM_ASM({
  emscriptenGetAudioObject($0).connect(emscriptenGetAudioObject($1).destination)
}, wasmAudioWorklet, audioContext);

Every example or usage needs this to play audio. Granted, more complex graphs will add more node types and emscriptenGetAudioObject() will have its uses, but for the general use case a simpler call without JS is preferred:

emscripten_audio_worklet_node_connect(audioContext, wasmAudioWorklet);

See here for how it's done in the examples:

https://github.com/emscripten-core/emscripten/blob/7df48f680e2e901b9bf1f5312d27ca939753548d/test/webaudio/audioworklet.c#L50

I updated the examples and the documentation.

@juj Comments or ideas?

cwoffenden avatar Oct 02 '24 16:10 cwoffenden

Perhaps the title should something like "[AUDIO_WORKLET] Add new emscripten_audio_worklet_node_connect API"?

Good point. I’ll take a look tomorrow, plus I might roll it in with some other quality of life changes.

cwoffenden avatar Oct 02 '24 19:10 cwoffenden

Perhaps the title should something like "[AUDIO_WORKLET] Add new emscripten_audio_worklet_node_connect API"?

Good point. I’ll take a look tomorrow, plus I might roll it in with some other quality of life changes.

As always, I'd love to see separate smaller PR that kind of thing rather than trying combine stuff.

sbc100 avatar Oct 02 '24 19:10 sbc100

This sounds fine.

Originally the connection was in JS since I did not want to start building a full Emscripten Web Audio API wrapper, but show developers in the examples how to do the JS side tasks on JS side.

However I can see that users who don't have a JS side web audio engine, might want to have a pure C API without needing to dip to JS side at all.

If we do this, I think it would be good to be forward-looking and generic, so instead of having a function

emscripten_audio_worklet_node_connect(EMSCRIPTEN_WEBAUDIO_T audioContext, EMSCRIPTEN_AUDIO_WORKLET_NODE_T audioWorkletNode);

let's instead have a function

emscripten_audio_node_connect(EMSCRIPTEN_WEBAUDIO_T sourceNode, int outputIndex, EMSCRIPTEN_WEBAUDIO_T destinationNode, int inputIndex);

and in the implementation of that function, do

sourceNode.connect(destinationNode.destination || destinationNode, outputIndex, inputIndex);

(https://developer.mozilla.org/en-US/docs/Web/API/AudioNode/connect)

This way the same function can be used to connect to AudioContext destination, but also to connect any arbitrary audio nodes together.

While we don't have capability to manipulate audio nodes in C side, maybe someone in the future might rationale to want to add that, so then the shape of the API would be generic to be compatible with that kind of expansion.

Would that make sense?

juj avatar Oct 02 '24 20:10 juj

If we do this, I think it would be good to be forward-looking and generic, so instead of having a function

I made something more generic that takes any two nodes, and added the indices as params as suggested.

I'll create a few smaller PRs with other proposed changes.

cwoffenden avatar Oct 03 '24 16:10 cwoffenden

I'll wait for @juj to chime in again before landing.

This PR also has the much smaller #22681 as a sibling for the public API parts I’d like to add to. Anything else will be internal.

cwoffenden avatar Oct 08 '24 07:10 cwoffenden