Merge pull request #337 from b-ma/refactor/message-passing

Refactor message passing

Author: orottier

src/node/biquad_filter.rs

@@ -1,8 +1,9 @@
 //! The biquad filter control and renderer parts
-use num_complex::Complex;
+use std::any::Any;
 use std::f64::consts::{PI, SQRT_2};
 use std::sync::atomic::{AtomicU32, Ordering};
-use std::sync::Arc;
+
+use num_complex::Complex;
 
 use crate::context::{AudioContextRegistration, AudioParamId, BaseAudioContext};
 use crate::param::{AudioParam, AudioParamDescriptor};
@@ -309,7 +310,7 @@ pub struct BiquadFilterNode {
     /// filter, depends on the `BiquadFilterType`
     gain: AudioParam,
     /// `BiquadFilterType` represented as u32
-    type_: Arc<AtomicU32>,
+    type_: AtomicU32,
 }
 
 impl AudioNode for BiquadFilterNode {
@@ -341,23 +342,32 @@ impl BiquadFilterNode {
         context.register(move |registration| {
             let sample_rate = context.sample_rate();
 
-            let q_options = AudioParamDescriptor {
+            let BiquadFilterOptions {
+                q,
+                detune,
+                frequency,
+                gain,
+                type_,
+                channel_config,
+            } = options;
+
+            let q_param_options = AudioParamDescriptor {
                 min_value: f32::MIN,
                 max_value: f32::MAX,
                 default_value: 1.,
                 automation_rate: crate::param::AutomationRate::A,
             };
-            let (q_param, q_proc) = context.create_audio_param(q_options, &registration);
-            q_param.set_value(options.q);
+            let (q_param, q_proc) = context.create_audio_param(q_param_options, &registration);
+            q_param.set_value(q);
 
-            let detune_options = AudioParamDescriptor {
+            let detune_param_options = AudioParamDescriptor {
                 min_value: -153_600.,
                 max_value: 153_600.,
                 default_value: 0.,
                 automation_rate: crate::param::AutomationRate::A,
             };
-            let (d_param, d_proc) = context.create_audio_param(detune_options, &registration);
-            d_param.set_value(options.detune);
+            let (d_param, d_proc) = context.create_audio_param(detune_param_options, &registration);
+            d_param.set_value(detune);
 
             let freq_options = AudioParamDescriptor {
                 min_value: 0.,
@@ -366,7 +376,7 @@ impl BiquadFilterNode {
                 automation_rate: crate::param::AutomationRate::A,
             };
             let (f_param, f_proc) = context.create_audio_param(freq_options, &registration);
-            f_param.set_value(options.frequency);
+            f_param.set_value(frequency);
 
             let gain_options = AudioParamDescriptor {
                 min_value: f32::MIN,
@@ -375,16 +385,14 @@ impl BiquadFilterNode {
                 automation_rate: crate::param::AutomationRate::A,
             };
             let (g_param, g_proc) = context.create_audio_param(gain_options, &registration);
-            g_param.set_value(options.gain);
-
-            let type_ = Arc::new(AtomicU32::new(options.type_ as u32));
+            g_param.set_value(gain);
 
             let renderer = BiquadFilterRenderer {
                 gain: g_proc,
                 detune: d_proc,
                 frequency: f_proc,
                 q: q_proc,
-                type_: Arc::clone(&type_),
+                type_,
                 x1: Vec::with_capacity(MAX_CHANNELS),
                 x2: Vec::with_capacity(MAX_CHANNELS),
                 y1: Vec::with_capacity(MAX_CHANNELS),
@@ -393,8 +401,8 @@ impl BiquadFilterNode {
 
             let node = Self {
                 registration,
-                channel_config: options.channel_config.into(),
-                type_,
+                channel_config: channel_config.into(),
+                type_: AtomicU32::new(type_ as u32),
                 q: q_param,
                 detune: d_param,
                 frequency: f_param,
@@ -432,7 +440,7 @@ impl BiquadFilterNode {
     /// Returns the biquad filter type
     #[must_use]
     pub fn type_(&self) -> BiquadFilterType {
-        self.type_.load(Ordering::SeqCst).into()
+        self.type_.load(Ordering::Acquire).into()
     }
 
     /// biquad filter type setter
@@ -441,7 +449,8 @@ impl BiquadFilterNode {
     ///
     /// * `type_` - the biquad filter type (lowpass, highpass,...)
     pub fn set_type(&self, type_: BiquadFilterType) {
-        self.type_.store(type_ as u32, Ordering::SeqCst);
+        self.type_.store(type_ as u32, Ordering::Release);
+        self.registration.post_message(type_);
     }
 
     /// Returns the frequency response for the specified frequencies
@@ -533,8 +542,8 @@ struct BiquadFilterRenderer {
     /// boost/attenuation (dB) - its impact on the frequency response of the filter
     /// depends on the `BiquadFilterType`
     gain: AudioParamId,
-    /// `BiquadFilterType` represented as u32
-    type_: Arc<AtomicU32>,
+    /// `BiquadFilterType`
+    type_: BiquadFilterType,
     // keep filter state for each channel
     x1: Vec<f64>,
     x2: Vec<f64>,
@@ -596,7 +605,7 @@ impl AudioProcessor for BiquadFilterRenderer {
         }
 
         // get a-rate parameters
-        let type_: BiquadFilterType = self.type_.load(Ordering::SeqCst).into();
+        let type_ = self.type_;
         let frequency = params.get(&self.frequency);
         let detune = params.get(&self.detune);
         let q = params.get(&self.q);
@@ -671,6 +680,15 @@ impl AudioProcessor for BiquadFilterRenderer {
 
         true
     }
+
+    fn onmessage(&mut self, msg: &mut dyn Any) {
+        if let Some(&type_) = msg.downcast_ref::<BiquadFilterType>() {
+            self.type_ = type_;
+            return;
+        }
+
+        log::warn!("BiquadFilterRenderer: Dropping incoming message {msg:?}");
+    }
 }
 
 #[cfg(test)]

src/node/constant_source.rs

@@ -11,12 +11,14 @@ use super::{AudioNode, AudioScheduledSourceNode, ChannelConfig};
 // dictionary ConstantSourceOptions {
 //   float offset = 1;
 // };
+// https://webaudio.github.io/web-audio-api/#ConstantSourceOptions
 //
 // @note - Does not extend AudioNodeOptions because AudioNodeOptions are
-// useless for source nodes as they instruct how to upmix the inputs.
-// This is a common source of confusion, see e.g. https://github.com/mdn/content/pull/18472
+// useless for source nodes, because they instruct how to upmix the inputs.
+// This is a common source of confusion, see e.g. mdn/content#18472
 #[derive(Clone, Debug)]
 pub struct ConstantSourceOptions {
+    /// Initial parameter value of the constant signal
     pub offset: f32,
 }
 
@@ -114,14 +116,16 @@ impl AudioScheduledSourceNode for ConstantSourceNode {
 impl ConstantSourceNode {
     pub fn new<C: BaseAudioContext>(context: &C, options: ConstantSourceOptions) -> Self {
         context.register(move |registration| {
-            let param_opts = AudioParamDescriptor {
+            let ConstantSourceOptions { offset } = options;
+
+            let param_options = AudioParamDescriptor {
                 min_value: f32::MIN,
                 max_value: f32::MAX,
                 default_value: 1.,
                 automation_rate: AutomationRate::A,
             };
-            let (param, proc) = context.create_audio_param(param_opts, &registration);
-            param.set_value(options.offset);
+            let (param, proc) = context.create_audio_param(param_options, &registration);
+            param.set_value(offset);
 
             let render = ConstantSourceRenderer {
                 offset: proc,

src/node/convolver.rs

@@ -1,18 +1,19 @@
-use super::{AudioNode, ChannelConfig, ChannelConfigOptions, ChannelInterpretation};
+use std::any::Any;
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::{Arc, Mutex};
+
+use realfft::{num_complex::Complex, ComplexToReal, RealFftPlanner, RealToComplex};
+
 use crate::buffer::AudioBuffer;
 use crate::context::{AudioContextRegistration, BaseAudioContext};
 use crate::render::{AudioParamValues, AudioProcessor, AudioRenderQuantum, RenderScope};
 use crate::RENDER_QUANTUM_SIZE;
 
-use crossbeam_channel::{Receiver, Sender};
-use realfft::{num_complex::Complex, ComplexToReal, RealFftPlanner, RealToComplex};
-use std::sync::{
-    atomic::{AtomicBool, Ordering},
-    Arc, Mutex,
-};
+use super::{AudioNode, ChannelConfig, ChannelConfigOptions, ChannelInterpretation};
 
 /// Scale buffer by an equal-power normalization
-fn normalization(buffer: &AudioBuffer) -> f32 {
+// see - <https://webaudio.github.io/web-audio-api/#dom-convolvernode-normalize>
+fn normalize_buffer(buffer: &AudioBuffer) -> f32 {
     let gain_calibration = 0.00125;
     let gain_calibration_sample_rate = 44100.;
     let min_power = 0.000125;
@@ -116,8 +117,6 @@ pub struct ConvolverNode {
     normalize: AtomicBool,
     /// The response buffer, nullable
     buffer: Mutex<Option<AudioBuffer>>,
-    /// Message bus to the renderer
-    sender: Sender<ConvolverRendererInner>,
 }
 
 impl AudioNode for ConvolverNode {
@@ -151,33 +150,31 @@ impl ConvolverNode {
     /// Panics when an AudioBuffer is provided via the `ConvolverOptions` with a sample rate
     /// different from the audio context sample rate.
     pub fn new<C: BaseAudioContext>(context: &C, options: ConvolverOptions) -> Self {
-        context.base().register(move |registration| {
-            let ConvolverOptions {
-                buffer,
-                disable_normalization,
-                channel_config,
-            } = options;
-
-            // Channel to send buffer channels references to the renderer.  A capacity of 1
-            // suffices, it will simply block the control thread when used concurrently
-            let (sender, receiver) = crossbeam_channel::bounded(1);
+        let ConvolverOptions {
+            buffer,
+            disable_normalization,
+            channel_config,
+        } = options;
 
-            let renderer = ConvolverRenderer::new(receiver);
+        let node = context.base().register(move |registration| {
+            let renderer = ConvolverRenderer { inner: None };
 
             let node = Self {
                 registration,
                 channel_config: channel_config.into(),
                 normalize: AtomicBool::new(!disable_normalization),
-                sender,
                 buffer: Mutex::new(None),
             };
 
-            if let Some(buffer) = buffer {
-                node.set_buffer(buffer);
-            }
-
             (node, Box::new(renderer))
-        })
+        });
+
+        // renderer has been sent to render thread, we can send it messages
+        if let Some(buffer) = buffer {
+            node.set_buffer(buffer);
+        }
+
+        node
     }
 
     /// Get the current impulse response buffer
@@ -199,7 +196,7 @@ impl ConvolverNode {
 
         // normalize before padding because the length of the buffer affects the scale
         let scale = if self.normalize() {
-            normalization(&buffer)
+            normalize_buffer(&buffer)
         } else {
             1.
         };
@@ -219,9 +216,9 @@ impl ConvolverNode {
             .collect();
 
         let padded_buffer = AudioBuffer::from(samples, sample_rate);
-
         let convolve = ConvolverRendererInner::new(padded_buffer);
-        let _ = self.sender.send(convolve); // can fail when render thread shut down
+
+        self.registration.post_message(Some(convolve));
 
         *self.buffer.lock().unwrap() = Some(buffer);
     }
@@ -304,20 +301,6 @@ impl Fft {
     }
 }
 
-struct ConvolverRenderer {
-    receiver: Receiver<ConvolverRendererInner>,
-    inner: Option<ConvolverRendererInner>,
-}
-
-impl ConvolverRenderer {
-    fn new(receiver: Receiver<ConvolverRendererInner>) -> Self {
-        Self {
-            receiver,
-            inner: None,
-        }
-    }
-}
-
 struct ConvolverRendererInner {
     num_ir_blocks: usize,
     h: Vec<Complex<f32>>,
@@ -414,6 +397,10 @@ impl ConvolverRendererInner {
     }
 }
 
+struct ConvolverRenderer {
+    inner: Option<ConvolverRendererInner>,
+}
+
 impl AudioProcessor for ConvolverRenderer {
     fn process(
         &mut self,
@@ -427,11 +414,6 @@ impl AudioProcessor for ConvolverRenderer {
         let output = &mut outputs[0];
         output.force_mono();
 
-        // handle new impulse response buffer, if any
-        if let Ok(msg) = self.receiver.try_recv() {
-            self.inner = Some(msg);
-        }
-
         let convolver = match &mut self.inner {
             None => {
                 // no convolution buffer set, passthrough
@@ -455,6 +437,16 @@ impl AudioProcessor for ConvolverRenderer {
 
         true
     }
+
+    fn onmessage(&mut self, msg: &mut dyn Any) {
+        if let Some(convolver) = msg.downcast_mut::<Option<ConvolverRendererInner>>() {
+            // Avoid deallocation in the render thread by swapping the convolver.
+            std::mem::swap(&mut self.inner, convolver);
+            return;
+        }
+
+        log::warn!("ConvolverRenderer: Dropping incoming message {msg:?}");
+    }
 }
 
 #[cfg(test)]
@@ -473,6 +465,36 @@ mod tests {
         assert_eq!(&input, &[4, 5, 6, 7, 8, 9, 10, 0, 0, 0]);
     }
 
+    #[test]
+    fn test_constructor_options_buffer() {
+        let sample_rate = 44100.;
+        let context = OfflineAudioContext::new(1, 10, sample_rate);
+
+        let ir = vec![1.];
+        let calibration = 0.00125;
+        let channel_data = vec![0., 1., 0., -1., 0.];
+        let expected = vec![0., calibration, 0., -calibration, 0., 0., 0., 0., 0., 0.];
+
+        // identity ir
+        let ir = AudioBuffer::from(vec![ir; 1], sample_rate);
+        let options = ConvolverOptions {
+            buffer: Some(ir),
+            ..ConvolverOptions::default()
+        };
+        let conv = ConvolverNode::new(&context, options);
+        conv.connect(&context.destination());
+
+        let buffer = AudioBuffer::from(vec![channel_data; 1], sample_rate);
+        let src = context.create_buffer_source();
+        src.connect(&conv);
+        src.set_buffer(buffer);
+        src.start();
+
+        let output = context.start_rendering_sync();
+
+        assert_float_eq!(output.get_channel_data(0), &expected[..], abs_all <= 1E-6);
+    }
+
     fn test_convolve(signal: &[f32], impulse_resp: Option<Vec<f32>>, length: usize) -> AudioBuffer {
         let sample_rate = 44100.;
         let context = OfflineAudioContext::new(1, length, sample_rate);