LV2 parameter name/range can't be edited

[agraef]

This was actually done on purpose (cf. https://github.com/plugdata-team/plugdata/commit/00ef8358258d7d75d7b231d451807d2a92f0fcb8), because JUCE's LV2 implementation currently doesn't seem to allow the use of LV2's dynamic manifest extension, which means that all parameter names and ranges are hardcoded into the plugdata.lv2 and plugdata-fx.lv2 Turtle (.ttl) files, specifically dsp.ttl. So this is actually a JUCE bug, or rather an omission, which should be fixed upstream (AFAICT).

But it's conceivable that LV2's dynamic manifests eventually find their way into JUCE, and even if it they don't then it might be fixable downstream (i.e., in plugdata) by using a custom juce_lv2_helper program. This is the program which loads a plugin and writes its static manifest into the plugin directory (the source code is in juce_LV2ManifestHelper.cpp).

I've written this kind of stuff myself a long time ago for faust-lv2, and while I've forgotten all the gory details, it's certainly possible to massage the LV2 plugin and helper code so that it produces ttl files for dynamic manifests instead. faust-lv2 is proof of that. So I'm tempted to look at this myself, because I've done it all before and I understand the quirks and idiosyncrasies of LV2 pretty well I'd say. I could also get in touch with @falkTX who's written the LV2 backend for JUCE if I'm not mistaken and discuss with him what might be done about this. Or maybe there is already some support for this in JUCE and it's just hard to find. I doubt this, though, since neither Projucer nor the juce_lv2_helper program have any corresponding options.

This will need some research, though, and I have other, more pressing items on my TODO list for the summer term holidays. But maybe you could leave this ticket open until we have a definitive resolution for this issue, even if the resolution is that it's not feasible or just too much work, given that VST3 and CLAP are now readily available as open-source alternatives for LV2.

[agraef]

@falkTX, if you happen to watch this spot, do you remember why dynamic manifests weren't used, at least as an option, in the JUCE LV2 implementation? I recall that a few years back support for that extension was still a bit thin on the ground, was that the reason? But I think that Ardour (which is the most prominent cross-platform DAW with LV2 support) has supported dynamic manifests for quite some time now, so it might be time to reconsider that decision.

The reason that we need the dynamic manifests for plugdata is that its automation parameters aren't fixed, but can be created in the plugin editor on the fly. All other plugin formats that plugdata uses (AU, CLAP, VST3) can do this out of the box, but LV2 needs to use dynamic manifests to make that work.

Any comments appreciated. :)

[falkTX]

dynamic manifests are evil, they go against the ideas of LV2 for fast scanning and can easily crash the host. most DAWs nowadays build with dynamic manifest disabled, on purpose, as we previously had cases of crashes on start due to bad plugins.

and dynamic manifest wont help for what you are trying to do. plugins are still required to not change their ports, if you do that you need a new plugin under and new URI.

note that even for VST3/CLAP you cant just randomly change IO and expect things to keep working fine on all hosts. The latest VST3 spec has adopted a similar strategy of LV2 where part of the plugin information is described in a text format, which has the potential to break these dynamic approach of PlugData.

the only proper way I see out of this is by creating a plugin that can truly be dynamic based on what exists in a file of the same name (say myplug.so would look into myplug.pd to fetch its info), but this would be something exportable from within PlugData and not the PlugData plugin itself. so you keep PlugData as plugin a mostly static thing, fixed IO and all that, useful for quick experiments; then once a patch is ready to become its own plugin have some export option that takes care of these more advanced details. in theory this is doable using JUCE too, the key here is that a manual export step is done, which generates its own static plugin.

[agraef]

Ok, I see, thanks for chiming in. It's not as if everything is going to change dynamically in plugdata, the number of parameters is in fact fixed. But you also want to set the names and ranges of some parameters, in the LV2 case that would be rdfs:label, rdfs:range, lv2:minimum, lv2:maximum. I fail to see what's evil about this. It's not as if your plugin is suddenly going to crash just because a parameter changes its name.

But that will of course also depend on the host implementation of the plugin format at hand. It's certainly possible in the current incarnation of VST3, Ardour at least picks up these changes from within the VST3 plugin without much ado.

But of course the hosts dictate what's possible, and if LV2 dynamic manifests aren't possible or highly deprecated, then this ticket is pretty much dead on arrival. Yes, it would be possible to implement such an export option which just creates a new plugin under a new URI, but that's a really clunky way of doing things for the end user IMHO.

[falkTX]

the "evil" is plugin discovery needing to run binary code, for which the host has no control over and cannot predict what kind of shenanigans the plugin will try to do (like trying to open full license manager guis during scanning, or just crashing the whole DAW). restricting plugins to static metadata text files for discovery prevents any sort of behaviour, avoids crashes or other runtime memory issues, and as a bonus it allows the scanning to be seriously fast (compared to having to open each plugin binary one by one, fetching its info, closing plugin, then repeat for all existing binaries)

For simply changing labels, I think we can still do it without having to deal with dynamic manifest. LV2 is extensible, whatever it is missing we can add it. We can propose a mechanism to inform the LV2 host about plugin parameter changes, the LV2 parameters extension (that superseeds the old and simple LV2 control ports) enforces that each parameter has its own URI which means we can set properties on them at runtime. If we define that a "patch:Set" with a parameter URI and "rdfs:label" as type can/should be used to change that name on runtime, it is a matter of getting hosts to implement such thing. It is not something unreasonable in my opinion, and last time I spoke with Robin about something related to this he was open to the idea.

Changing ranges is a bit more tricky, unlike CLAP and VST3 we store individual states in LV2 instead of the full "chunk" in 1 go, so it needs some discussion and planning.

But I still am on the opinion that this is better handled as a separate plugin binary thing, that way it would work already as-is without needing hosts to implement something new.

[agraef]

the "evil" is plugin discovery needing to run binary code

Yes, I understand that. I also understand that discovery becomes much faster without it. And for the vast majority of plugins, the static manifests are all they'll ever need. plugdata is one of the rare cases where this isn't good enough.

For simply changing labels, I think we can still do it without having to deal with dynamic manifest. LV2 is extensible, whatever it is missing we can add it.

That's not a bad idea. But that might take substantial time and effort to design and implement, both in LV2 (and maybe JUCE?) and in the hosts (Ardour and Reaper, basically; but Reaper is closed-source, and the plugdata lv2 doesn't seem to work all that well in Reaper anyway). As much as I love LV2, I doubt that anyone will embark on this, for just that single use case (Ardour+plugdata.lv2), especially considering that Ardour+plugdata.vst3 already covers that use case.

@drobilla, do you have an opinion on this? Would you consider an extension to adjust LV2 parameter names and ranges on the fly (while a plugin is running) worthwhile? If so, then we can maybe take the discussion to devel@lists.lv2plug.in.

But I still am on the opinion that this is better handled as a separate plugin binary thing, that way it would work already as-is without needing hosts to implement something new.

That's simply not a palatable solution for a highly dynamic environment like plugdata. Pd patches keep changing all the time, and you want to be able to quickly change the parameters along with them, without being forced to go through a compile step each time you make a little change. Not even to mention that you then have the corpses of all these little iterations littering your hard disk. ;-) That's not going to fly.

[drobilla]

I think it is a good idea to have a dynamic mechanism for changing data like this, and that dynmanifest is not a particularly great way to do this in general.

At the same time, I am not a fan of overly-specific APIs being grafted on for this, for example a change_control_label(const char*) thing, because there are many things like this and there will be more, so just continually grafting on APIs for each individual thing feels like it will collapse under its own weight to me.

The solution to this is a pretty common theme in LV2 stuff (where it makes sense): think in a more data-centric way and specify properties, generally as parameters. There is an advantage of the data model here that we could take better advantage of: everything has some sort of ID, and all information about everything has the same basic structure: properties. The options interface is getting close to this: https://lv2plug.in/c/html/group__options.html#structLV2__Options__Interface

More generally/simply, I imagine the way to deal with this problem in general is a sort of property_changed(key, value) callback thing (or maybe a message?) so plugins could say "hey, the label for this control has changed" or whatever.

I haven't fleshed this out in detail though, or really figured out how well a simple generic solution here could deal with all the specific cases of plugins wanting to change this or that over time (where "this or that" is a thing that's currently just written in the data file). It's a decent bit of work, but it's doable. The big questions there that are unclear to me are mainly threading rules kind of stuff: when are plugins "allowed" to announce such things? Is this best done as a literal C callback with a bunch of threading rules, or should be just avoid those problems entirely by using some new kind of message that goes over the existing channel and is inherently synchronous with audio? Conceptually, we have a very simple data model (in a structural sense) where you can easily talk about anything by specifying 3 parameters/arguments/fields/whatever, and that "set the label of this control to 'gain'" is absolutely the general way to go about doing this in my opinion, but the mechanics are up in the air because we can talk about the same stuff in different ways: Turtle, callbacks, atom-based messages (like falktx's example up there), etc. At the moment I'm not clued in to this particular issue to have much useful to say there, but in general I think addressing this need for dynamically changing data is a good thing we should do anyway.

In practice, we do already have those patch messages which you can use to talk about whatever, so if that works for this case and someone wants to make a plugin that sends such messages and implement support in a host, that's a pretty easy way to go that has a very low entry cost and is easy for implementors to tackle as they get around to it without requiring us to invent any grand new concepts or APIs that people don't already have to deal with anyway... but it needs some thought. Feel free to start a thread on the mailing list, I think it's a good topic to hash out.

[agraef]

David, thanks a lot for responding, it's much appreciated.

In practice, we do already have those patch messages which you can use to talk about whatever ... that's a pretty easy way to go that has a very low entry cost and is easy for implementors to tackle as they get around to it without requiring us to invent any grand new concepts or APIs...

Then that's probably the way to go. I'm all for using existing facilities if possible. If those patch messages allow to send something to the effect of plugin->rename_param(char *current_name, char *new_name) and plugin->change_param_range(char *param_name, float minval, float maxval) to the host then that should do the trick.

Of course this would have to be implemented in the corresponding JUCE method (presumably that's ParameterStorage::audioProcessorChanged() in juce_audio_plugin_client_LV2.cpp:210, which currently doesn't do anything), and on the Ardour side in the LV2 host code which needs to listen for those messages and then do whatever is necessary to implement those changes (we should be able to snitch this from the corresponding VST3 host code). Maybe we'll also need some extra plugin initialization code which calls those messages right after plugin instantiation in plugdata, but Tim should be able to help with that.

Feel free to start a thread on the mailing list, I think it's a good topic to hash out.

Will do. I also think that it is a worthwhile endeavor to have this working in LV2 even if we already have that functionality in VST3. I'm not sure when I'll get around doing the actual coding, as there's still a lot of other stuff on my TODO list for this summer and autumn. But it would be good to get some discussion going, and maybe collect some helpful code snippets. I believe that @x42 is on that list as well, it will be helpful to discuss the Ardour side of things with him, so that I can find the relevant code parts there.

[agraef]

Ok, my post to devel@lists.lv2plug.in is now up at http://lists.lv2plug.in/pipermail/devel-lv2plug.in/2023-August/002109.html. Thanks for everybody who chimed in here. Tim, I'd like to suggest that we keep this ticket open for the time being, as a reminder that this is being worked on.

[x42]

I believe that @x42 is on that list as well, it will be helpful to discuss the Ardour side of things with him, so that I can find the relevant code parts there.

Most of Ardour's LV2 support was written by Dr. David "LV2" Robillard himself.

That being said in VST3 land it was rather simple https://github.com/ardour/ardour/commit/649ad0f0520 https://github.com/ardour/ardour/commit/595e2e29146289e9e2b0a24966866fd03d36b65a

and the GUI will call Plugin::describe_parameter() again to update the name. In LV2's case that'll be

https://github.com/Ardour/ardour/blob/92fbee63125113007fd2cb5b5277045fa9e2222e/libs/ardour/lv2_plugin.cc#L2505-L2538

[agraef]

Ok, it seems that people just prefer the web interface here over the mailing list, I can understand that. Sorry, David, at least I tried. ;-)

Thanks, Robin, that was very helpful. So, if I understand this correctly, the LV2Plugin::get_parameter_descriptor() and LV2Plugin::describe_parameter() methods are being called by various parts in Ardour if they need to get all the metadata about an automation parameter. Right now, these go straight to the lilv API to get that information.

Since it's not possible to modify that information on a per-plugin instance basis (at least I don't see any "setters" in the lilv port API, only "getters", so I assume that's by design), we'd have to keep the meta data that we need to modify, such as the name, in the private part of the LV2Plugin class just like, say, _defaults. The above methods would then have to be modified so that they can pick up that data if present.

Then we need to get that data into the LV2Plugin class instance somehow, as we don't have a dedicated callback a la VST3 for that. David suggested to use patch messages to these ends, and I see that there's already some code in LV2Plugin::connect_and_run() which seems to deal with these kinds of messages. So I guess that's where we should add the code that processes patch messages like "change port \<id>'s name to \<name>", modify the private meta data accordingly, and then call that processors_changed() method like in https://github.com/Ardour/ardour/commit/595e2e29146289e9e2b0a24966866fd03d36b65a to get the GUI updated.

Does that sound about right? Or am I completely off track there?

[agraef]

Ok, the first part was easy, I think that the following should do the trick. At least it compiles without any hitches. :)

commit 31c0c5d74baca8784ed42d55a0a6f8b8da3c600f
Author: Albert Graef <aggraef@gmail.com>
Date:   Thu Aug 10 13:13:46 2023 +0200

    LV2Plugin: Add private _names array to make port names modifiable

diff --git a/libs/ardour/ardour/lv2_plugin.h b/libs/ardour/ardour/lv2_plugin.h
index 8014202322..ede10559ac 100644
--- a/libs/ardour/ardour/lv2_plugin.h
+++ b/libs/ardour/ardour/lv2_plugin.h
@@ -209,6 +209,7 @@ class LIBARDOUR_API LV2Plugin : public ARDOUR::Plugin, public ARDOUR::Workee
        float*        _control_data;
        float*        _shadow_data;
        float*        _defaults;
+       char**        _names;
        LV2_Evbuf**   _ev_buffers;
        LV2_Evbuf**   _atom_ev_buffers;
        float*        _bpm_control_port; ///< Special input set by ardour
diff --git a/libs/ardour/lv2_plugin.cc b/libs/ardour/lv2_plugin.cc
index a09cfd883e..5735cd61e1 100644
--- a/libs/ardour/lv2_plugin.cc
+++ b/libs/ardour/lv2_plugin.cc
@@ -869,6 +869,8 @@ LV2Plugin::init(const void* c_plugin, samplecnt_t rate)
        _control_data = new float[num_ports];
        _shadow_data  = new float[num_ports];
        _defaults     = new float[num_ports];
+       _names        = new char*[num_ports];
+       memset(_names, 0, sizeof(char*) * num_ports);
        _ev_buffers   = new LV2_Evbuf*[num_ports];
        memset(_ev_buffers, 0, sizeof(LV2_Evbuf*) * num_ports);

@@ -2371,7 +2373,7 @@ LV2Plugin::get_parameter_descriptor(uint32_t which, ParameterDescriptor& desc) c
        desc.toggled      = lilv_port_has_property(_impl->plugin, port, _world.lv2_toggled);
        desc.logarithmic  = lilv_port_has_property(_impl->plugin, port, _world.ext_logarithmic);
        desc.sr_dependent = lilv_port_has_property(_impl->plugin, port, _world.lv2_sampleRate);
-       desc.label        = lilv_node_as_string(lilv_port_get_name(_impl->plugin, port)); // XXX leaks
+       desc.label        = _names[which] ? _names[which] : lilv_node_as_string(lilv_port_get_name(_impl->plugin, port)); // XXX leaks
        desc.normal       = def ? lilv_node_as_float(def) : 0.0f;
        desc.lower        = min ? lilv_node_as_float(min) : 0.0f;
        desc.upper        = max ? lilv_node_as_float(max) : 1.0f;
@@ -2531,11 +2533,16 @@ LV2Plugin::describe_parameter(Evoral::Parameter which)
                        return X_("latency");
                }

-               LilvNode* name = lilv_port_get_name(_impl->plugin,
-                                                   lilv_plugin_get_port_by_index(_impl->plugin, which.id()));
-               string ret(lilv_node_as_string(name));
-               lilv_node_free(name);
-               return ret;
+               if (_names[which.id()]) {
+                 string ret(_names[which.id()]);
+                 return ret;
+               } else {
+                 LilvNode* name = lilv_port_get_name(_impl->plugin,
+                                                     lilv_plugin_get_port_by_index(_impl->plugin, which.id()));
+                 string ret(lilv_node_as_string(name));
+                 lilv_node_free(name);
+                 return ret;
+               }
        } else {
                return "??";
        }

Now I need to figure out how to actually register those patch set messages so that Ardour will recognize them. As I noted earlier, there is already some code in place in lv2_plugin.cc for this. Looking at LV2Plugin::load_supported_properties(), it seems that the properties being registered there need to have that writable bit set. But how do I do that? A typical parameter entry in the plugdata ttl file looks like this:

plug:param1
    a lv2:Parameter ;
    rdfs:label "param1" ;
    rdfs:range atom:Float ;
    lv2:default 0 ;
    lv2:minimum 0 ;
    lv2:maximum 1 .

So how do I declare that the rdfs:label there should be patchable? Do I need to do this at all? Or should I modify LV2Plugin::load_supported_properties() instead, so that it will recognize those additional patch set messages? As you can tell, I'm thoroughly confused. :) David, can you help with this?

[x42]

I highly recommend to use std::vector<std::string> instead of allocating char**

[agraef]

Yes, you're right. I'm also currently leaking memory on these arrays since I forgot to delete them in the destructor. :) But that's a minor implementation detail that I can easily fix later. I'd like to do a quick proof of concept first, to see how all the parts are supposed to work together.

[x42]

it seems that the properties being registered there need to have that writable bit set. But how do I do that?

add patch:writable have a look at https://github.com/x42/property_example.lv2 specifically https://github.com/x42/property_example.lv2/blob/3bc58875c37323aa8788fb1ac5ed1aac0f39dea5/lv2ttl/property_example.ttl.in#L46-L47

[falkTX]

juce already uses writable params, the only thing needed here is an atom event as patch:Set message, with the parameter URI to change as the subject, type as atom:string and value as the new name to use.

[agraef]

Ah yes, I missed that because it's a bit further down in the dsp.ttl file, thanks Filipe!

Well, it seems that there's been a bit of refactoring of this code for JUCE 7.0.6, and Tim recently downgraded plugdata HEAD to JUCE 7.0.5 again. So I'll have to stick with JUCE 7.0.5 for now, where the code is a bit different. (I know that this wrapper has been in development for a long time, but this seems to be the version as it landed in JUCE last year.)

Anyway, in JUCE 7.0.5 the LV2PluginInstance::audioProcessorChanged() callback looks like this:

    void audioProcessorChanged (AudioProcessor*, const ChangeDetails& details) override
    {
        // Only check for non-parameter state here because:
        // - Latency is automatically written every block.
        // - There's no way for an LV2 plugin to report an internal program change.
        // - Parameter info is hard-coded in the plugin's turtle description.
        if (details.nonParameterStateChanged)
            shouldSendStateChange = true;
    }

Looking at the corresponding method in the VST3 wrapper, it seems that I need to change this to something like:

    void audioProcessorChanged (AudioProcessor*, const ChangeDetails& details) override
    {
        if (details.nonParameterStateChanged)
            shouldSendStateChange = true;
        else if (details.parameterInfoChanged) {
            for (auto param : /*FIXME 1:*/ list_of_all_params()) {
                if (/*FIXME 2:*/ param.name_changed()) {
                    // FIXME 3: create a patch:Set message for the name change
                    // FIXME 4: output the message on the seqOutput port
                }
            }
        }
    }

(Or maybe I'll need to set some flag like in the nonParameterStateChanged case and then execute that for loop some time later.)

Yeah, that's just pidgin code right now. I don't know how to do any of these things in actual code, so I'm calling for help again. :stuck_out_tongue_winking_eye: I need to know:

1. How to get the list of all parameters. I guess that to get this in the context of the LV2PluginInstance, I should be able to use the ParameterStorage parameters member, but I'm not sure how.
2. How to check that a parameter name has changed. I don't see any ready-made methods or flags for this.
3. How to create a patch:Set message. Yeah, that should be pretty basic, but I don't know how to do this, and I've not been able to find any code examples.
4. How to find that seqOutput port that the LV2 wrapper apparently creates for such purposes, and how to output the message computed in step 3 on that port.

Item 3 is probably LV2 101 for most of you, so I hope that someone can just point me to an existing example. Items 1, 2, and 4 are about the LV2 wrapper, I might still be able to figure these out myself after diving into the code some more.

Sorry, I suspect that all this stuff might seem trivial to you, but it isn't for me. I'll readily admit that I don't know the JUCE API very well, and thus the LV2 wrapper code is way over my head at present. So please bear with me.

I guess that once this has been sorted out, going back to Ardour to receive and process the patch.Set message should hopefully be a walk in the park, comparatively.

[agraef]

Ok, it looks like the Atom Forge is the answer to item 3. That's actually in the LV2 wrapper.

It's in the LV2PluginInstance::run() method at juce_LV2_Client.cpp:603. This apparently loops over the parameters and emits a message for each value change. Right now, this doesn't deal with parameter name changes (the only option values implemented there are newClientValue, gestureBegan, and gestureEnded), but presumably something could be added for a name change there. I'm just not sure yet how and where exactly.

This in fact answers items 1, 3, and 4 in one fell swoop. Now I just need to figure out which callback lets me detect the name changes. I suspect that this is audioProcessorChanged() at line 209 which is currently a no-op, I probably need to check for ChangeDetails::parameterInfoChanged there and then set a corresponding flag in the options bitset and make sure that the corresponding messages get emitted in LV2PluginInstance::run().

Kudos to whoever wrote that code, it's full of C++ lambdas and makes my brain hurt. :)

[agraef]

Ok, I have something which I think will work on my testing branch (based on JUCE 7.0.5) over at https://github.com/agraef/JUCE/tree/testing. Specifically https://github.com/agraef/JUCE/commit/2995f5f353f5c6e1930b4dd4dce8b335c48090dc (the other commit is just debugging code). A plugdata branch which uses this JUCE version is up at https://github.com/agraef/plugdata/tree/testing.

I still need to go back to the Ardour code where those additional patch.Set messages will need to be processed, before I can actually test this. But the debugging messages I'm getting from plugdata in Ardour look right to me, so there's hope. :)

I'll have to leave it at that for now, since we're travelling starting tomorrow. But I'll get back to this asap.

[agraef]

I still need to go back to the Ardour code where those additional patch.Set messages will need to be processed, before I can actually test this. But the debugging messages I'm getting from plugdata in Ardour look right to me, so there's hope. :)

Well, that was a bit overly optimistic. I gave it the good old college try, and what I can do is pick up the patch:set message on the Ardour side and update the _property_descriptors table accordingly (https://github.com/agraef/ardour/commit/6815302a908d254d1e1efb790e87c20a83a0a0d5). After a GUI update, as Robin suggested, the changed control label will then be displayed correctly in the generic plugin controls.

That's some progress, but right now this doesn't work at initialization time, when the initial patch.set messages sent from the plugin get lost somewhere, probably due to the corresponding event buffers not being set up yet, or being reset before the connect_and_run() method has a chance to see them.

That might be fixable, but the real trouble is that the parameter name never gets updated in the automation menu, and that seems to be a limitation of the current LV2 plugin implementation in Ardour. There's a strange mix of automatable ports and automatable plugin properties at work there, and the latter are never updated even after an update of the _property_descriptors table. Unfortunately, those are the parameters that we're actually interested in.

If anyone has an idea on how to proceed, please let me know. However, I suspect that a major rewrite of lv2_plugin.cpp might be in order if we want to fix these issues.

[agraef]

There's a strange mix of automatable ports and automatable plugin properties at work there

Tim, incidentally that's probably the reason why the latter kind never shows in Reaper at all (it only shows the automatable ports). You mentioned this before in some other ticket.