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

Closed
wants to merge 117 commits into from
Closed

LV2 support for Anklang #32

wants to merge 117 commits into from

Conversation

swesterfeld
Copy link
Collaborator

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 (rncbc/qtractor#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.

Signed-off-by: Stefan Westerfeld <[email protected]>
Signed-off-by: Stefan Westerfeld <[email protected]>
Signed-off-by: Stefan Westerfeld <[email protected]>
Signed-off-by: Stefan Westerfeld <[email protected]>
git submodule add --name lilv https://github.com/swesterfeld/lilv external/lilv
git -C external/lilv checkout lilv-for-anklang
git submodule add --name zix https://github.com/drobilla/zix.git external/zix
git -C external/zix checkout v0.4.2
git submodule add --name serd https://github.com/drobilla/serd.git external/serd
git -C external/serd checkout v0.32.0
git submodule add --name sord https://github.com/drobilla/sord.git external/sord
git -C external/sord checkout v0.16.16
git submodule add --name lv2 https://github.com/drobilla/lv2.git external/lv2
git -C external/lv2 checkout v1.18.10
git submodule add --name sratom https://github.com/drobilla/sratom.git external/sratom
git -C external/sratom checkout v0.6.16

Signed-off-by: Stefan Westerfeld <[email protected]>
Signed-off-by: Stefan Westerfeld <[email protected]>
Build directory is deleted before build anyway.

Signed-off-by: Stefan Westerfeld <[email protected]>
@swesterfeld
Copy link
Collaborator Author

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

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 (a1ca1ed 558e317) and Arch CI works. I'm assigning this branch to you for code review.

@tim-janik tim-janik added this to the v0.4.0 milestone Jan 27, 2024
@tim-janik
Copy link
Owner

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:

#35 (comment)

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

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:

  • code quality is good enough
  • branch does improve something
  • merging the branch doesn't break anything
  • merging the branch shouldn't break anything in the future

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.

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

Successfully merging this pull request may close these issues.

2 participants