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

WIP: LV2 using patch extensions #1773

Closed
wants to merge 3 commits into from

Conversation

jpcima
Copy link
Contributor

@jpcima jpcima commented May 1, 2020

#1619

Hello it's the work in progress of lv2 patch extensions that help to resolve current problems.
This is not working yet, I think there is a blocking in carla, where the output port indicates an overflow problem regardless of the capacity I give it (?)
The state does not restore right among things, hence making it a WIP.

This sets up at least the framework to handle events according to semantics of LV2 patch.
The sendParameterAutomated logic is different whether it's UI or DSP that makes the call.

@baconpaul sorry that I'm a bit destroyed at the moment, I hope also that things are going well at your side, thanks.

@baconpaul
Copy link
Collaborator

Hi @jpcima sorry to hear things have been busy - hope all is well. I also fixed up that unit test failure which was unrelated to your change.

I'll give this a pull in the next week and see if I can also get a bit further but will keep it not merged indeed.

Thank you!!

@baconpaul
Copy link
Collaborator

So I built this hoping I could help but

  1. it doesn't stream any values into an ardour session indeed so save/restore doesn't work
  2. but the dynamic adjustments like change oscillator type does work in the ui
  3. but i can't record any automation in ardour

and it's beyond me

any idea if ardour will support vst3 anytime soon? I wonder if just using vst3 as our FOSS is the solution practically?

@baconpaul
Copy link
Collaborator

Hi @jpcima and @falkTX

just a quick note on timing. We've been adding a lot of features and fixes to surge in May and will keep pushing that pace through June I suspect, then use July to stabilize ready for an August release of 1.7.0 which will have loads of new stuff (skins! new effects! way improved ui! VST3 on Linux! cmake! We are actually pretty excited). That release (just like the current nightlies) will also stop distributing a binary VST2. You will have to build your own if you want it using your own version of the VST2SDK.

I would love the LV2 to be working when we do the 1.7.0 release and it seems like the primary problem we are facing is this param change issue, which I have no idea how to fix, and this PR doesn't seem to fix it either.

So just wanted to give you a ping with this timing. Either I need a whole bunch more information about the right way to have the DSP engine update parameters, or (better) you guys could finish the design you discussed in time for a july stabilization / august release sort of cadence.

Alternately we can go to 1.7.0 with the LV2 the way it is. Except for Ardour I think our linux community would be fine with the VST3 to be honest, and VST3 is FOSS.

Lemme know if I can do anything to help!

@falkTX
Copy link
Contributor

falkTX commented May 31, 2020

The PR is going in the right direction.
I did not try it yet, just checked the code.
@jpcima Seems like it is complete aside from finding and fixing bugs, or I am missing something?
Is there anything in particular you know it is missing/wrong?

@jpcima
Copy link
Contributor Author

jpcima commented May 31, 2020

There possibly remains some problems yes.
The audio cycle runs is subdivisions of some constant BLOCK_SIZE.

Before, the control ports will receive updates of values at the beginning of cycle.
With parameters this changed, now it may receive updated at each subdivision of the cycle.

I think there is possibility of trouble with LV2-specific parameter automations inside of Surge core.
For example in case of loading a whole patch, and the DSP will send all parameter updates which correspond to the patch.

If this resend occurs in a middle subdivision and a stream of input parameters, I have a suspicion that some bad behavior may occur as result of this.

@falkTX
Copy link
Contributor

falkTX commented May 31, 2020

I think there is possibility of trouble with LV2-specific parameter automations inside of Surge core.
For example in case of loading a whole patch, and the DSP will send all parameter updates which correspond to the patch.

If this resend occurs in a middle subdivision and a stream of input parameters, I have a suspicion that some bad behavior may occur as result of this.

what about a different approach?
when using lv2 parameters, we can make the dsp report to the host about parameter changes (just like other formats do).
so instead of sending every parameter on patch change from UI side, simply send a "this patch was loaded from gui" to the dsp, and let the dsp report to the host about changes made because of this action.
this will make the lv2 resemble more the other formats, correct?

there will still be issues of host automation + gui patch changes happening concurrently, but I think that is true for any format.

@baconpaul
Copy link
Collaborator

The problem is we do have pure parameter changes without a UI in other situations.

For instance: We can automate the oscillator type. That changes the meaning, type, and value of the oscillator parameters. This doesn't require a UI - just a DAW automation message to param 1 changes param 2-9. See what I mean?

@falkTX
Copy link
Contributor

falkTX commented May 31, 2020

that is exactly the problem that changing to lv2 parameter api is trying to solve.
we cannot change properties of the parameter, but at least we can change its value from the plugin/DSP side.

@baconpaul
Copy link
Collaborator

Cool! I had put (in a TARGET_LV2) block some calls to invalidation routines when those changes happen (osc fx and patch changes are really the class). That calls a bunch of setParameterAutomated which isn’t the right thing but those spots should be well isolated for when we have the right api.

And thanks for your help here. Especially with VST2 no longer in our build and VST3 support in the free daws lagging on linux I am sure demand for the LV2 will increase when we release 1.7.

@baconpaul
Copy link
Collaborator

Hey folks

We are going to feature-freeze the main branch around July 4, do a month of beta testing, and then release 1.7.0 in early august. If you think you might be able to get these LV2 changes in in the next couple of weeks we can include them, otherwise we will end up shipping 1.7.0 with the same LV2 bugs as 1.6.6.

To make our milestone management easier I moved all the LV2 bugs to a “1.7.LV2” milestone and out of the 1.7.0 (and 1.7.1 and so on) milestone. https://github.com/surge-synthesizer/surge/milestone/17

1.7.0 is going to no longer have a binary VST2. We’ve worked to make the VST3 work in many hosts on Linux; Ardour is the outlier it seem. users who want to use Ardour will either have to build a VST2 themselves with their own VST2 compatible SDK or run the LV2 with the limitations.

Thanks as always for your help

@falkTX
Copy link
Contributor

falkTX commented Jun 25, 2020

Why the vst2 removal? Tracktion/Waveform and Renoise are 2 commercial DAWs for Linux with no support for VST3

@baconpaul
Copy link
Collaborator

baconpaul commented Jun 25, 2020

None of us are VST2 Licensees, and we wanted Surge to be FOSS

@falkTX
Copy link
Contributor

falkTX commented Jun 25, 2020

right, licensing..
Better to get that lv2 version up to shape then, I will see if I can help

@baconpaul
Copy link
Collaborator

Thank you very much

And yeah licensing. You know the story

The code will still build against vst2 sdk if you have one, and we aren’t removing that ability. Just no binary in the deb. So if you want vst2 get the sdk and build yourself. Honestly if someone did that and redistributed the binary none of us would push them on the gpl3 problem either. Just we don’t want to be License ambiguous with our substantially new release. (With 1.7 we are no longer branding the synth as vember audio for instance)

But the lv2 being healthier would be the ideal situation. That’s why we’ve done so much work on the Linux vst3

@tank-trax
Copy link
Collaborator

I've contacted Tracktion asking them about VST3 and they've indicated that support will probably come after the JUCE team make JUCE6 the main supported version of JUCE. At the moment Tracktion builds Waveform against the "develop" branch of JUCE.

The developer of Radium has also replied with a similar response.

Uncertain whether or not Renoise will add VST3 support. There have been a few requests in the Renoise forum.

cheers!

@falkTX
Copy link
Contributor

falkTX commented Jul 1, 2020

FYI I am not going to have time to look into this so soon.
I am thinking instead of (later in the month) updating my juce lv2 wrapper to use lv2 parameter api instead of control ports, as a way to test plugins and hosts and also get more familiar with the process.
Then I should be more comfortable changing Surge to match what juce has, knowing that host-support is well tested.
A big problem with lv2 parameter API is that there are simply not enough plugins using them, so good host support is somewhat unknown. It should work correctly, but we mostly have only been testing with demo/example plugins so far.

@baconpaul
Copy link
Collaborator

OK

Can you give me any hint on what that API is? Maybe I can look at making it work ardour using that. If you guys give me the sketch of where to start maybe I could crack it while we are in our beta period (assuming our beta period doesn't have a bunch of problems). I'm happy to give it a whirl, just I don't know what "it" is :)

Thanks

@falkTX
Copy link
Contributor

falkTX commented Jul 1, 2020

The new API is basically what this PR is all about.

We used to have each surge parameter as an lv2 control port. Something that is under the control of the host and not allowed to be modified by the plugin (DSP) side. So we were having troubles in surge due to this, as surge (coming from VST) expects that the DSP side can modify its own parameters.
Note that the UI can still modify these control ports, but this passes through the host. Basically the UI asks the host to change a value, which it can still refuse to do (if the parameter already has an automation lane for example)
Pretty much all existing plugins use this control port approach.

Now the new LV2 parameter API uses "patch" events to get, set, update, etc the plugin parameters.
Instead of the host managing the parameter state via control ports, the plugin has the parameter state internally which must update when receiving a "patch" message from the host (note: changes can also come from the ui, but end up at the same place)
The great thing about this new parameter API is that we can define the parameters as readable, writable or both.
So for surge this would mean making all parameters read+write, so that surge can read changes from the host, but suge can also inform the host about changes.

The best resource (to me) that exists right now about this is https://github.com/x42/property_example.lv2
It is a fully working plugin, simplistic but has effects on audio and parameters. It does not report any changes to the host though.

A second good resource is the official lv2 example of parameter API https://github.com/lv2/lv2/tree/master/plugins/eg-params.lv2
This one reports changes to the host.

@baconpaul
Copy link
Collaborator

Thank you very much! This is great.

@baconpaul
Copy link
Collaborator

baconpaul commented Jul 2, 2020

So @falkTX my current thinking is

  1. We are going to release 1.7.0 with a million awesome features but
  2. The LV2 doesn't work very well

so what we were discussing is make the LV2 like the VST2. It's not in the official binary distribution. But it is buildable from source if you want. Just it is buildable using only FOSS.

I really want our binary distribution to only include (1) working (2) high quality (3) FOSS. The VST3 and AU are all three; the VST2 fails on the third test, and unfortunately the LV2 will fail on the first if we release in the next few weeks.

Of course KX and Arch and all the other build distress can build it and we will make it clear on the website how to do so.

Reactions?

@falkTX
Copy link
Contributor

falkTX commented Jul 2, 2020

Sounds good to me.
I will keep the kxstudio vst2 build though, as it is just too useful at the moment. plus I really hate breaking stuff like that in an update. The LV2 I guess I will remove for the moment, until we sort this out, as current version is broken anyway.

@baconpaul
Copy link
Collaborator

Yeah cool. I don’t know if you have updated KX to the cmake build formula yet but it makes it way way easier. I’ll drop a note on kxstudio repo when we finish 1.7.0 with full instructions.

@baconpaul
Copy link
Collaborator

OK I marked it experimental, removed it from the binary

https://surge-synthesizer.github.io/formats

if you want me to add something about kx to that page I am happy to.

@falkTX
Copy link
Contributor

falkTX commented Jul 6, 2020

there are some small wording issues that would be nice to get right.

  • Lv2 should be LV2
  • are incompatible in a couple of way => are incompatible in a couple of ways
  • the Lv2 assumption that all parameter chances come only from the UI => the LV2 assumption that plugins never modify their own control input ports
  • As a result, the LV2 - especially in Ardour - unreliably saves and restores state. (please add) The solution is to use new LV2 APIs that allow current Surge behaviour (and match what other plugin specs like AU and VST do).

@baconpaul
Copy link
Collaborator

Thanks! Added those.

@baconpaul baconpaul closed this Oct 10, 2020
@baconpaul
Copy link
Collaborator

Guys just an FYI. We're closing this PR but keeping the issues for the LV2 open in the milestone

Seems that the VST3 works fine in Ardour now - which was our top demand point for LV2. If you choose to return to the LV2 we can reopen this any time of course.

@reuk
Copy link

reuk commented Feb 11, 2021

Sorry to revive an old thread! I'm currently investigating using the LV2 'patch' API to expose plugin parameters, which works well in simple cases. However, it's not clear to me how to implement more advanced functionality with the patch API. In particular, the 'touch' extension to the 'UI' feature seems to expect that the grabbed parameter is backed by a unique port, making it incompatible with the patch API. It's also not clear whether port properties like enumeration and 'scale points' may be applied to parameters as well as ports.

I've checked out the patch examples mentioned earlier (which were really helpful!) but these don't implement the more advanced features I'm interested in.

I'm wondering whether these issues have impacted the LV2 build of Surge, and what workarounds you've settled on if so. Also, do you know of any newer examples of the patch API?

@baconpaul
Copy link
Collaborator

The primary workaround we’ve settled on is “work with daw manufacturers and testers to make the VST3 work on linux”. We do not recommend the lv2, don’t distribute the binary of it, and in our future surge xt pport to juce I expect we will discontinue it since juce mainline doesn’t have lv2 support right now

@falkTX
Copy link
Contributor

falkTX commented Feb 11, 2021

Once the port is done, I will try/revisit the lv2 version since I maintain a juce lv2 wrapper.
So for the moment, work on lv2 in surge is a bit useless.

@baconpaul what is the time frame on juce things?

@baconpaul
Copy link
Collaborator

@falkTX

1: the codebase has the alpha of surge-xt in it already (check out the 'port to juce' pinned issue here on directions) and the nightly has a super-pre-alpha binary in it today. So we have the start of a working juce surge. #3737 is the working issue.

2: We've taken to keeping a status issue pinned here but short version is: we are working towards a 1.9 release with current code base in April or so and then will commit 100% to juce for Surge XT. The pinned issue #3677 is one I update from time to time so devs know the status.

3: If you let me know how to try your wrapper I can give it a go. Right now we are using juce 6.0.7 with cmake so I presume I would just point at a different juce and add LV2 true or what not?

@falkTX
Copy link
Contributor

falkTX commented Feb 11, 2021

3: If you let me know how to try your wrapper I can give it a go. Right now we are using juce 6.0.7 with cmake so I presume I would just point at a different juce and add LV2 true or what not?

I dont know cmake enough to help, but for the lv2 version you basically just swap stock/upstream juce with my fork (in case you use submodules) or manually copy the lv2 files in your repo. then it should be mostly just setting up which files to build. we will have to see how to deal with generating/exporting the ttl, but that could be a 2nd step.

@baconpaul
Copy link
Collaborator

so you assume projucer builds? we've moved 100% to the juce cmake framework with the 6.0 release.

@falkTX
Copy link
Contributor

falkTX commented Feb 11, 2021

neither. I am using meson now. already one step ahead 🚀

@baconpaul
Copy link
Collaborator

ok! I'll leave it for another day then :)

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.

5 participants