Implement host request_restart on standalone #417
Conversation
However, macOS is untested
|
Thanks. Unfortunately this is not enough, audio/midi must be stopped during the restart. Your PR will probably lead to crashes and also they should trigger clap-helpers sanity checks. |
|
why do you say that timo? my concern about this was more that the activate doesn't wait for an inflight process to finish after setting audio running so basically you do sah->running = false but if there's a process running with a large block size, that activate may happen before the process finishes and loops around to check running again |
|
We do mean the same thing. The host must make sure that it is not in a process call and also does not enter it whilst re-activating. Therefore the processing must be stopped or bypassed. |
|
right so this will need to do a little spin and process will have to set a skipped atomic. |
|
oh. right. silly me. i was certain that a bug or oversight existed in my code, as i only skimmed the codebase for like 20 minutes before making this pr. i made this quick change to see if it worked, and i honestly i was uncertain if i should make an issue or a pr. i did notice the existence of that fairly obvious unaddressed concurrency problem some time after submitting this. probably what's causing that weird audio glitch on my end. er, i can try to implement the spinlock. i saw spinlock code somewhere in this project, i suppose i could find it and reappropriate it for usage here. also, while audio is addressed i'm not sure if midi processing should be a concern. is it or is it not? or does implementing this fix fix whatever problems might occur with midi? |
|
luckily in clap all events run through process so if you get that locked off properly then indeed midi note expression etc... will all be taken care of. |
|
i think this is okay |
|
|
||
| if (!running) | ||
| { | ||
| memset(f, 0, frameCount * 2 * sizeof(float)); |
There was a problem hiding this comment.
instead of the magic number "2" here should be the actual number of channels even if the current standalone implementation only assumes stereo.
|
@baconpaul I would be okay with that for now, WDYT? |
|
@pkhead please check on the failed workflow issues. If you don't have time, let me know. |
|
I pushed up a change to the clang format and misnamed macOS variable in AppDelegate.mm. Lets see how CI does now. |
|
I also changed the base branch to next |
|
thanks a lot - will merge to next |
3 participants
This PR fully implements the clap_host_t request_restart method for Windows and macOS standalone, although the macOS implementation is currently untested.