Skip to content

ext/params: clarify flush() dispatch expectations when active - #493

Draft
Yashika0724 wants to merge 2 commits into
free-audio:nextfrom
Yashika0724:fix/params-flush-thread-dispatch
Draft

ext/params: clarify flush() dispatch expectations when active#493
Yashika0724 wants to merge 2 commits into
free-audio:nextfrom
Yashika0724:fix/params-flush-thread-dispatch

Conversation

@Yashika0724

Copy link
Copy Markdown
Contributor

This updates the documentation around flush() and request_flush() in ext/params.h to clarify the intended threading expectations when a plugin is active but the audio engine is temporarily idle (for example, when the transport is stopped).

The existing comments already state that flush() must not run concurrently with process(). This change makes that requirement more explicit in the “active but not processing” case to avoid ambiguity about which thread should dispatch flush().

No functional changes are introduced. There are no ABI, struct layout, or behavioral modifications — this is purely a documentation clarification.

When a plugin is active but not processing (e.g. transport stopped),
flush() must still be called from the audio-thread, not the main-thread.
Dispatching flush() from the main-thread in that state violates the
"must not be called concurrently to process()" invariant if transport
restarts before the flush completes.

Also corrects the rationale in request_flush(): the audio-thread
prohibition is about re-entrancy, not solely about an ongoing process().

Signed-off-by: Yashika0724 <ssyashika1311@gmail.com>
@CLAassistant

CLAassistant commented Mar 1, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Yashika0724

Copy link
Copy Markdown
Contributor Author

Hi @abique,
While going through ext/params.h I thought the threading expectations around flush() in the “active but transport stopped” case could benefit from a bit more explicit wording.
This PR only aims to clarify that scenario and preserve the existing non-concurrency requirement between flush() and process().

If you think the current wording is already clear enough, I’m happy to adjust or drop the change.
Thank you for taking a look.

@defiantnerd

Copy link
Copy Markdown
Contributor

Usually an audio engine is still running, although transport is stopped. The term would be "processing is stopped" which process is not being called anymore for some time.

@Yashika0724

Copy link
Copy Markdown
Contributor Author

Thanks for the clarification, that makes sense.

I’ll update the wording to refer to “processing is stopped” instead of “transport is stopped” to better match the intended terminology.

Signed-off-by: Yashika0724 <ssyashika1311@gmail.com>
@Yashika0724

Copy link
Copy Markdown
Contributor Author

Thanks!
I’ve updated the wording to use “processing is stopped” instead.

Comment thread include/clap/ext/params.h
// - clap_plugin.process()
// - clap_plugin_params.flush()
//
// This function is always safe to use and should not be called from an [audio-thread] as the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me the previous paragraph said everything and shortly.
We could replace the should not by must not.

@abique

abique commented May 20, 2026

Copy link
Copy Markdown
Contributor

@Yashika0724 thank you again :-)

@defiantnerd @baconpaul what do you think of this PR?

You know that I prefer shorter documentation where you have the strict minimum info and you can deduce everything else.

But overtime, we moved to a more detailed style of documentation and I'm probably not the right person to comment on that PR, so let me know what you think please :-)

@abique abique assigned abique and baconpaul and unassigned abique May 20, 2026
@abique
abique changed the base branch from main to next May 20, 2026 11:36
@baconpaul

Copy link
Copy Markdown
Collaborator

So the audio thread is a single thread so I don’t understand the comment that process can’t be concurrent with flush when active since they are both audio thread and there’s only one audio thread. So I don’t think the first clause is that useful

the second clause really says “active but not currently processing is still active and when active this is documented as audio thread”. I think that’s a fine clarification as a “for avoidance of doubt” type thing. I would say “for avoidance of doubt an active plugin must still use the audio thread for this even if not processing. This is only available as a main thread function for plugins which are not activated by the host”.

@abique
abique marked this pull request as draft May 20, 2026 13:19
@abique
abique force-pushed the next branch 7 times, most recently from 1e83856 to 275a785 Compare May 23, 2026 09:25
@abique
abique force-pushed the next branch 2 times, most recently from e71b4fa to 195b42a Compare July 13, 2026 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants