RustAudio / coreaudio-sys

Raw bindings to the OSX CoreAudio framework generated by bindgen (see coreaudio-rs for a more rust-esque wrapper).
MIT License
69 stars 38 forks source link

Fixes iOS compiling. #33

Closed simlay closed 4 years ago

simlay commented 4 years ago

It's unclear if this finishes #29 but this deals with a pretty annoying edge case when building foraarch64-apple-ios as described here: rust-lang/rust-bindgen#1211.

Also, you might notice that rustup target list | grep ios yields something like:

aarch64-apple-ios (installed)
armv7-apple-ios (installed)
armv7s-apple-ios (installed)
i386-apple-ios (installed)
x86_64-apple-ios (installed)

The i386 and x86_64 targets don't have architecture support in the iPhoneOS13.2.sdk it seems. I can share the stack trace if you'd like. For now compiling for aarch64, armv7 and armv7s I think is more than enough.

simlay commented 4 years ago

I've updated this PR to use the objective-c feature found in https://github.com/rust-lang/rust-bindgen/pull/1702. We should wait for that to land and be pushed to crates.io to actually merge.

catt-io commented 4 years ago

Hi @simlay, cheers for looking into iOS support. When I try to build this for aarch64-apple-ios I get the following error:

error: failed to run custom build command for `coreaudio-sys v0.2.3 (https://github.com/simlay/coreaudio-sys.git?branch=add-ios-support#be5a43d2)`

Caused by:
  process didn't exit successfully: `.../target/release/build/coreaudio-sys-abd2e647f3203db2/build-script-build` (signal: 6, SIGABRT: process abort signal)
--- stderr
dyld: Library not loaded: @rpath/libclang.dylib
  Referenced from: .../target/release/build/coreaudio-sys-abd2e647f3203db2/build-script-build
  Reason: image not found

Xcode version is 11.3 and xcrun output is:

xcrun --sdk iphoneos --show-sdk-path
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.2.sdk

Same error with x86_64-apple-darwin, which the master branch is building fine for.

simlay commented 4 years ago

Hi @simlay, cheers for looking into iOS support. When I try to build this for aarch64-apple-ios I get the following error: Hmm... That looks like an issue with llvm/clang. Maybe brew reinstall llvm?

mitchmindtree commented 4 years ago

Thanks so much for your work on iOS support @simlay!

Unfortunately I don't have the means to test macOS and iOS at the moment. If we can get travis building successfully and a tick of approval from another macOS/iOS user I'd be happy to merge!

Also, would you be interested in being added as a collaborator at both the coreaudio-rs and coreaudio-sys crates? I realise it can be tricky finding iOS testers/reviewers and prefer you weren't endlessly blocked by this!

simlay commented 4 years ago

I’ll fix the build. This PR is dependent on a new release of rust-bindgen to be published on crates.io as more objective-c support has been added.

slmjkdbtl commented 4 years ago

Is there any update on this? @simlay 's fixes works and I wonder when can this get merged?

simlay commented 4 years ago

Is there any update on this? @simlay 's fixes works and I wonder when can this get merged?

So, I've added a ton to rust-bindgen in the last 8 months that makes the sys bindings much better but would also be breaking to whatever uses it to generate. If you're familiar with UIKit, I've got a WIP bindings crate for uikit-sys.

I'd like to wait until we have https://github.com/rust-lang/rust-bindgen/pull/1847 and https://github.com/rust-lang/rust-bindgen/pull/1784 as those kinda complete the objective-c binding generation from what I can tell.

slmjkdbtl commented 4 years ago

@simlay I see. Thanks a lot for your work on those iOS related subjects!

simlay commented 4 years ago

I've tested this feature branch out again with Xcode 12 (using the latest iOS SDK) on macOS 10.15.6 and for some unclear reason, I get the following stacktrace when compiling for aarch64-apple-ios:

~/projects/coreaudio-sys (add-ios-support)$ cargo build --target aarch64-apple-ios                                                                                                                                                                          
   Compiling coreaudio-sys v0.2.3 (/Users/simlay/projects/coreaudio-sys)
error[E0204]: the trait `Copy` may not be implemented for this type
     --> /Users/simlay/projects/coreaudio-sys/target/aarch64-apple-ios/debug/build/coreaudio-sys-b95864b357b6f856/out/coreaudio.rs:31119:17
      |
31119 | #[derive(Debug, Copy, Clone)]
      |                 ^^^^
31120 | pub struct AudioUnitRenderContext {
31121 |     pub workgroup: os_workgroup_t,
      |     ----------------------------- this field does not implement `Copy`
      |
      = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

For more information about this error, try `rustc --explain E0204`.
error: could not compile `coreaudio-sys`.

To learn more, run the command again with --verbose.

I don't have this issue when targeting x86_64-apple-ios which is pretty annoying.

MichaelHills commented 4 years ago

@simlay here's the patch of this PR I used when I got audio working on bevy -> rodio -> cpal. I rebased on master, which was 8d27848335dea18129dffe60643572497cc42d6f, and then disabled the objective C stuff. So given that this works, maybe we can push to land this as a first pass?

+        //builder = builder.clang_args(&["-x", "objective-c", "-fblocks"]);
+        builder = builder.objc_extern_crate(false);
+        builder = builder.generate_block(false);
+        builder = builder.block_extern_crate(false);

Full diff

diff --git a/Cargo.toml b/Cargo.toml
index cac8dc3..55779a5 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -10,8 +10,12 @@ homepage = "https://github.com/RustAudio/coreaudio-sys"
 repository = "https://github.com/RustAudio/coreaudio-sys.git"
 build = "build.rs"

+[target.'cfg(target_os = "ios")'.dependencies]
+objc = "0.2.7"
+block = "0.1.6"
+
 [build-dependencies.bindgen]
-version = "0.53"
+version = "0.55"
 default-features = false
 features = ["runtime"]

diff --git a/build.rs b/build.rs
index 3a7a80e..e1494c8 100644
--- a/build.rs
+++ b/build.rs
@@ -11,7 +11,12 @@ fn sdk_path(target: &str) -> Result<String, std::io::Error> {

     let sdk = if target.contains("apple-darwin") {
         "macosx"
-    } else if target.contains("apple-ios") {
+    } else if target == "x86_64-apple-ios" || target == "i386-apple-ios" {
+        "iphonesimulator"
+    } else if target == "aarch64-apple-ios"
+        || target == "armv7-apple-ios"
+        || target == "armv7s-apple-ios"
+    {
         "iphoneos"
     } else {
         unreachable!();
@@ -37,7 +42,7 @@ fn build(sdk_path: Option<&str>, target: &str) {
     use std::env;
     use std::path::PathBuf;

-    let mut headers: Vec<&'static str> = vec![];
+    let mut headers: Vec<&str> = vec![];

     #[cfg(feature = "audio_toolbox")]
     {
@@ -47,14 +52,19 @@ fn build(sdk_path: Option<&str>, target: &str) {

     #[cfg(feature = "audio_unit")]
     {
-        println!("cargo:rustc-link-lib=framework=AudioUnit");
+        println!("cargo:rustc-link-lib=framework=AudioToolbox");
         headers.push("AudioUnit/AudioUnit.h");
     }

     #[cfg(feature = "core_audio")]
     {
         println!("cargo:rustc-link-lib=framework=CoreAudio");
-        headers.push("CoreAudio/CoreAudio.h");
+
+        if target.contains("apple-ios") {
+            headers.push("CoreAudio/CoreAudioTypes.h");
+        } else {
+            headers.push("CoreAudio/CoreAudio.h");
+        }
     }

     #[cfg(feature = "open_al")]
@@ -81,11 +91,31 @@ fn build(sdk_path: Option<&str>, target: &str) {

     builder = builder.size_t_is_usize(true);

+    // See https://github.com/rust-lang/rust-bindgen/issues/1211
+    // Technically according to the llvm mailing list, the argument to clang here should be
+    // -arch arm64 but it looks cleaner to just change the target.
+    let target = if target == "aarch64-apple-ios" {
+        "arm64-apple-ios"
+    } else {
+        target
+    };
+
     builder = builder.clang_args(&[&format!("--target={}", target)]);

     if let Some(sdk_path) = sdk_path {
         builder = builder.clang_args(&["-isysroot", sdk_path]);
     }
+    if target.contains("apple-ios") {
+        //builder = builder.clang_args(&["-x", "objective-c", "-fblocks"]);
+        builder = builder.objc_extern_crate(false);
+        builder = builder.generate_block(false);
+        builder = builder.block_extern_crate(false);
+        builder = builder.rustfmt_bindings(true);
+        // time.h as has a variable called timezone that conflicts with some of the objective-c
+        // calls from NSCalendar.h in the Foundation framework. This removes that one variable.
+        builder = builder.blacklist_item("timezone");
+        builder = builder.blacklist_item("objc_object");
+    }

     let meta_header: Vec<_> = headers
         .iter()
@@ -95,9 +125,7 @@ fn build(sdk_path: Option<&str>, target: &str) {
     builder = builder.header_contents("coreaudio.h", &meta_header.concat());

     // Generate the bindings.
-    builder = builder
-        .trust_clang_mangling(false)
-        .derive_default(true);
+    builder = builder.trust_clang_mangling(false).derive_default(true);

     let bindings = builder.generate().expect("unable to generate bindings");
mitchmindtree commented 4 years ago

Thanks so much for all your work on this folks!

@simlay & @MichaelHills, just let me know if you'd like to become a co-maintainer of the repo and crate - I don't want my lack of time or the fact I don't have a Mac anymore to hold any progress back here.

MichaelHills commented 4 years ago

@mitchmindtree probably a good idea for one or both of us to be a co-maintainer especially since you don't have a Mac. @simlay is very active in the Rust iOS community and has been for a long time it seems. :) I have been focused on just getting the bevy game engine fully working on iOS and audio is the last piece of the puzzle.

Given the amount of effort I've spent getting audio working on iOS, it makes sense for me to help continue to refine it. Would love to have someone else help me test though!

In case you didn't see it, I have this up on coreaudio-rs https://github.com/RustAudio/coreaudio-rs/pull/72 Which RustAudio projects do you currently maintain?

mitchmindtree commented 4 years ago

Thanks @MichaelHills, I've invited you and @simlay to a rustaudio:coreaudio-maintainers group which should give maintainer access to both coreaudio repos along with the ability to publish to both as well.

Also, manually publishing coreaudio-sys should rarely be necessary as we have github actions that should auto-publish when a commit lands in master that updates the version - it might be worth moving coreaudio-rs from travis onto a similar github workflow?

MichaelHills commented 4 years ago

Thanks @mitchmindtree, auto publish for coreaudio-rs sounds good though not necessary. I'm a bit time poor after the big push I did on bevy iOS so will focus my efforts on coreaudio-rs and cpal actual code changes for the time being. Might chase up some testers to help me test. :)

MichaelHills commented 4 years ago

@simlay we should publish a new version so that https://github.com/RustAudio/coreaudio-rs/pull/72 can be landed and subsequently https://github.com/RustAudio/cpal/pull/485.

I'm not familiar with the release process of rust crates and versioning. What should the new version number be?

MichaelHills commented 4 years ago

Might be nice to also clean up all the warnings. One thing at a time I guess.

simlay commented 4 years ago

Might be nice to also clean up all the warnings. One thing at a time I guess.

You mean the 128 warnings emitted that are pretty much all warning:externblock uses typeu128, which is not FFI-safe? I'm pretty sure the reason why those are emitted is because of https://github.com/rust-lang/rust/issues/54341

I'm not familiar with the release process of rust crates and versioning. What should the new version number be?

Well, for everyone else in the non-iOS rust ecosystem this is a pretty small change and it's non breaking to anything. I'd say going from 0.2.5 to 0.2.6 is reasonable.

MichaelHills commented 4 years ago

Might be nice to also clean up all the warnings. One thing at a time I guess.

You mean the 128 warnings emitted that are pretty much all warning:externblock uses typeu128, which is not FFI-safe? I'm pretty sure the reason why those are emitted is because of rust-lang/rust#54341

Oh maybe I meant in coreaudio-rs, there's a bunch of try! and dyn trait warnings. Looks like there's nothing we can do about the u128 ones at the moment?