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

Please implement clap_plugin.reset() for your plugins. #488

Open
colugomusic opened this issue Dec 29, 2024 · 20 comments
Open

Please implement clap_plugin.reset() for your plugins. #488

colugomusic opened this issue Dec 29, 2024 · 20 comments

Comments

@colugomusic
Copy link

Hello!

https://github.com/free-audio/clap/blob/main/include/clap/plugin.h

typedef struct clap_plugin {
   ...
   // - Clears all buffers, performs a full reset of the processing state (filters, oscillators,
   //   envelopes, lfo, ...) and kills all voices.
   // - The parameter's value remain unchanged.
   // - clap_process.steady_time may jump backward.
   //
   // [audio-thread & active]
   void(CLAP_ABI *reset)(const struct clap_plugin *plugin);

CLAP hosts have to rely on plugin developers to implement this in order to revert the plugin DSP back to a "clean" DSP state. An example of where this is needed is the "panic button" that many DAWs provide which will kill active reverb tails, delay feedback, etc.

@falkTX
Copy link
Contributor

falkTX commented Dec 29, 2024

there is no call on DPF side that this can map to, so there is nothing to implement here.

CLAP should be smart enough to have a fallback in case this is not implemented on the plugin side, which is as simple as deactivating and activating the plugin again.
Since this is something on the "audio thread", we cannot do that on DPF side, as it would break audio/realtime conditions.

@colugomusic
Copy link
Author

colugomusic commented Dec 29, 2024

there is no call on DPF side that this can map to, so there is nothing to implement here.

I can see you already have a function named reset inside MVerb.h which I assume does exactly what needs to be implemented (i.e. clear the reverb tail.) Other plugins which don't have long running feedback are perhaps less important but they should still implement this function.

CLAP should be smart enough to have a fallback in case this is not implemented on the plugin side, which is as simple as deactivating and activating the plugin again.

CLAP doesn't have any API that allows the host to query whether or not a plugin is implemented correctly. The host assumes that when it calls reset() the plugin will do what the clap documentation says it should do.

Since this is something on the "audio thread", we cannot do that on DPF side, as it would break audio/realtime conditions.

There is no need to do anything realtime unsafe. At most you are just setting an array of floats back to zero.

@falkTX
Copy link
Contributor

falkTX commented Dec 30, 2024

there is no call on DPF side that this can map to, so there is nothing to implement here.

I can see you already have a function named reset inside MVerb.h which I assume does exactly what needs to be implemented (i.e. clear the reverb tail.) Other plugins which don't have long running feedback are perhaps less important but they should still implement this function.

the issue is that many plugins already released which do not have this function and if exposed will misbehave (clap host sees the function available but then it does nothing..)

CLAP doesn't have any API that allows the host to query whether or not a plugin is implemented correctly. The host assumes that when it calls reset() the plugin will do what the clap documentation says it should do.

but it does have query possible, the function being null means its not implemented.

Since this is something on the "audio thread", we cannot do that on DPF side, as it would break audio/realtime conditions.

There is no need to do anything realtime unsafe. At most you are just setting an array of floats back to zero.

I dont think I was clear in my point... if DPF would do the quick approach of implementing a fallback reset that triggers deactivate + activate, that would break RT requirements because those calls are not meant to be RT safe.
but if the host is in control it can check if the reset function is null, and in that case do the re-activation instead.

@falkTX falkTX transferred this issue from DISTRHO/DPF-Plugins Dec 30, 2024
@colugomusic
Copy link
Author

the issue is that many plugins already released which do not have this function and if exposed will misbehave (clap host sees the function available but then it does nothing..)

Well yes that is exactly the issue I am reporting. Calling reset() doesn't do anything.

but it does have query possible, the function being null means its not implemented.

Says who? Is that documented in the CLAP headers somewhere? As far as I know reset is a required function. Plugins can return null when an extension is queried but that is extensions, not core functions. But I might have missed some documentation somewhere. In any case none of the DPF plugins set that function to null.

I dont think I was clear in my point... if DPF would do the quick approach of implementing a fallback reset that triggers deactivate + activate, that would break RT requirements because those calls are not meant to be RT safe. but if the host is in control it can check if the reset function is null, and in that case do the re-activation instead.

Why not simply implement reset()?

@falkTX
Copy link
Contributor

falkTX commented Dec 30, 2024

Says who? Is that documented in the CLAP headers somewhere? As far as I know reset is a required function.

is it stated somewhere that the plugin struct calls are all mandatory?

Why not simply implement reset()?

just being very conservative on any additions to DPF base API, to not get it bloated. this request seems reasonable, but we have to keep in mind backwards compatibility.
looking around clap apis I think its possible to keep backwards compat with https://github.com/free-audio/clap/blob/main/include/clap/host.h#L30 while adding a new function (though it will need to be guarded by a feature macro again, so we only involve the host restart on old/non-updated plugins)

@colugomusic
Copy link
Author

Says who? Is that documented in the CLAP headers somewhere? As far as I know reset is a required function.

is it stated somewhere that the plugin struct calls are all mandatory?

Is it stated somewhere that they're not? CLAP doesn't have a proper specification yet so a lot of stuff like this is unspecified. I've cried a lot about it here: free-audio/clap#414

Personally I would assume that all functions are mandatory unless explicitly specified as being optional. What do you think?

Why not simply implement reset()?

just being very conservative on any additions to DPF base API, to not get it bloated. this request seems reasonable, but we have to keep in mind backwards compatibility. looking around clap apis I think its possible to keep backwards compat with https://github.com/free-audio/clap/blob/main/include/clap/host.h#L30 while adding a new function (though it will need to be guarded by a feature macro again, so we only involve the host restart on old/non-updated plugins)

The function is already there. It's just a no-op

static void CLAP_ABI clap_plugin_reset(const clap_plugin_t*)

@falkTX
Copy link
Contributor

falkTX commented Dec 30, 2024

well I added something for this in 800583a which might serve as the initial step.

if we introduce a dedicated reset function on DPF plugin API then we could make use of that instead of doing this restart.
but I think for the moment this is a nice quick fall-back solution, assuming it works..

@colugomusic
Copy link
Author

well I added something for this in 800583a which might serve as the initial step.

if we introduce a dedicated reset function on DPF plugin API then we could make use of that instead of doing this restart. but I think for the moment this is a nice quick fall-back solution, assuming it works..

Thanks for looking into this, but this is not correct! "reset" and "restart" are different concepts. The plugin shouldn't request the host to restart it when reset() is called. All it needs to do is reset its own DSP state back to zero.

@falkTX
Copy link
Contributor

falkTX commented Dec 30, 2024

Thanks for looking into this, but this is not correct! "reset" and "restart" are different concepts. The plugin shouldn't request the host to restart it when reset() is called. All it needs to do is reset its own DSP state back to zero.

I know and understand the concept, but in many (all?) plugins the re-activation triggers the same. it just so happens that deactivate + activate does a few more extra things.

And this is just a fallback anyway

@colugomusic
Copy link
Author

Okay I get it. Sorry I am a bit tired but I understand your reasoning now. Thanks for looking into it!

@falkTX
Copy link
Contributor

falkTX commented Dec 30, 2024

testing would be much appreciated. do you know of other plugin APIs that have similar concepts? it would add extra motivation/reasons to add it as plugin API.

I recall AU having this, dont think VST2 has it, LV2 for sure doesnt. maybe VST3?

@colugomusic
Copy link
Author

I can trying building your develop branch from source tomorrow and see if your fallback works ok. I think the main motivation to add it should be that CLAP specifies that it should be added but I admit that CLAP is still under-specified so you are not morally required to do anything. The argument that hosts may simply call stop_processing or deactivate instead isn't good because many plugins don't even perform a DSP reset in those functions either. That's why I will likely be bugging more plugin developers about implementing reset() properly.

I'm not overly familiar with VST3 but if I understand right, the VST host has the flexibility to destroy and recreate the audio component without affecting the GUI, so they don't need to rely on the plugin developer to implement a DSP reset. CLAP's approach is probably cleaner but unfortunately doesn't work very well currently because it requires the plugin developers to all be playing by the rules

@baconpaul
Copy link

baconpaul commented Dec 30, 2024

juce has juce::AudioProcessor::reset - a user function to reset

VST3 has IAudioProcessor::setProcessing which is required to reset buffers if called with false. Juce calls reset from here.

AU has ComponentResult MusicDeviceBase::Reset (AudioUnitScope inScope, AudioUnitElement inElement) override

clap has (as mentioned) void clap_plugin_t::reset() noexcept

From perusing the JUCE code it seems Unity has a similar API, but VST2 and AAX do not, or if they do, they are not implemented by JUCE. But that was just a quick scan.

@colugomusic
Copy link
Author

I'd like to help you test this but I'm not actually sure how to build the DPF plugins with your 'develop' branch instead of 'main'. But if it is easy for you to send me an MVerb.clap binary for Windows then I can at least give it a quick test to check if the reverb tail is stopping now. But there is no rush, I can wait until your next official release

@dromer
Copy link
Contributor

dromer commented Dec 30, 2024

@colugomusic simply checkout develop branch and build?

@colugomusic
Copy link
Author

@colugomusic simply checkout develop branch and build?

https://github.com/DISTRHO/DPF-Plugins has no develop branch

@dromer
Copy link
Contributor

dromer commented Dec 30, 2024

@colugomusic ah sorry, didn't realize you meant that repo and I also didn't know that this one has dpf fully vendored instead of as a submodule.

@colugomusic
Copy link
Author

Ah I see what you mean. I may be able to just checkout develop and drop it in locally then.

@falkTX
Copy link
Contributor

falkTX commented Dec 30, 2024

there is a script on that repo to update the plugins, anyhow I run it just now and updated the plugins + dpf

@falkTX
Copy link
Contributor

falkTX commented Dec 30, 2024

there is an experimental implementation as per 576b507

instead of a new function I went with a custom parameter designation, alike bypass.

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

No branches or pull requests

4 participants