diff --git a/src/wrapper/clap/context.rs b/src/wrapper/clap/context.rs index 4adae5bb..e4d42850 100644 --- a/src/wrapper/clap/context.rs +++ b/src/wrapper/clap/context.rs @@ -189,6 +189,6 @@ impl GuiContext for WrapperGuiContext

{ } fn set_state(&self, state: crate::wrapper::state::PluginState) { - self.wrapper.set_state_object(state) + self.wrapper.set_state_object_from_gui(state) } } diff --git a/src/wrapper/clap/wrapper.rs b/src/wrapper/clap/wrapper.rs index 42d72290..e5bf2459 100644 --- a/src/wrapper/clap/wrapper.rs +++ b/src/wrapper/clap/wrapper.rs @@ -1649,7 +1649,7 @@ impl Wrapper

{ /// Update the plugin's internal state, called by the plugin itself from the GUI thread. To /// prevent corrupting data and changing parameters during processing the actual state is only /// updated at the end of the audio processing cycle. - pub fn set_state_object(&self, mut state: PluginState) { + pub fn set_state_object_from_gui(&self, mut state: PluginState) { // Use a loop and timeouts to handle the super rare edge case when this function gets called // between a process call and the host disabling the plugin loop { @@ -1682,27 +1682,7 @@ impl Wrapper

{ } else { // Otherwise we'll set the state right here and now, since this function should be // called from a GUI thread - unsafe { - state::deserialize_object::

( - &mut state, - self.params.clone(), - state::make_params_getter(&self.param_by_hash, &self.param_id_to_hash), - self.current_buffer_config.load().as_ref(), - ); - } - - let audio_io_layout = self.current_audio_io_layout.load(); - if let Some(buffer_config) = self.current_buffer_config.load() { - // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks - let mut init_context = self.make_init_context(); - let mut plugin = self.plugin.lock(); - plugin.initialize(&audio_io_layout, &buffer_config, &mut init_context); - process_wrapper(|| plugin.reset()); - } - - let task_posted = self.schedule_gui(Task::ParameterValuesChanged); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); - + self.set_state_inner(&mut state); break; } } @@ -1747,6 +1727,68 @@ impl Wrapper

{ } } + /// Immediately set the plugin state. Returns `false` if the deserialization failed. The plugin + /// state is set from a couple places, so this function aims to deduplicate that. Includes + /// `permit_alloc()`s around the deserialization and initialization for the use case where + /// `set_state_object_from_gui()` was called while the plugin is process audio. + /// + /// Implicitly emits `Task::ParameterValuesChanged`. + /// + /// # Notes + /// + /// `self.plugin` must _not_ be locked while calling this function or it will deadlock. + pub fn set_state_inner(&self, state: &mut PluginState) -> bool { + let audio_io_layout = self.current_audio_io_layout.load(); + let buffer_config = self.current_buffer_config.load(); + + // FIXME: This is obviously not realtime-safe, but loading presets without doing this could + // lead to inconsistencies. It's the plugin's responsibility to not perform any + // realtime-unsafe work when the initialize function is called a second time if it + // supports runtime preset loading. `state::deserialize_object()` normally never + // allocates, but if the plugin has persistent non-parameter data then its + // `deserialize_fields()` implementation may still allocate. + let mut success = permit_alloc(|| unsafe { + state::deserialize_object::

( + state, + self.params.clone(), + state::make_params_getter(&self.param_by_hash, &self.param_id_to_hash), + self.current_buffer_config.load().as_ref(), + ) + }); + if !success { + nih_debug_assert_failure!("Deserializing plugin state from a state object failed"); + return false; + } + + // If the plugin was already initialized then it needs to be reinitialized + if let Some(buffer_config) = buffer_config { + // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks + let mut init_context = self.make_init_context(); + let mut plugin = self.plugin.lock(); + + // See above + success = permit_alloc(|| { + plugin.initialize(&audio_io_layout, &buffer_config, &mut init_context) + }); + if success { + process_wrapper(|| plugin.reset()); + } + } + + nih_debug_assert!( + success, + "Plugin returned false when reinitializing after loading state" + ); + + // Reinitialize the plugin after loading state so it can respond to the new parameter values + let task_posted = self.schedule_gui(Task::ParameterValuesChanged); + nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + + // TODO: Check for GUI size changes and resize accordingly + + success + } + unsafe extern "C" fn init(plugin: *const clap_plugin) -> bool { check_null_ptr!(false, plugin, (*plugin).plugin_data); let wrapper = &*((*plugin).plugin_data as *const Self); @@ -2362,39 +2404,7 @@ impl Wrapper

{ // doesn't do that let updated_state = permit_alloc(|| wrapper.updated_state_receiver.try_recv()); if let Ok(mut state) = updated_state { - // FIXME: This is obviously not realtime-safe, but loading presets without doing - // this could lead to inconsistencies. It's the plugin's responsibility to - // not perform any realtime-unsafe work when the initialize function is - // called a second time if it supports runtime preset loading. - // `state::deserialize_object()` normally never allocates, but if the plugin - // has persistent non-parameter data then its `deserialize_fields()` - // implementation may still allocate. - permit_alloc(|| { - state::deserialize_object::

( - &mut state, - wrapper.params.clone(), - state::make_params_getter( - &wrapper.param_by_hash, - &wrapper.param_id_to_hash, - ), - wrapper.current_buffer_config.load().as_ref(), - ); - }); - - // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks - let mut init_context = wrapper.make_init_context(); - let audio_io_layout = wrapper.current_audio_io_layout.load(); - let buffer_config = wrapper.current_buffer_config.load().unwrap(); - let mut plugin = wrapper.plugin.lock(); - // See above - permit_alloc(|| { - plugin.initialize(&audio_io_layout, &buffer_config, &mut init_context) - }); - plugin.reset(); - - // Reinitialize the plugin after loading state so it can respond to the new parameter values - let task_posted = wrapper.schedule_gui(Task::ParameterValuesChanged); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + wrapper.set_state_inner(&mut state); // We'll pass the state object back to the GUI thread so deallocation can happen // there without potentially blocking the audio thread @@ -3243,35 +3253,17 @@ impl Wrapper

{ } read_buffer.set_len(length as usize); - let success = state::deserialize_json::

( - &read_buffer, - wrapper.params.clone(), - state::make_params_getter(&wrapper.param_by_hash, &wrapper.param_id_to_hash), - wrapper.current_buffer_config.load().as_ref(), - ); - if !success { - return false; + match state::deserialize_json(&read_buffer) { + Some(mut state) => { + let success = wrapper.set_state_inner(&mut state); + if success { + nih_trace!("Loaded state ({} bytes)", read_buffer.len()); + } + + success + } + None => false, } - - let audio_io_layout = wrapper.current_audio_io_layout.load(); - if let Some(buffer_config) = wrapper.current_buffer_config.load() { - // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks - let mut init_context = wrapper.make_init_context(); - let mut plugin = wrapper.plugin.lock(); - plugin.initialize(&audio_io_layout, &buffer_config, &mut init_context); - // TODO: This also goes for the VST3 version, but should we call reset here? Won't the - // host always restart playback? Check this with a couple of hosts and remove the - // duplicate reset if it's not needed. - process_wrapper(|| plugin.reset()); - } - - // Reinitialize the plugin after loading state so it can respond to the new parameter values - let task_posted = wrapper.schedule_gui(Task::ParameterValuesChanged); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); - - nih_trace!("Loaded state ({length} bytes)"); - - true } unsafe extern "C" fn ext_tail_get(plugin: *const clap_plugin) -> u32 { diff --git a/src/wrapper/standalone/context.rs b/src/wrapper/standalone/context.rs index 5c6001dc..37791be8 100644 --- a/src/wrapper/standalone/context.rs +++ b/src/wrapper/standalone/context.rs @@ -138,6 +138,6 @@ impl> GuiContext for WrapperGuiContext { } fn set_state(&self, state: crate::wrapper::state::PluginState) { - self.wrapper.set_state_object(state) + self.wrapper.set_state_object_from_gui(state) } } diff --git a/src/wrapper/standalone/wrapper.rs b/src/wrapper/standalone/wrapper.rs index d6480417..12e7299e 100644 --- a/src/wrapper/standalone/wrapper.rs +++ b/src/wrapper/standalone/wrapper.rs @@ -411,7 +411,7 @@ impl> Wrapper { /// Update the plugin's internal state, called by the plugin itself from the GUI thread. To /// prevent corrupting data and changing parameters during processing the actual state is only /// updated at the end of the audio processing cycle. - pub fn set_state_object(&self, state: PluginState) { + pub fn set_state_object_from_gui(&self, state: PluginState) { match self.updated_state_sender.send(state) { Ok(_) => { // As mentioned above, the state object will be passed back to this thread @@ -517,38 +517,7 @@ impl> Wrapper { // alternative that doesn't do that let updated_state = permit_alloc(|| self.updated_state_receiver.try_recv()); if let Ok(mut state) = updated_state { - // FIXME: This is obviously not realtime-safe, but loading presets without - // doing this could lead to inconsistencies. It's the plugin's - // responsibility to not perform any realtime-unsafe work when the - // initialize function is called a second time if it supports - // runtime preset loading. `state::deserialize_object()` normally - // never allocates, but if the plugin has persistent non-parameter - // data then its `deserialize_fields()` implementation may still - // allocate. - permit_alloc(|| unsafe { - state::deserialize_object::

( - &mut state, - self.params.clone(), - |param_id| self.param_id_to_ptr.get(param_id).copied(), - Some(&self.buffer_config), - ); - }); - - // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks - let mut init_context = self.make_init_context(); - let mut plugin = self.plugin.lock(); - // See above - permit_alloc(|| { - plugin.initialize( - &self.audio_io_layout, - &self.buffer_config, - &mut init_context, - ) - }); - plugin.reset(); - - let task_posted = self.schedule_gui(Task::ParameterValuesChanged); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + self.set_state_inner(&mut state); // We'll pass the state object back to the GUI thread so deallocation can // happen there without potentially blocking the audio thread @@ -594,4 +563,68 @@ impl> Wrapper { transport, } } + + /// Immediately set the plugin state. Returns `false` if the deserialization failed. In other + /// wrappers state is set from a couple places, so this function is here to be consistent and to + /// centralize all of this behavior. Includes `permit_alloc()`s around the deserialization and + /// initialization for the use case where `set_state_object_from_gui()` was called while the + /// plugin is process audio. + /// + /// Implicitly emits `Task::ParameterValuesChanged`. + /// + /// # Notes + /// + /// `self.plugin` must _not_ be locked while calling this function or it will deadlock. + fn set_state_inner(&self, state: &mut PluginState) -> bool { + // FIXME: This is obviously not realtime-safe, but loading presets without doing this could + // lead to inconsistencies. It's the plugin's responsibility to not perform any + // realtime-unsafe work when the initialize function is called a second time if it + // supports runtime preset loading. `state::deserialize_object()` normally never + // allocates, but if the plugin has persistent non-parameter data then its + // `deserialize_fields()` implementation may still allocate. + let mut success = permit_alloc(|| unsafe { + state::deserialize_object::

( + state, + self.params.clone(), + |param_id| self.param_id_to_ptr.get(param_id).copied(), + Some(&self.buffer_config), + ) + }); + if !success { + nih_debug_assert_failure!("Deserializing plugin state from a state object failed"); + return false; + } + + // If the plugin was already initialized then it needs to be reinitialized + { + // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks + let mut init_context = self.make_init_context(); + let mut plugin = self.plugin.lock(); + + // See above + success = permit_alloc(|| { + plugin.initialize( + &self.audio_io_layout, + &self.buffer_config, + &mut init_context, + ) + }); + if success { + process_wrapper(|| plugin.reset()); + } + } + + nih_debug_assert!( + success, + "Plugin returned false when reinitializing after loading state" + ); + + // Reinitialize the plugin after loading state so it can respond to the new parameter values + let task_posted = self.schedule_gui(Task::ParameterValuesChanged); + nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + + // TODO: Check for GUI size changes and resize accordingly + + success + } } diff --git a/src/wrapper/state.rs b/src/wrapper/state.rs index 52077d81..22707465 100644 --- a/src/wrapper/state.rs +++ b/src/wrapper/state.rs @@ -240,23 +240,13 @@ pub(crate) unsafe fn deserialize_object( true } -/// Deserialize a plugin's state from a vector containing (compressed) JSON data. This can (and -/// should) be shared across plugin formats. Returns `false` and logs an error if the state could -/// not be deserialized. If the `zstd` feature is enabled, then this can -/// -/// Make sure to reinitialize plugin after deserializing the state so it can react to the new -/// parameter values. The smoothers have already been reset by this function. -/// -/// The [`Plugin`] argument is used to call [`Plugin::filter_state()`] just before loading the -/// state. -pub(crate) unsafe fn deserialize_json( - state: &[u8], - plugin_params: Arc, - params_getter: impl Fn(&str) -> Option, - current_buffer_config: Option<&BufferConfig>, -) -> bool { +/// Deserialize a plugin's state from a vector containing (compressed) JSON data. Doesn't load the +/// plugin state since doing so should be accompanied by calls to `Plugin::init()` and +/// `Plugin::reset()`, and this way all of that behavior can be encapsulated so it can be reused in +/// multiple places. The returned state object can be passed to [`deserialize_object()`]. +pub(crate) unsafe fn deserialize_json(state: &[u8]) -> Option { #[cfg(feature = "zstd")] - let mut state: PluginState = match zstd::decode_all(state) { + let result: Option = match zstd::decode_all(state) { Ok(decompressed) => match serde_json::from_slice(decompressed.as_slice()) { Ok(s) => { let state_bytes = decompressed.len(); @@ -267,11 +257,11 @@ pub(crate) unsafe fn deserialize_json( ({compression_ratio:.1}% compression ratio)" ); - s + Some(s) } Err(err) => { nih_debug_assert_failure!("Error while deserializing state: {}", err); - return false; + None } }, // Uncompressed state files can still be loaded after enabling this feature to prevent @@ -279,7 +269,7 @@ pub(crate) unsafe fn deserialize_json( Err(zstd_err) => match serde_json::from_slice(state) { Ok(s) => { nih_trace!("Older uncompressed state found"); - s + Some(s) } Err(json_err) => { nih_debug_assert_failure!( @@ -288,24 +278,19 @@ pub(crate) unsafe fn deserialize_json( zstd_err, json_err ); - return false; + None } }, }; #[cfg(not(feature = "zstd"))] - let mut state: PluginState = match serde_json::from_slice(state) { - Ok(s) => s, + let result: Option = match serde_json::from_slice(state) { + Ok(s) => Some(s), Err(err) => { nih_debug_assert_failure!("Error while deserializing state: {}", err); - return false; + None } }; - deserialize_object::

( - &mut state, - plugin_params, - params_getter, - current_buffer_config, - ) + result } diff --git a/src/wrapper/vst3/context.rs b/src/wrapper/vst3/context.rs index c01f2cbe..0194deb2 100644 --- a/src/wrapper/vst3/context.rs +++ b/src/wrapper/vst3/context.rs @@ -193,6 +193,6 @@ impl GuiContext for WrapperGuiContext

{ } fn set_state(&self, state: PluginState) { - self.inner.set_state_object(state) + self.inner.set_state_object_from_gui(state) } } diff --git a/src/wrapper/vst3/inner.rs b/src/wrapper/vst3/inner.rs index 7dc72c70..75893803 100644 --- a/src/wrapper/vst3/inner.rs +++ b/src/wrapper/vst3/inner.rs @@ -24,6 +24,7 @@ use crate::midi::{MidiConfig, PluginNoteEvent}; use crate::params::internals::ParamPtr; use crate::params::{ParamFlags, Params}; use crate::plugin::{Plugin, ProcessStatus, TaskExecutor, Vst3Plugin}; +use crate::util::permit_alloc; use crate::wrapper::state::{self, PluginState}; use crate::wrapper::util::{hash_param_id, process_wrapper}; @@ -476,7 +477,7 @@ impl WrapperInner

{ /// Update the plugin's internal state, called by the plugin itself from the GUI thread. To /// prevent corrupting data and changing parameters during processing the actual state is only /// updated at the end of the audio processing cycle. - pub fn set_state_object(&self, mut state: PluginState) { + pub fn set_state_object_from_gui(&self, mut state: PluginState) { // Use a loop and timeouts to handle the super rare edge case when this function gets called // between a process call and the host disabling the plugin loop { @@ -509,27 +510,7 @@ impl WrapperInner

{ } else { // Otherwise we'll set the state right here and now, since this function should be // called from a GUI thread - unsafe { - state::deserialize_object::

( - &mut state, - self.params.clone(), - state::make_params_getter(&self.param_by_hash, &self.param_id_to_hash), - self.current_buffer_config.load().as_ref(), - ); - } - - let audio_io_layout = self.current_audio_io_layout.load(); - if let Some(buffer_config) = self.current_buffer_config.load() { - // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks - let mut init_context = self.make_init_context(); - let mut plugin = self.plugin.lock(); - plugin.initialize(&audio_io_layout, &buffer_config, &mut init_context); - process_wrapper(|| plugin.reset()); - } - - let task_posted = self.schedule_gui(Task::ParameterValuesChanged); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); - + self.set_state_inner(&mut state); break; } } @@ -555,6 +536,68 @@ impl WrapperInner

{ nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); } } + + /// Immediately set the plugin state. Returns `false` if the deserialization failed. The plugin + /// state is set from a couple places, so this function aims to deduplicate that. Includes + /// `permit_alloc()`s around the deserialization and initialization for the use case where + /// `set_state_object_from_gui()` was called while the plugin is process audio. + /// + /// Implicitly emits `Task::ParameterValuesChanged`. + /// + /// # Notes + /// + /// `self.plugin` must _not_ be locked while calling this function or it will deadlock. + pub fn set_state_inner(&self, state: &mut PluginState) -> bool { + let audio_io_layout = self.current_audio_io_layout.load(); + let buffer_config = self.current_buffer_config.load(); + + // FIXME: This is obviously not realtime-safe, but loading presets without doing this could + // lead to inconsistencies. It's the plugin's responsibility to not perform any + // realtime-unsafe work when the initialize function is called a second time if it + // supports runtime preset loading. `state::deserialize_object()` normally never + // allocates, but if the plugin has persistent non-parameter data then its + // `deserialize_fields()` implementation may still allocate. + let mut success = permit_alloc(|| unsafe { + state::deserialize_object::

( + state, + self.params.clone(), + state::make_params_getter(&self.param_by_hash, &self.param_id_to_hash), + buffer_config.as_ref(), + ) + }); + if !success { + nih_debug_assert_failure!("Deserializing plugin state from a state object failed"); + return false; + } + + // If the plugin was already initialized then it needs to be reinitialized + if let Some(buffer_config) = buffer_config { + // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks + let mut init_context = self.make_init_context(); + let mut plugin = self.plugin.lock(); + + // See above + success = permit_alloc(|| { + plugin.initialize(&audio_io_layout, &buffer_config, &mut init_context) + }); + if success { + process_wrapper(|| plugin.reset()); + } + } + + nih_debug_assert!( + success, + "Plugin returned false when reinitializing after loading state" + ); + + // Reinitialize the plugin after loading state so it can respond to the new parameter values + let task_posted = self.schedule_gui(Task::ParameterValuesChanged); + nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + + // TODO: Check for GUI size changes and resize accordingly + + success + } } impl MainThreadExecutor> for WrapperInner

{ diff --git a/src/wrapper/vst3/wrapper.rs b/src/wrapper/vst3/wrapper.rs index 370d1327..88ff1d69 100644 --- a/src/wrapper/vst3/wrapper.rs +++ b/src/wrapper/vst3/wrapper.rs @@ -20,7 +20,7 @@ use vst3_sys::vst::{ use vst3_sys::VST3; use widestring::U16CStr; -use super::inner::{ProcessEvent, Task, WrapperInner}; +use super::inner::{ProcessEvent, WrapperInner}; use super::note_expressions::{self, NoteExpressionController}; use super::util::{ u16strlcpy, VstPtr, VST3_MIDI_CCS, VST3_MIDI_NUM_PARAMS, VST3_MIDI_PARAMS_START, @@ -497,35 +497,17 @@ impl IComponent for Wrapper

{ return kResultFalse; } - let success = state::deserialize_json::

( - &read_buffer, - self.inner.params.clone(), - state::make_params_getter(&self.inner.param_by_hash, &self.inner.param_id_to_hash), - self.inner.current_buffer_config.load().as_ref(), - ); - if !success { - return kResultFalse; + match state::deserialize_json(&read_buffer) { + Some(mut state) => { + if self.inner.set_state_inner(&mut state) { + nih_trace!("Loaded state ({} bytes)", read_buffer.len()); + kResultOk + } else { + kResultFalse + } + } + None => kResultFalse, } - - if let Some(buffer_config) = self.inner.current_buffer_config.load() { - // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks - let mut init_context = self.inner.make_init_context(); - let audio_io_layout = self.inner.current_audio_io_layout.load(); - let mut plugin = self.inner.plugin.lock(); - plugin.initialize(&audio_io_layout, &buffer_config, &mut init_context); - // TODO: This also goes for the CLAP version, but should we call reset here? Won't the - // host always restart playback? Check this with a couple of hosts and remove the - // duplicate reset if it's not needed. - process_wrapper(|| plugin.reset()); - } - - // Reinitialize the plugin after loading state so it can respond to the new parameter values - let task_posted = self.inner.schedule_gui(Task::ParameterValuesChanged); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); - - nih_trace!("Loaded state ({} bytes)", read_buffer.len()); - - kResultOk } unsafe fn get_state(&self, state: SharedVstPtr) -> tresult { @@ -1795,38 +1777,7 @@ impl IAudioProcessor for Wrapper

{ // doesn't do that let updated_state = permit_alloc(|| self.inner.updated_state_receiver.try_recv()); if let Ok(mut state) = updated_state { - // FIXME: This is obviously not realtime-safe, but loading presets without doing - // this could lead to inconsistencies. It's the plugin's responsibility to - // not perform any realtime-unsafe work when the initialize function is - // called a second time if it supports runtime preset loading. - // `state::deserialize_object()` normally never allocates, but if the plugin - // has persistent non-parameter data then its `deserialize_fields()` - // implementation may still allocate. - permit_alloc(|| { - state::deserialize_object::

( - &mut state, - self.inner.params.clone(), - state::make_params_getter( - &self.inner.param_by_hash, - &self.inner.param_id_to_hash, - ), - self.inner.current_buffer_config.load().as_ref(), - ); - }); - - // NOTE: This needs to be dropped after the `plugin` lock to avoid deadlocks - let mut init_context = self.inner.make_init_context(); - let audio_io_layout = self.inner.current_audio_io_layout.load(); - let buffer_config = self.inner.current_buffer_config.load().unwrap(); - let mut plugin = self.inner.plugin.lock(); - // See above - permit_alloc(|| { - plugin.initialize(&audio_io_layout, &buffer_config, &mut init_context) - }); - plugin.reset(); - - let task_posted = self.inner.schedule_gui(Task::ParameterValuesChanged); - nih_debug_assert!(task_posted, "The task queue is full, dropping task..."); + self.inner.set_state_inner(&mut state); // We'll pass the state object back to the GUI thread so deallocation can happen // there without potentially blocking the audio thread