[libcamera-devel] [PATCH 15/23] libcamera: IPAManager: Fetch IPAProxy corresponding to pipeline

[paul.elder at ideasonboard.com]

Hi Niklas,

On Sat, Sep 19, 2020 at 01:49:05PM +0200, Niklas Söderlund wrote:
> Hi Paul,
> 
> On 2020-09-15 23:20:30 +0900, Paul Elder wrote:
> > Now that each pipeline handler has its own IPAProxy implementation, make
> > the IPAManager fetch the IPAProxy based on the pipeline handler name.
> > Also, since the IPAProxy is used regardless of isolation or no
> > isolation, remove the isolation check from the proxy selection.
> > 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  src/libcamera/ipa_manager.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> > index 046fd5c6..2d0ea242 100644
> > --- a/src/libcamera/ipa_manager.cpp
> > +++ b/src/libcamera/ipa_manager.cpp
> > @@ -275,8 +275,8 @@ std::unique_ptr<IPAProxy> IPAManager::createIPA(PipelineHandler *pipe,
> >  	 *
> >  	 * \todo Implement a better proxy selection
> >  	 */
> > -	const char *proxyName = self_->isSignatureValid(m)
> > -			      ? "IPAProxyThread" : "IPAProxyLinux";
> > +	std::string pipeName(pipe->name());
> > +	const char *proxyName = pipeName.replace(0, 15, "IPAProxy").c_str();
> 
> Bikeshedding time, is it work keeping the IPAProxy prefix? No strong 
> opinion just asking ;-)

I think it is. There are quite a few components involved in a
pipeline-IPA pairing (with isolation):

pipeline handler - IPA proxy - IPAIPC - IPA proxy worker - IPA interface

And without isolation:

pipeline handler - IPA proxy - (thread) - IPA interface

Out of these five components, the pipeline handler, IPA proxy, IPA proxy
worker, and IPA interface are all pipeline-specific, therefore they
should have the pipeline name in their names. In addition, to
differentiate which component they are, there should be the component
name in their names as well.

IPAIPCs are dependent on the IPC mechanism, therefore they have the
mechanism name in their name, like IPAIPCUnixSocket.

I suppose IPA interface might have multiple names for different
implementations... but that's a different issue with a different
solution.


Paul

> >  	IPAProxyFactory *pf = nullptr;
> >  
> >  	for (IPAProxyFactory *factory : IPAProxyFactory::factories()) {
> > -- 
> > 2.27.0
> > 
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> 
> -- 
> Regards,
> Niklas Söderlund