Skip to content

Commit 701be64

Browse files
committed
No Clone for AudioContextRegistration because the Drop has side effects
Rewrite the AudioProcessingEvent a bit to circumvent this limitation
1 parent 37c75a3 commit 701be64

File tree

3 files changed

+18
-10
lines changed

3 files changed

+18
-10
lines changed

src/context/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,8 @@ impl From<u8> for AudioContextState {
8888
/// Only when implementing the AudioNode trait manually, this struct is of any concern.
8989
///
9090
/// This object allows for communication with the render thread and dynamic lifetime management.
91-
//
92-
// The only way to construct this object is by calling [`BaseAudioContext::register`]
93-
#[derive(Clone)]
91+
// The only way to construct this object is by calling [`BaseAudioContext::register`].
92+
// This struct should not derive Clone because of the Drop handler.
9493
pub struct AudioContextRegistration {
9594
/// the audio context in which nodes and connections lives
9695
context: ConcreteBaseAudioContext,

src/events.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::context::ConcreteBaseAudioContext;
12
use crate::context::{AudioContextState, AudioNodeId};
23
use crate::{AudioBuffer, AudioRenderCapacityEvent};
34

@@ -52,13 +53,17 @@ pub struct AudioProcessingEvent {
5253
/// The time when the audio will be played in the same time coordinate system as the
5354
/// AudioContext's currentTime.
5455
pub playback_time: f64,
55-
pub(crate) registration: Option<Arc<crate::context::AudioContextRegistration>>,
56+
pub(crate) registration: Option<(ConcreteBaseAudioContext, AudioNodeId)>,
5657
}
5758

5859
impl Drop for AudioProcessingEvent {
5960
fn drop(&mut self) {
60-
if let Some(registration) = self.registration.take() {
61-
registration.post_message(self.output_buffer.clone());
61+
if let Some((context, id)) = self.registration.take() {
62+
let wrapped = crate::message::ControlMessage::NodeMessage {
63+
id,
64+
msg: llq::Node::new(Box::new(self.output_buffer.clone())),
65+
};
66+
context.send_control_msg(wrapped);
6267
}
6368
}
6469
}

src/node/script_processor.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use crate::render::{
77
use crate::{AudioBuffer, RENDER_QUANTUM_SIZE};
88

99
use std::any::Any;
10-
use std::sync::Arc;
1110

1211
/// Options for constructing an [`ScriptProcessorNode`]
1312
#[derive(Clone, Debug)]
@@ -138,20 +137,25 @@ impl ScriptProcessorNode {
138137
/// the inputBuffer attribute. The audio data which is the result of the processing (or the
139138
/// synthesized data if there are no inputs) is then placed into the outputBuffer.
140139
///
140+
/// The output buffer is shipped back to the render thread when the AudioProcessingEvent goes
141+
/// out of scope, so be sure not to store it somewhere.
142+
///
141143
/// Only a single event handler is active at any time. Calling this method multiple times will
142144
/// override the previous event handler.
143145
pub fn set_onaudioprocess<F: FnMut(AudioProcessingEvent) + Send + 'static>(
144146
&self,
145147
mut callback: F,
146148
) {
147-
// TODO, hack: use Arc to prevent drop of AudioContextRegistration
148-
let registration = Arc::new(self.registration.clone());
149+
// We need these fields to ship the output buffer to the render thread
150+
let base = self.registration().context().clone();
151+
let id = self.registration().id();
152+
149153
let callback = move |v| {
150154
let mut payload = match v {
151155
EventPayload::AudioProcessing(v) => v,
152156
_ => unreachable!(),
153157
};
154-
payload.registration = Some(Arc::clone(&registration));
158+
payload.registration = Some((base.clone(), id));
155159
callback(payload);
156160
};
157161

0 commit comments

Comments
 (0)