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

Added initial changes for iOS support. #69

Closed
wants to merge 1 commit into from

Conversation

simlay
Copy link
Member

@simlay simlay commented Jan 7, 2020

The only real change here I think is the usage of:
kAudioSessionProperty_CurrentHardwareIOBufferDuration rather than kAudioDevicePropertyBufferFrameSize which I found at https://stackoverflow.com/questions/13157523/kaudiodevicepropertybufferframesize-replacement-for-ios

Most of these changes are conditional compilation things and mapping the names of the iOS version of the bindings.

This is obviously dependent on RustAudio/coreaudio-sys#33

@endragor
Copy link

I've tried to use this commit along with RustAudio/coreaudio-sys#33, but compilation failed for me with:

coreaudio.rs:20550:5
      |
20550 |     pub isa: Class,
      |     ^^^^^^^^^^^^^^ `Class` cannot be formatted using `{:?}`
      |
      = help: the trait `std::fmt::Debug` is not implemented for `Class`
      = note: add `#[derive(Debug)]` or manually implement `std::fmt::Debug`
      = note: required because of the requirements on the impl of `std::fmt::Debug` for `&Class`
      = note: required for the cast to the object type `dyn std::fmt::Debug`
      = 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

Will this be fixed along with the rust-bindgen fixes you mentioned in RustAudio/coreaudio-sys#33? Is there a workaround you could suggest?

Thank you for these PRs! I hope they will be merged soon.

@simlay
Copy link
Member Author

simlay commented Aug 18, 2020

Will this be fixed along with the rust-bindgen fixes you mentioned in RustAudio/coreaudio-sys#33? Is there a workaround you could suggest?

Hmm. @endragor, I think the issue is resolved with blacklisting objc_object but you're getting a slightly different stack trace than me.

I'm seeing this:

   Compiling coreaudio-sys v0.2.3 (/Users/simlay/projects/rust-audio/coreaudio-sys)
error[E0204]: the trait `Copy` may not be implemented for this type
     --> /Users/simlay/projects/rust-audio/coreaudio-rs/target/x86_64-apple-ios/debug/build/coreaudio-sys-4d76fa2d6fd973c6/out/coreaudio.rs:36699:17
      |
36699 | #[derive(Debug, Copy, Clone)]
      |                 ^^^^
36700 | pub struct objc_object {
36701 |     pub isa: objc::runtime::Object,
      |     ------------------------------ 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.

The line numbers for the generated bindings will depend a lot on your version of macOS, iOS SDKs, and LLVM version. The different iOS SDKs have different new or deprecated or deleted bits of the objective-c framework headers.

I wish I could tell you what objc_object does but I can't find a use for it so unlikely to be an issue. This was kind of introduce due to rust-lang/rust-bindgen#1722. I think it's well worth the benefits of the structured generated bindings. I try to test out most of my objective-c additions to bindgen in simlay/bindgen-test-ios-frameworks.

I've updated my add-ios-support branch to blacklist objc_object as well as use rust-lang/rust-bindgen#1784. I've been playing around with this stuff in uikit-sys and try to make sure it works. If there's an issue, I'm interested in knowing. I'm all for having more knowledge around what's not working.

Anyway, I've been testing this branch out with cargo-bundle in the example/sine.rs by adding:

[package.metadata.bundle.example.sine]
name = "sine"
identifier = "com.coreaudio.rs.sine"
category = "Utility"
short_description = "An example of a bundled application"
long_description = """
A trivial application that just displays a blank window with
a title bar.  It serves as an example of an application that
can be bundled with cargo-bundle, as well as a test-case for
cargo-bundle's support for bundling crate examples.
"""

But have been having the error of:
Error: NoMatchingDefaultAudioUnitFound in the console. So, I'm not so sure how well things will work here yet.

@MichaelHills
Copy link
Contributor

@simlay here's the patch I used of this PR for when I got audio working on bevy -> rodio -> cpal. I trimmed out all unnecessary lines, and I tried to implement the replacement of kAudioDevicePropertyBufferFrameSize with kAudioSessionProperty_CurrentHardwareIOBufferDuration correctly, but honestly I don't know if it works properly. I don't think this function set_input_callback got exercised during my testing. I have the feeling this is for recording microphone input?

diff --git a/src/audio_unit/render_callback.rs b/src/audio_unit/render_callback.rs
index e0c5668..46ced27 100644
--- a/src/audio_unit/render_callback.rs
+++ b/src/audio_unit/render_callback.rs
@@ -482,8 +482,20 @@ impl AudioUnit {
         // Pre-allocate a buffer list for input stream.
         //
         // First, get the current buffer size for pre-allocating the `AudioBuffer`s.
-        let id = sys::kAudioDevicePropertyBufferFrameSize;
-        let mut buffer_frame_size: u32 = self.get_property(id, Scope::Global, Element::Output)?;
+        #[cfg(target_os = "macos")]
+        let mut buffer_frame_size: u32 = {
+            let id = sys::kAudioDevicePropertyBufferFrameSize;
+            let buffer_frame_size: u32 = self.get_property(id, Scope::Global, Element::Output)?;
+            buffer_frame_size
+        };
+        #[cfg(target_os = "ios")]
+        let mut buffer_frame_size: u32 = {
+            let id = sys::kAudioSessionProperty_CurrentHardwareIOBufferDuration;
+            let seconds: f32 = self.get_property(id, Scope::Global, Element::Output)?;
+            let id = sys::kAudioSessionProperty_CurrentHardwareSampleRate;
+            let sample_rate: f64 = self.get_property(id, Scope::Global, Element::Output)?;
+            (sample_rate * seconds as f64).round() as u32
+        };
         let mut data: Vec<u8> = vec![];
         let sample_bytes = stream_format.sample_format.size_in_bytes();
         let n_channels = stream_format.channels_per_frame;

@simlay
Copy link
Member Author

simlay commented Sep 27, 2020

@MichaelHills Could you push that code to a branch/fork? You've mentioned that you've rebased so applying the raw diff seems error prone. I'm also fine with closing this PR and you can open a new PR. Thoughts?

@MichaelHills
Copy link
Contributor

I have opened up a draft #72 until I can test the changes. I'll just need to figure out how to exercise this code, perhaps by getting an example going using this crate directly rather than using cpal and rodio.

@simlay
Copy link
Member Author

simlay commented Sep 28, 2020

I'll just need to figure out how to exercise this code, perhaps by getting an example going using this crate directly rather than using cpal and rodio.

I'd probably publish PRs to rodio and/or cpal and have an example Xcode project that uses them showing the changes to coreaudio-sys and coreaudio-rs work. There are pros and cons to this because when a given update to Xcode gets published, the coreaudio-sys bindings will change bit by bit and if CI is 3 crates up, it'll be hard to track down. There eventually be enough people that when it breaks, someone will notice and figure out how to fix it.

For now, given that there's no iOS support for all four crates, I think you should have an example project that builds as part of CI either rodio or cpal depending on how ambitious you're feeling.

Closing in favor of #72. @MichaelHills got more context and knows how this works better than I do at this point.

@simlay simlay closed this Sep 28, 2020
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

Successfully merging this pull request may close these issues.

3 participants