Help on mod gui

[rominator1983]

@falkTX I made a mod gui for the NAM based on the original plugin (0.7.3 has a new look) here https://github.com/rominator1983/neural-amp-modeler-lv2

But I have one issues with it.

At the moment it does not seem to load the last model from the state into the UI. Though as I see it it just does not get loaded in the UI.
If I load a pedalboard and change the input gain and save it, the state of the effect in the pedalboard shows the old model but it never showed up in the UI (also the basic lv2 UI not just the new UI).

Knobs need to be painted to look like the original plugin. That is ongoing.

@mikeoliphant sorry to post this here but at least I suppose that is where I can talk to you two.

[rominator1983]

@mikeoliphant thinking about it. That might as well be an issue of the LV2 plugin itself without the MOD gui.

You might want to check that in reaper?

[falkTX]

that is a great start! the path update stuff should be handled automatically assuming we have defined all the right properties.
an example: https://github.com/moddevices/lv2-data-creative-commons/blob/master/plugin-data/distrho-a-fluidsynth.lv2/modgui/icon-die-fluidsynth.html#L45

(I havent looked at your code yet, will do that very soon)

[falkTX]

@rominator1983 I have added your modgui to https://github.com/moddevices/mod-lv2-data, seems best to continue discussion there. please send PRs for changes, will happily merge them quickly when I am online.

I have pushed the current plugin to the store as beta, as a way to verify there are no blockers.
seems all good \o/ https://pedalboards.mod.audio/plugins/aHR0cDovL2dpdGh1Yi5jb20vbWlrZW9saXBoYW50L25ldXJhbC1hbXAtbW9kZWxlci1sdjI=

[falkTX]

actually maybe easier to talk in the team chat, @rominator1983 any way to contact you outside of github?

[rominator1983]

Wow. blown away that my stuff from tonight is already in the store.

Other than the GUI I haven't touched anything on the plugin side (c/c++ code). Except for build steps and metadata which I was basically for building locally. But that's all in the past. I'll change my setup to do PRs on your repository then.

So the issue I described could really be in the plugin code in this repository.
@mikeoliphant has in the meantime removed the state from the plugin-definition. 29e0cd2 Also don't know if that's the right thing for MOD since the cabsim (which I copied some stuff from) does have a state defined.

So I will check things again with all the new code/setup maybe tonight or tomorrow. I will also send you a PR to get the newest code from THIS repository. I think this is in the .mk-file of plugin-builder? I have to check. Last time I saw it, the git-URL was pointing to some other fork but not here.

I still have to make correct screenshots/thumbnails and the knob-stuff.

Signal might be best. I'll send you an e-mail with my contact.

[rominator1983]

I will also send you a PR to get the newest code from THIS repository. I think this is in the .mk-file of plugin-builder? I have to check. Last time I saw it, the git-URL was pointing to some other fork but not here.

Somebody already took care of that.

[falkTX]

I still have to make correct screenshots/thumbnails and the knob-stuff.

I took care of the screenshots, easy to do on my side so dont worry about it. (I have an in progress tool that auto-generates these)

[mikeoliphant]

A couple of commments:

• Are you repurposing assets from the NAM VST plugin? If so, you might want to get permission from the designer. Technically, the NAM plugin repo is MIT-licensed, but it is probably still a good idea to be safe.
• You've got the level meters baked into the background, but they aren't functional.

[rominator1983]

@mikeoliphant you're right with both points of course.
I did of course check the license upfront but sent @stadkinson an e-mail now anyways to get a formal permission.
Thanks for having an eye on such things.

I did first remove the meters but it ended up looking worse so I went with the version with meters.

[rominator1983]

@mikeoliphant @falkTX I am stuck with checking out the load error.

I can confirm that the MOD GUI (and probably other lv2-hosts) does not show the loaded model after an application start or a pedalboard load.

The model is loaded (which I can confirm by looking at the MOD console and the trace ouputs) but this does not reflect in the GUI.

I have seen this exact thing working with cabsim and with the adia-x-plugin (there seems to be some duplicated code here too but I could not figure out what's going on).

[mikeoliphant]

I can confirm that the MOD GUI (and probably other lv2-hosts) does not show the loaded model after an application start or a pedalboard load.

Yes, I'm seeing that as well. The model also isn't showing as selected in the default gui interface.

The was originally an issue in other hosts before @rerdavies submitted patch #17. That fixed it in Ardour. And it works for me in Reaper.

[falkTX]

maybe just missing reporting the file change to the host when it happens after changing state (either pedalboard load or preset).
on aida-x this happens during work-restore https://github.com/AidaDSP/aidadsp-lv2/blob/main/rt-neural-generic/src/rt-neural-generic.cpp#L778 (which is called by the host during run/RT, so the atom sequence buffers are still valid)

[mikeoliphant]

maybe just missing reporting the file change to the host when it happens after changing state (either pedalboard load or preset).
on aida-x this happens during work-restore https://github.com/AidaDSP/aidadsp-lv2/blob/main/rt-neural-generic/src/rt-neural-generic.cpp#L778 (which is called by the host during run/RT, so the atom sequence buffers are still valid)

It looks to me like this is being done. LV2 atom code makes my head hurt, though, so there could be an issue with it.

[rerdavies]

The plugin currently sends an LV2_STATE__Changed message after the filename property changes in response to LV2_PATCH__Set,. It assumes that the host will issue an LV2_PATCH__Get if it wants to, and will do so to get the current filename after a state is loaded. Both Ardor and Reaper are satisfied with that.

The procedure being used in the code you cited seems to send either an unsolicited LV2_PATCH__Set or a custom notification (I can't find the source for it, but they seem to send the pathname). I don't think it's appropriate for a plugin to send an unsolicited LV2_PATCH__Set message. And the authors of the Aida-DSP plugin seem to be concerned enough about it that they saw fit to compile the code conditionally.

I'm open to arguments to the contrary; but I think the AIDA_DSP code is purpose-built for MOD GUI, and isn't generically correct. The host should be looking for a STATE_Changed message as notification that a property has changed due to a PATCH__Set; and the host should be issuing a PATCH__Get if knows that a state restore has been executed.

For what it's worth, I'm pretty sure that a MOD UI can do a PATCH__Get to retrieve a property, and that's probably a good path for you if you're trying to write a MOD UI for this plugin.

[falkTX]

I'm open to arguments to the contrary; but I think the AIDA_DSP code is purpose-built for MOD GUI, and isn't generically correct. The host should be looking for a STATE_Changed message as notification that a property has changed in due to a PATCH__Set; and and should be issuing a PATCH__Get if knows that a state restore has been called.

if the state has changed, I dont think the roundabout waiting for the host to send a patch-get should be used.
the "state changed" flag should be for stuff that the host cannot possibly know, like internal state not saved as control ports or patch parameters (so it is only accessible via state functions)

from my experience building 2 hosts (carla and mod-host) the extra step to fetch state is superfulous and introduces a sort of race condition because there is a step between a parameter being loaded/changed and the host knowing what that parameter changed to. by the time a state changed request is received and host asks about the plugin state, the plugin might have initiated another parameter change already.

but I do not really see a reason NOT to give this info back to the host.
if a parameter change is triggered via input port, then plugin can change things and dont bother report about it because it came from the host. if a parameter change happens due to something triggered during state load, the host cant really know about it in advance and so it should be notified.
that makes the most sense to me

it is obvious that after a state restore, the plugin state will have changed. sending "state changed" flag to the host after a state restore, only for the host to then listen to that and request the current state... just seem a layer of roundabout for no good reason. when the host calls state restore, it can expect the plugin state to have changed.

[rerdavies]

Rather difficult to argue with your impeccable credentials, sir. :-)

I'll make the changes in my plugins (ToobAmp). You want an LV2__PATCH__Set notification, right?

@mikeoliphant Do you want me to put together a pull request for you, or do you want to do it yourself. If the latter, you should also probably modify the code to issue the PATCH__Set when the message is received, rather than when the load completes.

[falkTX]

I'll make the changes in my plugins (ToobAmp). You want an LV2__PATCH__Set notification, right?

only when changed by the plugin, for now I think that only happens during state restore (which is called both during pedalboard load and preset load)

[rerdavies]

@falkTX: Ok, while we're here, walk me through your feelings on the plugin sending STATE_Changed in response to PATCH_Set. The argument being that a host can't assume that a setting a property changes the saveable state.

[falkTX]

if the host cant assume that, then a lot of things break. how can the host even know that a simple float parameter value was set correctly? the patch API is similar to a REST one used in HTTP, for those there is a clear response for each request. so in my opinion if we want a response to check the validity of a patch-set, the plugin ought to give something in return with a matching "call id" or whatever we define to match that patch-set.

I mean, if the host tries to change 5 parameters at once, and one fails. should the plugin send state-changed? seems the wrong approach and the wrong thing to use, there must be some another mechanism for it.
the state-changed is something that shouldnt be mixed in with all this.
but what the right approach is for validating patch-set requests I also do not know right now. I have always went with the idea that patch-set requests cant be ignored.

[mikeoliphant]

@mikeoliphant Do you want me to put together a pull request for you

That would be much appreciated - thanks!

[rominator1983]

@mikeoliphant I have written confirmation from @statkinson regarding the license for the resources.

Hi, Roman.

Thanks for reaching out! You're correct--as far as it relates to the repos that I maintain, yes, you're welcome to use it.

Cheers,

Steve

[rerdavies]

@falkTX: I have implemented your suggestions in ToobAmp convolution reverb, in preparation for porting the code into NAM (easier for me to debug). But I'm having problems with saving and restoring carla workspaces. The paths don't round-trip property. The file path is broken after opening the saved Carla project. I've made a detailed report in the Carla github repository: falkTX/Carla#1784 (comment)

[falkTX]

@rerdavies for the case of NAM, does the issue also happen? there is only 1 piece missing from NAM-lv2 side for the report of file changes, so I am guessing it doesn't apply here.
Asking because if you already have a bit of code to handle this then I don't have to do it myself. Will be happy to test

[rerdavies]

The pull request has been sent. I have not tested on MOD. Let me know if it works.

[rominator1983]

linking to here: #32

[mikeoliphant]

Model "sticking" in the gui resolved via #34