Rework worker and cleanup

[falkTX]

This PR reworks the way LV2 worker was implemented, relying on the host to do cleanup and making it less racy in general.
Also introduces "options" extension in order to know the maximum processing buffer size, helps in reducing xruns on the first run of the plugin due to NAM core resizing its buffers.

I had to "downgrade" the C++ smart pointer usage into raw pointers though, because they can't be represented as C-style POD data and thus can't be directly placed into LV2 worker APIs.
The previous worker implementation relied on class variables for a "model to delete" and "staging model" which is awkward to work with and prone to issues. It is best to let the non-RT workers do their thing without ever accessing plugin data, which prevents all race conditions.
With the implementation from this PR the only time class variables are used in workers is during "work response" which runs in the audio thread directly after "process"/"run", and so we can safely swap pointers and other RT things because it will be in sync with the audio processing. We can use that same "work response" time to give the old model pointer for a final non-rt worker for deletion.

Let me know what you think, and if you spot any potential issues.

[mikeoliphant]

The changes to the .ttl file cause the plugin to no longer load in Reaper.

It complains about "prefix not found for token" at byte 1561 - which is the "options" part of this line:

lv2:extensionData work:interface, state:interface, options:interface;

Ardour doesn't seem to like it either.

I think it just needs an @prefix line for "options".

[mikeoliphant]

Adding an options @prefix fixes the load issue.

Unfortunately, the model no longer saves properly in Reaper.

[falkTX]

my bad on the "options", it should be "opts". I pushed to a local copy but forgot to do that here. fix coming right away...

[falkTX]

I had some bad usage of the forge frame time, while cleaning that up I removed the state-changed notification since it is no longer needed here (plugin notifies the host of the file change, and has no other state or hidden settings, so yeah there is no need for it)
Should fix the issues in reaper, was happening in jalv too.

[mikeoliphant]

my bad on the "options", it should be "opts". I pushed to a local copy but forgot to do that here. fix coming right away...

That loads in Reaper now.

The model saving in Reaper may be another issue - I'm looking into it.

[mikeoliphant]

The Reaper issue seems to be unrelated. For some reason Reaper doesn't restore the loaded model properly if you start it without a valid audio interface running. Which is strange, but I don't think related to these changes.

[falkTX]

The Reaper issue seems to be unrelated. For some reason Reaper doesn't restore the loaded model properly if you start it without a valid audio interface running. Which is strange, but I don't think related to these changes.

not related to this, but still related to LV2 workers.
if the audio engine is not running, then perhaps reaper is not letting the workers do their thing since part of it runs in realtime.
I consider this a reaper bug. if no audio is running the workers should still do their thing, imitating a "offline" render mode where it short-circuits and runs worker stuff directly.

[mikeoliphant]

I think the code you added to run the model on max frames after loading is probably helpful, but won't really solve the problem if Eigen's resize() operations free data when shrinking. There is an issue for the core problem here, but I haven't had a chance to dig into it yet: sdatkinson/NeuralAmpModelerCore#49

[falkTX]

I agree, it is mostly a way to get things going for now until NAM core has some better APIs for it.
For MOD and other hosts where the buffer size does not change, the introduced bits already help.

At minimum we need the NAM core side to not resize to lower sizes, shrinking the data as you say.

Still to do on this LV2 plugin side is to resize buffers when the max-buffer-size changes, I didn't want to do a preroll in there again as it doesnt seem right.

[mikeoliphant]

I think I am good merging this. Anything else on your end?

[falkTX]

No, but would be nice to confirm with @rerdavies if the changes also work on their end.

I only tested with mod-host and jalv. I guess you tested with Reaper. That is already 3 hosts.

Leave the decision up to you, if you perhaps think this is a better starting point for the plugin then let's go for it and then fix whatever new issues appear.

[mikeoliphant]

No, but would be nice to confirm with @rerdavies if the changes also work on their end.

Yep - already planned to do that on the other PR thread. I'm thinking that if there are any issues it is probably easier to address them with this PR already in as a baseline.

[mikeoliphant]

Btw, I tested Ardour as well, and all looks good there.

[rerdavies]

@falkTX:

perhaps reaper is not letting the workers do their thing since part of it runs in realtime.
I consider this a reaper bug. if no audio is running the workers should still do their thing,
imitating a "offline" render mode where it short-circuits and runs worker stuff directly.

Probably this issue:

https://github.com/mikeoliphant/neural-amp-modeler-lv2/issues/38

Pull request made for the fix.

Apologies for the delay in responding. I've been implementing changes that came out of our earlier discussions in PiPedal and ToobAmp, and noticed the issue while retesting NAM.

In particular you may want to review changes made to MOD branches, particularly with respect to the missing lv2_atom_forge_pop(&atom_forge, &sequence_frame); which causes most hosts to go parsing through uninitialized output buffers -- most of which will be empty atoms if your host as zeroed out the buffer. It's a dangerous omission.