From 683c96bca0a8541c13077a643a3e04c08f6e125c Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 31 Jan 2023 16:31:29 +0100 Subject: [PATCH] Avoid duplicate parameter callbacks and smoothing REAPER seems to spam value set events when an automation lane is active, and it also always sends CLAP automation events twice. --- src/params.rs | 15 ++++++++--- src/params/boolean.rs | 34 +++++++++++++++--------- src/params/enums.rs | 12 ++++----- src/params/float.rs | 34 +++++++++++++++--------- src/params/integer.rs | 34 +++++++++++++++--------- src/params/internals.rs | 4 +-- src/wrapper/clap/wrapper.rs | 44 ++++++++++++++++++------------- src/wrapper/standalone/wrapper.rs | 16 +++++++---- src/wrapper/state.rs | 14 +++++++--- src/wrapper/vst3/inner.rs | 19 ++++++------- 10 files changed, 138 insertions(+), 88 deletions(-) diff --git a/src/params.rs b/src/params.rs index b9ee9771..a016e018 100644 --- a/src/params.rs +++ b/src/params.rs @@ -198,24 +198,33 @@ pub(crate) trait ParamMut: Param { /// [`modulate_value()`][Self::modulate_value()] has previously been called with a non zero /// value then this offset is taken into account to form the effective value. /// + /// Returns whether or not the value has changed. Any parameter callbacks are only run the value + /// has actually changed. + /// /// This does **not** update the smoother. - fn set_plain_value(&self, plain: Self::Plain); + fn set_plain_value(&self, plain: Self::Plain) -> bool; /// Set this parameter based on a normalized value. The normalized value will be snapped to the /// step size for continuous parameters (i.e. [`FloatParam`]). If /// [`modulate_value()`][Self::modulate_value()] has previously been called with a non zero /// value then this offset is taken into account to form the effective value. /// + /// Returns whether or not the value has changed. Any parameter callbacks are only run the value + /// has actually changed. + /// /// This does **not** update the smoother. - fn set_normalized_value(&self, normalized: f32); + fn set_normalized_value(&self, normalized: f32) -> bool; /// Add a modulation offset to the value's unmodulated value. This value sticks until this /// function is called again with a 0.0 value. Out of bound values will be clamped to the /// parameter's range. The normalized value will be snapped to the step size for continuous /// parameters (i.e. [`FloatParam`]). /// + /// Returns whether or not the value has changed. Any parameter callbacks are only run the value + /// has actually changed. + /// /// This does **not** update the smoother. - fn modulate_value(&self, modulation_offset: f32); + fn modulate_value(&self, modulation_offset: f32) -> bool; /// Update the smoother state to point to the current value. Also used when initializing and /// restoring a plugin so everything is in sync. In that case the smoother should completely diff --git a/src/params/boolean.rs b/src/params/boolean.rs index 97f28570..ba807263 100644 --- a/src/params/boolean.rs +++ b/src/params/boolean.rs @@ -168,7 +168,7 @@ impl Param for BoolParam { } impl ParamMut for BoolParam { - fn set_plain_value(&self, plain: Self::Plain) { + fn set_plain_value(&self, plain: Self::Plain) -> bool { let unmodulated_value = plain; let unmodulated_normalized_value = self.preview_normalized(plain); @@ -182,20 +182,28 @@ impl ParamMut for BoolParam { (self.preview_plain(normalized_value), normalized_value) }; - self.value.store(value, Ordering::Relaxed); - self.normalized_value - .store(normalized_value, Ordering::Relaxed); - self.unmodulated_value - .store(unmodulated_value, Ordering::Relaxed); - self.unmodulated_normalized_value - .store(unmodulated_normalized_value, Ordering::Relaxed); + // REAPER spams automation events with the same value. This prevents callbacks from firing + // multiple times. This can be problematic when they're used to trigger expensive + // computations when a parameter changes. + let old_value = self.value.swap(value, Ordering::Relaxed); + if value != old_value { + self.normalized_value + .store(normalized_value, Ordering::Relaxed); + self.unmodulated_value + .store(unmodulated_value, Ordering::Relaxed); + self.unmodulated_normalized_value + .store(unmodulated_normalized_value, Ordering::Relaxed); + if let Some(f) = &self.value_changed { + f(value); + } - if let Some(f) = &self.value_changed { - f(value); + true + } else { + false } } - fn set_normalized_value(&self, normalized: f32) { + fn set_normalized_value(&self, normalized: f32) -> bool { // NOTE: The double conversion here is to make sure the state is reproducible. State is // saved and restored using plain values, and the new normalized value will be // different from `normalized`. This is not necessary for the modulation as these @@ -203,12 +211,12 @@ impl ParamMut for BoolParam { self.set_plain_value(self.preview_plain(normalized)) } - fn modulate_value(&self, modulation_offset: f32) { + fn modulate_value(&self, modulation_offset: f32) -> bool { self.modulation_offset .store(modulation_offset, Ordering::Relaxed); // TODO: This renormalizes this value, which is not necessary - self.set_plain_value(self.unmodulated_plain_value()); + self.set_plain_value(self.unmodulated_plain_value()) } fn update_smoother(&self, _sample_rate: f32, _init: bool) { diff --git a/src/params/enums.rs b/src/params/enums.rs index 5fb5e0da..b913df16 100644 --- a/src/params/enums.rs +++ b/src/params/enums.rs @@ -294,15 +294,15 @@ impl Param for EnumParamInner { } impl ParamMut for EnumParam { - fn set_plain_value(&self, plain: Self::Plain) { + fn set_plain_value(&self, plain: Self::Plain) -> bool { self.inner.set_plain_value(T::to_index(plain) as i32) } - fn set_normalized_value(&self, normalized: f32) { + fn set_normalized_value(&self, normalized: f32) -> bool { self.inner.set_normalized_value(normalized) } - fn modulate_value(&self, modulation_offset: f32) { + fn modulate_value(&self, modulation_offset: f32) -> bool { self.inner.modulate_value(modulation_offset) } @@ -312,15 +312,15 @@ impl ParamMut for EnumParam { } impl ParamMut for EnumParamInner { - fn set_plain_value(&self, plain: Self::Plain) { + fn set_plain_value(&self, plain: Self::Plain) -> bool { self.inner.set_plain_value(plain) } - fn set_normalized_value(&self, normalized: f32) { + fn set_normalized_value(&self, normalized: f32) -> bool { self.inner.set_normalized_value(normalized) } - fn modulate_value(&self, modulation_offset: f32) { + fn modulate_value(&self, modulation_offset: f32) -> bool { self.inner.modulate_value(modulation_offset) } diff --git a/src/params/float.rs b/src/params/float.rs index 922278f7..3f80169a 100644 --- a/src/params/float.rs +++ b/src/params/float.rs @@ -202,7 +202,7 @@ impl Param for FloatParam { } impl ParamMut for FloatParam { - fn set_plain_value(&self, plain: Self::Plain) { + fn set_plain_value(&self, plain: Self::Plain) -> bool { let unmodulated_value = plain; let unmodulated_normalized_value = self.preview_normalized(plain); @@ -216,20 +216,28 @@ impl ParamMut for FloatParam { (self.preview_plain(normalized_value), normalized_value) }; - self.value.store(value, Ordering::Relaxed); - self.normalized_value - .store(normalized_value, Ordering::Relaxed); - self.unmodulated_value - .store(unmodulated_value, Ordering::Relaxed); - self.unmodulated_normalized_value - .store(unmodulated_normalized_value, Ordering::Relaxed); + // REAPER spams automation events with the same value. This prevents callbacks from firing + // multiple times. This can be problematic when they're used to trigger expensive + // computations when a parameter changes. + let old_value = self.value.swap(value, Ordering::Relaxed); + if value != old_value { + self.normalized_value + .store(normalized_value, Ordering::Relaxed); + self.unmodulated_value + .store(unmodulated_value, Ordering::Relaxed); + self.unmodulated_normalized_value + .store(unmodulated_normalized_value, Ordering::Relaxed); + if let Some(f) = &self.value_changed { + f(value); + } - if let Some(f) = &self.value_changed { - f(value); + true + } else { + false } } - fn set_normalized_value(&self, normalized: f32) { + fn set_normalized_value(&self, normalized: f32) -> bool { // NOTE: The double conversion here is to make sure the state is reproducible. State is // saved and restored using plain values, and the new normalized value will be // different from `normalized`. This is not necessary for the modulation as these @@ -237,12 +245,12 @@ impl ParamMut for FloatParam { self.set_plain_value(self.preview_plain(normalized)) } - fn modulate_value(&self, modulation_offset: f32) { + fn modulate_value(&self, modulation_offset: f32) -> bool { self.modulation_offset .store(modulation_offset, Ordering::Relaxed); // TODO: This renormalizes this value, which is not necessary - self.set_plain_value(self.unmodulated_plain_value()); + self.set_plain_value(self.unmodulated_plain_value()) } fn update_smoother(&self, sample_rate: f32, reset: bool) { diff --git a/src/params/integer.rs b/src/params/integer.rs index ad5d51e5..d6ecc22d 100644 --- a/src/params/integer.rs +++ b/src/params/integer.rs @@ -182,7 +182,7 @@ impl Param for IntParam { } impl ParamMut for IntParam { - fn set_plain_value(&self, plain: Self::Plain) { + fn set_plain_value(&self, plain: Self::Plain) -> bool { let unmodulated_value = plain; let unmodulated_normalized_value = self.preview_normalized(plain); @@ -196,20 +196,28 @@ impl ParamMut for IntParam { (self.preview_plain(normalized_value), normalized_value) }; - self.value.store(value, Ordering::Relaxed); - self.normalized_value - .store(normalized_value, Ordering::Relaxed); - self.unmodulated_value - .store(unmodulated_value, Ordering::Relaxed); - self.unmodulated_normalized_value - .store(unmodulated_normalized_value, Ordering::Relaxed); + // REAPER spams automation events with the same value. This prevents callbacks from firing + // multiple times. This can be problematic when they're used to trigger expensive + // computations when a parameter changes. + let old_value = self.value.swap(value, Ordering::Relaxed); + if value != old_value { + self.normalized_value + .store(normalized_value, Ordering::Relaxed); + self.unmodulated_value + .store(unmodulated_value, Ordering::Relaxed); + self.unmodulated_normalized_value + .store(unmodulated_normalized_value, Ordering::Relaxed); + if let Some(f) = &self.value_changed { + f(value); + } - if let Some(f) = &self.value_changed { - f(value); + true + } else { + false } } - fn set_normalized_value(&self, normalized: f32) { + fn set_normalized_value(&self, normalized: f32) -> bool { // NOTE: The double conversion here is to make sure the state is reproducible. State is // saved and restored using plain values, and the new normalized value will be // different from `normalized`. This is not necessary for the modulation as these @@ -217,12 +225,12 @@ impl ParamMut for IntParam { self.set_plain_value(self.preview_plain(normalized)) } - fn modulate_value(&self, modulation_offset: f32) { + fn modulate_value(&self, modulation_offset: f32) -> bool { self.modulation_offset .store(modulation_offset, Ordering::Relaxed); // TODO: This renormalizes this value, which is not necessary - self.set_plain_value(self.unmodulated_plain_value()); + self.set_plain_value(self.unmodulated_plain_value()) } fn update_smoother(&self, sample_rate: f32, reset: bool) { diff --git a/src/params/internals.rs b/src/params/internals.rs index aa76e022..9ba92069 100644 --- a/src/params/internals.rs +++ b/src/params/internals.rs @@ -74,8 +74,8 @@ impl ParamPtr { param_ptr_forward!(pub unsafe fn string_to_normalized_value(&self, string: &str) -> Option); param_ptr_forward!(pub unsafe fn flags(&self) -> ParamFlags); - param_ptr_forward!(pub(crate) unsafe fn set_normalized_value(&self, normalized: f32)); - param_ptr_forward!(pub(crate) unsafe fn modulate_value(&self, modulation_offset: f32)); + param_ptr_forward!(pub(crate) unsafe fn set_normalized_value(&self, normalized: f32) -> bool); + param_ptr_forward!(pub(crate) unsafe fn modulate_value(&self, modulation_offset: f32) -> bool); param_ptr_forward!(pub(crate) unsafe fn update_smoother(&self, sample_rate: f32, reset: bool)); // These functions involve casts since the plugin formats only do floating point types, so we diff --git a/src/wrapper/clap/wrapper.rs b/src/wrapper/clap/wrapper.rs index af7eb8be..b47c8fc6 100644 --- a/src/wrapper/clap/wrapper.rs +++ b/src/wrapper/clap/wrapper.rs @@ -866,17 +866,20 @@ impl Wrapper

{ let normalized_value = clap_plain_value as f32 / unsafe { param_ptr.step_count() }.unwrap_or(1) as f32; - // Also update the parameter's smoothing if applicable - unsafe { param_ptr.set_normalized_value(normalized_value) }; - if let Some(sample_rate) = sample_rate { - unsafe { param_ptr.update_smoother(sample_rate, false) }; - } + if unsafe { param_ptr.set_normalized_value(normalized_value) } { + if let Some(sample_rate) = sample_rate { + unsafe { param_ptr.update_smoother(sample_rate, false) }; + } - // The GUI needs to be informed about the changed parameter value. This - // triggers an `Editor::param_value_changed()` call on the GUI thread. - let task_posted = - self.schedule_gui(Task::ParameterValueChanged(hash, normalized_value)); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + // The GUI needs to be informed about the changed parameter value. This + // triggers an `Editor::param_value_changed()` call on the GUI thread. + let task_posted = self + .schedule_gui(Task::ParameterValueChanged(hash, normalized_value)); + nih_debug_assert!( + task_posted, + "The task queue is full, dropping task..." + ); + } true } @@ -884,15 +887,20 @@ impl Wrapper

{ let normalized_delta = clap_plain_delta as f32 / unsafe { param_ptr.step_count() }.unwrap_or(1) as f32; - // Also update the parameter's smoothing if applicable - unsafe { param_ptr.modulate_value(normalized_delta) }; - if let Some(sample_rate) = sample_rate { - unsafe { param_ptr.update_smoother(sample_rate, false) }; - } + if unsafe { param_ptr.modulate_value(normalized_delta) } { + if let Some(sample_rate) = sample_rate { + unsafe { param_ptr.update_smoother(sample_rate, false) }; + } - let task_posted = self - .schedule_gui(Task::ParameterModulationChanged(hash, normalized_delta)); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + let task_posted = self.schedule_gui(Task::ParameterModulationChanged( + hash, + normalized_delta, + )); + nih_debug_assert!( + task_posted, + "The task queue is full, dropping task..." + ); + } true } diff --git a/src/wrapper/standalone/wrapper.rs b/src/wrapper/standalone/wrapper.rs index 7c80dceb..b2780545 100644 --- a/src/wrapper/standalone/wrapper.rs +++ b/src/wrapper/standalone/wrapper.rs @@ -522,11 +522,17 @@ impl Wrapper { while let Some((param_ptr, normalized_value)) = self.unprocessed_param_changes.pop() { - unsafe { param_ptr.set_normalized_value(normalized_value) }; - unsafe { param_ptr.update_smoother(sample_rate, false) }; - let task_posted = self - .schedule_gui(Task::ParameterValueChanged(param_ptr, normalized_value)); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + if unsafe { param_ptr.set_normalized_value(normalized_value) } { + unsafe { param_ptr.update_smoother(sample_rate, false) }; + let task_posted = self.schedule_gui(Task::ParameterValueChanged( + param_ptr, + normalized_value, + )); + nih_debug_assert!( + task_posted, + "The task queue is full, dropping task..." + ); + } } // After processing audio, we'll check if the editor has sent us updated plugin diff --git a/src/wrapper/state.rs b/src/wrapper/state.rs index 913076d4..8fff90bb 100644 --- a/src/wrapper/state.rs +++ b/src/wrapper/state.rs @@ -182,14 +182,20 @@ pub(crate) unsafe fn deserialize_object( }; match (param_ptr, param_value) { - (ParamPtr::FloatParam(p), ParamValue::F32(v)) => (*p).set_plain_value(*v), - (ParamPtr::IntParam(p), ParamValue::I32(v)) => (*p).set_plain_value(*v), - (ParamPtr::BoolParam(p), ParamValue::Bool(v)) => (*p).set_plain_value(*v), + (ParamPtr::FloatParam(p), ParamValue::F32(v)) => { + (*p).set_plain_value(*v); + } + (ParamPtr::IntParam(p), ParamValue::I32(v)) => { + (*p).set_plain_value(*v); + } + (ParamPtr::BoolParam(p), ParamValue::Bool(v)) => { + (*p).set_plain_value(*v); + } // Enums are either serialized based on the active variant's index (which may not be the // same as the discriminator), or a custom set stable string ID. The latter allows the // variants to be reordered. (ParamPtr::EnumParam(p), ParamValue::I32(variant_idx)) => { - (*p).set_plain_value(*variant_idx) + (*p).set_plain_value(*variant_idx); } (ParamPtr::EnumParam(p), ParamValue::String(id)) => { let deserialized_enum = (*p).set_from_id(id); diff --git a/src/wrapper/vst3/inner.rs b/src/wrapper/vst3/inner.rs index e58a652e..92426b7d 100644 --- a/src/wrapper/vst3/inner.rs +++ b/src/wrapper/vst3/inner.rs @@ -448,18 +448,15 @@ impl WrapperInner

{ ) -> tresult { match self.param_by_hash.get(&hash) { Some(param_ptr) => { - // Also update the parameter's smoothing if applicable - match (param_ptr, sample_rate) { - (_, Some(sample_rate)) => unsafe { - param_ptr.set_normalized_value(normalized_value); - param_ptr.update_smoother(sample_rate, false); - }, - _ => unsafe { param_ptr.set_normalized_value(normalized_value) }, - } + if unsafe { param_ptr.set_normalized_value(normalized_value) } { + if let Some(sample_rate) = sample_rate { + unsafe { param_ptr.update_smoother(sample_rate, false) }; + } - let task_posted = - self.schedule_gui(Task::ParameterValueChanged(hash, normalized_value)); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + let task_posted = + self.schedule_gui(Task::ParameterValueChanged(hash, normalized_value)); + nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + } kResultOk }