Enable deriving from HostProxy + code adjustments#85
Enable deriving from HostProxy + code adjustments#85Trinitou wants to merge 15 commits intofree-audio:nextfrom
Conversation
* * Add host getExtension() handler * Ignore generated files when used as a meson subproject * host/plugin: check overriden extension access before built-ins * host.hh: remove string_view include; not used. * plugin: call extension on the 'self' object
* Add base implementation of surround extension * Comment out windows clang test due to broken env * Remove wrong check free-audio#84#discussion_r2192490266 * Remove wrong check free-audio#84#discussion_r2192491651
| class HostProxy { | ||
| public: | ||
| HostProxy(const clap_host *host); | ||
| virtual ~HostProxy() = default; |
There was a problem hiding this comment.
So my real question is: why do you need virtual inheritance here.
That is you could do
struct MyHostProxy : HostProxy<A,B>
{
void init() {
getExtension(foo);
getExtension(bar);
doWhateverIWant();
HostProxy<A,B>::init();
}
};
There was a problem hiding this comment.
That would also let you add the extra constructors you want below.
There was a problem hiding this comment.
yes, you're right and I have done that. This would be mainly for convenience. You think it's bad?
| protected: | ||
| Plugin(const clap_plugin_descriptor *desc, const clap_host *host); | ||
| virtual ~Plugin() = default; | ||
| PluginBase(const clap_plugin_descriptor &desc, HostProxy<h, l> &hostProxy); |
There was a problem hiding this comment.
why change as opposed to add two constructors?
There was a problem hiding this comment.
the current version has the (non-reference) member HostProxy<h, l> _host;. It is protected so there might be some implementations relying on it so I can't just replace it how I want.
But I want to pass a reference to a custom-implemented HostProxy and have the Plugin only hold a reference. So my solution was to extract the class.
Maybe 2 constructors could be done by changing the _host into a reference and putting a std::unique_ptr next to it:
std::unique_ptr<HostProxy<h, l>> _hostProxy;
HostProxy<h, l>& _host;
Plugin(const clap_plugin_descriptor &desc, const clap_host *host)
: _hostProxy(std::make_unique<HostProxy<h, l>>(host))
, _host(*_hostProxy.get())
{
}
Plugin(const clap_plugin_descriptor &desc, std::unique_ptr<HostProxy<h, l>> hostProxy)
: _hostProxy(std::move(hostProxy))
, _host(*_hostProxy.get())
{
}Has the disadvantage that it needs an additional member. Also I'm not sure whether everybody is happy with the std::unique_ptr (I mean, why not? but who knows)
What do you think?
There was a problem hiding this comment.
ah, or maybe like this?
std::unique_ptr<HostProxy<h, l>> _hostProxy;
HostProxy<h, l>& _host;
Plugin(const clap_plugin_descriptor &desc, const clap_host *host)
: _hostProxy(std::make_unique<HostProxy<h, l>>(host))
, _host(*_hostProxy.get())
{
}
Plugin(const clap_plugin_descriptor &desc, HostProxy<h, l> &hostProxy)
: _host(hostProxy)
{
}so the first constructor would use the std::unique_ptr _hostProxy and the second one would leave it empty and unused.
Is that what you meant, @baconpaul ?
There was a problem hiding this comment.
this is really confusing. changing types and changing values to references which changes the lifecycle. or changing them to unique ptrs which changes every client from . to ->
why are we changing the type of Plugin::_host at all? What's wrong with it being a value?
Maybe take a step back. What's the purpose of any of these changes. You want to allow HostProxy to have additional extensions via subclassing?
There was a problem hiding this comment.
Yes, deriving from HostProxy is the main thought behind it.
There was a problem hiding this comment.
hey @baconpaul :
- I added a test so we can see/prove everything more hands-on
- I added the test first (a3926ed)
- then reverted my previous changes
- and then adapted the test to show the difference (acf44c3)
- so you can see it is not possible to inherit from
clap::helpers::HostProxyanymore -> instead I have to make its thread-check andclap_hostmembers public in order to use them for custom extensions
- so you can see it is not possible to inherit from
There was a problem hiding this comment.
So looking at acf44c3 it really seems like the only change you need is to add a clapHost accessor to the host proxy
I dint understand why protected isn’t a good enough protection though. Protected allows calls from derived classes. Why public?
but from the second commit seems like a 20 line change is in order
There was a problem hiding this comment.
public because I don't derive from HostProxy anymore.
I can't override the HostProxy Plugin::_host with a derived class (or can you tell me a way to do it?).
And I don't want to to have two host proxies (one derived + the inevitable HostProxy Plugin::_host)
There was a problem hiding this comment.
Ahh right you want the Plugin::_host to be a derived class.
so Plugin is already a template. You could either (1) add a host proxy type which defaults to HostProxy<cl,tl> or (2) use a trait/specialization trick which lets you override at your creation time. But I don't understand why you don't derive from HostProxy
You know this is too confusing on GitHub from diffs. I'm just going to have to pull your test and see what it's doing and try and figure out what the problem is. I'm not helping anyone unless I do that. Let me make some time for that in the morning.
873cc70 to
a61bcdf
Compare
I wanted to derive from
HostProxyto try out some custom host extensions. And it would be good to be able to derive fromHostProxymyself and then pass a reference toPlugin-> So I extracted the
Pluginclass for compatibility and introduced a newPluginBaseclass