free-audio / clap

Audio Plugin API
https://cleveraudio.org/
MIT License
1.77k stars 100 forks source link

Documentation ambiguity about params flush #147

Closed baconpaul closed 1 year ago

baconpaul commented 2 years ago

in params.h we see

We should also clarify that language ambiguity though.

   // Request a parameter flush.
   //
   // The host will then schedule a call to either:
   // - clap_plugin.process()
   // - clap_plugin_params->flush()
   //

This language is ambiguous. You can meet this spec by always calling flush, for instance, even if processing, especially since flush has the signature

  // Flushes a set of parameter changes.
   // This method must not be called concurrently to clap_plugin->process().
   //
   // [active ? audio-thread : main-thread]

So a host could, while processing, according to the spec, do

start processing
while (processing)
   if uithread_requested_flush
       params flush
  process

and meet the spec as written. But I think the intent was really to call one of flush or process in response to a request flush. That is, if you are processing, the next call to process will do what you need.

This came up because reaper 664 0804 calls param flush in a way similar to above, and surge assumes param flush means processing isn't happening so mis-updates internal state. I have a defensive change to surge for that and I think reaper will change to more the intended one-of-not-both approach but we should update the doc I think.

it seems that should say

   // Request a parameter flush.
   //
   // The host will then schedule a call to either:
   // - clap_plugin.process() if processing is active
   // - clap_plugin_params->flush() if processing is not active
   //

but it could also say something like schedule a call to either, which when processing could mean flush then process

baconpaul commented 2 years ago

As I mentioned in discord, the problem with the outline of calling params flush before process is you lose sample accurate processing for the param changes which get flushed there from the inbound events. So the host would have to very carefully deal with any events it wanted to send to the plugin. I really think we want to give this a bit of thought.

Bremmers commented 2 years ago

// Request a parameter flush. // // The host will then schedule a call to either: // - clap_plugin.process() if processing is active // - clap_plugin_params->flush() if processing is not active //

+1

abique commented 2 years ago

// Request a parameter flush. // // The host will then schedule a call to either: // - clap_plugin.process() if processing is active // - clap_plugin_params->flush() if processing is not active //

+1

This changes the spec, clap_plugin_params->flush() is allowed even if the plugin is processing and active.

Yes, one may loose the sample offset when using flush, but if the plugin may produce record-able events, maybe it should request processing and prevent the host from sleeping it?

@baconpaul I think the loop that you pointed out is correct, but reflects a partial understanding of how clap works rather than a problem in the spec. I'll try to add some clarification here.