[libcamera-devel] [PATCH 2/3] libcamera: ipa_manager: Split common code out of createIPA()

[Kieran Bingham]

Hi Laurent,

On 12/07/2021 00:15, Laurent Pinchart wrote:
> The createIPA() template function starts with code that doesn't depend
> on the template parameters. Split it to a non-template function to avoid
> code duplication in the binary.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/internal/ipa_manager.h | 13 ++++---------
>  src/libcamera/ipa_manager.cpp            | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> index 42201839901b..0687842e5c06 100644
> --- a/include/libcamera/internal/ipa_manager.h
> +++ b/include/libcamera/internal/ipa_manager.h
> @@ -34,15 +34,7 @@ public:
>  					    uint32_t minVersion,
>  					    uint32_t maxVersion)
>  	{
> -		IPAModule *m = nullptr;
> -
> -		for (IPAModule *module : self_->modules_) {
> -			if (module->match(pipe, minVersion, maxVersion)) {
> -				m = module;
> -				break;
> -			}
> -		}
> -
> +		IPAModule *m = self_->module(pipe, minVersion, maxVersion);
>  		if (!m)
>  			return nullptr;
>  
> @@ -62,6 +54,9 @@ private:
>  		      std::vector<std::string> &files);
>  	unsigned int addDir(const char *libDir, unsigned int maxDepth = 0);
>  
> +	IPAModule *module(PipelineHandler *pipe, uint32_t minVersion,
> +			  uint32_t maxVersion);
> +
>  	bool isSignatureValid(IPAModule *ipa) const;
>  
>  	std::vector<IPAModule *> modules_;
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index b4606c6159e5..9533c8fadea6 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -245,6 +245,23 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth)
>  	return count;
>  }
>  
> +/**
> + * \brief Retrieve and IPA module that matches a given pipeline handler

This actually finds the 'first' matching IPA module (for however the
list is sorted).

We can already have multiple potentially matching IPA modules available
on a system. Should this be more explicit on what is matched?



> + * \param[in] pipe The pipeline handler
> + * \param[in] minVersion Minimum acceptable version of IPA module
> + * \param[in] maxVersion Maximum acceptable version of IPA module
> + */
> +IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,
> +			      uint32_t maxVersion)
> +{
> +	for (IPAModule *module : modules_) {
> +		if (module->match(pipe, minVersion, maxVersion))
> +			return module;
> +	}
> +
> +	return nullptr;
> +}
> +
>  /**
>   * \fn IPAManager::createIPA()
>   * \brief Create an IPA proxy that matches a given pipeline handler
>