tim-janik / anklang

MIDI and Audio Synthesizer and Composer
https://anklang.testbit.eu/
Mozilla Public License 2.0
51 stars 3 forks source link

LV2: preset loading stalls DSP thread #37

Open swesterfeld opened 5 months ago

swesterfeld commented 5 months ago

Before I describe the actual problem, let me give you some context. LV2 has a preset mechanism which allows sharing presets between applications. Some plugins also ship with presets. The idea is that a LV2 plugin state is stored in a .ttl file, either in the /usr/lib/lv2 or ~/.lv2 directory. Presets generated by one application (and stored in ~/.lv2) can be loaded by another application. Ardour UI provides a menu with all available presets above the custom UI and a big "+" button to store a new preset. Preset store/load is done by liblilv, so that the actual preset I/O code is compatible between all LV2 hosts.

preset

When I started to implement LV2 support, I wanted to have preset support mainly to test LV2 plugins which have filename properties (such as the liquidsfz or sfizz LV2 plugins). Since we have no string properties, but with preset loading support I could change the filename for the plugin by changing the preset. However, I did not want to mess with the Anklang UI here, so I decided to simply create a choice property and put a list of all presets into the choice. Whenever the choice property is changed by the user, the new preset is now loaded in my LV2 code. Note that this does not cover saving presets (in a way that they could be used by other plugins) at all. So in the Anklang UI, the same preset menu looks like this (which is probably not the final form, but still acceptable from a usability point of view):

anklang

Now to the actual problem: whenever the choice value is changed by the user, a new preset needs to be loaded. Right now, this is implemented by directly switching into the Gtk thread from the DSP thread, and waiting until the preset has been loaded. This is also because we are never allowed to change state while the DSP thread is processing audio.

https://lv2plug.in/c/html/group__state.html documents the restore function (which needs to be called for loading presets) as

"This function (restore) is in the "Instantiation" threading class as defined by LV2. This means it MUST NOT be called concurrently with any other function on the same plugin instance."

Ok, since we obviously do not want to stall the audio thread during preset loading, the way we deal with preset loading needs to be changed. Again here are options.

Option 1

Since presets will be - at some point - implemented properly in Anklang, with the correct UI support - we could for now disable the preset loading code, merge the stuff without it. LV2 support without load/save presets is probably a lot better than no LV2 support.

Option 2

Since we do not want to stall the DSP thread for loading a preset, we could do the following changes:

Since option 2 is quite a bit of coding work, I'd like to have a second opinion here. Should we do this? Should we simply postpone the problem to a later point in time (option 1)? Is there a third way I haven't thought of yet?

swesterfeld commented 5 months ago

Btw, here is a link to the current code in my LV2 branch, in case you want to look at this:

https://github.com/swesterfeld/anklang/blob/d2653b1a08acff0a960fd31b3adc4835ee618e77/ase/lv2device.cc#L2144

tim-janik commented 5 months ago

Since we do not want to stall the DSP thread for loading a preset, we could do the following changes:

  • once the preset is changed in the DSP thread, we send a message to the Anklang thread

    • at the same time we set a flag on the LV2 device to avoid calling run again until the new preset has been loaded; if the render method on the LV2 device is called while a preset is loaded the render method would have to fill all output buffers with zeros

    • in the Anklang thread we can then load the preset while the plugin is essentially disabled and reset the flag

    • we can stall the Anklang thread while the preset is being loaded to ensure that no other LV2 functions run at the same time

    • as soon as the flag is no longer set, the DSP thread can resume calling the LV2 processing function during render

I don't quite understand why the Anklang thread is involved here (and stalling it is not an option). AFAIU, the LV2 code should only be executed from render() calls, and from the gtk-thread, so plugins can rely on being called only from within a) the "main" loop which for LV2 plugins is the glib/gtk-thread, b) the "dsp" thread, which only ever calls render().

Apart from that, this touches on the more general issue of how handling of plugin functions should be implemented that MUST NOT be called in parallel to render(). This also affects CLAP. Ideally, plugins should be able to load new state in the background and queue that into their DSP logic once loading is ready (like the liquidsfz sample loading). But if they cannot do that, the Anklang AudioProcessor logic needs to implement Mute logic. The plugin in question is muted (i.e. AudioProcessor short cuts inputs->outputs) and suspended (in CLAP, this is stop_processing, deactivate), presets/samples are loaded and then audio processing is reactivated.

It is quite possible that you need to implement full Mute + Suspend logic in the AudioProcessor first (+ a UI toggle to also request this explicitly from the front end).

swesterfeld commented 5 months ago

I don't quite understand why the Anklang thread is involved here (and stalling it is not an option).

At the very least we need to use send_param() for all parameters after loading a LV2 preset. Changing the preset usually involves changing all parameters of the LV2 plugin. The AudioProcessor::send_param() has assert_return (this_thread_is_ase(), false); // main_loop thread so I guess this absolutely has to be done from the Anklang thread.

As for whether we absolutely need to stall the Anklang thread, I am somewhat optimistic. That is we could probably do it in these steps

  1. user selects preset
  2. lv2 device sets muted flag
  3. lv2 device sends message to gtk thread
  4. gtk thread loads preset
  5. gtk thread notifies anklang thread with new preset parameters
  6. anklang thread loops over all parameters and sets them using send_param()
  7. anklang thread unsets muted flag
  8. lv2 device resumes processing with new preset

That way we would stall the Gtk thread but not the Anklang thread (which would involve a longer stall) while loading a new preset, and only a minimal amount of work would be done in the Anklang thread after loading the preset.

Ideally, plugins should be able to load new state in the background and queue that into their DSP logic once loading is ready (like the liquidsfz sample loading). But if they cannot do that, the Anklang AudioProcessor logic needs to implement Mute logic.

LV2 allows plugins that do support thread safe restore to declare this in their .ttl file using

https://lv2plug.in/ns/ext/state#threadSafeRestore

In that case the host can call the restore() function while calling the processing function in the DSP thread at the same time. However, not all LV2 plugins support this (for instance SpectMorph does not).

It is quite possible that you need to implement full Mute + Suspend logic in the AudioProcessor first (+ a UI toggle to also request this explicitly from the front end).

If it is a general infrastructure problem, then I am not sure if I am qualified to solve this alone. I could probably make it work just for LV2 with the steps above. But maybe it could be the better decision to focus on making everything else ready-to-merge. Merge the support without preset loading and come back to it later.

swesterfeld commented 5 months ago

Btw, for LV2 plugins that do not have the restore() function defined, it would probably be better to just do

  1. lv2 device sends message to gtk thread
  2. gtk thread loads preset
  3. gtk thread notifies anklang thread with new preset parameters
  4. anklang thread loops over all parameters and sets them using send_param()

without muting, because for instance for the case of a compressor, muting the plugin while loading the preset will cause audible glitches, whereas just setting the parameters (best would be atomically in this case) might at least minimize the audible glitches.

tim-janik commented 5 months ago

LV2 allows plugins that do support thread safe restore to declare this in their .ttl file using

https://lv2plug.in/ns/ext/state#threadSafeRestore

In that case the host can call the restore() function while calling the processing function in the DSP thread at the same time. However, not all LV2 plugins support this (for instance SpectMorph does not).

It is quite possible that you need to implement full Mute + Suspend logic in the AudioProcessor first (+ a UI toggle to also request this explicitly from the front end).

If it is a general infrastructure problem, then I am not sure if I am qualified to solve this alone. I could probably make it work just for LV2 with the steps above.

You are perfectly capable of producing great infrastructure patches, track mute/solo is a good example, and e.g. SpectMorph is an impressive peice of software, all thanks to your excellent programming and design skills!

Also, your recent patch using gtk_init_check is another example showcasing that initial fear is generally unfounded. Be assured, that I will provide any information you could possibly need to work your way through this.

Take a step back, and consider how to implement Mue:On/Off and Activate/Deactive at the UI for BlepSynth and CLAP plugins. Once that is in place, it also nicely solves the LV2 needs we discussed above.

As for AudioProcessor.send_param(), the only existing use cases are BlepSynth/FreeVerb and CLAP plugins, for those send_param() doesn't make sense in the DSP threads, which only leaves the Ase main thread. Thus the assertion you quoted above. Depending on how LV2 is implemented, you might need to call that from the glib&gtk thread or not. Again, take a step back, consider what the right, future proof way to implementing this is, and then give it a try. The assertion can certainly be adapted when it makes sense. Considering your last comment, you might need to implement a vectorized send_param() overload anyway, so you can atomically submit state changes.

Note that all of this is v0.4 milestone material anyway. For now the focus is on getting v0.3 issues finished. After that we can merge On/Off + Activate/Deactive and then figure if LV2 support can also make it into v0.4.

swesterfeld commented 4 months ago

Ok, I have an implementation now which avoids stalling the DSP thread. Maybe this can be improved, but it works. Here is the source code comment I added, which describes how it works.

Asynchronous preset loading: a user can select a new preset in the UI - since this is a choice property of the LV2Processor, the audio thread will get notified during render() that a preset should be loaded, but we don't want to stall the audio thread to actually load the preset (which can take a while, depending on the plugin).

So loading a preset works like this:

Since LV2 does not allow preset loading and calling the run function at the same time, the AudioProcessor checks if the std::atomic contains PRESET_STATE_READY before calling the run function. Only if it does, it calls the run() function. Otherwise, it fills all output buffers with zeros.