Skip to content

refactor: fix clippy warnings, replace Arc<Mutex> with Rc<RefCell> in CoreAudio #998

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.

Sign up for GitHub

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

Merged
roderickvd merged 6 commits into from
Aug 5, 2025
Merged

refactor: fix clippy warnings, replace Arc<Mutex> with Rc<RefCell> in CoreAudio #998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
719a44a
refactor: use Rust 1.58+ format strings and update CoreAudio locking
roderickvd Aug 5, 2025
d7788c1
style: cargo fmt
roderickvd Aug 5, 2025
6d14e57
docs: update changelog for CoreAudio Rc<RefCell> refactor
roderickvd Aug 5, 2025
43e0a9d
fix: handle borrow errors when pausing on device disconnect
roderickvd Aug 5, 2025
8aa9a70
docs: clarify error handling comments for stream borrow failure
roderickvd Aug 5, 2025
355cf6c
git commit -m "docs(changelog): remove internal refactor entry
roderickvd Aug 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::env;
const CPAL_ASIO_DIR: &str = "CPAL_ASIO_DIR";

fn main() {
println!("cargo:rerun-if-env-changed={}", CPAL_ASIO_DIR);
println!("cargo:rerun-if-env-changed={CPAL_ASIO_DIR}");

// If ASIO directory isn't set silently return early
// otherwise set the asio config flag
Expand Down
4 changes: 2 additions & 2 deletions examples/beep.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ fn main() -> anyhow::Result<()> {
println!("Output device: {}", device.name()?);

let config = device.default_output_config().unwrap();
println!("Default output config: {:?}", config);
println!("Default output config: {config:?}");

match config.sample_format() {
cpal::SampleFormat::I8 => run::<i8>(&device, &config.into()),
Expand Down Expand Up @@ -108,7 +108,7 @@ where
(sample_clock * 440.0 * 2.0 * std::f32::consts::PI / sample_rate).sin()
};

let err_fn = |err| eprintln!("an error occurred on stream: {}", err);
let err_fn = |err| eprintln!("an error occurred on stream: {err}");

let stream = device.build_output_stream(
config,
Expand Down
14 changes: 7 additions & 7 deletions examples/enumerate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ use cpal::traits::{DeviceTrait, HostTrait};
fn main() -> Result<(), anyhow::Error> {
println!("Supported hosts:\n {:?}", cpal::ALL_HOSTS);
let available_hosts = cpal::available_hosts();
println!("Available hosts:\n {:?}", available_hosts);
println!("Available hosts:\n {available_hosts:?}");

for host_id in available_hosts {
println!("{}", host_id.name());
let host = cpal::host_from_id(host_id)?;

let default_in = host.default_input_device().map(|e| e.name().unwrap());
let default_out = host.default_output_device().map(|e| e.name().unwrap());
println!(" Default Input Device:\n {:?}", default_in);
println!(" Default Output Device:\n {:?}", default_out);
println!(" Default Input Device:\n {default_in:?}");
println!(" Default Output Device:\n {default_out:?}");

let devices = host.devices()?;
println!(" Devices: ");
Expand All @@ -24,12 +24,12 @@ fn main() -> Result<(), anyhow::Error> {

// Input configs
if let Ok(conf) = device.default_input_config() {
println!(" Default input stream config:\n {:?}", conf);
println!(" Default input stream config:\n {conf:?}");
}
let input_configs = match device.supported_input_configs() {
Ok(f) => f.collect(),
Err(e) => {
println!(" Error getting supported input configs: {:?}", e);
println!(" Error getting supported input configs: {e:?}");
Vec::new()
}
};
Expand All @@ -47,12 +47,12 @@ fn main() -> Result<(), anyhow::Error> {

// Output configs
if let Ok(conf) = device.default_output_config() {
println!(" Default output stream config:\n {:?}", conf);
println!(" Default output stream config:\n {conf:?}");
}
let output_configs = match device.supported_output_configs() {
Ok(f) => f.collect(),
Err(e) => {
println!(" Error getting supported output configs: {:?}", e);
println!(" Error getting supported output configs: {e:?}");
Vec::new()
}
};
Expand Down
7 changes: 2 additions & 5 deletions examples/feedback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,7 @@ fn main() -> anyhow::Result<()> {
};

// Build streams.
println!(
"Attempting to build both streams with f32 samples and `{:?}`.",
config
);
println!("Attempting to build both streams with f32 samples and `{config:?}`.");
let input_stream = input_device.build_input_stream(&config, input_data_fn, err_fn, None)?;
let output_stream = output_device.build_output_stream(&config, output_data_fn, err_fn, None)?;
println!("Successfully built streams.");
Expand All @@ -173,5 +170,5 @@ fn main() -> anyhow::Result<()> {
}

fn err_fn(err: cpal::StreamError) {
eprintln!("an error occurred on stream: {}", err);
eprintln!("an error occurred on stream: {err}");
}
6 changes: 3 additions & 3 deletions examples/record_wav.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fn main() -> Result<(), anyhow::Error> {
let config = device
.default_input_config()
.expect("Failed to get default input config");
println!("Default input config: {:?}", config);
println!("Default input config: {config:?}");

// The WAV file we're recording to.
const PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/recorded.wav");
Expand All @@ -97,7 +97,7 @@ fn main() -> Result<(), anyhow::Error> {
let writer_2 = writer.clone();

let err_fn = move |err| {
eprintln!("an error occurred on stream: {}", err);
eprintln!("an error occurred on stream: {err}");
};

let stream = match config.sample_format() {
Expand Down Expand Up @@ -138,7 +138,7 @@ fn main() -> Result<(), anyhow::Error> {
std::thread::sleep(std::time::Duration::from_secs(3));
drop(stream);
writer.lock().unwrap().take().unwrap().finalize()?;
println!("Recording {} complete!", PATH);
println!("Recording {PATH} complete!");
Ok(())
}

Expand Down
6 changes: 3 additions & 3 deletions examples/synth_tones.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub fn host_device_setup(
println!("Output device : {}", device.name()?);

let config = device.default_output_config()?;
println!("Default output config : {:?}", config);
println!("Default output config : {config:?}");

Ok((host, device, config))
}
Expand All @@ -142,10 +142,10 @@ where
current_sample_index: 0.0,
frequency_hz: 440.0,
};
let err_fn = |err| eprintln!("Error building output sound stream: {}", err);
let err_fn = |err| eprintln!("Error building output sound stream: {err}");

let time_at_start = std::time::Instant::now();
println!("Time at start: {:?}", time_at_start);
println!("Time at start: {time_at_start:?}");

let stream = device.build_output_stream(
config,
Expand Down
6 changes: 3 additions & 3 deletions src/host/coreaudio/macos/enumerate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl Devices {
match audio_devices() {
Ok(devices) => devices,
Err(os_status) => {
let description = format!("{}", os_status);
let description = format!("{os_status}");
let err = BackendSpecificError { description };
return Err(err.into());
}
Expand Down Expand Up @@ -105,7 +105,7 @@ pub fn default_input_device() -> Option<Device> {
NonNull::from(&mut audio_device_id).cast(),
)
};
if status != kAudioHardwareNoError as i32 {
if status != kAudioHardwareNoError {
return None;
}

Expand Down Expand Up @@ -135,7 +135,7 @@ pub fn default_output_device() -> Option<Device> {
NonNull::from(&mut audio_device_id).cast(),
)
};
if status != kAudioHardwareNoError as i32 {
if status != kAudioHardwareNoError {
return None;
}

Expand Down
47 changes: 26 additions & 21 deletions src/host/coreaudio/macos/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ use objc2_core_audio::{
use objc2_core_audio_types::{
AudioBuffer, AudioBufferList, AudioStreamBasicDescription, AudioValueRange,
};
use std::cell::RefCell;
use std::fmt;
use std::mem;
use std::ptr::{null, NonNull};
use std::rc::Rc;
use std::slice;
use std::sync::mpsc::{channel, RecvTimeoutError};
use std::sync::{Arc, Mutex};
Expand Down Expand Up @@ -275,10 +277,7 @@ impl Device {
kAudioObjectPropertyScopeInput => Ok(true),
kAudioObjectPropertyScopeOutput => Ok(false),
_ => Err(BackendSpecificError {
description: format!(
"unexpected scope (neither input nor output): {:?}",
scope
),
description: format!("unexpected scope (neither input nor output): {scope:?}"),
}),
}?;
let audio_unit = audio_unit_from_device(self, input)?;
Expand Down Expand Up @@ -359,7 +358,7 @@ impl Device {
Err(DefaultStreamConfigError::DeviceNotAvailable)
}
err => {
let description = format!("{}", err);
let description = format!("{err}");
let err = BackendSpecificError { description };
Err(err.into())
}
Expand Down Expand Up @@ -411,10 +410,7 @@ impl Device {
kAudioObjectPropertyScopeInput => Ok(true),
kAudioObjectPropertyScopeOutput => Ok(false),
_ => Err(BackendSpecificError {
description: format!(
"unexpected scope (neither input nor output): {:?}",
scope
),
description: format!("unexpected scope (neither input nor output): {scope:?}"),
}),
}?;
let audio_unit = audio_unit_from_device(self, input)?;
Expand Down Expand Up @@ -465,7 +461,7 @@ impl StreamInner {
fn play(&mut self) -> Result<(), PlayStreamError> {
if !self.playing {
if let Err(e) = self.audio_unit.start() {
let description = format!("{}", e);
let description = format!("{e}");
let err = BackendSpecificError { description };
return Err(err.into());
}
Expand All @@ -477,7 +473,7 @@ impl StreamInner {
fn pause(&mut self) -> Result<(), PauseStreamError> {
if self.playing {
if let Err(e) = self.audio_unit.stop() {
let description = format!("{}", e);
let description = format!("{e}");
let err = BackendSpecificError { description };
return Err(err.into());
}
Expand All @@ -497,8 +493,8 @@ fn add_disconnect_listener<E>(
where
E: FnMut(StreamError) + Send + 'static,
{
let stream_inner_weak = Arc::downgrade(&stream.inner);
let mut stream_inner = stream.inner.lock().unwrap();
let stream_inner_weak = Rc::downgrade(&stream.inner);
let mut stream_inner = stream.inner.borrow_mut();
stream_inner._disconnect_listener = Some(AudioObjectPropertyListener::new(
stream_inner.device_id,
AudioObjectPropertyAddress {
Expand All @@ -508,8 +504,17 @@ where
},
move || {
if let Some(stream_inner_strong) = stream_inner_weak.upgrade() {
let mut stream_inner = stream_inner_strong.lock().unwrap();
let _ = stream_inner.pause();
match stream_inner_strong.try_borrow_mut() {
Ok(mut stream_inner) => {
let _ = stream_inner.pause();
}
Err(_) => {
// Could not acquire mutable borrow. This can occur if there are
// overlapping borrows, if the stream is already in use, or if a panic
// occurred during a previous borrow. Still notify about device
// disconnection even if we can't pause.
}
}
(error_callback.lock().unwrap())(StreamError::DeviceNotAvailable);
}
},
Expand Down Expand Up @@ -664,7 +669,7 @@ impl Device {
add_disconnect_listener(&stream, error_callback_disconnect)?;
}

stream.inner.lock().unwrap().audio_unit.start()?;
stream.inner.borrow_mut().audio_unit.start()?;

Ok(stream)
}
Expand Down Expand Up @@ -769,7 +774,7 @@ impl Device {
add_disconnect_listener(&stream, error_callback_disconnect)?;
}

stream.inner.lock().unwrap().audio_unit.start()?;
stream.inner.borrow_mut().audio_unit.start()?;

Ok(stream)
}
Expand Down Expand Up @@ -930,26 +935,26 @@ fn set_sample_rate(

#[derive(Clone)]
pub struct Stream {
inner: Arc<Mutex<StreamInner>>,
inner: Rc<RefCell<StreamInner>>,
}

impl Stream {
fn new(inner: StreamInner) -> Self {
Self {
inner: Arc::new(Mutex::new(inner)),
inner: Rc::new(RefCell::new(inner)),
}
}
}

impl StreamTrait for Stream {
fn play(&self) -> Result<(), PlayStreamError> {
let mut stream = self.inner.lock().unwrap();
let mut stream = self.inner.borrow_mut();

stream.play()
}

fn pause(&self) -> Result<(), PauseStreamError> {
let mut stream = self.inner.lock().unwrap();
let mut stream = self.inner.borrow_mut();

stream.pause()
}
Expand Down
6 changes: 3 additions & 3 deletions src/host/coreaudio/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub use self::macos::{
Device, Host, Stream,
};

/// Common helper methods used by both macOS and iOS
// Common helper methods used by both macOS and iOS

fn check_os_status(os_status: OSStatus) -> Result<(), BackendSpecificError> {
match coreaudio::Error::from_os_status(os_status) {
Expand Down Expand Up @@ -105,7 +105,7 @@ impl From<coreaudio::Error> for BuildStreamError {

impl From<coreaudio::Error> for SupportedStreamConfigsError {
fn from(err: coreaudio::Error) -> SupportedStreamConfigsError {
let description = format!("{}", err);
let description = format!("{err}");
let err = BackendSpecificError { description };
// Check for possible DeviceNotAvailable variant
SupportedStreamConfigsError::BackendSpecific { err }
Expand All @@ -114,7 +114,7 @@ impl From<coreaudio::Error> for SupportedStreamConfigsError {

impl From<coreaudio::Error> for DefaultStreamConfigError {
fn from(err: coreaudio::Error) -> DefaultStreamConfigError {
let description = format!("{}", err);
let description = format!("{err}");
let err = BackendSpecificError { description };
// Check for possible DeviceNotAvailable variant
DefaultStreamConfigError::BackendSpecific { err }
Expand Down
Loading