Add "audio-port-activation" and "param-indication" extensions #27
Conversation
- ambisonic - audio ports activation - configurable audio ports - context menu - param indication (also implemented) - preset load - remote controls - state context - surround - track info
|
Could you also add |
- sets `CLAP_PARAM_IS_ENUM` and `CLAP_PARAM_IS_STEPPED` together, as the CLAP spec requires them to be set together anyways.
|
Hi, and thanks for your contribution! I think this is quite a big amount of work to both update I will update clap-sys separately, and if that is okay with you I'd like to change your PR to integrate only the extensions you have implemented, and I can implement the others separately. |
|
100% on board with that. Thanks! |
# Conflicts: # extensions/Cargo.toml # extensions/src/lib.rs # extensions/src/params.rs # host/src/process/audio_buffers.rs
prokopyl
changed the title
prokopyl
left a comment
prokopyl
left a comment
There was a problem hiding this comment.
Alright, I just finished getting your work back onto main, and I've only kept the extensions you implemented. 🙂
There's a couple issues that need fixing, but other than that this looks good!
| is_input: bool, | ||
| port_index: u32, | ||
| is_active: bool, | ||
| sample_size: u32, |
There was a problem hiding this comment.
I think you meant to use [SampleSize] here 🙂
| #[inline] | ||
| pub fn set_active( | ||
| &self, | ||
| plugin: &mut PluginSharedHandle, |
There was a problem hiding this comment.
Using the Shared type is incorrect here, as it allows the function to be called on any thread, as well as it being reentrant, both of which are forbidden by the CLAP spec for this function, which will lead to UB.
The CLAP spec requires the [active ? audio-thread : main-thread] pattern, which is a bit tricky to implement.
For that reason, I've just added a new InactivePluginMainThreadHandle handle type to clack-host. You can see the implementation of the params.flush function for reference, but the quick version is this:
- On the plugin side you'll need two traits: one for the
AudioProcessorand another for theMainThread, both requiring to implement the sameset_activefunction.- In the implementation of the C callback, you can use the
PluginWrapper::audio_processoraccessor, which will do the activation check for you and returnOkonly if it is active, which allows you to forward the call to the correct implementation depending on the plugin's state
- In the implementation of the C callback, you can use the
- On the host side you'll need two functions on the
PluginAudioPortsActivation, one for the main thread and one for the audio thread, which will takeInactivePluginMainThreadHandleandPluginAudioProcessorHandlerespectively.
(I should document that stuff somewhere 😅 )
| where | ||
| for<'a> P::Shared<'a>: PluginAudioPortsActivationSharedImpl, | ||
| { | ||
| let sample_size = SampleSize::from_raw(sample_size).unwrap(); |
There was a problem hiding this comment.
You want this line to be inside the handle callback: panicking in a C callback called via FFI is Undefined Behavior and the host won't be able to handle it.
The PluginWrapper::handle function wraps the given callback in catch_unwind and automatically logs everything if a panic happens, and it just returns None. 🙂
| param_id: ClapId, | ||
| has_mapping: bool, | ||
| color: clap_color, | ||
| label: &str, |
There was a problem hiding this comment.
You should use &CStr here instead of &str. The CLAP spec has no requirement of those strings to be strict UTF-8, and it also saves an allocation (from CString::new) as well as removes the error path (which currently leads to a panic).
| ) where | ||
| for<'a> P::MainThread<'a>: PluginParamIndicationImpl, | ||
| { | ||
| let param_id = ClapId::new(param_id); |
There was a problem hiding this comment.
Same as above for panicking outside the handle callback. 🙂
3 participants
This corresponds to CLAP v1.2.2. CLAP v1.2.0 did add a number of new stable extensions:
because for the most part, all these extensions are backwards compatible with their last draft versions, right now it is an open question to extend the extensions crate definition of "stable extension" to "stable extension and any compatible draft version". For now, I've opted not to include the draft version (
CLAP_EXT_..._COMPATin general).