-
Notifications
You must be signed in to change notification settings - Fork 4
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
Comments
Btw, here is a link to the current code in my LV2 branch, in case you want to look at this: |
I don't quite understand why the Anklang thread is involved here (and stalling it is not an option). 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. 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). |
At the very least we need to use 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
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.
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
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. |
Btw, for LV2 plugins that do not have the restore() function defined, it would probably be better to just do
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. |
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 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>k 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. |
See tim-janik#37. Signed-off-by: Stefan Westerfeld <[email protected]>
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 So loading a preset works like this:
Since LV2 does not allow preset loading and calling the run function at the same time, the |
See tim-janik#37. Signed-off-by: Stefan Westerfeld <[email protected]>
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.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):
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?
The text was updated successfully, but these errors were encountered: