Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow plugins to request custom extensions #136

Merged
merged 7 commits into from
Oct 17, 2023

Conversation

jatinchowdhury18
Copy link
Collaborator

@baconpaul I still need to clean up the example code, but I wanted to ask about the pattern I used for making the extensionGet function pointer available to the plugin. The "problem" with the function pointer pattern that we've used previously is that the pointer is not yet set when the plugin constructor is called, so the plugin can't query for extensions until sometime after the constructor has been called.

So I tried to do something similar to what was done for the is_clap field in clap_properties. I'm not sure if that's the right pattern to use here, or what the potential issues are with that approach? I can't think of anything better at the moment, but if you've got any ideas, that would be great too!

@baconpaul
Copy link
Collaborator

I wonder if we should do something like make the default version of extensionget something like

{
// don’t call this in the constructor it is too early
Jassertfalse
Return nullptr
}

So you at least get that clearly in debug?

@jatinchowdhury18
Copy link
Collaborator Author

I wonder if we should do something like make the default version of extensionget something like

{ // don’t call this in the constructor it is too early Jassertfalse Return nullptr }

So you at least get that clearly in debug?

Yeah, I had thought about that too... my concern is that plugins will probably want to make decisions in their constructor based on which extensions are or aren't available. Like the other logical spots to check for extension availability would be prepareToPlay() or when you construct the plugin editor? It's just a little frustrating that clap_create_plugin passes the host as an argument, so it seems like it should be easy to call get_extension on it, but maybe not :(.

@baconpaul
Copy link
Collaborator

Ahh clap allows it in the constructor and its just we can't thread it? sorry I had mis-understood.

Then yes we can do some finagling since we know there's only one construction path at a time. Set a guard where we create the audio processor with the current host available and reset it afterwards or something. So the construct looks like

   clap_static::host = host;
   auto p = new yourAudioProcessor();
   p->cla-bits_host = host;
   clap_staic::host = nullptr

type thing then getExtension() does if (clap_static::host)... else this->host

that's how you are thinking?

@jatinchowdhury18
Copy link
Collaborator Author

Yeah, that's about what I was thinking, although your little snippet is a lot cleaner than what I had going on :). I'll re-work it later today.

@jatinchowdhury18 jatinchowdhury18 changed the title Allow plugins to request custom extensions [Draft] Allow plugins to request custom extensions Oct 15, 2023
@jatinchowdhury18
Copy link
Collaborator Author

@baconpaul this is ready for a real review now :)

@nofishonfriday
Copy link

nofishonfriday commented Oct 15, 2023

@jatinchowdhury18 Thanks for making this (I'm the original requester)!

I can't build your PR to check for myself atm but looking at the code I have a thought:
https://github.com/free-audio/clap-juce-extensions/pull/136/files#diff-0f1a87f718925de65656f0d89835b615adb368ed8e1c0b497bed064e7528f888R16

Wouldn't it make more sense to do this check for the master track (using GetMasterTrack())?
Master track is always available in every Reaper project by default (unlike track 0, which has to be created manually by the user), so if someone tries to load this CLAP plugin on the master track in an otherwise empty project the check would fail even though it'd actually work fine.
Does it make sense what I'm saying? :)

@jatinchowdhury18
Copy link
Collaborator Author

jatinchowdhury18 commented Oct 16, 2023

@nofishonfriday yes, that's a good call! I hadn't though about what would happen if the user loaded the plugin on the master track.

Update: unfortunately, Reaper won't let us change the master track colour, so I'll keep using track 0 for that part. I just added a check so that we won't crash if track 0 doesn't exist yet.

@baconpaul
Copy link
Collaborator

The CJE changes look great to me. Happy for you to merge!

@jatinchowdhury18 jatinchowdhury18 merged commit 5f369ad into free-audio:main Oct 17, 2023
18 checks passed
@jatinchowdhury18 jatinchowdhury18 deleted the reaper-ext branch October 17, 2023 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants