Only apply VPIO properties while running

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.

src/backend/mod.rs

@@ -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(
@@ -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;
@@ -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(())
@@ -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>,
@@ -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,
@@ -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,
@@ -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.
@@ -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() {
@@ -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() {
@@ -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();
 
@@ -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);
@@ -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() {
@@ -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;

src/backend/tests/interfaces.rs

@@ -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;