Vibe -- fix undefined behavior, gargabe output

[x42]

Ideally apply on top of #36

This fixes various issues with rkr-vibe due to uninitialized variables in the filters. Depending on the initial state, it may produce constant garbage output and even NaN or Inf.

[x42]

PS. Other effects may have similar issue, but before updating those, there'd ideally be a consistent API to ::activate() a plugin. That should called only once when leaving bypass to clear and reset stat, separate from ::cleanup().

[ssj71]

so you are saying ::cleanup should not be called when bypassing, and instead there should be a proper ::activate? I think I just implemented the same logic as was in the standalone rakarrack code to call cleanup on bypass. Doesn't make it right, I'm just trying to understand.

I'm fine with merging this, but I'd like to understand it a bit better.

[x42]

so you are saying ::cleanup should not be called when bypassing.

Ideally it would called only once. Right now every run_() call periodically calls the cleanup method when a plugin is bypassed `if(plug->bypass_p && plug->prev_bypass)` is true after the x-fade completed.

[x42]

The main part of the fix is the fparams() constructor in src/Vibe.h that initializes x1 = y1 = 0; Previously those were uninitialized variables, producing garbage audio output.

After a bypass/re-enable those variable should be cleared, as well. The current patch clears them every cycle. It's not expensive to do that, so it's fine for the time being.

so you are saying ::cleanup should not be called when bypassing, and instead there should be a proper ::activate?

Yes. Long term it would be nice to only reset the state once.

[x42]

context: https://discourse.ardour.org/t/all-of-the-lv2-vibe-plugins-are-broken-underlying-issue/101628

[ssj71]

ok, I think I should go through and fix the logic to only call ::cleanup excatly once after the xfade out. That was the intended result but I can see that its not correct. I can hopefully find enough time to do that in the next few days, that shouldn't necessarily change this PR though IIUC. Sound alright?

[x42]

Ideally it'd be called once before fade_in: When activating a plugin, including the first run.

There's usually no need to reset state during cleanup.

[ssj71]

I'm thinking like #38

It should get the job done, only that it will run an uncessary call to cleanup() when the plugin is bypassed then removed. Changing it to run the other way (cleanup on activate) is just a bit more work that I'm not sure is worth it. Am I missing something?