nih-plug icon indicating copy to clipboard operation
nih-plug copied to clipboard

Safe way to set many Params at once? (VST3 threading issue)

Open PolyVector opened this issue 2 years ago • 3 comments

Hello! I'm working on a preset browser for my second NIH-plug + egui plugin, and I'm running into issues trying to restore a preset from the UI thread. Many VST3 hosts seem to crash when modifying ~150 plugin params all at once. I've seen it so far in Waveform11/12, Ardour, and Bitwig Studio.

My initial attempt used ParamSetter:

for (param, value) in list_of_params {
        setter.begin_set_parameter(param);
        setter.set_parameter(param, value);
        setter.end_set_parameter(param);
}

... that appeared to work in Standalone and CLAP, but VST3 crashes in the hosts I tried.

My second attempt used the GuiContext directly like this:

let ctx = &setter.raw_context;
let mut state = ctx.get_state().clone();
// Update the state.params here...
ctx.set_state(state);

... that had the same outcome, but also appeared to be executing on the wrong thread according to this Carla error:

JUCE Assertion failure in juce_VST3PluginFormat.cpp:3636

Is there a safe way to set all these parameters?

PolyVector avatar Nov 09 '23 22:11 PolyVector

Don't try to load presets using automation. Hosts definitely shouldn't crash, but they won't appreciate it either. Load the preset the normal way instead: https://nih-plug.robbertvanderhelm.nl/nih_plug/context/gui/trait.GuiContext.html#tymethod.set_state

robbert-vdh avatar Nov 11 '23 15:11 robbert-vdh

set_state crashes the same way for me when used in VST3, so maybe I'm doing something else wrong. I'll have to figure out my debugger this weekend, heh. Thank you.

PolyVector avatar Nov 11 '23 16:11 PolyVector

Hey Robbert, I switched to set_state() for everything, thanks for the guidance, but I've still been having issues with various Linux hosts. After a ton of head scratching and digging around, I believe I was hitting 3 separate issues. I put together a quick repro project for you, and I'll try to explain things here.

  1. There's a race condition that is crashing Linux Bitwig when using set_state() to swap presets. In src/wrapper/vst3/view.rs in the post_task() function, it appears that the task is pushed before the data is written. I see there's a comment about exactly this, but in my tests pushing the task at the end of the function is much safer:
let notify_value = 1i8;
const NOTIFY_VALUE_SIZE: usize = std::mem::size_of::<i8>();
assert_eq!(
    unsafe {
        libc::write(
            self.socket_write_fd,
            &notify_value as *const _ as *const c_void,
            NOTIFY_VALUE_SIZE,
        )
    },
    NOTIFY_VALUE_SIZE as isize
);

self.tasks.push(task)?; // Seems to be much safer here

Ok(())

When I make this change, Bitwig is totally stable.

  1. Task::TriggerRestart(kParamValuesChanged) is called on the wrong thread on Linux, causing a warning in Carla/JUCE In src/wrapper/vst3/inner.rs at the end of the set_state_object_from_gui() function:
// After the state has been updated, notify the host about the new parameter values
let task_posted =
    self.schedule_gui(Task::TriggerRestart( // This makes Carla/JUCE happy
            RestartFlags::kParamValuesChanged as i32,
        ));   

I believe this should call self.schedule_gui() and not the event_loop.schedule_gui() directly. When I make that change, Carla no longer complains.

  1. Perhapse not an issue, but I was getting allocation assertions (that I mistook for bugs) when running my tests. in src/wrapper/vst3/wrapper.rs and src/wrapper/standalone/wrapper.rs there are similar lines:
        // We'll pass the state object back to the GUI thread so deallocation can happen
        // there without potentially blocking the audio thread
        if let Err(err) = self.inner.updated_state_sender.send(state) {
            nih_debug_assert_failure!(
                "Failed to send state object back to GUI thread: {}",
                err
            );
        };

I think the updated_state_sender.send() command is allocating, and wrapping it in a permit_alloc() helped.

Anyways, I hope some of this is useful.

NIH-plug Repro SetState.zip

PolyVector avatar Nov 15 '23 23:11 PolyVector