From ea024e8f053b60ec45865e9d68851ade82be65de Mon Sep 17 00:00:00 2001 From: Otto Date: Thu, 11 Apr 2024 15:55:36 +0200 Subject: [PATCH 1/7] AudioNode::disconnect_from now panics when connection does not exist This means we need to store the connections on the ConcreteBaseAudioContext as well. Fixes #471 --- src/context/concrete_base.rs | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/context/concrete_base.rs b/src/context/concrete_base.rs index 087b5aac..9d0b0c7c 100644 --- a/src/context/concrete_base.rs +++ b/src/context/concrete_base.rs @@ -14,6 +14,7 @@ use crate::spatial::AudioListenerParams; use crate::AudioListener; use crossbeam_channel::{SendError, Sender}; +use std::collections::HashSet; use std::sync::atomic::{AtomicU64, AtomicU8, Ordering}; use std::sync::{Arc, Mutex, RwLock, RwLockWriteGuard}; @@ -109,6 +110,8 @@ struct ConcreteBaseAudioContextInner { event_loop: EventLoop, /// Sender for events that will be handled by the EventLoop event_send: Sender, + /// Current audio graph connections + connections: Mutex>, } impl BaseAudioContext for ConcreteBaseAudioContext { @@ -147,6 +150,7 @@ impl ConcreteBaseAudioContext { state, event_loop, event_send, + connections: Mutex::new(HashSet::new()), }; let base = Self { inner: Arc::new(base_inner), @@ -393,6 +397,11 @@ impl ConcreteBaseAudioContext { /// Connects the output of the `from` audio node to the input of the `to` audio node pub(crate) fn connect(&self, from: AudioNodeId, to: AudioNodeId, output: usize, input: usize) { + self.inner + .connections + .lock() + .unwrap() + .insert((from, output, to, input)); let message = ControlMessage::ConnectNode { from, to, @@ -406,6 +415,8 @@ impl ConcreteBaseAudioContext { /// /// It is not performed immediately as the `AudioNode` is not registered at this point. pub(super) fn queue_audio_param_connect(&self, param: &AudioParam, audio_node: AudioNodeId) { + // no need to store these type of connections in self.inner.connections + let message = ControlMessage::ConnectNode { from: param.registration().id(), to: audio_node, @@ -416,13 +427,32 @@ impl ConcreteBaseAudioContext { } /// Disconnects all outputs of the audio node that go to a specific destination node. + /// + /// # Panics + /// + /// Panics if this node was not connected to the target node pub(crate) fn disconnect_from(&self, from: AudioNodeId, to: AudioNodeId) { + // check if the node was connected, otherwise panic + let mut connections = self.inner.connections.lock().unwrap(); + let prev_len = connections.len(); + connections.retain(|c| c.0 != from || c.2 != to); + let len = connections.len(); + drop(connections); + if prev_len == len { + panic!("InvalidAccessError - attempting to disconnect unconnected nodes"); + } + let message = ControlMessage::DisconnectNode { from, to }; self.send_control_msg(message); } /// Disconnects all outgoing connections from the audio node. pub(crate) fn disconnect(&self, from: AudioNodeId) { + self.inner + .connections + .lock() + .unwrap() + .retain(|c| c.0 != from); let message = ControlMessage::DisconnectAll { from }; self.send_control_msg(message); } @@ -470,6 +500,7 @@ impl ConcreteBaseAudioContext { #[cfg(test)] mod tests { use super::*; + use crate::context::OfflineAudioContext; #[test] fn test_provide_node_id() { @@ -481,4 +512,29 @@ mod tests { assert_eq!(provider.get().0, 0); // reused assert_eq!(provider.get().0, 2); // newly assigned } + + #[test] + fn test_connect_disconnect() { + let context = OfflineAudioContext::new(1, 128, 48000.); + let node1 = context.create_constant_source(); + let node2 = context.create_gain(); + + node1.disconnect(); // never panic for plain disconnect calls + + node1.connect(&node2); + node1.disconnect(); + + node1.connect(&node2); + node1.disconnect_from(&node2); + } + + #[test] + #[should_panic] + fn test_disconnect_not_existing() { + let context = OfflineAudioContext::new(1, 128, 48000.); + let node1 = context.create_constant_source(); + let node2 = context.create_gain(); + + node1.disconnect_from(&node2); + } } From 63efe2f5f3f791bfc5ceaeb4709af02aaacc993a Mon Sep 17 00:00:00 2001 From: Otto Date: Sat, 13 Apr 2024 12:19:20 +0200 Subject: [PATCH 2/7] Implement AudioNode::disconnect_at and fix disconnect `disconnect()` would previously clear all connections, but it should only clear the node's *outgoing* connections. --- src/context/concrete_base.rs | 65 +++++++++++++++++++++++++++++------ src/context/offline.rs | 7 +++- src/message.rs | 10 +++--- src/node/audio_node.rs | 6 ++++ src/render/graph.rs | 66 ++++-------------------------------- src/render/thread.rs | 15 +++++--- 6 files changed, 88 insertions(+), 81 deletions(-) diff --git a/src/context/concrete_base.rs b/src/context/concrete_base.rs index 9d0b0c7c..d359294c 100644 --- a/src/context/concrete_base.rs +++ b/src/context/concrete_base.rs @@ -110,7 +110,7 @@ struct ConcreteBaseAudioContextInner { event_loop: EventLoop, /// Sender for events that will be handled by the EventLoop event_send: Sender, - /// Current audio graph connections + /// Current audio graph connections (from node, output port, to node, input port) connections: Mutex>, } @@ -433,17 +433,29 @@ impl ConcreteBaseAudioContext { /// Panics if this node was not connected to the target node pub(crate) fn disconnect_from(&self, from: AudioNodeId, to: AudioNodeId) { // check if the node was connected, otherwise panic + let mut has_disconnected = false; let mut connections = self.inner.connections.lock().unwrap(); - let prev_len = connections.len(); - connections.retain(|c| c.0 != from || c.2 != to); - let len = connections.len(); + connections.retain(|&(c_from, output, c_to, input)| { + let retain = c_from != from || c_to != to; + if !retain { + has_disconnected = true; + let message = ControlMessage::DisconnectNode { + from, + to, + input, + output, + }; + self.send_control_msg(message); + } + retain + }); + + // make sure to drop the MutexGuard before the panic to avoid poisoning drop(connections); - if prev_len == len { + + if !has_disconnected { panic!("InvalidAccessError - attempting to disconnect unconnected nodes"); } - - let message = ControlMessage::DisconnectNode { from, to }; - self.send_control_msg(message); } /// Disconnects all outgoing connections from the audio node. @@ -452,9 +464,40 @@ impl ConcreteBaseAudioContext { .connections .lock() .unwrap() - .retain(|c| c.0 != from); - let message = ControlMessage::DisconnectAll { from }; - self.send_control_msg(message); + .retain(|&(c_from, output, to, input)| { + let retain = c_from != from; + if !retain { + let message = ControlMessage::DisconnectNode { + from, + to, + input, + output, + }; + self.send_control_msg(message); + } + retain + }); + } + + /// Disconnects all outgoing connections at the given output port from the audio node. + pub(crate) fn disconnect_at(&self, from: AudioNodeId, output: usize) { + self.inner + .connections + .lock() + .unwrap() + .retain(|&(c_from, c_output, to, input)| { + let retain = c_from != from || c_output != output; + if !retain { + let message = ControlMessage::DisconnectNode { + from, + to, + input, + output, + }; + self.send_control_msg(message); + } + retain + }); } /// Connect the `AudioListener` to a `PannerNode` diff --git a/src/context/offline.rs b/src/context/offline.rs index 9350bd5d..0e725765 100644 --- a/src/context/offline.rs +++ b/src/context/offline.rs @@ -459,21 +459,26 @@ mod tests { #[test] fn test_suspend_sync() { + use crate::node::ConstantSourceNode; + use std::sync::OnceLock; + let len = RENDER_QUANTUM_SIZE * 4; let sample_rate = 48000_f64; let mut context = OfflineAudioContext::new(1, len, sample_rate as f32); + static SOURCE: OnceLock = OnceLock::new(); context.suspend_sync(RENDER_QUANTUM_SIZE as f64 / sample_rate, |context| { assert_eq!(context.state(), AudioContextState::Suspended); let mut src = context.create_constant_source(); src.connect(&context.destination()); src.start(); + SOURCE.set(src).unwrap(); }); context.suspend_sync((3 * RENDER_QUANTUM_SIZE) as f64 / sample_rate, |context| { assert_eq!(context.state(), AudioContextState::Suspended); - context.destination().disconnect(); + SOURCE.get().unwrap().disconnect(); }); let output = context.start_rendering_sync(); diff --git a/src/message.rs b/src/message.rs index b00d089c..22db376c 100644 --- a/src/message.rs +++ b/src/message.rs @@ -28,10 +28,12 @@ pub(crate) enum ControlMessage { }, /// Clear the connection between two given nodes in the audio graph - DisconnectNode { from: AudioNodeId, to: AudioNodeId }, - - /// Disconnect this node from the audio graph (drop all its connections) - DisconnectAll { from: AudioNodeId }, + DisconnectNode { + from: AudioNodeId, + to: AudioNodeId, + input: usize, + output: usize, + }, /// Notify the render thread this node is dropped in the control thread ControlHandleDropped { id: AudioNodeId }, diff --git a/src/node/audio_node.rs b/src/node/audio_node.rs index f9c6099f..ad9e0700 100644 --- a/src/node/audio_node.rs +++ b/src/node/audio_node.rs @@ -306,6 +306,12 @@ pub trait AudioNode { self.context().disconnect(self.registration().id()); } + /// Disconnects all outgoing connections at the given output port from the AudioNode. + fn disconnect_at(&self, output: usize) { + self.context() + .disconnect_at(self.registration().id(), output); + } + /// The number of inputs feeding into the AudioNode. For source nodes, this will be 0. fn number_of_inputs(&self) -> usize; diff --git a/src/render/graph.rs b/src/render/graph.rs index 1d03ba31..8e227d9e 100644 --- a/src/render/graph.rs +++ b/src/render/graph.rs @@ -212,28 +212,13 @@ impl Graph { self.ordered.clear(); // void current ordering } - pub fn remove_edge(&mut self, source: AudioNodeId, dest: AudioNodeId) { + pub fn remove_edge(&mut self, source: (AudioNodeId, usize), dest: (AudioNodeId, usize)) { self.nodes - .get_unchecked_mut(source) + .get_unchecked_mut(source.0) .outgoing_edges - .retain(|edge| edge.other_id != dest); - - self.ordered.clear(); // void current ordering - } - - pub fn remove_edges_from(&mut self, source: AudioNodeId) { - // Remove outgoing edges - self.nodes.get_unchecked_mut(source).outgoing_edges.clear(); - - // Remove incoming edges - we need to traverse all nodes - self.nodes.values_mut().for_each(|node| { - // Retain edge when - // - not connected to this node, or - // - when this is an audioparam edge (only disconnect true audio nodes) - node.get_mut() - .outgoing_edges - .retain(|edge| edge.other_id != source || edge.other_index == usize::MAX); - }); + .retain(|edge| { + edge.other_id != dest.0 || edge.self_index != source.1 || edge.other_index != dest.1 + }); self.ordered.clear(); // void current ordering } @@ -635,7 +620,7 @@ mod tests { assert!(pos2 < pos1); // node 1 depends on node 2 // Detach node 1 (and thus node 2) from the root node - graph.remove_edge(AudioNodeId(1), AudioNodeId(0)); + graph.remove_edge((AudioNodeId(1), 0), (AudioNodeId(0), 0)); graph.order_nodes(); // sorting is not deterministic, but this should uphold: @@ -653,45 +638,6 @@ mod tests { assert!(pos2 < pos1); // node 1 depends on node 2 } - #[test] - fn test_remove_all() { - let mut graph = Graph::new(llq::Queue::new().split().0); - - let node = Box::new(TestNode { tail_time: false }); - add_node(&mut graph, 0, node.clone()); - add_node(&mut graph, 1, node.clone()); - add_node(&mut graph, 2, node); - - // link 1->0, 1->2 and 2->0 - add_edge(&mut graph, 1, 0); - add_edge(&mut graph, 1, 2); - add_edge(&mut graph, 2, 0); - - graph.order_nodes(); - - assert_eq!( - graph.ordered, - vec![AudioNodeId(1), AudioNodeId(2), AudioNodeId(0)] - ); - - graph.remove_edges_from(AudioNodeId(1)); - graph.order_nodes(); - - // sorting is not deterministic, but this should uphold: - assert_eq!(graph.ordered.len(), 3); // all nodes present - let pos0 = graph - .ordered - .iter() - .position(|&n| n == AudioNodeId(0)) - .unwrap(); - let pos2 = graph - .ordered - .iter() - .position(|&n| n == AudioNodeId(2)) - .unwrap(); - assert!(pos2 < pos0); // node 1 depends on node 0 - } - #[test] fn test_cycle() { let mut graph = Graph::new(llq::Queue::new().split().0); diff --git a/src/render/thread.rs b/src/render/thread.rs index ba6eb862..2e7b3ce6 100644 --- a/src/render/thread.rs +++ b/src/render/thread.rs @@ -151,11 +151,16 @@ impl RenderThread { .unwrap() .add_edge((from, output), (to, input)); } - DisconnectNode { from, to } => { - self.graph.as_mut().unwrap().remove_edge(from, to); - } - DisconnectAll { from } => { - self.graph.as_mut().unwrap().remove_edges_from(from); + DisconnectNode { + from, + output, + to, + input, + } => { + self.graph + .as_mut() + .unwrap() + .remove_edge((from, output), (to, input)); } ControlHandleDropped { id } => { self.graph.as_mut().unwrap().mark_control_handle_dropped(id); From 67f2c55e12e3bb481c9c5574c74c3a58fed00fbc Mon Sep 17 00:00:00 2001 From: Otto Date: Sat, 13 Apr 2024 20:26:27 +0200 Subject: [PATCH 3/7] AudioNode::disconnect, disconnect_dest, disconnect_output and more All variants of https://webaudio.github.io/web-audio-api/#AudioNode disconnect methods. Changed: renamed disconnect_from -> disconnect_dest Fix: disconnect methods should not return any value --- src/context/concrete_base.rs | 75 +++++++------------------ src/node/audio_node.rs | 106 +++++++++++++++++++++++++++++++---- src/node/delay.rs | 102 +++++++++++++++++++++++++++++++-- 3 files changed, 210 insertions(+), 73 deletions(-) diff --git a/src/context/concrete_base.rs b/src/context/concrete_base.rs index d359294c..0e885387 100644 --- a/src/context/concrete_base.rs +++ b/src/context/concrete_base.rs @@ -426,24 +426,29 @@ impl ConcreteBaseAudioContext { self.inner.queued_messages.lock().unwrap().push(message); } - /// Disconnects all outputs of the audio node that go to a specific destination node. - /// - /// # Panics - /// - /// Panics if this node was not connected to the target node - pub(crate) fn disconnect_from(&self, from: AudioNodeId, to: AudioNodeId) { + /// Disconnects outputs of the audio node, possibly filtered by output node, input, output. + pub(crate) fn disconnect( + &self, + from: AudioNodeId, + output: Option, + to: Option, + input: Option, + ) { // check if the node was connected, otherwise panic let mut has_disconnected = false; let mut connections = self.inner.connections.lock().unwrap(); - connections.retain(|&(c_from, output, c_to, input)| { - let retain = c_from != from || c_to != to; + connections.retain(|&(c_from, c_output, c_to, c_input)| { + let retain = c_from != from + || c_output != output.unwrap_or(c_output) + || c_to != to.unwrap_or(c_to) + || c_input != input.unwrap_or(c_input); if !retain { has_disconnected = true; let message = ControlMessage::DisconnectNode { from, - to, - input, - output, + to: c_to, + input: c_input, + output: c_output, }; self.send_control_msg(message); } @@ -453,53 +458,11 @@ impl ConcreteBaseAudioContext { // make sure to drop the MutexGuard before the panic to avoid poisoning drop(connections); - if !has_disconnected { + if !has_disconnected && to.is_some() { panic!("InvalidAccessError - attempting to disconnect unconnected nodes"); } } - /// Disconnects all outgoing connections from the audio node. - pub(crate) fn disconnect(&self, from: AudioNodeId) { - self.inner - .connections - .lock() - .unwrap() - .retain(|&(c_from, output, to, input)| { - let retain = c_from != from; - if !retain { - let message = ControlMessage::DisconnectNode { - from, - to, - input, - output, - }; - self.send_control_msg(message); - } - retain - }); - } - - /// Disconnects all outgoing connections at the given output port from the audio node. - pub(crate) fn disconnect_at(&self, from: AudioNodeId, output: usize) { - self.inner - .connections - .lock() - .unwrap() - .retain(|&(c_from, c_output, to, input)| { - let retain = c_from != from || c_output != output; - if !retain { - let message = ControlMessage::DisconnectNode { - from, - to, - input, - output, - }; - self.send_control_msg(message); - } - retain - }); - } - /// Connect the `AudioListener` to a `PannerNode` pub(crate) fn connect_listener_to_panner(&self, panner: AudioNodeId) { self.connect(LISTENER_NODE_ID, panner, 0, usize::MAX); @@ -568,7 +531,7 @@ mod tests { node1.disconnect(); node1.connect(&node2); - node1.disconnect_from(&node2); + node1.disconnect_dest(&node2); } #[test] @@ -578,6 +541,6 @@ mod tests { let node1 = context.create_constant_source(); let node2 = context.create_gain(); - node1.disconnect_from(&node2); + node1.disconnect_dest(&node2); } } diff --git a/src/node/audio_node.rs b/src/node/audio_node.rs index ad9e0700..2a36c858 100644 --- a/src/node/audio_node.rs +++ b/src/node/audio_node.rs @@ -288,28 +288,112 @@ pub trait AudioNode { dest } + /// Disconnects all outgoing connections from the AudioNode. + fn disconnect(&self) { + self.context() + .disconnect(self.registration().id(), None, None, None); + } + /// Disconnects all outputs of the AudioNode that go to a specific destination AudioNode. - fn disconnect_from<'a>(&self, dest: &'a dyn AudioNode) -> &'a dyn AudioNode { + /// + /// # Panics + /// + /// This function will panic when + /// - the AudioContext of the source and destination does not match + /// - the source node was not connected to the destination node + fn disconnect_dest(&self, dest: &dyn AudioNode) { assert!( self.context() == dest.context(), "InvalidAccessError - Attempting to disconnect nodes from different contexts" ); - self.context() - .disconnect_from(self.registration().id(), dest.registration().id()); + self.context().disconnect( + self.registration().id(), + None, + Some(dest.registration().id()), + None, + ); + } - dest + /// Disconnects all outgoing connections at the given output port from the AudioNode. + /// + /// # Panics + /// + /// This function will panic when + /// - if the output port is out of bounds for this node + fn disconnect_output(&self, output: usize) { + assert!( + self.number_of_outputs() > output, + "IndexSizeError - output port {} is out of bounds", + output + ); + + self.context() + .disconnect(self.registration().id(), Some(output), None, None); } - /// Disconnects all outgoing connections from the AudioNode. - fn disconnect(&self) { - self.context().disconnect(self.registration().id()); + /// Disconnects a specific output of the AudioNode to a specific destination AudioNode + /// + /// # Panics + /// + /// This function will panic when + /// - the AudioContext of the source and destination does not match + /// - if the output port is out of bounds for the source node + /// - the source node was not connected to the destination node + fn disconnect_dest_output(&self, dest: &dyn AudioNode, output: usize) { + assert!( + self.context() == dest.context(), + "InvalidAccessError - Attempting to disconnect nodes from different contexts" + ); + + assert!( + self.number_of_outputs() > output, + "IndexSizeError - output port {} is out of bounds", + output + ); + + self.context().disconnect( + self.registration().id(), + Some(output), + Some(dest.registration().id()), + None, + ); } - /// Disconnects all outgoing connections at the given output port from the AudioNode. - fn disconnect_at(&self, output: usize) { - self.context() - .disconnect_at(self.registration().id(), output); + /// Disconnects a specific output of the AudioNode to a specific input of some destination + /// AudioNode + /// + /// # Panics + /// + /// This function will panic when + /// - the AudioContext of the source and destination does not match + /// - if the input port is out of bounds for the destination node + /// - if the output port is out of bounds for the source node + /// - the source node was not connected to the destination node + fn disconnect_dest_output_input(&self, dest: &dyn AudioNode, output: usize, input: usize) { + assert!( + self.context() == dest.context(), + "InvalidAccessError - Attempting to disconnect nodes from different contexts" + ); + + assert!( + self.number_of_outputs() > output, + "IndexSizeError - output port {} is out of bounds", + output + ); + + assert!( + dest.number_of_inputs() > input, + "IndexSizeError - input port {} is out of bounds", + input + ); + + self.context().disconnect( + self.registration().id(), + Some(output), + Some(dest.registration().id()), + Some(input), + ); } /// The number of inputs feeding into the AudioNode. For source nodes, this will be 0. diff --git a/src/node/delay.rs b/src/node/delay.rs index 8e86070d..94d2e1b6 100644 --- a/src/node/delay.rs +++ b/src/node/delay.rs @@ -160,22 +160,112 @@ impl AudioNode for DelayNode { dest } + /// Disconnects all outgoing connections from the AudioNode. + fn disconnect(&self) { + self.context() + .disconnect(self.reader_registration.id(), None, None, None); + } + /// Disconnects all outputs of the AudioNode that go to a specific destination AudioNode. - fn disconnect_from<'a>(&self, dest: &'a dyn AudioNode) -> &'a dyn AudioNode { + /// + /// # Panics + /// + /// This function will panic when + /// - the AudioContext of the source and destination does not match + /// - the source node was not connected to the destination node + fn disconnect_dest(&self, dest: &dyn AudioNode) { assert!( self.context() == dest.context(), "InvalidAccessError - Attempting to disconnect nodes from different contexts" ); + self.context().disconnect( + self.reader_registration.id(), + None, + Some(dest.registration().id()), + None, + ); + } + + /// Disconnects all outgoing connections at the given output port from the AudioNode. + /// + /// # Panics + /// + /// This function will panic when + /// - if the output port is out of bounds for this node + fn disconnect_output(&self, output: usize) { + assert!( + self.number_of_outputs() > output, + "IndexSizeError - output port {} is out of bounds", + output + ); + self.context() - .disconnect_from(self.reader_registration.id(), dest.registration().id()); + .disconnect(self.reader_registration.id(), Some(output), None, None); + } - dest + /// Disconnects a specific output of the AudioNode to a specific destination AudioNode + /// + /// # Panics + /// + /// This function will panic when + /// - the AudioContext of the source and destination does not match + /// - if the output port is out of bounds for the source node + /// - the source node was not connected to the destination node + fn disconnect_dest_output(&self, dest: &dyn AudioNode, output: usize) { + assert!( + self.context() == dest.context(), + "InvalidAccessError - Attempting to disconnect nodes from different contexts" + ); + + assert!( + self.number_of_outputs() > output, + "IndexSizeError - output port {} is out of bounds", + output + ); + + self.context().disconnect( + self.reader_registration.id(), + Some(output), + Some(dest.registration().id()), + None, + ); } - /// Disconnects all outgoing connections from the AudioNode. - fn disconnect(&self) { - self.context().disconnect(self.reader_registration.id()); + /// Disconnects a specific output of the AudioNode to a specific input of some destination + /// AudioNode + /// + /// # Panics + /// + /// This function will panic when + /// - the AudioContext of the source and destination does not match + /// - if the input port is out of bounds for the destination node + /// - if the output port is out of bounds for the source node + /// - the source node was not connected to the destination node + fn disconnect_dest_output_input(&self, dest: &dyn AudioNode, output: usize, input: usize) { + assert!( + self.context() == dest.context(), + "InvalidAccessError - Attempting to disconnect nodes from different contexts" + ); + + assert!( + self.number_of_outputs() > output, + "IndexSizeError - output port {} is out of bounds", + output + ); + + assert!( + dest.number_of_inputs() > input, + "IndexSizeError - input port {} is out of bounds", + input + ); + + self.context().disconnect( + self.reader_registration.id(), + Some(output), + Some(dest.registration().id()), + Some(input), + ); } } From 4f5749c3aa32dbdde957c7f52ec660559ae9e018 Mon Sep 17 00:00:00 2001 From: Otto Date: Sun, 14 Apr 2024 08:12:53 +0200 Subject: [PATCH 4/7] Clear control side connections when node is dropped (id may be recycled) --- src/context/concrete_base.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/context/concrete_base.rs b/src/context/concrete_base.rs index 0e885387..71d48f24 100644 --- a/src/context/concrete_base.rs +++ b/src/context/concrete_base.rs @@ -287,17 +287,23 @@ impl ConcreteBaseAudioContext { self.inner.render_channel.write().unwrap() } - /// Inform render thread that the control thread `AudioNode` no langer has any handles pub(super) fn mark_node_dropped(&self, id: AudioNodeId) { - // do not drop magic nodes - let magic = id == DESTINATION_NODE_ID - || id == LISTENER_NODE_ID - || LISTENER_PARAM_IDS.contains(&id.0); - - if !magic { - let message = ControlMessage::ControlHandleDropped { id }; - self.send_control_msg(message); + // Ignore magic nodes + if id == DESTINATION_NODE_ID || id == LISTENER_NODE_ID || LISTENER_PARAM_IDS.contains(&id.0) + { + return; } + + // Inform render thread that the control thread AudioNode no langer has any handles + let message = ControlMessage::ControlHandleDropped { id }; + self.send_control_msg(message); + + // Clear the connection administration for this node, the node id may be recycled later + self.inner + .connections + .lock() + .unwrap() + .retain(|&(from, _output, to, _input)| from != id && to != id); } /// Inform render thread that this node can act as a cycle breaker From 9f0613d554a10e5d200de6dbcf222a0a0727451f Mon Sep 17 00:00:00 2001 From: Otto Date: Sun, 14 Apr 2024 08:24:32 +0200 Subject: [PATCH 5/7] Add tests for ConcreteBaseAudioContext connections list --- src/context/concrete_base.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/context/concrete_base.rs b/src/context/concrete_base.rs index 71d48f24..acef2ec5 100644 --- a/src/context/concrete_base.rs +++ b/src/context/concrete_base.rs @@ -531,13 +531,24 @@ mod tests { let node1 = context.create_constant_source(); let node2 = context.create_gain(); + // connection list starts empty + assert!(context.base().inner.connections.lock().unwrap().is_empty()); + node1.disconnect(); // never panic for plain disconnect calls node1.connect(&node2); + + // connection should be registered + assert_eq!(context.base().inner.connections.lock().unwrap().len(), 1); + node1.disconnect(); + assert!(context.base().inner.connections.lock().unwrap().is_empty()); node1.connect(&node2); + assert_eq!(context.base().inner.connections.lock().unwrap().len(), 1); + node1.disconnect_dest(&node2); + assert!(context.base().inner.connections.lock().unwrap().is_empty()); } #[test] @@ -549,4 +560,18 @@ mod tests { node1.disconnect_dest(&node2); } + + #[test] + fn test_mark_node_dropped() { + let context = OfflineAudioContext::new(1, 128, 48000.); + + let node1 = context.create_constant_source(); + let node2 = context.create_gain(); + + node1.connect(&node2); + context.base().mark_node_dropped(node1.registration().id()); + + // dropping should clear connections administration + assert!(context.base().inner.connections.lock().unwrap().is_empty()); + } } From 55bbd2e0c96ae7976e373b0dc0eec8f98f76bf76 Mon Sep 17 00:00:00 2001 From: Otto Date: Sun, 14 Apr 2024 11:44:34 +0200 Subject: [PATCH 6/7] Rename AudioNode connect/disconnect methods with inputs/outputs --- examples/constant_source.rs | 4 ++-- examples/merger.rs | 4 ++-- examples/multichannel.rs | 2 +- src/node/audio_node.rs | 13 +++++++++---- src/node/channel_merger.rs | 8 ++++---- src/node/channel_splitter.rs | 2 +- src/node/delay.rs | 11 ++++++++--- 7 files changed, 27 insertions(+), 17 deletions(-) diff --git a/examples/constant_source.rs b/examples/constant_source.rs index 511076bb..01adf8d0 100644 --- a/examples/constant_source.rs +++ b/examples/constant_source.rs @@ -33,7 +33,7 @@ fn main() { // left branch let gain_left = context.create_gain(); gain_left.gain().set_value(0.); - gain_left.connect_at(&merger, 0, 0); + gain_left.connect_from_output_to_input(&merger, 0, 0); let mut src_left = context.create_oscillator(); src_left.frequency().set_value(200.); @@ -43,7 +43,7 @@ fn main() { // right branch let gain_right = context.create_gain(); gain_right.gain().set_value(0.); - gain_right.connect_at(&merger, 0, 1); + gain_right.connect_from_output_to_input(&merger, 0, 1); let mut src_right = context.create_oscillator(); src_right.frequency().set_value(300.); diff --git a/examples/merger.rs b/examples/merger.rs index deaab2bf..561d1f61 100644 --- a/examples/merger.rs +++ b/examples/merger.rs @@ -46,9 +46,9 @@ fn main() { let merger = context.create_channel_merger(2); // connect left osc to left input of the merger - left.connect_at(&merger, 0, 0); + left.connect_from_output_to_input(&merger, 0, 0); // connect right osc to left input of the merger - right.connect_at(&merger, 0, 1); + right.connect_from_output_to_input(&merger, 0, 1); // Connect the merger to speakers merger.connect(&context.destination()); diff --git a/examples/multichannel.rs b/examples/multichannel.rs index c24ecd16..75e2fe52 100644 --- a/examples/multichannel.rs +++ b/examples/multichannel.rs @@ -57,7 +57,7 @@ fn main() { let now = context.current_time(); let mut osc = context.create_oscillator(); - osc.connect_at(&merger, 0, output_channel); + osc.connect_from_output_to_input(&merger, 0, output_channel); osc.frequency().set_value(200.); osc.start_at(now); osc.stop_at(now + 1.); diff --git a/src/node/audio_node.rs b/src/node/audio_node.rs index 2a36c858..a7ad96f2 100644 --- a/src/node/audio_node.rs +++ b/src/node/audio_node.rs @@ -245,7 +245,7 @@ pub trait AudioNode { /// This function will panic when /// - the AudioContext of the source and destination does not match fn connect<'a>(&self, dest: &'a dyn AudioNode) -> &'a dyn AudioNode { - self.connect_at(dest, 0, 0) + self.connect_from_output_to_input(dest, 0, 0) } /// Connect a specific output of this AudioNode to a specific input of another node. @@ -256,7 +256,7 @@ pub trait AudioNode { /// - the AudioContext of the source and destination does not match /// - if the input port is out of bounds for the destination node /// - if the output port is out of bounds for the source node - fn connect_at<'a>( + fn connect_from_output_to_input<'a>( &self, dest: &'a dyn AudioNode, output: usize, @@ -340,7 +340,7 @@ pub trait AudioNode { /// - the AudioContext of the source and destination does not match /// - if the output port is out of bounds for the source node /// - the source node was not connected to the destination node - fn disconnect_dest_output(&self, dest: &dyn AudioNode, output: usize) { + fn disconnect_dest_from_output(&self, dest: &dyn AudioNode, output: usize) { assert!( self.context() == dest.context(), "InvalidAccessError - Attempting to disconnect nodes from different contexts" @@ -370,7 +370,12 @@ pub trait AudioNode { /// - if the input port is out of bounds for the destination node /// - if the output port is out of bounds for the source node /// - the source node was not connected to the destination node - fn disconnect_dest_output_input(&self, dest: &dyn AudioNode, output: usize, input: usize) { + fn disconnect_dest_from_output_to_input( + &self, + dest: &dyn AudioNode, + output: usize, + input: usize, + ) { assert!( self.context() == dest.context(), "InvalidAccessError - Attempting to disconnect nodes from different contexts" diff --git a/src/node/channel_merger.rs b/src/node/channel_merger.rs index ca198d56..1263914f 100644 --- a/src/node/channel_merger.rs +++ b/src/node/channel_merger.rs @@ -213,12 +213,12 @@ mod tests { let mut src1 = context.create_constant_source(); src1.offset().set_value(2.); - src1.connect_at(&merger, 0, 0); + src1.connect_from_output_to_input(&merger, 0, 0); src1.start(); let mut src2 = context.create_constant_source(); src2.offset().set_value(3.); - src2.connect_at(&merger, 0, 1); + src2.connect_from_output_to_input(&merger, 0, 1); src2.start(); let buffer = context.start_rendering_sync(); @@ -242,12 +242,12 @@ mod tests { let mut src1 = context.create_constant_source(); src1.offset().set_value(2.); - src1.connect_at(&merger, 0, 0); + src1.connect_from_output_to_input(&merger, 0, 0); src1.start(); let mut src2 = context.create_constant_source(); src2.offset().set_value(3.); - src2.connect_at(&merger, 0, 1); + src2.connect_from_output_to_input(&merger, 0, 1); src2.start(); context.suspend_sync(disconnect_at, move |_| src2.disconnect()); diff --git a/src/node/channel_splitter.rs b/src/node/channel_splitter.rs index 993d85e9..f582efa6 100644 --- a/src/node/channel_splitter.rs +++ b/src/node/channel_splitter.rs @@ -265,7 +265,7 @@ mod tests { let splitter = context.create_channel_splitter(2); // connect the 2nd output to the destination - splitter.connect_at(&context.destination(), 1, 0); + splitter.connect_from_output_to_input(&context.destination(), 1, 0); // create buffer with sample value 1. left, value -1. right let audio_buffer = AudioBuffer::from(vec![vec![1.], vec![-1.]], 48000.); diff --git a/src/node/delay.rs b/src/node/delay.rs index 94d2e1b6..1697f7b2 100644 --- a/src/node/delay.rs +++ b/src/node/delay.rs @@ -127,7 +127,7 @@ impl AudioNode for DelayNode { } /// Connect a specific output of this AudioNode to a specific input of another node. - fn connect_at<'a>( + fn connect_from_output_to_input<'a>( &self, dest: &'a dyn AudioNode, output: usize, @@ -212,7 +212,7 @@ impl AudioNode for DelayNode { /// - the AudioContext of the source and destination does not match /// - if the output port is out of bounds for the source node /// - the source node was not connected to the destination node - fn disconnect_dest_output(&self, dest: &dyn AudioNode, output: usize) { + fn disconnect_dest_from_output(&self, dest: &dyn AudioNode, output: usize) { assert!( self.context() == dest.context(), "InvalidAccessError - Attempting to disconnect nodes from different contexts" @@ -242,7 +242,12 @@ impl AudioNode for DelayNode { /// - if the input port is out of bounds for the destination node /// - if the output port is out of bounds for the source node /// - the source node was not connected to the destination node - fn disconnect_dest_output_input(&self, dest: &dyn AudioNode, output: usize, input: usize) { + fn disconnect_dest_from_output_to_input( + &self, + dest: &dyn AudioNode, + output: usize, + input: usize, + ) { assert!( self.context() == dest.context(), "InvalidAccessError - Attempting to disconnect nodes from different contexts" From 4d0553c000f19d3090280996cf9ea48d00d888a3 Mon Sep 17 00:00:00 2001 From: Otto Date: Sun, 14 Apr 2024 11:45:32 +0200 Subject: [PATCH 7/7] Spelling --- src/context/concrete_base.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/context/concrete_base.rs b/src/context/concrete_base.rs index acef2ec5..8f654aea 100644 --- a/src/context/concrete_base.rs +++ b/src/context/concrete_base.rs @@ -294,7 +294,7 @@ impl ConcreteBaseAudioContext { return; } - // Inform render thread that the control thread AudioNode no langer has any handles + // Inform render thread that the control thread AudioNode no longer has any handles let message = ControlMessage::ControlHandleDropped { id }; self.send_control_msg(message);