Bug 1530715 - P28: Create a Mixer module. r=padenot

Using an Mixer struct to operate all the mixing APIs is a way to make
the code clearer. In addition, we can avoid calling mix function by
borrowing the AudioUnitStream as a mutable. It will help to avoid
potential borrowing-twice issues in the later mutex replacement.

Differential Revision: https://phabricator.services.mozilla.com/D34061

--HG--
extra : moz-landing-system : lando

media/libcubeb/cubeb-coreaudio-rs/README_MOZILLA

@@ -3,4 +3,4 @@ git repository using the update.sh script.
 
 The cubeb-coreaudio-rs git repository is: https://github.com/ChunMinChang/cubeb-coreaudio-rs
 
-The git commit ID used was 4154c3ca7c55d5e3f4fc7bda262f25cdd34aad33 (2019-06-25 11:32:22 -0700)
+The git commit ID used was 5c9b94910dabb1f64b1c6e6d5907304d08c53d44 (2019-06-25 11:32:22 -0700)

media/libcubeb/cubeb-coreaudio-rs/src/backend/mixer.rs

@@ -0,0 +1,118 @@
+use super::auto_release::*;
+use super::utils::cubeb_sample_size;
+use cubeb_backend::{ffi, ChannelLayout, SampleFormat};
+use std::mem;
+use std::os::raw::{c_int, c_void};
+use std::ptr;
+
+#[derive(Debug)]
+pub struct Mixer {
+    mixer: AutoRelease<ffi::cubeb_mixer>,
+    format: SampleFormat,
+    in_channels: u32,
+    in_layout: ChannelLayout,
+    out_channels: u32,
+    out_layout: ChannelLayout,
+    // Only accessed from callback thread.
+    buffer: Vec<u8>,
+}
+
+impl Mixer {
+    pub fn new(
+        format: SampleFormat,
+        in_channels: u32,
+        in_layout: ChannelLayout,
+        out_channels: u32,
+        out_layout: ChannelLayout,
+    ) -> Self {
+        let raw_mixer = unsafe {
+            ffi::cubeb_mixer_create(
+                format.into(),
+                in_channels,
+                in_layout.into(),
+                out_channels,
+                out_layout.into(),
+            )
+        };
+        assert!(!raw_mixer.is_null(), "Failed to create mixer");
+        Self {
+            mixer: AutoRelease::new(raw_mixer, ffi::cubeb_mixer_destroy),
+            format,
+            in_channels,
+            in_layout,
+            out_channels,
+            out_layout,
+            buffer: Vec::new(),
+        }
+    }
+
+    pub fn update_buffer_size(&mut self, frames: usize) -> bool {
+        let size_needed = frames * self.in_channels as usize * cubeb_sample_size(self.format);
+        let elements_needed = size_needed / mem::size_of::<u8>();
+        if self.buffer.len() < elements_needed {
+            self.buffer.resize(elements_needed, 0);
+            true
+        } else {
+            false
+        }
+    }
+
+    pub fn get_buffer_mut_ptr(&mut self) -> *mut u8 {
+        self.buffer.as_mut_ptr()
+    }
+
+    fn get_buffer_info(&self) -> (*const u8, usize) {
+        (
+            self.buffer.as_ptr(),
+            self.buffer.len() * mem::size_of::<u8>(),
+        )
+    }
+
+    // `update_buffer_size` must be called before this.
+    pub fn mix(
+        &mut self,
+        frames: usize,
+        dest_buffer: *mut c_void,
+        dest_buffer_size: usize,
+    ) -> c_int {
+        let size_needed = frames * self.in_channels as usize * cubeb_sample_size(self.format);
+        let (src_buffer_ptr, src_buffer_size) = self.get_buffer_info();
+        assert!(src_buffer_size >= size_needed);
+        unsafe {
+            ffi::cubeb_mixer_mix(
+                self.mixer.as_mut(),
+                frames,
+                src_buffer_ptr as *const c_void,
+                src_buffer_size,
+                dest_buffer,
+                dest_buffer_size,
+            )
+        }
+    }
+
+    fn destroy(&mut self) {
+        if !self.mixer.as_ptr().is_null() {
+            self.mixer.reset(ptr::null_mut());
+        }
+    }
+}
+
+impl Drop for Mixer {
+    fn drop(&mut self) {
+        self.destroy();
+    }
+}
+
+impl Default for Mixer {
+    fn default() -> Self {
+        Self {
+            mixer: AutoRelease::new(ptr::null_mut(), ffi::cubeb_mixer_destroy),
+            format: SampleFormat::Float32NE,
+            in_channels: 0,
+            in_layout: ChannelLayout::UNDEFINED,
+            out_channels: 0,
+            out_layout: ChannelLayout::UNDEFINED,
+            buffer: Vec::default(),
+        }
+    }
+}

media/libcubeb/cubeb-coreaudio-rs/src/backend/mod.rs

@@ -11,6 +11,7 @@ extern crate libc;
 mod aggregate_device;
 mod auto_array;
 mod auto_release;
+mod mixer;
 mod owned_critical_section;
 mod property_address;
 mod resampler;
@@ -26,6 +27,7 @@ use self::coreaudio_sys_utils::cf_mutable_dict::*;
 use self::coreaudio_sys_utils::dispatch::*;
 use self::coreaudio_sys_utils::string::*;
 use self::coreaudio_sys_utils::sys::*;
+use self::mixer::*;
 use self::owned_critical_section::*;
 use self::property_address::*;
 use self::resampler::*;
@@ -498,20 +500,15 @@ extern "C" fn audiounit_output_callback(
     }
 
     // Get output buffer
-    if stm.mixer.as_ptr().is_null() {
+    output_buffer = match stm.mixer.as_mut() {
-        output_buffer = buffers[0].mData;
+        None => buffers[0].mData,
-    } else {
+        Some(mixer) => {
-        // If remixing needs to occur, we can't directly work in our final
+            // If remixing needs to occur, we can't directly work in our final
-        // destination buffer as data may be overwritten or too small to start with.
+            // destination buffer as data may be overwritten or too small to start with.
-        let size_needed = (output_frames * stm.output_stream_params.channels()) as usize
+            mixer.update_buffer_size(output_frames as usize);
-            * cubeb_sample_size(stm.output_stream_params.format());
+            mixer.get_buffer_mut_ptr() as *mut c_void
-        if stm.temp_buffer_size < size_needed {
-            stm.temp_buffer = allocate_array_by_size(size_needed);
-            assert_eq!(stm.temp_buffer.len() * mem::size_of::<u8>(), size_needed);
-            stm.temp_buffer_size = size_needed;
         }
-        output_buffer = stm.temp_buffer.as_mut_ptr() as *mut c_void;
+    };
-    }
 
     stm.frames_written
         .fetch_add(i64::from(output_frames), Ordering::SeqCst);
@@ -619,7 +616,7 @@ extern "C" fn audiounit_output_callback(
     }
 
     // Mixing
-    if stm.mixer.as_ptr().is_null() {
+    if stm.mixer.is_none() {
         // Pan stereo.
         if panning != 0.0 {
             unsafe {
@@ -639,17 +636,9 @@ extern "C" fn audiounit_output_callback(
             }
         }
     } else {
-        assert!(!stm.temp_buffer.is_empty());
+        assert!(buffers[0].mDataByteSize >= stm.output_desc.mBytesPerFrame * output_frames);
-        assert_eq!(
+        stm.mixer.as_mut().unwrap().mix(
-            stm.temp_buffer_size,
-            stm.temp_buffer.len() * mem::size_of::<u8>()
-        );
-        assert_eq!(output_buffer, stm.temp_buffer.as_mut_ptr() as *mut c_void);
-        let temp_buffer_size = stm.temp_buffer_size;
-        stm.mix_output_buffer(
             output_frames as usize,
-            output_buffer,
-            temp_buffer_size,
             buffers[0].mData,
             buffers[0].mDataByteSize as usize,
         );
@@ -2464,11 +2453,7 @@ struct AudioUnitStream<'ctx> {
     // This is true if a device change callback is currently running.
     switching_device: AtomicBool,
     // Mixer interface
-    mixer: AutoRelease<ffi::cubeb_mixer>,
+    mixer: Option<Mixer>,
-    // Buffer where remixing/resampling will occur when upmixing is required
-    // Only accessed from callback thread
-    temp_buffer: Vec<u8>,
-    temp_buffer_size: usize,
     // Listeners indicating what system events are monitored.
     default_input_listener: Option<device_property_listener>,
     default_output_listener: Option<device_property_listener>,
@@ -2528,9 +2513,7 @@ impl<'ctx> AudioUnitStream<'ctx> {
             current_latency_frames: AtomicU32::new(0),
             panning: atomic::Atomic::new(0.0_f32),
             switching_device: AtomicBool::new(false),
-            mixer: AutoRelease::new(ptr::null_mut(), ffi::cubeb_mixer_destroy),
+            mixer: None,
-            temp_buffer: Vec::new(),
-            temp_buffer_size: 0,
             default_input_listener: None,
             default_output_listener: None,
             input_alive_listener: None,
@@ -2656,37 +2639,6 @@ impl<'ctx> AudioUnitStream<'ctx> {
         NO_ERR
     }
 
-    fn mix_output_buffer(
-        &mut self,
-        output_frames: usize,
-        input_buffer: *mut c_void,
-        input_buffer_size: usize,
-        output_buffer: *mut c_void,
-        output_buffer_size: usize,
-    ) {
-        assert!(
-            input_buffer_size
-                >= cubeb_sample_size(self.output_stream_params.format())
-                    * self.output_stream_params.channels() as usize
-                    * output_frames
-        );
-        assert!(output_buffer_size >= self.output_desc.mBytesPerFrame as usize * output_frames);
-
-        let r = unsafe {
-            ffi::cubeb_mixer_mix(
-                self.mixer.as_mut(),
-                output_frames,
-                input_buffer,
-                input_buffer_size,
-                output_buffer,
-                output_buffer_size,
-            )
-        };
-        if r != 0 {
-            cubeb_log!("Remix error = {}", r);
-        }
-    }
-
     fn minimum_resampling_input_frames(&self, output_frames: i64) -> i64 {
         assert_ne!(self.input_hw_rate, 0_f64);
         assert_ne!(self.output_stream_params.rate(), 0);
@@ -2995,19 +2947,6 @@ impl<'ctx> AudioUnitStream<'ctx> {
         Ok(())
     }
 
-    fn init_mixer(&mut self) {
-        self.mixer.reset(unsafe {
-            ffi::cubeb_mixer_create(
-                self.output_stream_params.format().into(),
-                self.output_stream_params.channels(),
-                self.output_stream_params.layout().into(),
-                self.context.channels,
-                self.context.layout.load(atomic::Ordering::SeqCst).into(),
-            )
-        });
-        assert!(!self.mixer.as_ptr().is_null());
-    }
-
     fn layout_init(&mut self, side: io_side) {
         // We currently don't support the input layout setting.
         if side == io_side::INPUT {
@@ -3231,7 +3170,13 @@ impl<'ctx> AudioUnitStream<'ctx> {
                 != self.output_stream_params.layout()
         {
             cubeb_log!("Incompatible channel layouts detected, setting up remixer");
-            self.init_mixer();
+            self.mixer = Some(Mixer::new(
+                self.output_stream_params.format(),
+                self.output_stream_params.channels(),
+                self.output_stream_params.layout(),
+                self.context.channels,
+                self.context.layout.load(atomic::Ordering::SeqCst),
+            ));
             // We will be remixing the data before it reaches the output device.
             // We need to adjust the number of channels and other
             // AudioStreamDescription details.
@@ -3241,7 +3186,7 @@ impl<'ctx> AudioUnitStream<'ctx> {
             self.output_desc.mBytesPerPacket =
                 self.output_desc.mBytesPerFrame * self.output_desc.mFramesPerPacket;
         } else {
-            self.mixer.reset(ptr::null_mut());
+            self.mixer = None;
         }
 
         r = audio_unit_set_property(
@@ -3503,7 +3448,7 @@ impl<'ctx> AudioUnitStream<'ctx> {
             self.output_unit = ptr::null_mut();
         }
 
-        self.mixer.reset(ptr::null_mut());
+        self.mixer = None;
 
         {
             let mut stream_device = self.stream_device.lock().unwrap();