Merge pull request #402 from orottier/bugfix/fix-panic-with-some-constructor-options

Fix panic in AudioBufferSource and Oscillator when supplying some options

Author: orottier

src/node/audio_buffer_source.rs

@@ -159,16 +159,16 @@ impl AudioScheduledSourceNode for AudioBufferSourceNode {
 impl AudioBufferSourceNode {
     /// Create a new [`AudioBufferSourceNode`] instance
     pub fn new<C: BaseAudioContext>(context: &C, options: AudioBufferSourceOptions) -> Self {
-        context.register(move |registration| {
-            let AudioBufferSourceOptions {
-                buffer,
-                detune,
-                loop_,
-                loop_start,
-                loop_end,
-                playback_rate,
-            } = options;
-
+        let AudioBufferSourceOptions {
+            buffer,
+            detune,
+            loop_,
+            loop_start,
+            loop_end,
+            playback_rate,
+        } = options;
+
+        let mut node = context.register(move |registration| {
             // these parameters can't be changed to a-rate
             // @see - <https://webaudio.github.io/web-audio-api/#audioparam-automation-rate-constraints>
             let detune_param_options = AudioParamDescriptor {
@@ -214,7 +214,7 @@ impl AudioBufferSourceNode {
                 ended_triggered: false,
             };
 
-            let mut node = Self {
+            let node = Self {
                 registration,
                 channel_config: ChannelConfig::default(),
                 detune: d_param,
@@ -225,12 +225,15 @@ impl AudioBufferSourceNode {
                 source_started: false,
             };
 
-            if let Some(buf) = buffer {
-                node.set_buffer(buf);
-            }
-
             (node, Box::new(renderer))
-        })
+        });
+
+        // renderer has been sent to render thread, we can send it messages
+        if let Some(buf) = buffer {
+            node.set_buffer(buf);
+        }
+
+        node
     }
 
     /// Start the playback at the given time and with a given offset
@@ -789,6 +792,29 @@ mod tests {
 
     use super::*;
 
+    #[test]
+    fn test_construct_with_options_and_run() {
+        let sample_rate = 44100.;
+        let length = RENDER_QUANTUM_SIZE;
+        let context = OfflineAudioContext::new(1, length, sample_rate);
+
+        let buffer = AudioBuffer::from(vec![vec![1.; RENDER_QUANTUM_SIZE]], sample_rate);
+        let options = AudioBufferSourceOptions {
+            buffer: Some(buffer),
+            ..Default::default()
+        };
+        let mut src = AudioBufferSourceNode::new(&context, options);
+        src.connect(&context.destination());
+        src.start();
+        let res = context.start_rendering_sync();
+
+        assert_float_eq!(
+            res.channel_data(0).as_slice()[..],
+            &[1.; RENDER_QUANTUM_SIZE][..],
+            abs_all <= 0.
+        );
+    }
+
     #[test]
     fn test_playing_some_file() {
         let context = OfflineAudioContext::new(2, RENDER_QUANTUM_SIZE, 44_100.);

src/node/oscillator.rs

@@ -181,18 +181,18 @@ impl OscillatorNode {
     /// * `context` - The `AudioContext`
     /// * `options` - The OscillatorOptions
     pub fn new<C: BaseAudioContext>(context: &C, options: OscillatorOptions) -> Self {
-        context.register(move |registration| {
+        let OscillatorOptions {
+            type_,
+            frequency,
+            detune,
+            channel_config,
+            periodic_wave,
+        } = options;
+
+        let mut node = context.register(move |registration| {
             let sample_rate = context.sample_rate();
             let nyquist = sample_rate / 2.;
 
-            let OscillatorOptions {
-                type_,
-                frequency,
-                detune,
-                channel_config,
-                periodic_wave,
-            } = options;
-
             // frequency audio parameter
             let freq_param_options = AudioParamDescriptor {
                 name: String::new(),
@@ -229,21 +229,23 @@ impl OscillatorNode {
                 sine_table: precomputed_sine_table(),
             };
 
-            let mut node = Self {
+            let node = Self {
                 registration,
                 channel_config: channel_config.into(),
                 frequency: f_param,
                 detune: det_param,
                 type_,
             };
 
-            // if periodic wave has been given, init it
-            if let Some(p_wave) = periodic_wave {
-                node.set_periodic_wave(p_wave);
-            }
-
             (node, Box::new(renderer))
-        })
+        });
+
+        // renderer has been sent to render thread, we can send it messages
+        if let Some(p_wave) = periodic_wave {
+            node.set_periodic_wave(p_wave);
+        }
+
+        node
     }
 
     /// A-rate [`AudioParam`] that defines the fundamental frequency of the
@@ -579,7 +581,7 @@ mod tests {
 
         let context = OfflineAudioContext::new(2, 1, 44_100.);
 
-        let osc = context.create_oscillator();
+        let mut osc = context.create_oscillator();
 
         let freq = osc.frequency.value();
         assert_float_eq!(freq, default_freq, abs_all <= 0.);
@@ -588,6 +590,11 @@ mod tests {
         assert_float_eq!(det, default_det, abs_all <= 0.);
 
         assert_eq!(osc.type_(), default_type);
+
+        // should not panic when run
+        osc.start();
+        osc.connect(&context.destination());
+        let _ = context.start_rendering_sync();
     }
 
     #[test]
@@ -598,7 +605,7 @@ mod tests {
 
         let context = OfflineAudioContext::new(2, 1, 44_100.);
 
-        let osc = OscillatorNode::new(&context, OscillatorOptions::default());
+        let mut osc = OscillatorNode::new(&context, OscillatorOptions::default());
 
         let freq = osc.frequency.value();
         assert_float_eq!(freq, default_freq, abs_all <= 0.);
@@ -607,6 +614,11 @@ mod tests {
         assert_float_eq!(det, default_det, abs_all <= 0.);
 
         assert_eq!(osc.type_(), default_type);
+
+        // should not panic when run
+        osc.start();
+        osc.connect(&context.destination());
+        let _ = context.start_rendering_sync();
     }
 
     #[test]
@@ -630,9 +642,14 @@ mod tests {
             ..OscillatorOptions::default()
         };
 
-        let osc = OscillatorNode::new(&context, options);
+        let mut osc = OscillatorNode::new(&context, options);
 
         assert_eq!(osc.type_(), expected_type);
+
+        // should not panic when run
+        osc.start();
+        osc.connect(&context.destination());
+        let _ = context.start_rendering_sync();
     }
 
     #[test]
@@ -652,6 +669,11 @@ mod tests {
 
         osc.set_type(OscillatorType::Sine);
         assert_eq!(osc.type_(), expected_type);
+
+        // should not panic when run
+        osc.start();
+        osc.connect(&context.destination());
+        let _ = context.start_rendering_sync();
     }
 
     // # Test waveforms