free-audio / clap

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

Clarity on whether the "main thread" is a "symbolic thread" #195

Closed russellmcc closed 1 year ago

russellmcc commented 1 year ago

I've read the documentation of the threading in thread-check.h and I remain confused about one point.

It is clearly stated that the "audio thread" is what is described as a "symbolic thread", i.e., "there isn't one OS thread that remains the audio-thread for the plugin lifetime". This makes sense to me - it's sort of an "abstract thread" where the host is required to sequence all calls into it, but is not required to run all calls on the same OS thread. Is the same true of the main-thread? It seems so, since the intro to the thread-check.h comment says "CLAP defines two symbolic threads".

However, there is a main thread defined by the OS, which is the thread where GUI communications happen. It seems like gui.h assumes that that the "symbolic" main-thread corresponds exactly to the OS GUI thread. So, in this situation it seems it's not a "symbolic thread" at all but rather the OS GUI thread.

One concrete question I have about all this is, is the host allowed to load states on background threads? This is allowed explicitly in other formats, and many hosts take advantage of this to improve session load time by loading different plug-in's states concurrently.

If this project wants to support performance parity with other formats, I recommend making it clear that the main thread is another "symbolic thread" similar to the audio thread. This would require an additional documented restriction on the calls in gui.h - not only must they be synchronized with the main thread "symbolic thread", they must also occur on the OS GUI thread. Additionally, I recommend removing the following sentence in the thread-check.h documentation - "It is usually the thread on which the GUI receives its events" - if main-thread is "symbolic", it's actually entirely up to the host which OS thread the main-thread is run on, so guesses about what thread "it usually is on" aren't very helpful for plug-in authors and may be a source of confusion. Plug-in authors may accidentally build in an assumption that the main-thread is the GUI thread, only to have obscure bugs on hosts where that isn't always the case.

It also would be possible, if the project wants to emphasize threading simplicity over performance parity with other formats, to choose that the main-thread, unlike the audio-thread, is not "symbolic" but rather corresponds to a single OS thread, which must be the GUI thread if the GUI interface is supported (otherwise could be any single thread).

baconpaul commented 1 year ago

So the short version as I understand it is

So that combo is why the doc says the main thread is symbolic. On Linux and on win/mac without a ui event loop it has to be. But I agree it is symbolic in a special way

if my collaborators agree with this assessment perhaps we can put an abbreviated version of it in the doc. I agree this is a confusing and subtle issue

russellmcc commented 1 year ago

Thanks for the extra information on linux UI, I wasn't aware of that.

I guess as a practical matter the biggest question for me is the load function on the state interface on mac and windows. In other plug-in formats generally hosts are allowed to call the equivalent function on a non-GUI thread so that loading a session can make use of multi-core processing to load faster. Indeed many hosts do take advantage of this, for example Reaper. If in clap the host is required to call the load function on the main GUI thread, the result could be slower session loading than other formats since the host would not be able to take advantage of multicore processors.

russellmcc commented 1 year ago

Any thoughts on this matter? I think if the main thread really must be the GUI thread, it could be argued that CLAP is "less performant" than other formats, like VST3, since session loads couldn't take advantage of multithreading and would thus take longer. So, after thinking about this I'd recommend trying to allow the load call to be called off the GUI thread.

robbert-vdh commented 1 year ago

On Windows and macOS it must be the GUI thread, yes (or well, it needs to be the main thread). On Linux there's no concept of a main thread so hosts can in theory do whatever they want, but it's probably still a good idea to keep the behavior the same as on Windows and macOS to avoid surprises. With VST3 and with all other plugin formats you also need to load state from the main thread or you may run into plugin that end up freezing or crashing the host because they do something during state loading that may only be done from the main thread.

russellmcc commented 1 year ago

There are hosts that can do asynchronous loading of state - for example in this document, Apple states for AUv2:

When setting audio unit properties, don’t assume callbacks happen on the main thread. When updating the user interface, asynchronously dispatch to the main queue.

Indeed there are AUv2 hosts that will load saved state asynchronously, taking advantage of this note.

Outside of AUv2, there is another common format where background loading is explicitly allowed unless the plug-in opts out - perhaps this thread might provide some insight.

I think it's okay to say in CLAP, the load call really has to happen on the main thread, but this comes with a certain performance cost vs other plug-in formats, which might be acceptable. Perhaps to clarify matters gui.h and thread-check.h should include a note that the main thread in CLAP must correspond to the OS main thread on mac and windows.

abique commented 1 year ago

Can we close this issue?

russellmcc commented 1 year ago

This issue has not been resolved.

Probably the easiest resolution would be to add a note of clarification in gui.h stating that during the lifetime of the GUI (i.e. between create and destroy, inclusive), all calls to the plugin symbolic main-thread must actually occur on the OS main thread on Windows and Mac.

I can try to open a PR to this effect if this resolution is desirable.

russellmcc commented 1 year ago

I apologize for closing, I was confused by the github UI :-)

abique commented 1 year ago

I can try to open a PR to this effect if this resolution is desirable.

Please do so 👍

abique commented 1 year ago

Please add: Fixes #195 in the commit message, so it'll close this issue automatically once merged to main.

Trinitou commented 1 year ago

But now this could be closed as #255 has been merged, right?

russellmcc commented 1 year ago

Yes, thanks.