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

emscripten support is broken (since wasm-bindgen does not work there?) #810

Open
benjamin-sieffert opened this issue Nov 13, 2023 · 7 comments

Comments

@benjamin-sieffert
Copy link

benjamin-sieffert commented Nov 13, 2023

When I try to use cpal (via rodio) in an emscripten build, I first need to apply this patch:

--- a/src/lib.rs
+++ b/src/lib.rs
@@ -243,7 +243,7 @@ impl wasm_bindgen::describe::WasmDescribe for BufferSize {
 
 #[cfg(target_os = "emscripten")]
 impl wasm_bindgen::convert::IntoWasmAbi for BufferSize {
-    type Abi = wasm_bindgen::convert::WasmOption<u32>;
+    type Abi = Option<u32>;
     fn into_abi(self) -> Self::Abi {
         match self {
             Self::Default => None,

Then I can compile and run into the following error:
"cannot call wasm-bindgen imported functions on non-wasm targets" @ /home/ax/.cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.64/src/features/gen_AudioContext.rs:5
Afaik, wasm-bindgen fundamentally does not work on emscripten, at least not without jumping through a lot of hoops.

Before #713 , "stdweb" was used, which advertises direct emscripten support; so it probably (somewhat) worked at some point.
Unfortunately the old version(s) can no longer be compiled, due to various other changes in rustc/libc/emscripten.

According to the emscripten docs, "OpenAL" should be the best way to get audio in that environment: https://emscripten.org/docs/porting/Audio.html

@benjamin-sieffert
Copy link
Author

Since it appears I am the only developer on emscripten, I suggest simply removing the advertised emscripten support from the docs, to not bait any more victims. ;)
As for myself, I will see if I can’t just use the SDL2_mixer extension.

@alisomay
Copy link
Contributor

alisomay commented Nov 18, 2023

#809 (comment)
I realized that also and suggested a fix. I can make a PR but I don't use emscripten and I don't know how the optional type translation to wasm is used in the wild?
Do you mind checking it out?

@est31
Copy link
Member

est31 commented Nov 23, 2023

Maybe if @alisomay made a PR, @benjamin-sieffert could try it out? Then we can see if we can merge it or not.

@benjamin-sieffert
Copy link
Author

@alisomay @est31
I think this lone compilation failure is not the main issue with emscripten. The whole cpal-for-emscripten implementation is based on wasm-bindgen through and through, but that doesn’t work on emscripten at runtime!! It compiles because the compiler finds the right cpu arch ("wasm32"), but in practice the environment created by the emscripten wrapper inside the browser does look very different than the normal wasm environment.
https://rustwasm.github.io/wasm-bindgen/reference/rust-targets.html

The wasm-bindgen target does not support the wasm32-unknown-emscripten nor the asmjs-unknown-emscripten targets. There are currently no plans to support these targets either. All annotations work like other platforms on the targets, retaining exported functions and causing all imports to panic.

I can try your patch instead of mine, but I doubt it helps with the runtime failure.
My own patch honestly seems saner to me, as I assume WasmOption has been removed simply because support for plain Option was added somewhere in the stack; note that the trait requires type Abi to implement WasmAbi, so this is already implemented for Option<u32> somewhere and we probably don’t need to do any further gymnastics.

@alisomay
Copy link
Contributor

alisomay commented Nov 24, 2023

I see.. I'm also alien to emscripten and just tried to follow common sense quickly.
Please let me know how it goes.

@est31
Copy link
Member

est31 commented Dec 17, 2023

Good points, there is something deeply flawed in our CI jobs then... maybe we should just remove them?

@djtuBIG-MaliceX
Copy link

Just posting here to say that I am an occasional emscripten user myself exploring what's available for rust. Attempting to compile an application that uses rodio which therefore pulls in cpal using wasm32-unknown-emscripten target will fail with the following error:

   Compiling cpal v0.15.2
error[E0412]: cannot find type `WasmOption` in module `wasm_bindgen::convert`
   --> C:\Users\djtub\.cargo\registry\src\index.crates.io-6f17d22bba15001f\cpal-0.15.2\src\lib.rs:246:39
    |
246 |     type Abi = wasm_bindgen::convert::WasmOption<u32>;
    |                                       ^^^^^^^^^^ not found in `wasm_bindgen::convert`

error: aborting due to previous error

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

No branches or pull requests

4 participants