Skip to content

Commit

Permalink
Only apply VPIO properties while running
Browse files Browse the repository at this point in the history 
An Apple bug breaks, at least, the bypass property if it is set prior to
starting the VPIO unit. Broken here means that the property can be set
to any value later while running, without having an effect.

The VPIO defaults is bypass OFF and AGC ON. Starting like that and then
turning on bypass as requested is not going to cause an unexpected
glitch. We reset the VPIO to defaults before stopping so that on reuse
it doesn't risk causing a glitch either.
  • Loading branch information
@Pehrsons
Pehrsons committed Jun 17, 2024
1 parent f8a4ec0 commit 9ea3d34
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 26 deletions.
98 changes: 72 additions & 26 deletions src/backend/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,7 @@ fn set_input_processing_params(unit: AudioUnit, params: InputProcessingParams) -
let aec = params.contains(InputProcessingParams::ECHO_CANCELLATION);
let ns = params.contains(InputProcessingParams::NOISE_SUPPRESSION);
let agc = params.contains(InputProcessingParams::AUTOMATIC_GAIN_CONTROL);

// AEC and NS are active as soon as VPIO is not bypassed, therefore the only combinations
// of those we can explicitly support are {} and {aec, ns}.
if aec != ns {
// No control to turn on AEC without NS or vice versa.
return Err(Error::error());
}
assert_eq!(aec, ns);

let mut old_agc: u32 = 0;
let r = audio_unit_get_property(
Expand Down Expand Up @@ -419,6 +413,11 @@ fn set_input_processing_params(unit: AudioUnit, params: InputProcessingParams) -
);
return Err(Error::error());
}
cubeb_log!(
"set_input_processing_params on unit {:p} - set agc: {}",
unit,
agc
);
}

let mut old_bypass: u32 = 0;
Expand Down Expand Up @@ -455,6 +454,11 @@ fn set_input_processing_params(unit: AudioUnit, params: InputProcessingParams) -
);
return Err(Error::error());
}
cubeb_log!(
"set_input_processing_params on unit {:p} - set bypass: {}",
unit,
bypass
);
}

Ok(())
Expand Down Expand Up @@ -3015,6 +3019,7 @@ struct CoreStreamData<'ctx> {
input_processing_params: InputProcessingParams,
input_mute: bool,
input_buffer_manager: Option<BufferManager>,
units_running: bool,
// Listeners indicating what system events are monitored.
default_input_listener: Option<device_property_listener>,
default_output_listener: Option<device_property_listener>,
Expand Down Expand Up @@ -3064,6 +3069,7 @@ impl<'ctx> Default for CoreStreamData<'ctx> {
input_processing_params: InputProcessingParams::NONE,
input_mute: false,
input_buffer_manager: None,
units_running: false,
default_input_listener: None,
default_output_listener: None,
input_alive_listener: None,
Expand Down Expand Up @@ -3119,6 +3125,7 @@ impl<'ctx> CoreStreamData<'ctx> {
input_processing_params: InputProcessingParams::NONE,
input_mute: false,
input_buffer_manager: None,
units_running: false,
default_input_listener: None,
default_output_listener: None,
input_alive_listener: None,
Expand All @@ -3145,7 +3152,7 @@ impl<'ctx> CoreStreamData<'ctx> {
stm.queue.debug_assert_is_current();
}

fn start_audiounits(&self) -> Result<()> {
fn start_audiounits(&mut self) -> Result<()> {
self.debug_assert_is_on_stream_queue();
// Only allowed to be called after the stream is initialized
// and before the stream is destroyed.
Expand All @@ -3156,22 +3163,50 @@ impl<'ctx> CoreStreamData<'ctx> {
}
if self.using_voice_processing_unit() {
// Handle the VoiceProcessIO case where there is a single unit.

// Always try to remember the applied input processing params. If they cannot
// be applied in the new device pair, we notify the client of an error and it
// will have to open a new stream.
if let Err(r) =
set_input_processing_params(self.input_unit, self.input_processing_params)
{
cubeb_log!(
"({:p}) Failed to set params of voiceprocessing. Error: {}",
self.stm_ptr,
r
);
return Err(r);
}
return Ok(());
}
if !self.output_unit.is_null() {
start_audiounit(self.output_unit)?;
}
self.units_running = true;
Ok(())
}

fn stop_audiounits(&self) {
fn stop_audiounits(&mut self) {
self.debug_assert_is_on_stream_queue();
self.units_running = false;
if !self.input_unit.is_null() {
let r = stop_audiounit(self.input_unit);
assert!(r.is_ok());
}
if self.using_voice_processing_unit() {
// Handle the VoiceProcessIO case where there is a single unit.

// Always reset input processing params to VPIO defaults in case VPIO is reused later.
let vpio_defaults = InputProcessingParams::ECHO_CANCELLATION
| InputProcessingParams::AUTOMATIC_GAIN_CONTROL
| InputProcessingParams::NOISE_SUPPRESSION;
if let Err(r) = set_input_processing_params(self.input_unit, vpio_defaults) {
cubeb_log!(
"({:p}) Failed to reset params of voiceprocessing. Error: {}",
self.stm_ptr,
r
);
}
return;
}
if !self.output_unit.is_null() {
Expand Down Expand Up @@ -4095,20 +4130,6 @@ impl<'ctx> CoreStreamData<'ctx> {
);
return Err(r);
}

// Always try to remember the applied input processing params. If they cannot
// be applied in the new device pair, we notify the client of an error and it
// will have to open a new stream.
if let Err(r) =
set_input_processing_params(self.input_unit, self.input_processing_params)
{
cubeb_log!(
"({:p}) Failed to set params of voiceprocessing. Error: {}",
self.stm_ptr,
r
);
return Err(r);
}
}

if let Err(r) = self.install_system_changed_callback() {
Expand Down Expand Up @@ -4838,6 +4859,7 @@ impl<'ctx> StreamOps for AudioUnitStream<'ctx> {
// Execute start in serial queue to avoid racing with destroy or reinit.
let result = self
.queue
.clone()
.run_sync(|| self.core_stream_data.start_audiounits())
.unwrap();

Expand All @@ -4855,6 +4877,7 @@ impl<'ctx> StreamOps for AudioUnitStream<'ctx> {
if !self.stopped.swap(true, Ordering::SeqCst) {
// Execute stop in serial queue to avoid racing with destroy or reinit.
self.queue
.clone()
.run_sync(|| self.core_stream_data.stop_audiounits());

self.notify_state_changed(State::Stopped);
Expand Down Expand Up @@ -4997,6 +5020,20 @@ impl<'ctx> StreamOps for AudioUnitStream<'ctx> {
return Err(Error::invalid_parameter());
}

// AEC and NS are active as soon as VPIO is not bypassed, therefore the only combinations
// of those we can explicitly support are {} and {aec, ns}.
let aec = params.contains(InputProcessingParams::ECHO_CANCELLATION);
let ns = params.contains(InputProcessingParams::NOISE_SUPPRESSION);
if aec != ns {
// No control to turn on AEC without NS or vice versa.
cubeb_log!(
"Cubeb stream ({:p}) couldn't set input processing params {:?}. AEC != NS.",
self as *const AudioUnitStream,
params
);
return Err(Error::error());
}

// CUBEB_ERROR if params could not be applied
// note: only works with VoiceProcessingIO
if !self.core_stream_data.using_voice_processing_unit() {
Expand All @@ -5005,17 +5042,26 @@ impl<'ctx> StreamOps for AudioUnitStream<'ctx> {

// Execute set_input_processing_params in serial queue to avoid racing with destroy or reinit.
let mut result = Err(Error::error());
let set = &mut result;
let result_ = &mut result;
let mut deferred = false;
let deferred_ = &mut deferred;
let stream = &self;
self.queue.run_sync(move || {
*set = set_input_processing_params(stream.core_stream_data.input_unit, params);
if stream.core_stream_data.units_running {
*deferred_ = true;
*result_ = Ok(());
} else {
*deferred_ = false;
*result_ = set_input_processing_params(stream.core_stream_data.input_unit, params);
}
});

result?;

cubeb_log!(
"Cubeb stream ({:p}) set input processing params to {:?}.",
"Cubeb stream ({:p}) {} input processing params {:?}.",
self as *const AudioUnitStream,
if deferred { "deferred" } else { "set" },
params
);
self.core_stream_data.input_processing_params = params;
Expand Down
1 change: 1 addition & 0 deletions src/backend/tests/interfaces.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,7 @@ fn test_ops_timing_sensitive_multiple_duplex_voice_stream_params() {
|stream| {
let stm = unsafe { &mut *(stream as *mut AudioUnitStream) };
assert!(stm.core_stream_data.using_voice_processing_unit());
assert_eq!(unsafe { OPS.stream_start.unwrap()(stream) }, ffi::CUBEB_OK);
let queue = stm.queue.clone();
// Test that input processing params does not carry over when reusing vpio.
let mut bypass: u32 = 0;
Expand Down

0 comments on commit 9ea3d34

Please sign in to comment.