[libcamera-devel] [PATCH v3 5/7] libcamera: ipa_manager: use proxy

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

I think you missed my review on v2.

On Wed, Jul 10, 2019 at 03:44:48AM +0900, Paul Elder wrote:
> Make IPAManager isolate an IPA in a Proxy if the IPA's license is not
> open source, before returning the IPA to the caller. For now, only use
> the default Linux IPA proxy, and only LGPL 2.1+ is considered open
> source.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> Changes in v3:
> - license checking is done with SPDX license strings, and only LGPL 2.1+
>   is accepted for now
> 
> New in v2
> - replaces adding shims
> - since Proxies are not external shared objects like the shims in v1
>   were, there is no longer a list of shims that is treated like
>   IPAModules
> - instead the matching is done by searching the list of proxy factories
> 
>  src/libcamera/ipa_manager.cpp | 46 +++++++++++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index 532b77d..49a12ca 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -14,6 +14,7 @@
>  #include "ipa_module.h"
>  #include "log.h"
>  #include "pipeline_handler.h"
> +#include "ipa_proxy.h"
>  #include "utils.h"
>  
>  /**
> @@ -25,6 +26,18 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPAManager)
>  
> +namespace {
> +
> +bool isOpenSource(const char *license)
> +{
> +	if (!strcmp(license, "LGPL-2.1-or-later"))
> +		return true;
> +
> +	return false;
> +}
> +
> +} /* namespace */
> +
>  /**
>   * \class IPAManager
>   * \brief Manager for IPA modules
> @@ -129,7 +142,7 @@ int IPAManager::addDir(const char *libDir)
>   * \param[in] maxVersion Maximum acceptable version of IPA module
>   *
>   * \return A newly created IPA interface, or nullptr if no matching
> - * IPA module is found
> + * IPA module is found or if the IPA interface fails to initialize
>   */
>  std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,
>  						    uint32_t maxVersion,
> @@ -144,7 +157,36 @@ std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,
>  		}
>  	}
>  
> -	if (!m || !m->load())
> +	if (!m)
> +		return nullptr;
> +
> +	if (!isOpenSource(m->info().license)) {
> +		IPAProxyFactory *pf = nullptr;
> +		std::vector<IPAProxyFactory *> &factories = IPAProxyFactory::factories();
> +
> +		for (IPAProxyFactory *factory : factories) {
> +			/* TODO: Better matching */
> +			if (!strcmp(factory->name().c_str(), "IPAProxyLinux")) {
> +				pf = factory;
> +				break;
> +			}
> +		}
> +
> +		if (!pf) {
> +			LOG(IPAManager, Error) << "Failed to get proxy factory";
> +			return nullptr;
> +		}
> +
> +		std::unique_ptr<IPAProxy> proxy = pf->create(m);
> +		if (!proxy->isValid()) {
> +			LOG(IPAManager, Error) << "Failed to load proxy";
> +			return nullptr;
> +		}
> +
> +		return proxy;
> +	}
> +
> +	if (!m->load())
>  		return nullptr;
>  
>  	return m->createInstance();

-- 
Regards,

Laurent Pinchart