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

Use objc2 crates #128

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use objc2 crates #128

wants to merge 1 commit into from

Conversation

madsmtm
Copy link

@madsmtm madsmtm commented Jan 26, 2025

Use the objc2-* crates instead of coreaudio-sys (you knew it was coming @simlay 😉):

  • objc2-audio-toolbox.
  • objc2-core-audio.
  • objc2-core-audio-types.
  • objc2-core-midi.

Resolves every single coreaudio-sys issue1 and PR by no longer invoking bindgen, and instead generating the headers ahead of time. We should probably deprecate coreaudio-sys and archive the repo once this lands.

The bindings generated by objc2's tool header-translator are also a bit more type-safe as they include for example null-ability information, and is able to merge some enum constants into constants on the enum type.

Tested with sine example on:

  • macOS 10.12.6
  • macOS 14.7.1

Note: We've experienced problems with linking AudioUnix in the past, see RustAudio/coreaudio-sys#51, but that should be irrelevant since we now just always link AudioToolbox. This is fine, since AudioUnit has been an empty re-export of parts of AudioToolbox since macOS 10.10 (and rustc's minimum supported macOS version is 10.12) 2.

Note: I am unsure of your MSRV, but this will at least bump it to 1.71 (required for extern "C-unwind").

Footnotes

  1. Okay, maybe Docs failed to build for 0.2.5. coreaudio-sys#40 isn't quite resolved, since objc2-core-audio also failed building docs, but that's a minor issue.

  2. If you really want to, we could add #[link(name = "AudioUnit", kind = "framework")] extern "C" {} to still support older macOS versions.

Comment on lines +20 to +32
pub mod sys {
#[cfg(feature = "audio_toolbox")]
pub use objc2_audio_toolbox::*;
#[cfg(feature = "core_audio")]
pub use objc2_core_audio::*;
#[cfg(feature = "core_audio")]
pub use objc2_core_audio_types::*;
#[cfg(feature = "core_midi")]
pub use objc2_core_midi::*;

// MacTypes.h
pub type OSStatus = i32;
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of this module, ideally users should probably import the crates themselves, but it's useful to not break too much at once.

Comment on lines +52 to 63
let mut audio_device_id: AudioDeviceID = 0;
let data_size = mem::size_of::<AudioDeviceID>() as u32;
let status = unsafe {
AudioObjectGetPropertyData(
kAudioObjectSystemObject,
&property_address as *const _,
kAudioObjectSystemObject as AudioObjectID,
NonNull::from(&property_address),
0,
null(),
&data_size as *const _ as *mut _,
&audio_device_id as *const _ as *mut _,
NonNull::from(&data_size),
NonNull::from(&mut audio_device_id).cast(),
)
};
Copy link
Author

@madsmtm madsmtm Jan 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made sure to use &mut on the out pointer audio_device_id here (and in similar cases below), this was UB before (AudioObjectGetPropertyData was writing through an immutable reference).

Comment on lines +34 to +109
objc2-core-foundation = { version = "0.3", optional = true, default-features = false, features = [
"std",
"CFBase",
"CFString",
] }
objc2-audio-toolbox = { version = "0.3", optional = true, default-features = false, features = [
"std",
"bitflags",
"libc",
"objc2-core-foundation",
"AUAudioUnit",
"AUAudioUnitImplementation",
"AUCocoaUIView",
"AUComponent",
"AUGraph",
"AUParameters",
"AudioCodec",
"AudioComponent",
"AudioConverter",
"AudioFile",
"AudioFileStream",
"AudioFormat",
"AudioOutputUnit",
"AudioQueue",
"AudioServices",
"AudioSession",
"AudioUnit",
"AudioUnitCarbonView",
"AudioUnitParameters",
"AudioUnitProperties",
"AudioUnitUtilities",
"AudioWorkInterval",
"CAFFile",
"CAShow",
"DefaultAudioOutput",
"ExtendedAudioFile",
"MusicDevice",
"MusicPlayer",
"objc2-core-audio",
"objc2-core-audio-types",
] }
objc2-core-audio = { version = "0.3", optional = true, default-features = false, features = [
"std",
"objc2-core-audio-types",
"AudioHardware",
"AudioHardwareDeprecated",
"AudioServerPlugIn",
"HostTime",
] }
objc2-core-audio-types = { version = "0.3", optional = true, default-features = false, features = [
"std",
"bitflags",
"AudioSessionTypes",
"CoreAudioBaseTypes",
] }
objc2-core-midi = { version = "0.3", optional = true, default-features = false, features = [
"std",
"objc2-core-foundation",
"MIDIBluetoothConnection",
"MIDICIDevice",
"MIDICIDeviceManager",
"MIDICapabilityInquiry",
"MIDIDriver",
"MIDIMessages",
"MIDINetworkSession",
"MIDIServices",
"MIDISetup",
"MIDIThruConnection",
"MIDIUMPCI",
"MIDIUMPCIProfile",
"MIDIUMPEndpoint",
"MIDIUMPEndpointManager",
"MIDIUMPFunctionBlock",
"MIDIUMPMutableEndpoint",
"MIDIUMPMutableFunctionBlock",
] }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is unclear what features we want to enable here, so for now, I went with roughly "all features except block2 and objc2".

Copy link
Member

@simlay simlay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to do testing but I think this looks pretty good.

@simlay
Copy link
Member

simlay commented Feb 9, 2025

@mitchmindtree Would you be up to review this? You've got the most commit history on coreaudio-rs and coreaudio-sys so you might know this better than me. If you're unfamiliar with the objc2 crates, it's some work similar to bindgen but with higher level bindings. Mads has been working on the objective-c for a few years and slowly updating various crates. I find the bindings very nice.

@mitchmindtree
Copy link
Member

Nice work @madsmtm! Agree @simlay it definitely looks a lot nicer at a glance.

coreaudio-rs and coreaudio-sys were born out of necessity to get cpal cracking on macos like 10 yrs ago. Tbh, I have very little memory of what's going on 😅

My only hesitation is the sheer number of libraries, audio tools, game engines (including bevy), etc that use coreaudio-rs and coreaudio-sys now, though I guess most do transitively via cpal. If we can test this with cpal and confirm that it's working nicely (e.g. all examples still work at least) and that we're not breaking anything major, and that we're not including any excessive number of extra deps, I think I'd be happy to see this happen 👍

@madsmtm
Copy link
Author

madsmtm commented Feb 9, 2025

Thanks for the feedback!

Re deps, cpal currently:

$ cargo tree --edges no-dev
cpal v0.15.3
├── core-foundation-sys v0.8.7
├── coreaudio-rs v0.11.3
│   ├── bitflags v1.3.2
│   ├── core-foundation-sys v0.8.7
│   └── coreaudio-sys v0.2.16
│       [build-dependencies]
│       └── bindgen v0.70.1
│           ├── bitflags v2.8.0
│           ├── cexpr v0.6.0
│           │   └── nom v7.1.3
│           │       ├── memchr v2.7.4
│           │       └── minimal-lexical v0.2.1
│           ├── clang-sys v1.8.1
│           │   ├── glob v0.3.2
│           │   ├── libc v0.2.169
│           │   └── libloading v0.8.6
│           │       └── cfg-if v1.0.0
│           │   [build-dependencies]
│           │   └── glob v0.3.2
│           ├── itertools v0.12.1
│           │   └── either v1.13.0
│           ├── proc-macro2 v1.0.93
│           │   └── unicode-ident v1.0.15
│           ├── quote v1.0.38
│           │   └── proc-macro2 v1.0.93 (*)
│           ├── regex v1.11.1
│           │   ├── regex-automata v0.4.9
│           │   │   └── regex-syntax v0.8.5
│           │   └── regex-syntax v0.8.5
│           ├── rustc-hash v1.1.0
│           ├── shlex v1.3.0
│           └── syn v2.0.96
│               ├── proc-macro2 v1.0.93 (*)
│               ├── quote v1.0.38 (*)
│               └── unicode-ident v1.0.15
├── dasp_sample v0.11.0
└── mach2 v0.4.2
    └── libc v0.2.169

After RustAudio/cpal#943.

$ cargo tree --edges no-dev
cpal v0.15.3 (/Users/madsmtm/Desktop/rust/cpal)
├── coreaudio-rs v0.12.1 (/Users/madsmtm/Desktop/rust/coreaudio)
│   ├── bitflags v1.3.2
│   ├── libc v0.2.169
│   ├── objc2-audio-toolbox v0.3.0
│   │   ├── bitflags v2.8.0
│   │   ├── libc v0.2.169
│   │   ├── objc2 v0.6.0
│   │   │   └── objc2-encode v4.1.0
│   │   ├── objc2-core-audio v0.3.0
│   │   │   ├── objc2 v0.6.0 (*)
│   │   │   ├── objc2-core-audio-types v0.3.0
│   │   │   │   ├── bitflags v2.8.0
│   │   │   │   └── objc2 v0.6.0 (*)
│   │   │   └── objc2-core-foundation v0.3.0
│   │   │       ├── bitflags v2.8.0
│   │   │       └── objc2 v0.6.0 (*)
│   │   ├── objc2-core-audio-types v0.3.0 (*)
│   │   ├── objc2-core-foundation v0.3.0 (*)
│   │   └── objc2-foundation v0.3.0
│   │       ├── bitflags v2.8.0
│   │       └── objc2 v0.6.0 (*)
│   ├── objc2-core-audio v0.3.0 (*)
│   ├── objc2-core-audio-types v0.3.0 (*)
│   └── objc2-core-foundation v0.3.0 (*)
├── dasp_sample v0.11.0
└── mach2 v0.4.2
    └── libc v0.2.169

Summing up, it's basically bitflags, libc and the objc2-provided crates (which are split for organization and compile-time, but include effectively the same amount of code as core-audio-sys did). Weighed against the quite heavy bindgen compile-time dependency that core-audio-sys has.

I suspect even the libc dependency could be avoided (and compile-times brought more down) if we didn't expose coreaudio::sys, but I'd rather leave that to a later PR.

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