jpernst / alto

Idiomatic Rust bindings for OpenAL 1.1 and extensions (including EFX).
Apache License 2.0
80 stars 28 forks source link

Building alto on Emscripten #30

Closed yoanlcq closed 7 years ago

yoanlcq commented 7 years ago

I recently tried to build a dummy project listing alto as one of its dependencies, targeting Emscripten with --target asmjs-unknown-emscripten, and it failed because libloading does not (yet) support Emscripten.
I had reported this, but in the meantime, al-sys actually does not need the libloading dependency when target Emscripten, because there, "out-of-the-box" support for OpenAL is claimed (see this issue).

My suggestion : When the target is Emscripten, fall back to simply declaring the FFI bindings, and disable the libloading dependency so that it builds successfully.

jpernst commented 7 years ago

Yeah, this is a problem I've been meaning to tackle for a while now, but haven't gotten around to it yet. When I'm done with the redesign of rental I'm going to take another pass at alto/al-sys to update them, and I'll add support for this then, unless someone PRs it first.

jpernst commented 7 years ago

Ok, I've added support for the emscripten target to al-sys 0.4, it will now use static symbols for that target. I don't have the environment set up to test it, though. If you try it in your app and it works, then I'll close this.

yoanlcq commented 7 years ago

Alright, I've come up with a basic test for this, and it turns out that it works, with some caveats.
The thing is that Emscripten's OpenAL implementation appears to be slightly incomplete, (e.g alcGetEnumValue() isn't defined) and as a result, normal use of al-sys and alto may cause linker errors.

Otherwise, stuff appears to work. Running said test (after commenting out the lines that cause linker errors) using

cargo build --target asmjs-unknown-emscripten -v
node target/asmjs-unknown-emscripten/debug/alto-emscripten-example.js

gave me :

ALC Version: 1.1
Default device : emsc
OpenAL version : 1.1
OpenAL vendor : Emscripten
OpenAL renderer: WebAudio

Which looks pretty good. Didn't dig any further (e.g testing playback) because I don't have much time (embedding+loading files on Emscripten is a bit tedious even in C, etc).

Since the problem doesn't come from alto anymore, I think you can indeed close this, and I'll proceed to report an issue on Emscripten's repo. EDIT : Actually, perhaps this should be left open - It may not be alto's fault, but the core problem remains until Emscripten's OpenAL is complete. About this, it seems they would appreciate pull requests (relevant issues : this and this).

jpernst commented 7 years ago

Yeah, it's reasonable to leave this open until the issue is resolved, whatever the cause. I plan to explore emscripten more in the future, so perhaps I'll look into an upstream contribution at that point, unless someone beats me to it.

jpernst commented 7 years ago

I think at long last we can close this. Thanks for your help with the implementation!

yoanlcq commented 7 years ago

Thanks, but I wonder, was the pull request all there is to it ? I mean, does Alto have tests or examples for Emscripten which would prove that this issue is resolved for good ?

jpernst commented 7 years ago

I have a few emscripten specific tests, but they require a bunch of boilerplate to set up, so I don't really want to distribute them with alto itself. As far as alto is concerned emscripten is just another OpenAL impl like any other (now, at least). The only code specific to it is the special case for symbol lookup in al-sys.

ndarilek commented 6 years ago

Sorry to rehash this one, but what is the recommended current fix for:

          error: unresolved symbol: alcGetEnumValue

I'm hitting this one now and am scratching my head. Tried adding -lopenal to my linker arguments but it makes no difference, and I don't even know if I'm doing it correctly.

https://gitlab.com/ndarilek/rust-wasm-gamedev Trying to build with cargo-web 0.4.

Thanks.

yoanlcq commented 6 years ago

Hi, (I'm setting up my environment to try to reproduce this. It's taking quite a while. In the meantime...)

alcGetEnumValue() should not cause linker errors since our pull request to Emscripten was merged; if it does, it's likely to be the same for all other OpenAL functions, so the problem is at a lower level.

I think the following could help (perhaps I'm asking for too many irrelevant details, but it doesn't hurt):

Also, on our side, I see no #[link="openal"] (or something) whatsoever (the relevant part would be here).
It used to be that linking OpenAL was implicit in Emscripten, but not anymore as of the latest versions. Therefore, perhaps we do miss something. I'll try and dig further.

ndarilek commented 6 years ago

Thanks for the quick response!

  1. rustc 1.22.1 (05e2e1c41 2017-11-22) Stable.
  2. emcc (Emscripten gcc/clang-like replacement) 1.37.21 ()
  3. I've tried every target.
  4. Yes, though I think cargo-web may obsolete much of it as it downloads/sets up emscripten for you. Either way, I've tried with emsdk and cargo-web's own emscripten download, same results. I'm currently using the cargo-web implementation since it sets up in seconds, though if that doesn't work for whatever reason then I can switch back to emsdk. cargo-web's own setup seems complete and is much easier to maintain.
yoanlcq commented 6 years ago

So I'm using rustc 1.24.0 Nightly, emcc is 1.37.21 and target asmjs-unknown-emscripten (and cargo-web 0.4) and the issue does indeed show up.
This Emscripten version is quite recent (circa December 2017) so as a user I'd expect stuff to work fine; Therefore, I'd be in favor or re-opening this issue until we find a fix.

I'm not very up-to-date myself with what happened besides our PR, but it was merged by the end of September; I wonder if it "made it" to a public release (probably yes, but in all cases, using the upstream Emscripten is worth considering).
Still digging...

jpernst commented 6 years ago

Emscripten 1.37.21 was cut on August 31, so it's not surprising that our PR isn't in that release.

yoanlcq commented 6 years ago

Then that should be it - also, my bad, I actually inferred the version's release date by looking at v1.37.23 in the changelog, wrongly thinking that a difference of 2 in the patch version meant they ought to be close in time.

(I'll still try to see if I can get the project to compile using the latest Emscripten or something, for my own sake)

ndarilek commented 6 years ago

Hey, thanks for catching that! I wasn't sure when these changes had merged relative to the Emscripten release cycle.

Looks like cargo-web master supports the latest emscripten as of a few hours ago. I'll give that a shot.

ndarilek commented 6 years ago

Confirming that this works with cargo-web via git, as it installs the newest Emscripten. Thanks for catching my silly oversight! :)