From b177e36800ce9dbc93b2f28382afa6cb64436bae Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 18 Jun 2023 16:30:05 +0200 Subject: [PATCH] AudioParam: Reduce code duplication and Arc usage - Extract common fields into AudioParamBase - Extract atomic fields into AudioParamShared wrapped into a single Arc --- src/param.rs | 173 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 104 insertions(+), 69 deletions(-) diff --git a/src/param.rs b/src/param.rs index b31ef27e..0eaa9879 100644 --- a/src/param.rs +++ b/src/param.rs @@ -194,24 +194,23 @@ impl AudioParamEventTimeline { /// AudioParam controls an individual aspect of an AudioNode's functionality, such as volume. pub struct AudioParam { registration: AudioContextRegistration, - is_a_rate: Arc, - automation_rate_constrained: bool, + raw_parts: AudioParamRaw, +} + +// helper struct shared between the actual param and the processor +#[derive(Debug, Clone)] +pub(crate) struct AudioParamBase { + shared_parts: Arc, default_value: f32, // readonly min_value: f32, // readonly max_value: f32, // readonly - current_value: Arc, - sender: Sender, } // helper struct to attach / detach to context (for borrow reasons) #[derive(Clone)] pub(crate) struct AudioParamRaw { - is_a_rate: Arc, + base_parts: AudioParamBase, automation_rate_constrained: bool, - default_value: f32, - min_value: f32, - max_value: f32, - current_value: Arc, sender: Sender, } @@ -255,7 +254,13 @@ impl AudioNode for AudioParam { impl AudioParam { /// Current value of the automation rate of the AudioParam pub fn automation_rate(&self) -> AutomationRate { - if self.is_a_rate.load(Ordering::SeqCst) { + if self + .raw_parts + .base_parts + .shared_parts + .is_a_rate + .load(Ordering::SeqCst) + { AutomationRate::A } else { AutomationRate::K @@ -268,28 +273,32 @@ impl AudioParam { /// /// Some nodes have automation rate constraints and may panic when updating the value pub fn set_automation_rate(&self, value: AutomationRate) { - if self.automation_rate_constrained && value != self.automation_rate() { + if self.raw_parts.automation_rate_constrained && value != self.automation_rate() { panic!("InvalidStateError: automation rate cannot be changed for this param"); } let is_a_rate = value == AutomationRate::A; - self.is_a_rate.store(is_a_rate, Ordering::SeqCst); + self.raw_parts + .base_parts + .shared_parts + .is_a_rate + .store(is_a_rate, Ordering::SeqCst); } pub(crate) fn set_automation_rate_constrained(&mut self, value: bool) { - self.automation_rate_constrained = value; + self.raw_parts.automation_rate_constrained = value; } pub fn default_value(&self) -> f32 { - self.default_value + self.raw_parts.base_parts.default_value } pub fn min_value(&self) -> f32 { - self.min_value + self.raw_parts.base_parts.min_value } pub fn max_value(&self) -> f32 { - self.max_value + self.raw_parts.base_parts.max_value } /// Retrieve the current value of the `AudioParam`. @@ -304,7 +313,11 @@ impl AudioParam { // test_exponential_ramp_a_rate_multiple_blocks // test_exponential_ramp_k_rate_multiple_blocks pub fn value(&self) -> f32 { - self.current_value.load(Ordering::SeqCst) + self.raw_parts + .base_parts + .shared_parts + .current_value + .load(Ordering::SeqCst) } /// Set the value of the `AudioParam`. @@ -320,8 +333,15 @@ impl AudioParam { // cf. https://www.w3.org/TR/webaudio/#dom-audioparam-value pub fn set_value(&self, value: f32) -> &Self { // current_value should always be clamped - let clamped = value.clamp(self.min_value, self.max_value); - self.current_value.store(clamped, Ordering::SeqCst); + let clamped = value.clamp( + self.raw_parts.base_parts.min_value, + self.raw_parts.base_parts.max_value, + ); + self.raw_parts + .base_parts + .shared_parts + .current_value + .store(clamped, Ordering::SeqCst); // this event is meant to update param intrinsic value before any calculation // is done, will behave as SetValueAtTime with `time == block_timestamp` @@ -539,53 +559,48 @@ impl AudioParam { // helper function to detach from context (for borrow reasons) pub(crate) fn into_raw_parts(self) -> AudioParamRaw { - AudioParamRaw { - is_a_rate: self.is_a_rate, - automation_rate_constrained: self.automation_rate_constrained, - default_value: self.default_value, - min_value: self.min_value, - max_value: self.max_value, - current_value: self.current_value, - sender: self.sender, - } + let Self { + registration: _, + raw_parts, + } = self; + raw_parts } // helper function to attach to context (for borrow reasons) pub(crate) fn from_raw_parts( registration: AudioContextRegistration, - parts: AudioParamRaw, + raw_parts: AudioParamRaw, ) -> Self { Self { registration, - is_a_rate: parts.is_a_rate, - automation_rate_constrained: parts.automation_rate_constrained, - default_value: parts.default_value, - min_value: parts.min_value, - max_value: parts.max_value, - current_value: parts.current_value, - sender: parts.sender, + raw_parts, } } fn send_event(&self, event: AudioParamEvent) { if cfg!(test) { // bypass audiocontext enveloping of control messages for simpler testing - self.sender.send(event).unwrap(); + self.raw_parts.sender.send(event).unwrap(); } else { - self.context().pass_audio_param_event(&self.sender, event); + self.context() + .pass_audio_param_event(&self.raw_parts.sender, event); } } } +// Atomic fields of `AudioParam` that could be safely shared between threads +// when wrapped into an `Arc`. +#[derive(Debug)] +pub(crate) struct AudioParamShared { + current_value: AtomicF32, + is_a_rate: AtomicBool, +} + #[derive(Debug)] pub(crate) struct AudioParamProcessor { intrinsic_value: f32, - current_value: Arc, receiver: Receiver, - is_a_rate: Arc, - default_value: f32, - min_value: f32, - max_value: f32, + param: AudioParamBase, event_timeline: AudioParamEventTimeline, last_event: Option, buffer: Vec, @@ -632,13 +647,13 @@ impl AudioParamProcessor { let mut value = self.buffer[0]; if value.is_nan() { - value = self.default_value; + value = self.param.default_value; } output.set_single_valued(true); let output_channel = output.channel_data_mut(0); - output_channel[0] = value.clamp(self.min_value, self.max_value); + output_channel[0] = value.clamp(self.param.min_value, self.param.max_value); } else { // @note: we could add two other optimizations here: // - when buffer.len() == 1 and buffer[0] == 0., then we don't need to @@ -656,10 +671,10 @@ impl AudioParamProcessor { *o += p; if o.is_nan() { - *o = self.default_value; + *o = self.param.default_value; } - *o = o.clamp(self.min_value, self.max_value) + *o = o.clamp(self.param.min_value, self.param.max_value) }); } } @@ -767,7 +782,7 @@ impl AudioParamProcessor { // from the value at the beginning of the vent, while // Chrome just keeps the current intrinsic_value // The spec is not very clear there, but Firefox - // seems to be the more compliant: + // seems to be more compliant: // "Any active automations whose automation event // time is less than cancelTime are also cancelled, // and such cancellations may cause discontinuities @@ -986,13 +1001,18 @@ impl AudioParamProcessor { fn compute_buffer(&mut self, block_time: f64, dt: f64, count: usize) { // Set [[current value]] to the value of paramIntrinsicValue at the // beginning of this render quantum. - let clamped = self.intrinsic_value.clamp(self.min_value, self.max_value); - self.current_value.store(clamped, Ordering::SeqCst); + let clamped = self + .intrinsic_value + .clamp(self.param.min_value, self.param.max_value); + self.param + .shared_parts + .current_value + .store(clamped, Ordering::SeqCst); // clear the buffer for this block self.buffer.clear(); - let is_a_rate = self.is_a_rate.load(Ordering::SeqCst); + let is_a_rate = self.param.shared_parts.is_a_rate.load(Ordering::SeqCst); let is_k_rate = !is_a_rate; let next_block_time = dt.mul_add(count as f64, block_time); @@ -1530,37 +1550,52 @@ impl AudioParamProcessor { } pub(crate) fn audio_param_pair( - opts: AudioParamDescriptor, + descriptor: AudioParamDescriptor, registration: AudioContextRegistration, ) -> (AudioParam, AudioParamProcessor) { let (sender, receiver) = crossbeam_channel::unbounded(); - let current_value = Arc::new(AtomicF32::new(opts.default_value)); - let is_a_rate = Arc::new(AtomicBool::new(opts.automation_rate == AutomationRate::A)); - let param = AudioParam { - registration, - is_a_rate: is_a_rate.clone(), - automation_rate_constrained: false, - default_value: opts.default_value, - min_value: opts.min_value, - max_value: opts.max_value, - current_value: current_value.clone(), - sender, + let AudioParamDescriptor { + automation_rate, + default_value, + max_value, + min_value, + } = descriptor; + + let is_a_rate = match automation_rate { + AutomationRate::A => true, + AutomationRate::K => false, + }; + let shared_parts = AudioParamShared { + current_value: AtomicF32::new(default_value), + is_a_rate: AtomicBool::new(is_a_rate), + }; + + let base_parts = AudioParamBase { + shared_parts: Arc::new(shared_parts), + default_value, + min_value, + max_value, }; let render = AudioParamProcessor { - intrinsic_value: opts.default_value, - current_value, + intrinsic_value: default_value, receiver, - is_a_rate, - default_value: opts.default_value, - min_value: opts.min_value, - max_value: opts.max_value, + param: base_parts.clone(), event_timeline: AudioParamEventTimeline::new(), last_event: None, buffer: Vec::with_capacity(RENDER_QUANTUM_SIZE), }; + let param = AudioParam { + registration, + raw_parts: AudioParamRaw { + base_parts, + automation_rate_constrained: false, + sender, + }, + }; + (param, render) }