-
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
Basic support for running Emscripten code as AudioWorklets #12502
Conversation
358b422
to
17a72a5
Compare
Interesting @tklajnscek , but I'm a little lost here... can you please explain why this is needed? That is, is this because AudioWorklets are very different than normal environments (if so, what are the specific issues this fixes) or is this adding a new type of functionality entirely (using an AudioWorklet in some special way)? Or to ask the question in another way, what does this make possible that wasn't before? |
Sure, let me try to explain. Forgive me if some of this is stating the obvious 😅 Let me run through the main pain points first vs regular Workers:
This PR addresses this by:
All of the above makes it possible to run a shared memory instance of the program in AudioWorkletGlobalScope with the pthread environment initialized properly, just like a regular Worker. Which in turn makes it possible to execute all the program code in there which makes it super easy to run WASM code (=fast) in audio worklets and lets you port a bunch of existing C(++) audio stuff over much more easily. The developer only needs to register their AudioWorkletProcessor and call their exported functions from it and it JustWorks(TM). Without this you can't really run any Emscripten compiled code from an AudioWorkletProcessor unless you do all the steps manually anyways like transferring over the wasmInstance, getting the .js to evaluate, initializing the pthread environment manually to get the stack set up etc. So you end up running custom JS (or potentially, custom WASM+JS) in there with a bunch of glue code to get it to play along with your main program that does a bunch of processing already. We currently use this to run the final output mixer as an AudioWorklet and it works great and it's the same code as any other platform (with the addition of the AudioWorkletProcessor class that's calling into it). In addition to this we run audio decoders as regular Workers. I can also see this being very useful to anyone doing threaded/DPC audio output because it most likely maps 1:1 with what they have in there now. I hope that answers the question @kripken. Let me know if anything is unclear or if I missed something obvious somewhere :) |
I see thanks @tklajnscek ! This does sound like a useful thing to support, and I think I understand now why it takes so much work here. Some initial comments on the PR:
@juj , have you thought about worklet support? |
80379c3
to
1b1512a
Compare
@kripken I believe the latest changes should address all your points and it's rebased on latest master.
@sbc100 if you have any thoughts on this they'd be much appreciated :) |
First off, thanks for working on this kind of project, this is a very interesting development, one which I have had a TODO for close to three(!) years now to experiment with..
I've been going through this code back and forth for a few passes now, and some of the design questions on this front do boil down to the portability vs native web question. You mention portability, but there seems to be currently very little provided in terms of portability in the test code, since it is very JS-specific? The review and maintenance complexity of this PR is high, so we should be very careful about how to proceed. Some thoughts to get us further:
Maybe we'll need to flesh that out in documentation or a hello world test without test harness, to illustrate the expected API surface area.
My expectation for portability would be something akin to having a C based API, where one would call something like
To summarize, in order to proceed with this feature, I believe we should define in detail, what the "native web" API would look like to approach this feature, if portability to other platforms is not a concern. Then we can define what the "portable" API perspective would look like, if/when the added requirement of portability is necessary - and how should that API differ from the native web-like API? |
@juj I'll try to find some time to work on this in the next few days and we can discuss. For now here's a quick reply to your points.
Yeah, I think a clean sample would make sense. I'll put something somewhere that we can use a starting point for discussion.
The port existing code part of my comments above was referring to the ability to easily take existing C(++) audio processing code and make it work pretty much as-is under Emscripten when executed in an audio worklet context. My intention was not to create a generic, portable API for access to the platform-specific audio system which this would become. I didn't even attempt to wrap/abstract/hide the creation the AudioContext etc which immediately makes this a non-portable thing. I think that is totally beyond the scope of Emscripten and it would most likely be a total nightmare to maintain. What I was aiming for was to add a way of hooking into the audio callbacks like you can on other platforms (using platform specific APIs - XAudio/WASAPI/AU etc.) and use normal C++ code in there without having to modify the internals of Emscripten/pthreads/low level synchronization. This is what I was going for with portability. Because currently, you can't write a function that gets called from an audio worklet and actually any pthread/sync primitive at all. For example, if you have an existing app that renders audio you likely have a bunch of platform-independent code that processes all the audio/dsp logic. That's the bulk of the code. Then you have some platform-specific code that queries supported features/formats, creates the audio device and ends upsetting up some kind of DPC audio callback which is where the audio gets submitted to the os/driver for playback. And that should stay like that IMO. Before AudioWorklets this worked fine in Emscripten too. For the 'Emscripten platform' part of the code we used a ScriptAudioProcessor node and simply called our "native" DPC callback to fill the buffers from
Yeah, definitely. I'll make a pass on this to clean it up.
While I totally agree with the sentiment, this is more of a question of balance between code duplication and keeping code size down when disabled. @kripken asked me to deduplicate some of the code, which I also think is important to make it maintainable. But at the same time, doing this means that it because more difficult to keep the overhead to an absolute 0 when disabled without making the whole thing a hot ifdef-ridden mess. It becomes especially difficult in the futex implementation since just extracting a part of it into a common function will increase the code size even for non-audio-worklet builds. In any case, I'm confident we can get pretty damn close to 0, or you know, optimize something else in the process to offset the cost :) |
6481737
to
cadcdd9
Compare
Hey everyone, So I said in a few days but then life happened, so here we are 6 months later and I've finally been able to spend some more time on this and brought it up to speed with the latest revision and addressed some of the issues, mostly:
I'd appreciate any guidance on what needs doing so that I stand a chance of getting this merged as I really thing a lot of cool stuff can be done in the audio field with Emscripten and this just brings it way closer to native audio dev then it used to be. Note: Some of the SSE tests seem to be failing, but I'm not sure how this is related to my changes and it seems to be non-trivial for me to run them locally, but I can try if this is not a known issue with the test suite? |
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.
In general, I'd love to see some the complexity of the test/example reduced. It would seem odd to be if that was the smallest possible test/example of using a worklet.
Wow, thanks for the uber fast review! I'll jump on it first thing in the morning as it's quite late here already :) |
In addition the the stuff discussed in the diff comments I split up the worklet test into a futex test and a tone generator example. I'll amend this PR some more after I'm done playing around with MODULARIZE=0/1 and MINIMAL _RUNTIME.. have some ideas :) |
Let's see if the tests pass! Made everything work without requiring MODULARIZE=1! :) |
I think we're good to go here. Any further feedback is much appreciated! |
Some more fixes:
I think this is shaping up nicely if I do say so myself 😁 |
Ping :) Has anyone had time to look at this? |
This is my fortnightly reminder for an update here 😄 |
I tried to do another reading of this, but given all the discussion, II'm not sure where things stand. In particular, I see there was discussion of a possible C API and other options, for example - are those discussions still ongoing? Or is this PR in an intended final state? Skimming the code, it does look like it no longer duplicates a bunch of logic, which is good! |
bbc2a79
to
a92f951
Compare
Hey @Jonathhhan, yeah I haven't been merging in the latest changes lately... I've gone ahead and rebased everything on 2.0.34 so this should work for you:
This should do it. Then go into The tone generator sample should also build and run - in Let me know if it works for you :) |
@tklajnscek Thank you very much - I was looking for a solution like that for a long time. Open Frameworks is working with audioworklet now (had to make some small additional changes). I make an audiocallback from the audioworklet every 128 samples. A strange thing is that it only runs smooth with Chrome and Ubuntu, other browser / OS combinations have some audio artifacts / glitches - maybe the buffersize is too small, I will investigate on that. Here is my example: https://gameoflife3d.handmadeproductions.de/ Here is the same thing without audioworklet (and a buffersize of 4096): https://simplesequencer.handmadeproductions.de/ (will post my audioworkletcode later...) |
I set the Open Frameworks buffersize to 1024 and it is much better now (also compared to scripprocessornode) - not to say it works great.
This is the audioworklet:
My only issue is, that I need to edit the Emscripten generated Java Script file every time after compiling, because I do not know how to change the source code for that (a hint would be great).
|
Hey @Jonathhhan, I tried your demos and they both actually work fine for me in Chrome on Win 10 - couldn't hear any glitches :) Obviously you have to be careful with any code you run in the audio worklet processor as it has to be very fast to stay real-time. Also, as you already noticed, the buffer is settable and that helps, but as far as I understand, the browser is always supposed to figure the ideal buffer to use with audio worklets and that's the whole point of a fixed 128 sample quantum. If you can't make it work fast enough, then you might have to run the processing in another (multiple?) thread(s) and then just use the audio worklet processor to fill the output buffers. Finally, regarding the file packager thingy - we don't actually use that so I'm not going to be able to help you much there, but generally, the issue is that XHRs are not allowed in worklet contexts. The first solution is to fix The other, IMO better, option is switch to manual file fetches which are more flexible, but it requires you to know in the code that you need to download stuff and wait for it before using file.
Hope any of this helps :) |
Hey @tklajnscek
Edit: I did something wrong before, it works also very well with a buffer size of 64 (which is 128 for the audioworklet, because it is stereo - if i am right). And it worked with editing |
Hey @tklajnscek, do you maybe know, if it is possible to run audioworklets with your branch together with
|
Ha! It probably doesn't work because it needs code adding to proxy the call to the main thread, just like the regular pthread_create call does. There's also special case code for offscreen canvas support. Since I don't really have a use case for this and I have other priorities right now I won't really be able to help you out with this, but you're more than welcome to take my code and make any changes necessary to make it work. I guess you could just look at the proxying and offscreen canvas code paths in |
@tklajnscek I wanted to thank you again, your implementation works absolutely great with Open Frameworks and Pure Data (I think I fixed all the issues I had). I build a small demo where you can manipulate local audio files and audio input: https://emscriptenpdeffect.handmadeproductions.de/ |
@Jonathhhan that's really great! I'm happy this was useful to someone else! |
@tklajnscek actually even the filesystem error disappeared after removing -s ASSERTIONS (which seems a bit strange, but anyhow). I made one small change for recalculating mouse coordinates in fullscreen mode and one for offscreen canvas (but that has nothing to do with the audioWorklet implementation). |
Hey @tklajnscek, |
I'd just like to add that I'm using this branch for one of my projects and it has been absolutely fantastic so far. The project is relatively complex and shares a lot of data between the main/audio thread via the use of the moodycamel concurrent queues and it works just as I expect it to! |
From the discussion above I think the general agreement was to focus on @juj 's Wasm Workers PR and to add AudioWorklet support on top of that? If so, then helping in that direction would probably be better. Or is there not consensus on that direction? |
Oooh! I see there some action over at #12833 now! I was afraid it was dead after a few months of silence and was just about to start asking what's up :) Also, I'm glad someone else is finding the work here useful! 🥳 Anyways, my plan for this PR is to keep it reasonably updated to at least cover our needs for Emscripten updates until we hopefully end up building our thing on top of the web workers API. We usually take a new version every 3-4 months. For our code base it shouldn't be a huge problem since we already have a per-platform threading abstraction so this is a natural fit where I can plug in the wasm workers API. I guess this might not be that straightforward for others that have pthreads code sprinkled all over their codebase... It'll be a few months until I get around to actually trying the wasm workers though as we're in the midst of shipping a major version update 😅 |
@tklajnscek thanks, that sounds great.
I agree totally with that (only that my programming knowledge is a little limited - but I am happy to test and help where I can), and it would be great to have audioWorklet in the official branch. Its just, that at the moment this branch is the only I know where I can use audioWorklets together with Open Frameworks. |
Wasm Workers PR did land, although not sure if that was premature ( #12833 (comment) ). The PR #16449 is now up for reviewing and landing Audio Worklet support as well. Looking at the dates on the comment history here, I see it has taken quite some time, so thanks for the patience on this one! Please give the above PR a review for any comments you might have. |
Hello everyone. I am new to open source and have arrived on this pr by following the guidelines mentioned on the Chromium GSOC 2022 Project Proposal: Audio Worklet for Emscripten. I would really like to contribute to this project. Can somebody brief me how I can contribute to this thread or project. |
Hey folks, any update as to whats going on here? WASM workers appear to have been merged - should they be used to handle this instead, or is this PR still the best method? EDIT: Just took a look at the above PR (#16449). Disregard this message! |
Can this PR be closed now that #16449 has landed? |
Agreed! |
Basically, this PR lets you run AudioWorkletNodes that can nicely call into Emscripten-generated code.
It sets up a pthread environment with shared memory in the AudioWorkletGlobalScope that's shared by all the audio worklets.
Summary of major changes:
this.XXX
sincethis
doesn't exist in AudioWorkletGlobalScopeCurrent limitations:
Doesn't work with MINIMAL_RUNTIME as I haven't figured out how to export functions so that they can be called from the registered
AudioWorkletProcessor
.Examples
There's an example in
tests/audioworklet
that also tests the pthread environment setup and the futex implementation.We are using (a much less clean) version of this in production and it works great!
Any comments highly appreciated! :)