Skip to content
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

The latest Emscripten is broken for WASM examples #348

Closed
hoch opened this issue Nov 2, 2023 · 11 comments · Fixed by #349
Closed

The latest Emscripten is broken for WASM examples #348

hoch opened this issue Nov 2, 2023 · 11 comments · Fixed by #349
Assignees

Comments

@hoch
Copy link
Member

hoch commented Nov 2, 2023

The latest Emscripten compiles source code okay, but it fails on run time.

** Emscripten version **

$ emcc -v
emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.47 (431685f05c67f0424c11473cc16798b9587bb536)
clang version 18.0.0 (https://github.com/llvm/llvm-project 21030b9ab4487d845e29792063f5666d8c4b8e09)
Target: wasm32-unknown-emscripten
Thread model: posix

** make output **

~/.../design-pattern/wasm-supersaw on emscripten-test*
$ make
cache:INFO: generating system asset: symbol_lists/0abd322b1484ef22b9814d0eed790edcd69ecb21.json... (this will be cached in "<redacted>" for subsequent builds)
cache:INFO:  - ok
Build complete: ./synth.wasm.js

** Runtime error **

When running audio-worklet/design-pattern/wasm-supersaw/:

synth.wasm.js:3138 Uncaught ReferenceError: atob is not defined
    at intArrayFromBase64 (synth.wasm.js:3138:17)
    at tryParseAsDataURI (synth.wasm.js:3153:10)
    at getBinarySync (synth.wasm.js:571:16)
    at instantiateSync (synth.wasm.js:589:16)
    at createWasm (synth.wasm.js:651:16)
    at synth.wasm.js:3063:19

It looks like atob() is being used in AudioWorkletGlobalScope.

@hoch hoch self-assigned this Nov 2, 2023
@hoch
Copy link
Member Author

hoch commented Nov 2, 2023

I believe this needs a bit of attention from @kripken, @juj, and @padenot.

Some people reported Emscripten examples in this repository are failing due to the changes in Emsciprten. So I did some digging and realized that the new -sAUDIO_WORKLET brought a new set of issues.

  1. This option forces the usage of SharedArrayBuffer. As @rbdannenberg's blog post shows an example of these hurdles from developer's perspective, this narrows significantly deployable targets and environment.

  2. The usage of ShardArrayBuffer implies a ring buffer (or something similar) is being used under the hood. Is this design documented somewhere? What's the size of the buffer? How much of latency is expected? Or is there anyway to customize the size of the buffer?

  3. Why the abstraction of AudioContext was necessary? The significant effort went into this abstraction/wrapper. Althought I think it's brilliant, I would like to understand the rationale (why/use case) of the design.

  4. Although the Emscripten's AudioWorklet option is brilliant, I wish there's a simple way to use the AudioWorklet without pulling in other cutting edge APIs that require more security restrictions.

The patch was merged a while ago, so I do not expect responses from the team. But all these questions will still be relevant for people looking up WASM examples in this repository.

FWIW, this issue in question is fixed by making small changes to the make file:

  • Undefined atob() in AudioWorkletGlobalScope: the current ToT (3.1.48-git) fixes this bug.
  • Undefined 'Module._malloc(): using s EXPORTED_FUNCTIONS...` fixes this issue.

References:

@padenot
Copy link

padenot commented Nov 3, 2023

3. Why the abstraction of AudioContext was necessary? The significant effort went into this abstraction/wrapper. Althought I think it's brilliant, I would like to understand the rationale (why/use case) of the design.

This thin wrapper looks like bindings with a C ABI to interact with the Web Audio API them from native code, that's about it, no?

It's probably possible to use AudioWorklet in emscripten without SharedArrayBuffer, but this looks like it stems from requiring a build with WASM_WORKER.

I suppose it could work without it (much like we used AudioWorklet without SAB before) probably just a matter of someone doing it.

But in the grand scheme of things, taking the time to go native but only doing 50% of the job by doing all communications with postMessage feels a bit strange, as you don't really two things you're probably after: (1) execution speed (2) determinism. You only get WASM as a compilation target, that's it.

@juj
Copy link

juj commented Nov 3, 2023

This option forces the usage of SharedArrayBuffer.
The usage of ShardArrayBuffer implies a ring buffer (or something similar) is being used under the hood. Is this design documented somewhere? What's the size of the buffer? How much of latency is expected? Or is there anyway to customize the size of the buffer?

There is no ring buffer.

Overall, there seems to be a confusion here, let's wind back a bit.

The reason why the Audio Worklets support provided by Emscripten requires shared memory is because that is the only configuration that needs to be tended by Emscripten itself to become supportable.

To illustrate: if one wants to use Audio Worklets with Emscripten, there are generally three possible strategies or meanings that this could take:

  1. one writes the Audio Worklet code fully in JS as a standalone .js file (by hand), and then the main web page contains the Emscripten compiled Webassembly program. (unshared memory, message port postMessage() communication)
  2. one would compile the Audio Worklet code in .wasm using Emscripten, that would produce a standalone .wasm module that is loaded into the Audio Worklet, and then one compiles a separate Emscripten codebase for the main program .wasm. (two unshared memory modules, message port postMessage() communication)
  3. one builds a single Wasm module that is shared across the Audio Worklet and main web page, and uses low latency shared memory to communicate between the two. (shared memory, lock-free atomics on wasm shared mem communication)

The Wasm Audio Worklets feature is point 3. above.

If you wanted to instead have 1. above, there is nothing required in core Emscripten to support this. You can already implement this since a few years ago, go nuts and have fun. The general techniques of embedding an Emscripten compiled page to a surrounding web site and the existing --js-library, --pre-js and --shell-file mechanisms already allow you to fully realize this mode.

If you wanted to have 2. above, likewise these are just two separate Emscripten-compiled .wasm modules, and nothing again is required from Emscripten repository to realize this, except to cover for possible audio scope differences/"bugs" like that atob being missing in AudioWorkletScope is. Go nuts and have fun. (using -sSINGLE_FILE=1 mode, compiling to .js instead of .html and omitting main() will be needed here. -sMINIMAL_RUNTIME=1 can also help to get the code size squeezed down to absolute minimum).

So 1. and 2. should be covered under "you can already go and do it with existing techniques". If there are issues, then those could be looked as bug reports.

(Sidenote, a drawback of both modes above is that they will require resorting to only high latency postMessage()ing for communication, and a drawback of the second mode is that it results in two separate unshared .wasm modules, and doesn't allow for any sharing of code or memory. All shared code will be compiled twice into the web page. Both .wasm files will have their own memory spaces, so all buffers will need to be deep copied over. I would not recommend people to do 1. or 2. except for small-ish programs)

This leaves tending to mode 3. which is the only mode with hard dependencies that core integration from Emscripten is required. It can not be achieved by users unless Emscripten itself provides integrated support for it, because the way that shared memory and modules work, the core Emscripten runtime loader must account for it. However it does not mean that by Emscripten having 3. you couldn't go for 1. or 2. if you wanted to.

Maybe the fact that support for 3. landed, it got people thinking that that way *must*\ be used, and people couldn't do 1. or 2. above on their own?

To reiterate, the only reason why Emscripten provides 3. out of the box, and not 1. or 2. is because no developer can do 3. otherwise, unless they forked Emscripten and hacked its guts to pull it off. It just requires seamless integration from Emscripten to work.

@juj
Copy link

juj commented Nov 3, 2023

Why the abstraction of AudioContext was necessary?

WebAssembly code needs to have some kind of way to reference a JS AudioContext object in WebAssembly side. The canonical solution here is to use a handle-based approach where an array/map of objects is used to establish an integer handle namespace, that the Wasm side code will then use. For Audio Worklets code, this integer handle namespace is implemented in this dictionary EmAudio.

(there is Reference Types for WebAssembly proposal, though that is a separate story)

You don't need to use any of the provided C API functions highlighted here: https://github.com/emscripten-core/emscripten/blob/e2e62437243f4c0208b5c6a8e51654bfffe3ed73/system/include/emscripten/webaudio.h#L28-L60 if you don't wish to.

I wrote those functions to solve two problems in one go:
a) make it easier for native C/C++ developers to get started, because I know that if I didn't, then other set of people would complain "why do I need to write any JS code to make this work? I just want to write this all in C/C++".
b) to provide an API that the required Emscripten Audio Worklet unit tests can use

Rather than write ad hoc code just for test files that aren't supposed to be used, I wrote the test code in a way of an API that then at least some people could just reuse the same primitives that were part of the tests. Emscripten has a good history of providing "monkey copy-paste from test code examples" to the users, so this strategy helps here too.

If those functions don't fit your needs, then you are free to not use them. The only requirement you have is to use the JS function emscriptenRegisterAudioObject in your own JavaScript to acquire a handle integer to pass to wasm side that the wasm code would then understand.

Using these functions however is necessary, as they are the core part of the API to create wasm audio processors and nodes.

Undefined atob() in AudioWorkletGlobalScope

This was discussed also in WebAudio/web-audio-api#2553

Undefined 'Module._malloc()

This does not seem like an AudioWorklet issue. I took a great deal of care to make sure that the Wasm Audio Worklets implementation would not need to do any dynamic memory allocation. All Wasm Audio Worklet memory allocation is done by a lightning fast batched together stack allocation at https://github.com/emscripten-core/emscripten/blob/e2e62437243f4c0208b5c6a8e51654bfffe3ed73/src/audio_worklet.js#L56-L57 .

Also double-checking through the rest of the JS code, I don't see any uses of malloc having crept in there:

https://github.com/emscripten-core/emscripten/blob/main/src/audio_worklet.js
https://github.com/emscripten-core/emscripten/blob/main/src/library_webaudio.js

@rbdannenberg
Copy link

Emscripten has a good history of providing "monkey copy-paste from test code examples" to the users

I hope Emscripten doesn't think that very limited code examples are either adequate or a substitute for documentation. When you make statements like you are free not to use them, that implies that there are discoverable alternatives. Where's the documentation? For example, where would one learn the semantics of emscriptenRegisterAudioObject? The link is vague and incomplete. I guess the fact that Emscripten has so much potential is what makes it so frustrating that code surrounding it is so opaque.

@juj
Copy link

juj commented Nov 3, 2023

Tone check, please. Your complaints read to me like you are disappointed that good is not perfect. When your tone is plain negative and you assume ill intent, it is hard to find interest to help you solve your computing problems. You write as if you had contracted me to solve these issues for you to your satisfaction.

Emscripten is an open project, and you are free to contribute to improve it to become better.

@rbdannenberg
Copy link

I apologize if this sounded so negative. I even took care to acknowledge the clear value of emscripten. I did attempt to contribute by documenting practical problems in using emscripten (mentioned above as @rbdannenberg's blog post). Also, I should clarify when I said "code surrounding it [emscripten]," I didn't mean only emscripten -- it's true of almost everything I've seen describing audio worklets and browser internals. I'm genuinely curious how you or anyone working on this gets past this knowledge barrier. Is it just me?

@hoch
Copy link
Member Author

hoch commented Nov 6, 2023

@padenot

But in the grand scheme of things, taking the time to go native but only doing 50% of the job by doing all communications with postMessage feels a bit strange, as you don't really two things you're probably after: (1) execution speed (2) determinism. You only get WASM as a compilation target, that's it.

Can you explain a bit more on this? Is this comment for the examples in this repository or the Emscripten's WASM Audio Worklet API?

@hoch
Copy link
Member Author

hoch commented Nov 6, 2023

Also Re:@juj,

I should have started by expressing my gratitude for your contribution to Emscripten. I am confident that the WASM Audio Worklet API enables a wide range of use cases. The official documentation does not focus on "why this design", so I am happy to have your rationale here.

WebAssembly code needs to have some kind of way to reference a JS AudioContext object in WebAssembly side.

I am guessing this is for accessing destination, sampleRate, channelCount. Or are there something else?

If those functions don't fit your needs, then you are free to not use them. The only requirement you have is to use the JS function emscriptenRegisterAudioObject in your own JavaScript to acquire a handle integer to pass to wasm side that the wasm code would then understand.

Absolutely. This code location and your suggestion is immensely helpful here!

Also double-checking through the rest of the JS code, I don't see any uses of malloc having crept in there:
https://github.com/emscripten-core/emscripten/blob/main/src/audio_worklet.js
https://github.com/emscripten-core/emscripten/blob/main/src/library_webaudio.js

You're correct. The _malloc() issue was just an artifact from the latest Emscripten patch, not from WASM Audio Worklet API.

Like I said, I didn't expect a actionable response from this discussion. The latest Emscripten (including WASM AudioWorklet API) did break some of existing examples out there and those seemed ignored during the development of new capabilities. I hope this discussion thread is helpful for anyone having similar issues.

Thanks for your comment!

@hoch
Copy link
Member Author

hoch commented Nov 6, 2023

Re @rbdannenberg:

I didn't mean only emscripten -- it's true of almost everything I've seen describing audio worklets and browser internals. I'm genuinely curious how you or anyone working on this gets past this knowledge barrier. Is it just me?

I feel responsible for this point. Although I believe @padenot, myself, and a small group of people contributed to the documentations and examples for AudioWorklet, perhaps it was not good enough. If you have constructive feedback on how we can improve web.dev or MDN, we will definitely take notes from it.

@padenot
Copy link

padenot commented Nov 8, 2023

Can you explain a bit more on this? Is this comment for the examples in this repository or the Emscripten's WASM Audio Worklet API?

I meant that I personally feel that when going the emscripten (or otherwise WASM) route for real-time audio, using SharedArrayBuffer is a must. As such, emscripten using shared memory and making it a requirement if one wants to benefit from the built-in facilities does not shock me.

We've been using emscripten / wasm for real-time audio for a long time "manually" as @juj says, and afaik it still works well (my examples and demos still run at least).

So I guess it was more of a comment on emscripten's design (which I find sensible).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants