Merge pull request #465 from orottier/feature/audioscheduledsourcenode-start

AudioScheduledSourceNode can return tail_time false when not scheduled

Author: orottier

src/context/concrete_base.rs

@@ -306,7 +306,7 @@ impl ConcreteBaseAudioContext {
             || LISTENER_PARAM_IDS.contains(&id.0);
 
         if !magic {
-            let message = ControlMessage::FreeWhenFinished { id };
+            let message = ControlMessage::ControlHandleDropped { id };
             self.send_control_msg(message);
         }
     }

src/message.rs

@@ -34,7 +34,7 @@ pub(crate) enum ControlMessage {
     DisconnectAll { from: AudioNodeId },
 
     /// Notify the render thread this node is dropped in the control thread
-    FreeWhenFinished { id: AudioNodeId },
+    ControlHandleDropped { id: AudioNodeId },
 
     /// Mark node as a cycle breaker (DelayNode only)
     MarkCycleBreaker { id: AudioNodeId },

src/node/audio_buffer_source.rs

@@ -55,7 +55,7 @@ struct PlaybackInfo {
     k: f32,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Copy)]
 struct LoopState {
     pub is_looping: bool,
     pub start: f64,
@@ -214,7 +214,7 @@ impl AudioBufferSourceNode {
                 buffer: None,
                 detune: d_proc,
                 playback_rate: pr_proc,
-                loop_state: loop_state.clone(),
+                loop_state,
                 render_state: AudioBufferRendererState::default(),
                 ended_triggered: false,
             };
@@ -411,39 +411,43 @@ impl AudioProcessor for AudioBufferSourceRenderer {
         params: AudioParamValues<'_>,
         scope: &AudioWorkletGlobalScope,
     ) -> bool {
-        // single output node
+        // Single output node
         let output = &mut outputs[0];
 
         let sample_rate = scope.sample_rate as f64;
         let dt = 1. / sample_rate;
         let block_duration = dt * RENDER_QUANTUM_SIZE as f64;
         let next_block_time = scope.current_time + block_duration;
 
-        let LoopState {
-            is_looping,
-            start: loop_start,
-            end: loop_end,
-        } = self.loop_state.clone();
-
-        // these will only be used if `loop_` is true, so no need for `Option`
-        let mut actual_loop_start = 0.;
-        let mut actual_loop_end = 0.;
-
-        // return early if start_time is beyond this block
+        // Return early if start_time is beyond this block
         if self.start_time >= next_block_time {
             output.make_silent();
-            return true;
+            // #462 AudioScheduledSourceNodes that have not been scheduled to start can safely
+            // return tail_time false in order to be collected if their control handle drops.
+            return self.start_time != f64::MAX;
         }
 
         // If the buffer has not been set wait for it.
         let buffer = match &self.buffer {
             None => {
                 output.make_silent();
-                return true;
+                // #462 like the above arm, we can safely return tail_time false if this node has
+                // no buffer set.
+                return false;
             }
             Some(b) => b,
         };
 
+        let LoopState {
+            is_looping,
+            start: loop_start,
+            end: loop_end,
+        } = self.loop_state;
+
+        // these will only be used if `loop_` is true, so no need for `Option`
+        let mut actual_loop_start = 0.;
+        let mut actual_loop_end = 0.;
+
         // compute compound parameter at k-rate, these parameters have constraints
         // https://webaudio.github.io/web-audio-api/#audioparam-automation-rate-constraints
         let detune = params.get(&self.detune)[0];
@@ -456,15 +460,20 @@ impl AudioProcessor for AudioBufferSourceRenderer {
         // we just linearly interpolate, thus favoring performance vs quality
         let sampling_ratio = buffer.sample_rate() as f64 / sample_rate;
 
-        // In addition, if the buffer has more than one channel, then the
-        // AudioBufferSourceNode output must change to a single channel of silence
-        // at the beginning of a render quantum after the time at which any one of
-        // the following conditions holds:
+        // Load the buffer time from the render state.
+        // The render state has to be updated before leaving this method!
+        let mut buffer_time = self.render_state.buffer_time.load(Ordering::Relaxed);
 
+        // The output must change to a single channel of silence at the beginning of a render
+        // quantum after the time at which any one of the following conditions holds:
         // 1. the stop time has been reached.
         // 2. the duration has been reached.
+        // 3. the end of the buffer has been reached.
         if scope.current_time >= self.stop_time
             || self.render_state.buffer_time_elapsed >= self.duration
+            || !is_looping
+                && (computed_playback_rate > 0. && buffer_time >= buffer_duration
+                    || computed_playback_rate < 0. && buffer_time < 0.)
         {
             output.make_silent(); // also converts to mono
 
@@ -477,31 +486,6 @@ impl AudioProcessor for AudioBufferSourceRenderer {
             return false;
         }
 
-        // Load the buffer time from the render state.
-        // The render state has to be updated before leaving this method!
-        let mut buffer_time = self.render_state.buffer_time.load(Ordering::Relaxed);
-
-        // 3. the end of the buffer has been reached.
-        if !is_looping {
-            if computed_playback_rate > 0. && buffer_time >= buffer_duration {
-                output.make_silent(); // also converts to mono
-                if !self.ended_triggered {
-                    scope.send_ended_event();
-                    self.ended_triggered = true;
-                }
-                return false;
-            }
-
-            if computed_playback_rate < 0. && buffer_time < 0. {
-                output.make_silent(); // also converts to mono
-                if !self.ended_triggered {
-                    scope.send_ended_event();
-                    self.ended_triggered = true;
-                }
-                return false;
-            }
-        }
-
         output.set_number_of_channels(buffer.number_of_channels());
 
         // go through the algorithm described in the spec

src/node/constant_source.rs

@@ -192,7 +192,9 @@ impl AudioProcessor for ConstantSourceRenderer {
 
         if self.start_time >= next_block_time {
             output.make_silent();
-            return true;
+            // #462 AudioScheduledSourceNodes that have not been scheduled to start can safely
+            // return tail_time false in order to be collected if their control handle drops.
+            return self.start_time != f64::MAX;
         }
 
         output.force_mono();
@@ -314,6 +316,23 @@ mod tests {
         assert_float_eq!(channel[128..], vec![1.; 128][..], abs_all <= 0.);
     }
 
+    #[test]
+    fn test_start_in_the_future_while_dropped() {
+        let sample_rate = 48000.;
+        let mut context = OfflineAudioContext::new(1, 4 * 128, sample_rate);
+
+        let mut src = context.create_constant_source();
+        src.connect(&context.destination());
+        src.start_at(258. / sample_rate as f64); // in 3rd block
+        drop(src); // explicit drop
+
+        let buffer = context.start_rendering_sync();
+        let channel = buffer.get_channel_data(0);
+
+        assert_float_eq!(channel[0..258], vec![0.; 258][..], abs_all <= 0.);
+        assert_float_eq!(channel[258..], vec![1.; 254][..], abs_all <= 0.);
+    }
+
     #[test]
     #[should_panic]
     fn test_start_twice() {

src/node/oscillator.rs

@@ -373,7 +373,9 @@ impl AudioProcessor for OscillatorRenderer {
 
         if self.start_time >= next_block_time {
             output.make_silent();
-            return true;
+            // #462 AudioScheduledSourceNodes that have not been scheduled to start can safely
+            // return tail_time false in order to be collected if their control handle drops.
+            return self.start_time != f64::MAX;
         } else if self.stop_time < scope.current_time {
             output.make_silent();
 

src/render/graph.rs

@@ -54,7 +54,7 @@ pub struct Node {
     /// Outgoing edges: tuple of outcoming node reference, our output index and their input index
     outgoing_edges: SmallVec<[OutgoingEdge; 2]>,
     /// Indicates if the control thread has dropped this Node
-    free_when_finished: bool,
+    control_handle_dropped: bool,
     /// Indicates if the node has any incoming connections (for lifecycle management)
     has_inputs_connected: bool,
     /// Indicates if the node can act as a cycle breaker (only DelayNode for now)
@@ -68,7 +68,7 @@ impl std::fmt::Debug for Node {
             .field("processor", &self.processor)
             .field("channel_config", &self.channel_config)
             .field("outgoing_edges", &self.outgoing_edges)
-            .field("free_when_finished", &self.free_when_finished)
+            .field("control_handle_dropped", &self.control_handle_dropped)
             .field("cycle_breaker", &self.cycle_breaker)
             .finish_non_exhaustive()
     }
@@ -85,7 +85,7 @@ impl Node {
     fn can_free(&self, tail_time: bool) -> bool {
         // Only drop when the Control thread has dropped its handle.
         // Otherwise the node can be reconnected/restarted etc.
-        if !self.free_when_finished {
+        if !self.control_handle_dropped {
             return false;
         }
 
@@ -185,7 +185,7 @@ impl Graph {
                 outputs,
                 channel_config,
                 outgoing_edges: smallvec![],
-                free_when_finished: false,
+                control_handle_dropped: false,
                 has_inputs_connected: false,
                 cycle_breaker: false,
             }),
@@ -231,12 +231,12 @@ impl Graph {
         self.ordered.clear(); // void current ordering
     }
 
-    pub fn mark_free_when_finished(&mut self, index: AudioNodeId) {
+    pub fn mark_control_handle_dropped(&mut self, index: AudioNodeId) {
         // Issue #92, a race condition can occur for AudioParams. They may have already been
         // removed from the audio graph if the node they feed into was dropped.
         // Therefore, do not assume this node still exists:
         if let Some(node) = self.nodes.get_mut(index) {
-            node.get_mut().free_when_finished = true;
+            node.get_mut().control_handle_dropped = true;
         }
     }
 
@@ -509,7 +509,7 @@ impl Graph {
                     // - special node (destination = id 0, listener = id 1), or
                     // - not connected to this dropped node, or
                     // - if the control thread still has a handle to it.
-                    let retain = id.0 < 2 || !was_connected || !node.free_when_finished;
+                    let retain = id.0 < 2 || !was_connected || !node.control_handle_dropped;
 
                     if !retain {
                         self.reclaim_id_channel
@@ -735,7 +735,7 @@ mod tests {
         // dropped and the AudioNodeId(3) should be reclaimed
         add_node(&mut graph, 2, node.clone());
         // Mark the node as 'detached from the control thread', so it is allowed to drop
-        graph.nodes[AudioNodeId(2)].get_mut().free_when_finished = true;
+        graph.nodes[AudioNodeId(2)].get_mut().control_handle_dropped = true;
 
         // Connect the regular node to the AudioDestinationNode
         add_edge(&mut graph, 2, 0);
@@ -777,7 +777,7 @@ mod tests {
         // dropped and the AudioNodeId(3) should be reclaimed
         add_node(&mut graph, 2, node.clone());
         // Mark the node as 'detached from the control thread', so it is allowed to drop
-        graph.nodes[AudioNodeId(2)].get_mut().free_when_finished = true;
+        graph.nodes[AudioNodeId(2)].get_mut().control_handle_dropped = true;
 
         // Connect the regular node to the AudioDestinationNode
         add_edge(&mut graph, 2, 0);
@@ -786,7 +786,7 @@ mod tests {
         let param = Box::new(TestNode { tail_time: true }); // audio params have tail time true
         add_node(&mut graph, 3, param);
         // Mark the node as 'detached from the control thread', so it is allowed to drop
-        graph.nodes[AudioNodeId(3)].get_mut().free_when_finished = true;
+        graph.nodes[AudioNodeId(3)].get_mut().control_handle_dropped = true;
 
         // Connect the audioparam to the regular node
         add_audioparam(&mut graph, 3, 2);
@@ -828,7 +828,7 @@ mod tests {
         add_node(&mut graph, 2, node);
 
         // Mark the node as 'detached from the control thread', so it is allowed to drop
-        graph.nodes[AudioNodeId(2)].get_mut().free_when_finished = true;
+        graph.nodes[AudioNodeId(2)].get_mut().control_handle_dropped = true;
 
         // Render a single quantum
         let scope = AudioWorkletGlobalScope {

src/render/thread.rs

@@ -150,8 +150,8 @@ impl RenderThread {
                 DisconnectAll { from } => {
                     self.graph.as_mut().unwrap().remove_edges_from(from);
                 }
-                FreeWhenFinished { id } => {
-                    self.graph.as_mut().unwrap().mark_free_when_finished(id);
+                ControlHandleDropped { id } => {
+                    self.graph.as_mut().unwrap().mark_control_handle_dropped(id);
                 }
                 MarkCycleBreaker { id } => {
                     self.graph.as_mut().unwrap().mark_cycle_breaker(id);