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 support for Anklang #32

Closed swesterfeld closed 5 months ago

swesterfeld commented 6 months ago

This implements LV2 support for Anklang. It works for many plugins, but I also didn't test them all. But instrument plugins with midi input and stereo output and effect plugins with stereo input and stereo output should be fine, and custom UIs with different toolkits (via libsuil) and load/save should work.

I've left some TODO comments in the code where improvements are possible. Due to the modular/extensible nature of LV2, there is certainly also stuff this first iteration doesn't provide, so it makes sense to add stuff later on. Due to a lilv issue (https://github.com/rncbc/qtractor/issues/427), I recommend building against my fork of lilv with a fix https://github.com/swesterfeld/lilv/tree/lilv-for-anklang that has not been accepted upstream. Note that if you build your own liblilv and install it in a non-standard location, you may want to set something like

$ export LV2_PATH=/usr/lib/lv2:/usr/lib/x86_64-linux-gnu/lv2

so that liblilv will use the system LV2 plugins. If you need more plugins, I recommend the excellent https://kx.studio/Repositories that can be added to Ubuntu to get many LV2 plugins.

I'd be happy to get some feedback about any changes that need to be made for merging this.

swesterfeld commented 5 months ago

I recommend building against my fork of lilv

Update: I added the necessary git submodules and a script to build my forked lilv now. I also updated some things for the CI. I am not sure why the Arch CI fails, but it builds fine, it fails somewhere in the tests.

swesterfeld commented 5 months ago

I am not sure why the Arch CI fails

So the problem with the Arch CI was that the AnklangSynthEngine was started before Xvfb, so it didn't have a $DISPLAY; but it wants to have that in order to have the Gtk Thread running, which is needed for instantiating LV2 plugins. So I now start AnklangSynthEngine with a valid $DISPLAY (https://github.com/tim-janik/anklang/pull/32/commits/a1ca1edaeea23b30513a3f9001c81b495bd7626a https://github.com/tim-janik/anklang/pull/32/commits/558e3175e01c063f62e7f72f3cbeb3a787b16e6f) and Arch CI works. I'm assigning this branch to you for code review.

tim-janik commented 5 months ago

I am not sure why the Arch CI fails

So the problem with the Arch CI was that the AnklangSynthEngine was started before Xvfb, so it didn't have a $DISPLAY; but it wants to have that in order to have the Gtk Thread running, which is needed for instantiating LV2 plugins.

I have addressed the $DISPLAY issue here:

https://github.com/tim-janik/anklang/issues/35#issuecomment-1912515356

I've left some TODO comments in the code where improvements are possible.

I would really like to see PRs submitted when you think they are actually ready to be merged. Do you need an extension to synsmell.py to catch TODO comments also, or are you taking care of resolving all issues before submission? As for actual code review, I'm fine with being pointed at a branch in your repo for that, in case you need an eye on it before final submission.

swesterfeld commented 5 months ago

I've left some TODO comments in the code where improvements are possible.

I would really like to see PRs submitted when you think they are actually ready to be merged. Do you need an extension to synsmell.py to catch TODO comments also, or are you taking care of resolving all issues before submission? As for actual code review, I'm fine with being pointed at a branch in your repo for that, in case you need an eye on it before final submission.

TODO comments are not a problem to be catched by synsmell.py. In some cases they should simply remain and point out further work to be done later on.

In general I'd propose the following criteria for a branch before it can be merged:

In particular, for LV2 and further improvements, I'd like to follow a release early, release often scheme for PRs. We also did this successfully for other parts of the code, like BlepSynth (which still has possible improvements like displaying the wave form or portamento support) and the Piano Roll (which is not capable of entering triplets yet).

I'll try my best to ensure that a PR meets the criteria above and ask for clarification where I need help. Then it can simply be merged. In particular for LV2 this means that fancy stuff (like out-of-process stuff or being able to interpreting all possible hints and extensions) can wait. For now I'd just like to get the basic stuff right, which when released will already be a significant improvement in usability, because we'll have simple stuff like compressors, limiters, equalizers and so on covered. Also I'd like users to be able to test it and report problems and give feedback as soon as possible.

Also having PRs as soon as possible in "official" Anklang soon means that these features can be used in ScreenCasts.