Skip to content

Commit 63efe2f

Browse files
committed
Implement AudioNode::disconnect_at and fix disconnect
`disconnect()` would previously clear all connections, but it should only clear the node's *outgoing* connections.
1 parent ea024e8 commit 63efe2f

File tree

6 files changed

+88
-81
lines changed

6 files changed

+88
-81
lines changed

src/context/concrete_base.rs

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ struct ConcreteBaseAudioContextInner {
110110
event_loop: EventLoop,
111111
/// Sender for events that will be handled by the EventLoop
112112
event_send: Sender<EventDispatch>,
113-
/// Current audio graph connections
113+
/// Current audio graph connections (from node, output port, to node, input port)
114114
connections: Mutex<HashSet<(AudioNodeId, usize, AudioNodeId, usize)>>,
115115
}
116116

@@ -433,17 +433,29 @@ impl ConcreteBaseAudioContext {
433433
/// Panics if this node was not connected to the target node
434434
pub(crate) fn disconnect_from(&self, from: AudioNodeId, to: AudioNodeId) {
435435
// check if the node was connected, otherwise panic
436+
let mut has_disconnected = false;
436437
let mut connections = self.inner.connections.lock().unwrap();
437-
let prev_len = connections.len();
438-
connections.retain(|c| c.0 != from || c.2 != to);
439-
let len = connections.len();
438+
connections.retain(|&(c_from, output, c_to, input)| {
439+
let retain = c_from != from || c_to != to;
440+
if !retain {
441+
has_disconnected = true;
442+
let message = ControlMessage::DisconnectNode {
443+
from,
444+
to,
445+
input,
446+
output,
447+
};
448+
self.send_control_msg(message);
449+
}
450+
retain
451+
});
452+
453+
// make sure to drop the MutexGuard before the panic to avoid poisoning
440454
drop(connections);
441-
if prev_len == len {
455+
456+
if !has_disconnected {
442457
panic!("InvalidAccessError - attempting to disconnect unconnected nodes");
443458
}
444-
445-
let message = ControlMessage::DisconnectNode { from, to };
446-
self.send_control_msg(message);
447459
}
448460

449461
/// Disconnects all outgoing connections from the audio node.
@@ -452,9 +464,40 @@ impl ConcreteBaseAudioContext {
452464
.connections
453465
.lock()
454466
.unwrap()
455-
.retain(|c| c.0 != from);
456-
let message = ControlMessage::DisconnectAll { from };
457-
self.send_control_msg(message);
467+
.retain(|&(c_from, output, to, input)| {
468+
let retain = c_from != from;
469+
if !retain {
470+
let message = ControlMessage::DisconnectNode {
471+
from,
472+
to,
473+
input,
474+
output,
475+
};
476+
self.send_control_msg(message);
477+
}
478+
retain
479+
});
480+
}
481+
482+
/// Disconnects all outgoing connections at the given output port from the audio node.
483+
pub(crate) fn disconnect_at(&self, from: AudioNodeId, output: usize) {
484+
self.inner
485+
.connections
486+
.lock()
487+
.unwrap()
488+
.retain(|&(c_from, c_output, to, input)| {
489+
let retain = c_from != from || c_output != output;
490+
if !retain {
491+
let message = ControlMessage::DisconnectNode {
492+
from,
493+
to,
494+
input,
495+
output,
496+
};
497+
self.send_control_msg(message);
498+
}
499+
retain
500+
});
458501
}
459502

460503
/// Connect the `AudioListener` to a `PannerNode`

src/context/offline.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,21 +459,26 @@ mod tests {
459459

460460
#[test]
461461
fn test_suspend_sync() {
462+
use crate::node::ConstantSourceNode;
463+
use std::sync::OnceLock;
464+
462465
let len = RENDER_QUANTUM_SIZE * 4;
463466
let sample_rate = 48000_f64;
464467

465468
let mut context = OfflineAudioContext::new(1, len, sample_rate as f32);
469+
static SOURCE: OnceLock<ConstantSourceNode> = OnceLock::new();
466470

467471
context.suspend_sync(RENDER_QUANTUM_SIZE as f64 / sample_rate, |context| {
468472
assert_eq!(context.state(), AudioContextState::Suspended);
469473
let mut src = context.create_constant_source();
470474
src.connect(&context.destination());
471475
src.start();
476+
SOURCE.set(src).unwrap();
472477
});
473478

474479
context.suspend_sync((3 * RENDER_QUANTUM_SIZE) as f64 / sample_rate, |context| {
475480
assert_eq!(context.state(), AudioContextState::Suspended);
476-
context.destination().disconnect();
481+
SOURCE.get().unwrap().disconnect();
477482
});
478483

479484
let output = context.start_rendering_sync();

src/message.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,12 @@ pub(crate) enum ControlMessage {
2828
},
2929

3030
/// Clear the connection between two given nodes in the audio graph
31-
DisconnectNode { from: AudioNodeId, to: AudioNodeId },
32-
33-
/// Disconnect this node from the audio graph (drop all its connections)
34-
DisconnectAll { from: AudioNodeId },
31+
DisconnectNode {
32+
from: AudioNodeId,
33+
to: AudioNodeId,
34+
input: usize,
35+
output: usize,
36+
},
3537

3638
/// Notify the render thread this node is dropped in the control thread
3739
ControlHandleDropped { id: AudioNodeId },

src/node/audio_node.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,12 @@ pub trait AudioNode {
306306
self.context().disconnect(self.registration().id());
307307
}
308308

309+
/// Disconnects all outgoing connections at the given output port from the AudioNode.
310+
fn disconnect_at(&self, output: usize) {
311+
self.context()
312+
.disconnect_at(self.registration().id(), output);
313+
}
314+
309315
/// The number of inputs feeding into the AudioNode. For source nodes, this will be 0.
310316
fn number_of_inputs(&self) -> usize;
311317

src/render/graph.rs

Lines changed: 6 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -212,28 +212,13 @@ impl Graph {
212212
self.ordered.clear(); // void current ordering
213213
}
214214

215-
pub fn remove_edge(&mut self, source: AudioNodeId, dest: AudioNodeId) {
215+
pub fn remove_edge(&mut self, source: (AudioNodeId, usize), dest: (AudioNodeId, usize)) {
216216
self.nodes
217-
.get_unchecked_mut(source)
217+
.get_unchecked_mut(source.0)
218218
.outgoing_edges
219-
.retain(|edge| edge.other_id != dest);
220-
221-
self.ordered.clear(); // void current ordering
222-
}
223-
224-
pub fn remove_edges_from(&mut self, source: AudioNodeId) {
225-
// Remove outgoing edges
226-
self.nodes.get_unchecked_mut(source).outgoing_edges.clear();
227-
228-
// Remove incoming edges - we need to traverse all nodes
229-
self.nodes.values_mut().for_each(|node| {
230-
// Retain edge when
231-
// - not connected to this node, or
232-
// - when this is an audioparam edge (only disconnect true audio nodes)
233-
node.get_mut()
234-
.outgoing_edges
235-
.retain(|edge| edge.other_id != source || edge.other_index == usize::MAX);
236-
});
219+
.retain(|edge| {
220+
edge.other_id != dest.0 || edge.self_index != source.1 || edge.other_index != dest.1
221+
});
237222

238223
self.ordered.clear(); // void current ordering
239224
}
@@ -635,7 +620,7 @@ mod tests {
635620
assert!(pos2 < pos1); // node 1 depends on node 2
636621

637622
// Detach node 1 (and thus node 2) from the root node
638-
graph.remove_edge(AudioNodeId(1), AudioNodeId(0));
623+
graph.remove_edge((AudioNodeId(1), 0), (AudioNodeId(0), 0));
639624
graph.order_nodes();
640625

641626
// sorting is not deterministic, but this should uphold:
@@ -653,45 +638,6 @@ mod tests {
653638
assert!(pos2 < pos1); // node 1 depends on node 2
654639
}
655640

656-
#[test]
657-
fn test_remove_all() {
658-
let mut graph = Graph::new(llq::Queue::new().split().0);
659-
660-
let node = Box::new(TestNode { tail_time: false });
661-
add_node(&mut graph, 0, node.clone());
662-
add_node(&mut graph, 1, node.clone());
663-
add_node(&mut graph, 2, node);
664-
665-
// link 1->0, 1->2 and 2->0
666-
add_edge(&mut graph, 1, 0);
667-
add_edge(&mut graph, 1, 2);
668-
add_edge(&mut graph, 2, 0);
669-
670-
graph.order_nodes();
671-
672-
assert_eq!(
673-
graph.ordered,
674-
vec![AudioNodeId(1), AudioNodeId(2), AudioNodeId(0)]
675-
);
676-
677-
graph.remove_edges_from(AudioNodeId(1));
678-
graph.order_nodes();
679-
680-
// sorting is not deterministic, but this should uphold:
681-
assert_eq!(graph.ordered.len(), 3); // all nodes present
682-
let pos0 = graph
683-
.ordered
684-
.iter()
685-
.position(|&n| n == AudioNodeId(0))
686-
.unwrap();
687-
let pos2 = graph
688-
.ordered
689-
.iter()
690-
.position(|&n| n == AudioNodeId(2))
691-
.unwrap();
692-
assert!(pos2 < pos0); // node 1 depends on node 0
693-
}
694-
695641
#[test]
696642
fn test_cycle() {
697643
let mut graph = Graph::new(llq::Queue::new().split().0);

src/render/thread.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,16 @@ impl RenderThread {
151151
.unwrap()
152152
.add_edge((from, output), (to, input));
153153
}
154-
DisconnectNode { from, to } => {
155-
self.graph.as_mut().unwrap().remove_edge(from, to);
156-
}
157-
DisconnectAll { from } => {
158-
self.graph.as_mut().unwrap().remove_edges_from(from);
154+
DisconnectNode {
155+
from,
156+
output,
157+
to,
158+
input,
159+
} => {
160+
self.graph
161+
.as_mut()
162+
.unwrap()
163+
.remove_edge((from, output), (to, input));
159164
}
160165
ControlHandleDropped { id } => {
161166
self.graph.as_mut().unwrap().mark_control_handle_dropped(id);

0 commit comments

Comments
 (0)