Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LV2: preset loading stalls DSP thread #37

Open
swesterfeld opened this issue Jan 29, 2024 · 6 comments
Open

LV2: preset loading stalls DSP thread #37

swesterfeld opened this issue Jan 29, 2024 · 6 comments
Milestone

Comments

@swesterfeld
Copy link
Collaborator

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:

  • 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

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
Copy link
Collaborator Author

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
Copy link
Owner

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).

@tim-janik tim-janik added this to the v0.4.0 milestone Jan 29, 2024
@swesterfeld
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Owner

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 added a commit to swesterfeld/anklang that referenced this issue Feb 15, 2024
@swesterfeld
Copy link
Collaborator Author

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:

  • audio thread checks if the preset choice property has been changed by the user
  • audio thread sets preset_to_load_ with the new preset if choice has been changed
  • audio thread stores PRESET_STATE_LOAD to the preset_state_ std::atomic
  • gtk thread periodically scans all PluginInstance objects if std::atomic contains PRESET_STATE_LOAD
  • if so, it restores the preset preset_to_load_ using restore_preset()
  • gtk thread sets std::atomic to PRESET_STATE_FINALIZE
  • gtk thread wakes up ase thread
  • ase thread loops over all PluginInstance objects and checks if std::atomic contains PRESET_STATE_FINALIZE
  • ase thread applies the parameters to the plugin and resets std::atomic to PRESET_STATE_READY

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.

swesterfeld added a commit to swesterfeld/anklang that referenced this issue Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants