request_flush, processing, races, and when to call

[baconpaul]

This thread is a result of a conversation with @robbert-vdh @signaldust Urs and myself on KVR at https://www.kvraudio.com/forum/viewtopic.php?p=8453301#p8453301

The essence of the question was: When processing is stopped a plugin should call request_flush. How do you communicate processing is stopped so that can happen without a race.

One such approach is

1. Create a std::atomic<bool> which you update in start/stop processing
2. If that bool is false, call request flush

but @signaldust rightly points out that if processing resumes before the request flush, then the request flush will happen with processing active, which the spec says is not necessary.

I think the spec is ambiguous in this regard. The documentation says

   // Request the host to call clap_plugin_params->fush().
   // This is useful if the plugin has parameters value changes to report to the host but the plugin
   // is not processing.
   //
   // eg. the plugin has a USB socket to some hardware controllers and receives a parameter change
   // while it is not processing.
   //
   // This must not be called on the [audio-thread].
   //
   // [thread-safe]

I propose we add a sentence If processing is active when request_flush is called, the call is ignored. I think we should add that because (1) it is what happens I believe and (2) it allows you to easily write a race free implementation using an atomic and a timer, as discussed on that KVR thread

Reading the sample clap-host it seems that request_flush does the following (void PluginHost::clapParamsRequestFlush)

1. If the engine is inactive, flush immediatley
2. Else queue up a flush which the idle thread will force a flush if inactive

From this I conclude calling refresh_flush repeatedly with processing going will result in nothing happening until processing stops and then will have one call to flush from the idle thread. I think this is reasonable behavior, but the spec is silent.

I think making request_flush strictly a no-op when processing is going (which would mean basically untoggle that state in the idle loop I think in the clap host) is a better spec, I think its what we intended, and I think we can update the doc accordingly

But if it isn't a better spec, and isn't what we intended, then lets figure out what is and clean up the spec doc some for a future dot release!

[baconpaul]

In a discord convo with @abique he tells me that yes, the intent is definitely that request_flush is always safe, and so this should be a spec documentation update, not a host behavior update.

[signaldust]

Oh right, that makes sense.

[abique]

I've clarified the documentation on the next branch.

[baconpaul]

yeah put a little edit there.

now in other news, when I call request_flush, bws43b6 doesn't call my flush method when processing is off... :)

[abique]

This is a bug then.

[abique]

Maybe better to report in interop tracker then.

[baconpaul]

This is a bug then.

I know! :) I'll write it up in interop tracker once i make it reproducible with the little saw demo. Probably today maybe this weekend.

[baconpaul]

free-audio/interop-tracker#24