diff --git a/Cargo.toml b/Cargo.toml index 5fad2487..06e0349a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,6 @@ no_denormals = "0.1.2" num-complex = "0.4" realfft = "3.3" rubato = "0.14" -rustc-hash = "1.1" smallvec = "1.11" symphonia = { version = "0.5", default-features = false } vecmath = "1.0" diff --git a/src/context/concrete_base.rs b/src/context/concrete_base.rs index a5aed670..9adfcecc 100644 --- a/src/context/concrete_base.rs +++ b/src/context/concrete_base.rs @@ -17,6 +17,34 @@ use crossbeam_channel::{Receiver, SendError, Sender}; use std::sync::atomic::{AtomicU64, AtomicU8, Ordering}; use std::sync::{Arc, Mutex, RwLock, RwLockWriteGuard}; +/// This struct assigns new [`AudioNodeId`]s for [`AudioNode`]s +/// +/// It reuses the ids of decommissioned nodes to prevent unbounded growth of the audio graphs node +/// list (which is stored in a Vec indexed by the AudioNodeId). +struct AudioNodeIdProvider { + /// incrementing id + id_inc: AtomicU64, + /// receiver for decommissioned AudioNodeIds, which can be reused + id_consumer: Mutex>, +} + +impl AudioNodeIdProvider { + fn new(id_consumer: llq::Consumer) -> Self { + Self { + id_inc: AtomicU64::new(0), + id_consumer: Mutex::new(id_consumer), + } + } + + fn get(&self) -> AudioNodeId { + if let Some(available_id) = self.id_consumer.lock().unwrap().pop() { + llq::Node::into_inner(available_id) + } else { + AudioNodeId(self.id_inc.fetch_add(1, Ordering::Relaxed)) + } + } +} + /// The struct that corresponds to the Javascript `BaseAudioContext` object. /// /// This object is returned from the `base()` method on @@ -47,8 +75,8 @@ struct ConcreteBaseAudioContextInner { sample_rate: f32, /// max number of speaker output channels max_channel_count: usize, - /// incrementing id to assign to audio nodes - node_id_inc: AtomicU64, + /// provider for new AudioNodeIds + audio_node_id_provider: AudioNodeIdProvider, /// destination node's current channel count destination_channel_config: ChannelConfig, /// message channel from control to render thread @@ -83,9 +111,8 @@ impl BaseAudioContext for ConcreteBaseAudioContext { &self, f: F, ) -> T { - // create unique identifier for this node - let id = self.inner.node_id_inc.fetch_add(1, Ordering::SeqCst); - let id = AudioNodeId(id); + // create a unique id for this node + let id = self.inner.audio_node_id_provider.get(); let registration = AudioContextRegistration { id, context: self.clone(), @@ -97,6 +124,7 @@ impl BaseAudioContext for ConcreteBaseAudioContext { // pass the renderer to the audio graph let message = ControlMessage::RegisterNode { id, + reclaim_id: llq::Node::new(id), node: render, inputs: node.number_of_inputs(), outputs: node.number_of_outputs(), @@ -126,6 +154,7 @@ impl ConcreteBaseAudioContext { render_channel: Sender, event_channel: Option<(Sender, Receiver)>, offline: bool, + node_id_consumer: llq::Consumer, ) -> Self { let event_loop = EventLoop::new(); let (event_send, event_recv) = match event_channel { @@ -133,12 +162,14 @@ impl ConcreteBaseAudioContext { Some((send, recv)) => (Some(send), Some(recv)), }; + let audio_node_id_provider = AudioNodeIdProvider::new(node_id_consumer); + let base_inner = ConcreteBaseAudioContextInner { sample_rate, max_channel_count, render_channel: RwLock::new(render_channel), queued_messages: Mutex::new(Vec::new()), - node_id_inc: AtomicU64::new(0), + audio_node_id_provider, destination_channel_config: ChannelConfigOptions::default().into(), frames_played, queued_audio_listener_msgs: Mutex::new(Vec::new()), @@ -202,7 +233,10 @@ impl ConcreteBaseAudioContext { // Validate if the hardcoded node IDs line up debug_assert_eq!( - base.inner.node_id_inc.load(Ordering::Relaxed), + base.inner + .audio_node_id_provider + .id_inc + .load(Ordering::Relaxed), LISTENER_PARAM_IDS.end, ); @@ -420,3 +454,19 @@ impl ConcreteBaseAudioContext { self.inner.event_loop.clear_handler(event); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_provide_node_id() { + let (mut id_producer, id_consumer) = llq::Queue::new().split(); + let provider = AudioNodeIdProvider::new(id_consumer); + assert_eq!(provider.get().0, 0); // newly assigned + assert_eq!(provider.get().0, 1); // newly assigned + id_producer.push(llq::Node::new(AudioNodeId(0))); + assert_eq!(provider.get().0, 0); // reused + assert_eq!(provider.get().0, 2); // newly assigned + } +} diff --git a/src/context/offline.rs b/src/context/offline.rs index 5a8947ed..d49ce298 100644 --- a/src/context/offline.rs +++ b/src/context/offline.rs @@ -43,7 +43,8 @@ impl OfflineAudioContext { // unbounded is fine because it does not need to be realtime safe let (sender, receiver) = crossbeam_channel::unbounded(); - let graph = crate::render::graph::Graph::new(); + let (node_id_producer, node_id_consumer) = llq::Queue::new().split(); + let graph = crate::render::graph::Graph::new(node_id_producer); let message = crate::message::ControlMessage::Startup { graph }; sender.send(message).unwrap(); @@ -67,6 +68,7 @@ impl OfflineAudioContext { sender, None, true, + node_id_consumer, ); Self { diff --git a/src/context/online.rs b/src/context/online.rs index ff6918e2..4172080f 100644 --- a/src/context/online.rs +++ b/src/context/online.rs @@ -173,7 +173,8 @@ impl AudioContext { event_recv, } = control_thread_init; - let graph = Graph::new(); + let (node_id_producer, node_id_consumer) = llq::Queue::new().split(); + let graph = Graph::new(node_id_producer); let message = ControlMessage::Startup { graph }; ctrl_msg_send.send(message).unwrap(); @@ -184,6 +185,7 @@ impl AudioContext { ctrl_msg_send, Some((event_send, event_recv)), false, + node_id_consumer, ); base.set_state(AudioContextState::Running); diff --git a/src/media_devices/mod.rs b/src/media_devices/mod.rs index 0ce54326..3fe7d70b 100644 --- a/src/media_devices/mod.rs +++ b/src/media_devices/mod.rs @@ -4,7 +4,7 @@ //! //! -use rustc_hash::FxHasher; +use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; use crate::context::{AudioContextLatencyCategory, AudioContextOptions}; @@ -54,7 +54,7 @@ impl DeviceId { index, }; - let mut hasher = FxHasher::default(); + let mut hasher = DefaultHasher::new(); device_info.hash(&mut hasher); format!("{}", hasher.finish()) } diff --git a/src/message.rs b/src/message.rs index 58bd8e7a..f1bf7e59 100644 --- a/src/message.rs +++ b/src/message.rs @@ -14,6 +14,7 @@ pub(crate) enum ControlMessage { /// Register a new node in the audio graph RegisterNode { id: AudioNodeId, + reclaim_id: llq::Node, node: Box, inputs: usize, outputs: usize, diff --git a/src/render/graph.rs b/src/render/graph.rs index 2af19ebe..34b07111 100644 --- a/src/render/graph.rs +++ b/src/render/graph.rs @@ -4,10 +4,9 @@ use std::cell::RefCell; use std::panic::{self, AssertUnwindSafe}; use crate::context::AudioNodeId; -use rustc_hash::FxHashMap; use smallvec::{smallvec, SmallVec}; -use super::{Alloc, AudioParamValues, AudioProcessor, AudioRenderQuantum}; +use super::{Alloc, AudioParamValues, AudioProcessor, AudioRenderQuantum, NodeCollection}; use crate::node::ChannelConfig; use crate::render::RenderScope; @@ -23,6 +22,8 @@ struct OutgoingEdge { /// Renderer Node in the Audio Graph pub struct Node { + /// AudioNodeId, to be sent back to the control thread when this node is dropped + reclaim_id: Option>, /// Renderer: converts inputs to outputs processor: Box, /// Reusable input buffers @@ -76,10 +77,11 @@ impl Node { /// The audio graph pub(crate) struct Graph { /// Processing Nodes - nodes: FxHashMap>, + nodes: NodeCollection, /// Allocator for audio buffers alloc: Alloc, - + /// Message channel to notify control thread of reclaimable AudioNodeIds + reclaim_id_channel: llq::Producer, /// Topological ordering of the nodes ordered: Vec, /// Topological sorting helper @@ -93,15 +95,16 @@ pub(crate) struct Graph { } impl Graph { - pub fn new() -> Self { + pub fn new(reclaim_id_channel: llq::Producer) -> Self { Graph { - nodes: FxHashMap::default(), + nodes: NodeCollection::new(), + alloc: Alloc::with_capacity(64), + reclaim_id_channel, ordered: vec![], marked: vec![], marked_temp: vec![], in_cycle: vec![], cycle_breakers: vec![], - alloc: Alloc::with_capacity(64), } } @@ -114,6 +117,7 @@ impl Graph { pub fn add_node( &mut self, index: AudioNodeId, + reclaim_id: llq::Node, processor: Box, number_of_inputs: usize, number_of_outputs: usize, @@ -129,6 +133,7 @@ impl Graph { self.nodes.insert( index, RefCell::new(Node { + reclaim_id: Some(reclaim_id), processor, inputs, outputs, @@ -142,9 +147,7 @@ impl Graph { } pub fn add_edge(&mut self, source: (AudioNodeId, usize), dest: (AudioNodeId, usize)) { - self.nodes - .get_mut(&source.0) - .unwrap_or_else(|| panic!("cannot connect {:?} to {:?}", source, dest)) + self.nodes[source.0] .get_mut() .outgoing_edges .push(OutgoingEdge { @@ -157,9 +160,7 @@ impl Graph { } pub fn remove_edge(&mut self, source: AudioNodeId, dest: AudioNodeId) { - self.nodes - .get_mut(&source) - .unwrap_or_else(|| panic!("cannot remove the edge from {:?} to {:?}", source, dest)) + self.nodes[source] .get_mut() .outgoing_edges .retain(|edge| edge.other_id != dest); @@ -168,12 +169,7 @@ impl Graph { } pub fn remove_edges_from(&mut self, source: AudioNodeId) { - self.nodes - .get_mut(&source) - .unwrap_or_else(|| panic!("cannot remove edges from {:?}", source)) - .get_mut() - .outgoing_edges - .clear(); + self.nodes[source].get_mut().outgoing_edges.clear(); self.nodes.values_mut().for_each(|node| { node.get_mut() @@ -188,22 +184,17 @@ impl Graph { // 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) { + if let Some(node) = self.nodes.get_mut(index) { node.get_mut().free_when_finished = true; } } pub fn mark_cycle_breaker(&mut self, index: AudioNodeId) { - self.nodes.get_mut(&index).unwrap().get_mut().cycle_breaker = true; + self.nodes[index].get_mut().cycle_breaker = true; } pub fn route_message(&mut self, index: AudioNodeId, msg: &mut dyn Any) { - self.nodes - .get_mut(&index) - .unwrap() - .get_mut() - .processor - .onmessage(msg); + self.nodes[index].get_mut().processor.onmessage(msg); } /// Helper function for `order_nodes` - traverse node and outgoing edges @@ -226,7 +217,7 @@ impl Graph { let cycle_breaker_node = marked_temp .iter() .skip(pos) - .find(|node_id| self.nodes.get(node_id).unwrap().borrow().cycle_breaker); + .find(|&&node_id| self.nodes[node_id].borrow().cycle_breaker); match cycle_breaker_node { Some(&node_id) => { @@ -255,14 +246,7 @@ impl Graph { marked_temp.push(node_id); // Visit outgoing nodes, and call `visit` on them recursively - for edge in self - .nodes - .get(&node_id) - .unwrap() - .borrow() - .outgoing_edges - .iter() - { + for edge in self.nodes[node_id].borrow().outgoing_edges.iter() { let cycle_breaker_applied = self.visit( edge.other_id, marked, @@ -324,7 +308,7 @@ impl Graph { // since the audio graph could contain legs detached from the destination and those should // still be rendered. let mut cycle_breaker_applied = false; - for &node_id in self.nodes.keys() { + for node_id in self.nodes.keys() { cycle_breaker_applied = self.visit( node_id, &mut marked, @@ -343,8 +327,7 @@ impl Graph { // clear the outgoing edges of the nodes that have been recognized as cycle breaker let nodes = &mut self.nodes; cycle_breakers.iter().for_each(|node_id| { - let node = nodes.get_mut(node_id).unwrap(); - node.get_mut().outgoing_edges.clear(); + nodes[*node_id].get_mut().outgoing_edges.clear(); }); continue; @@ -385,7 +368,7 @@ impl Graph { // process every node, in topological sorted order self.ordered.iter().for_each(|index| { // acquire a mutable borrow of the current processing node - let mut node = nodes.get(index).unwrap().borrow_mut(); + let mut node = nodes[*index].borrow_mut(); // make sure all input buffers have the correct number of channels, this might not be // the case if the node has no inputs connected or the channel count has just changed @@ -396,7 +379,7 @@ impl Graph { .for_each(|i| i.mix(count, interpretation)); // let the current node process (catch any panics that may occur) - let params = AudioParamValues::from(&*nodes); + let params = AudioParamValues::from(nodes); scope.node_id.set(*index); let (success, tail_time) = { // We are abusing AssertUnwindSafe here, we cannot guarantee it upholds. @@ -420,7 +403,7 @@ impl Graph { // audio params are connected to the 'hidden' usize::MAX output, ignore them here .filter(|edge| edge.other_index != usize::MAX) .for_each(|edge| { - let mut output_node = nodes.get(&edge.other_id).unwrap().borrow_mut(); + let mut output_node = nodes[edge.other_id].borrow_mut(); output_node.has_inputs_connected = true; let signal = &node.outputs[edge.self_index]; let channel_config = &output_node.channel_config.clone(); @@ -446,25 +429,41 @@ impl Graph { // Check if we can decommission this node (end of life) if can_free { // Node is dropped, remove it from the node list - nodes.remove(index); + let mut node = nodes.remove(*index).into_inner(); + self.reclaim_id_channel + .push(node.reclaim_id.take().unwrap()); + drop(node); // And remove it from the ordering after we have processed all nodes nodes_dropped = true; // Nodes are only dropped when they do not have incoming connections. // But they may have AudioParams feeding into them, these can de dropped too. - nodes.retain(|id, n| { + nodes.retain(|id, node| { + let node = node.get_mut(); // unwrap the RefCell + // Check if this node was connected to the dropped node. In that case, it is // either an AudioParam (which can be dropped), or the AudioListener that feeds // into a PannerNode (which can be disconnected). - let outgoing_edges = &mut n.borrow_mut().outgoing_edges; - let prev_len = outgoing_edges.len(); - outgoing_edges.retain(|e| e.other_id != *index); - let was_connected = outgoing_edges.len() != prev_len; - - let special = id.0 < 2; // never drop Listener and Destination node - special || !was_connected - }); + let was_connected = { + let outgoing_edges = &mut node.outgoing_edges; + let prev_len = outgoing_edges.len(); + outgoing_edges.retain(|e| e.other_id != *index); + outgoing_edges.len() != prev_len + }; + + // Retain when + // - 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; + + if !retain { + self.reclaim_id_channel + .push(node.reclaim_id.take().unwrap()); + } + retain + }) } }); @@ -472,7 +471,7 @@ impl Graph { if nodes_dropped { let mut i = 0; while i < self.ordered.len() { - if !nodes.contains_key(&self.ordered[i]) { + if nodes.get(self.ordered[i]).is_none() { self.ordered.remove(i); } else { i += 1; @@ -481,12 +480,7 @@ impl Graph { } // Return the output buffer of destination node - &self - .nodes - .get_mut(&AudioNodeId(0)) - .unwrap() - .get_mut() - .outputs[0] + &self.nodes[AudioNodeId(0)].get_mut().outputs[0] } } @@ -495,7 +489,9 @@ mod tests { use super::*; #[derive(Debug, Clone)] - struct TestNode {} + struct TestNode { + tail_time: bool, + } impl AudioProcessor for TestNode { fn process( @@ -505,7 +501,7 @@ mod tests { _params: AudioParamValues<'_>, _scope: &RenderScope, ) -> bool { - false + self.tail_time } } @@ -518,19 +514,33 @@ mod tests { .into() } + fn add_node(graph: &mut Graph, id: u64, node: Box) { + let id = AudioNodeId(id); + let reclaim_id = llq::Node::new(id); + graph.add_node(id, reclaim_id, node, 1, 1, config()); + } + + fn add_edge(graph: &mut Graph, from: u64, to: u64) { + graph.add_edge((AudioNodeId(from), 0), (AudioNodeId(to), 0)); + } + + fn add_audioparam(graph: &mut Graph, from: u64, to: u64) { + graph.add_edge((AudioNodeId(from), 0), (AudioNodeId(to), usize::MAX)); + } + #[test] fn test_add_remove() { - let mut graph = Graph::new(); + let mut graph = Graph::new(llq::Queue::new().split().0); - let node = Box::new(TestNode {}); - graph.add_node(AudioNodeId(0), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(1), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(2), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(3), node, 1, 1, config()); + 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.clone()); + add_node(&mut graph, 3, node); - graph.add_edge((AudioNodeId(1), 0), (AudioNodeId(0), 0)); - graph.add_edge((AudioNodeId(2), 0), (AudioNodeId(1), 0)); - graph.add_edge((AudioNodeId(3), 0), (AudioNodeId(0), 0)); + add_edge(&mut graph, 1, 0); + add_edge(&mut graph, 2, 1); + add_edge(&mut graph, 3, 0); graph.order_nodes(); @@ -571,17 +581,17 @@ mod tests { #[test] fn test_remove_all() { - let mut graph = Graph::new(); + let mut graph = Graph::new(llq::Queue::new().split().0); - let node = Box::new(TestNode {}); - graph.add_node(AudioNodeId(0), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(1), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(2), node, 1, 1, config()); + 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 - graph.add_edge((AudioNodeId(1), 0), (AudioNodeId(0), 0)); - graph.add_edge((AudioNodeId(1), 0), (AudioNodeId(2), 0)); - graph.add_edge((AudioNodeId(2), 0), (AudioNodeId(0), 0)); + add_edge(&mut graph, 1, 0); + add_edge(&mut graph, 1, 2); + add_edge(&mut graph, 2, 0); graph.order_nodes(); @@ -610,21 +620,21 @@ mod tests { #[test] fn test_cycle() { - let mut graph = Graph::new(); + let mut graph = Graph::new(llq::Queue::new().split().0); - let node = Box::new(TestNode {}); - graph.add_node(AudioNodeId(0), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(1), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(2), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(3), node.clone(), 1, 1, config()); - graph.add_node(AudioNodeId(4), node, 1, 1, config()); + 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.clone()); + add_node(&mut graph, 3, node.clone()); + add_node(&mut graph, 4, node); // link 4->2, 2->1, 1->0, 1->2, 3->0 - graph.add_edge((AudioNodeId(4), 0), (AudioNodeId(2), 0)); - graph.add_edge((AudioNodeId(2), 0), (AudioNodeId(1), 0)); - graph.add_edge((AudioNodeId(1), 0), (AudioNodeId(0), 0)); - graph.add_edge((AudioNodeId(1), 0), (AudioNodeId(2), 0)); - graph.add_edge((AudioNodeId(3), 0), (AudioNodeId(0), 0)); + add_edge(&mut graph, 4, 2); + add_edge(&mut graph, 2, 1); + add_edge(&mut graph, 1, 0); + add_edge(&mut graph, 1, 2); + add_edge(&mut graph, 3, 0); graph.order_nodes(); @@ -642,4 +652,95 @@ mod tests { // a-cyclic part should be present assert!(pos3.unwrap() < pos0.unwrap()); } + + #[test] + fn test_lifecycle_and_reclaim() { + let (node_id_producer, mut node_id_consumer) = llq::Queue::new().split(); + let mut graph = Graph::new(node_id_producer); + + let node = Box::new(TestNode { tail_time: false }); + + // Destination Node is always node id 0, and should never drop + add_node(&mut graph, 0, node.clone()); + + // AudioListener Node is always node id 1, and should never drop + add_node(&mut graph, 1, node.clone()); + + // Add a regular node at id 3, it has tail time false so after rendering it should be + // 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; + + // Connect the regular node to the AudioDestinationNode + add_edge(&mut graph, 2, 0); + + // Render a single quantum + let scope = RenderScope { + current_frame: 0, + current_time: 0., + sample_rate: 48000., + node_id: std::cell::Cell::new(AudioNodeId(0)), + event_sender: None, + }; + graph.render(&scope); + + // The dropped node should be our regular node, not the AudioListener + let reclaimed = node_id_consumer + .pop() + .expect("should have decommisioned node"); + assert_eq!(reclaimed.0, 2); + + // No other dropped nodes + assert!(node_id_consumer.pop().is_none()); + } + + #[test] + fn test_audio_param_lifecycle() { + let (node_id_producer, mut node_id_consumer) = llq::Queue::new().split(); + let mut graph = Graph::new(node_id_producer); + + let node = Box::new(TestNode { tail_time: false }); + + // Destination Node is always node id 0, and should never drop + add_node(&mut graph, 0, node.clone()); + + // AudioListener Node is always node id 1, and should never drop + add_node(&mut graph, 1, node.clone()); + + // Add a regular node at id 3, it has tail time false so after rendering it should be + // 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; + + // Connect the regular node to the AudioDestinationNode + add_edge(&mut graph, 2, 0); + + // Add an AudioParam at id 4, it should be dropped alongside the regular node + 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; + + // Connect the audioparam to the regular node + add_audioparam(&mut graph, 3, 2); + + // Render a single quantum + let scope = RenderScope { + current_frame: 0, + current_time: 0., + sample_rate: 48000., + node_id: std::cell::Cell::new(AudioNodeId(0)), + event_sender: None, + }; + graph.render(&scope); + + // First the regular node should be dropped, then the audioparam + assert_eq!(node_id_consumer.pop().unwrap().0, 2); + assert_eq!(node_id_consumer.pop().unwrap().0, 3); + + // No other dropped nodes + assert!(node_id_consumer.pop().is_none()); + } } diff --git a/src/render/mod.rs b/src/render/mod.rs index d9b1a62f..b05642f7 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -11,4 +11,8 @@ pub(crate) use thread::*; mod processor; pub use processor::*; mod quantum; + +mod node_collection; +pub(crate) use node_collection::NodeCollection; + pub use quantum::*; diff --git a/src/render/node_collection.rs b/src/render/node_collection.rs new file mode 100644 index 00000000..69204abb --- /dev/null +++ b/src/render/node_collection.rs @@ -0,0 +1,106 @@ +use crate::context::AudioNodeId; +use crate::render::graph::Node; + +use std::cell::RefCell; +use std::ops::{Index, IndexMut}; + +pub(crate) struct NodeCollection { + nodes: Vec>>, +} + +impl NodeCollection { + pub fn new() -> Self { + let mut instance = Self { + nodes: Vec::with_capacity(64), + }; + instance.ensure_capacity(64); + instance + } + + pub fn is_empty(&self) -> bool { + self.nodes.is_empty() + } + + #[inline(always)] + fn ensure_capacity(&mut self, new_len: usize) { + self.nodes + .resize_with(new_len.max(self.nodes.len()), || None); + } + + #[inline(always)] + pub fn insert(&mut self, index: AudioNodeId, value: RefCell) { + let index = index.0 as usize; + self.ensure_capacity(index + 1); + self.nodes[index] = Some(value); + } + + #[inline(always)] + pub fn remove(&mut self, index: AudioNodeId) -> RefCell { + self.nodes[index.0 as usize] + .take() + .expect("Unable to remove non-existing Node in NodeCollection") + } + + #[inline(always)] + pub fn keys(&self) -> impl Iterator + '_ { + self.nodes + .iter() + .enumerate() + .filter_map(|(i, v)| v.as_ref().and(Some(AudioNodeId(i as u64)))) + } + + #[inline(always)] + pub fn values_mut(&mut self) -> impl Iterator> { + self.nodes.iter_mut().filter_map(Option::as_mut) + } + + #[inline(always)] + pub fn get(&self, index: AudioNodeId) -> Option<&RefCell> { + self.nodes[index.0 as usize].as_ref() + } + + #[inline(always)] + pub fn get_mut(&mut self, index: AudioNodeId) -> Option<&mut RefCell> { + self.nodes[index.0 as usize].as_mut() + } + + #[inline(always)] + pub fn retain(&mut self, mut f: F) + where + F: FnMut(AudioNodeId, &mut RefCell) -> bool, + { + self.nodes.iter_mut().enumerate().for_each(|(i, opt)| { + if let Some(v) = opt.as_mut() { + if !f(AudioNodeId(i as u64), v) { + *opt = None; + } + } + }) + } +} + +impl Index for NodeCollection { + type Output = RefCell; + + #[track_caller] + #[inline(always)] + fn index(&self, index: AudioNodeId) -> &Self::Output { + self.nodes + .get(index.0 as usize) + .unwrap_or_else(|| panic!("Unexpected index {} for NodeCollection", index.0)) + .as_ref() + .unwrap_or_else(|| panic!("Index {} for dropped Node in NodeCollection", index.0)) + } +} + +impl IndexMut for NodeCollection { + #[track_caller] + #[inline(always)] + fn index_mut(&mut self, index: AudioNodeId) -> &mut Self::Output { + self.nodes + .get_mut(index.0 as usize) + .unwrap_or_else(|| panic!("Unexpected index {} for NodeCollection", index.0)) + .as_mut() + .unwrap_or_else(|| panic!("Index {} for dropped Node in NodeCollection", index.0)) + } +} diff --git a/src/render/processor.rs b/src/render/processor.rs index 64d1d321..8fee65bb 100644 --- a/src/render/processor.rs +++ b/src/render/processor.rs @@ -3,11 +3,10 @@ use crate::context::{AudioNodeId, AudioParamId}; use crate::events::{ErrorEvent, EventDispatch}; use crate::{Event, RENDER_QUANTUM_SIZE}; -use super::{graph::Node, AudioRenderQuantum}; +use super::{graph::Node, AudioRenderQuantum, NodeCollection}; use crossbeam_channel::Sender; -use rustc_hash::FxHashMap; -use std::cell::{Cell, RefCell}; +use std::cell::Cell; use std::any::Any; use std::ops::Deref; @@ -140,11 +139,11 @@ impl Deref for DerefAudioRenderQuantumChannel<'_> { /// /// Provided to implementations of [`AudioProcessor`] in the render thread pub struct AudioParamValues<'a> { - nodes: &'a FxHashMap>, + nodes: &'a NodeCollection, } impl<'a> AudioParamValues<'a> { - pub(crate) fn from(nodes: &'a FxHashMap>) -> Self { + pub(crate) fn from(nodes: &'a NodeCollection) -> Self { Self { nodes } } @@ -155,7 +154,7 @@ impl<'a> AudioParamValues<'a> { /// provide a slice of length equal to the render quantum size (default: 128) #[allow(clippy::missing_panics_doc)] pub fn get(&self, index: &AudioParamId) -> impl Deref + '_ { - DerefAudioRenderQuantumChannel(self.nodes.get(&index.into()).unwrap().borrow()) + DerefAudioRenderQuantumChannel(self.nodes[index.into()].borrow()) } pub(crate) fn listener_params(&self) -> [impl Deref + '_; 9] { diff --git a/src/render/thread.rs b/src/render/thread.rs index dd4f73f9..606fb492 100644 --- a/src/render/thread.rs +++ b/src/render/thread.rs @@ -95,6 +95,7 @@ impl RenderThread { match msg { RegisterNode { id: node_id, + reclaim_id, node, inputs, outputs, @@ -102,6 +103,7 @@ impl RenderThread { } => { self.graph.as_mut().unwrap().add_node( node_id, + reclaim_id, node, inputs, outputs, diff --git a/tests/online.rs b/tests/online.rs index 67ef774a..7f9f7d5d 100644 --- a/tests/online.rs +++ b/tests/online.rs @@ -143,7 +143,34 @@ fn test_panner_node_drop_panic() { // A crashed thread will not fail the test (only if the main thread panics). // Instead inspect if there is progression of time in the audio context. + let time = context.current_time(); + std::thread::sleep(std::time::Duration::from_millis(200)); + assert!(context.current_time() >= time + 0.15); +} +#[test] +fn test_audioparam_outlives_audionode() { + let options = AudioContextOptions { + sink_id: "none".into(), + ..AudioContextOptions::default() + }; + let context = AudioContext::new(options); + + // Create a node with an audioparam, drop to node but keep the audioparam + let gain = context.create_gain(); + let gain_param = gain.gain().clone(); + drop(gain); + + // Start the audio graph, and give some time to drop the gain node (it has no inputs connected + // so dynamic lifetime will drop the node); + std::thread::sleep(std::time::Duration::from_millis(200)); + + // We still have a handle to the param, so that should not be removed from the audio graph. + // So by updating the value, the render thread should not crash. + gain_param.set_value(1.); + + // A crashed thread will not fail the test (only if the main thread panics). + // Instead inspect if there is progression of time in the audio context. let time = context.current_time(); std::thread::sleep(std::time::Duration::from_millis(200)); assert!(context.current_time() >= time + 0.15);