[libcamera-devel] [PATCH v3 4/6] libcamera: ipa_manager: Re-arrange IPA precedence

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Feb 20, 2020 at 04:57:02PM +0000, Kieran Bingham wrote:
> Setting a user environment path in LIBCAMERA_IPA_MODULE_PATH should take
> precedence over the system loading locations.
> 
> Adjust the IPA search orders accordingly.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/libcamera/ipa_manager.cpp | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index c30b4555290f..3b1d4c0b295e 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -98,24 +98,24 @@ IPAManager::IPAManager()
>  	unsigned int ipaCount = 0;
>  	int ret;
>  
> -	ret = addDir(IPA_MODULE_DIR);
> -	if (ret > 0)
> -		ipaCount += ret;
> -
> +	/* User specified paths take precedence. */

s/User specified/User-specified/ ?

>  	const char *modulePaths = utils::secure_getenv("LIBCAMERA_IPA_MODULE_PATH");
> -	if (!modulePaths) {
> +	if (modulePaths) {
> +		ipaCount += addPath(modulePaths);
> +
>  		if (!ipaCount)
>  			LOG(IPAManager, Warning)
> -				<< "No IPA found in '" IPA_MODULE_DIR "'";
> -		return;
> +				<< "No IPA found in '" << modulePaths << "'";
>  	}
>  
> -	ipaCount += addPath(modulePaths);
> +	/* Load IPAs from the installed system path. */
> +	ret = addDir(IPA_MODULE_DIR);
> +	if (ret > 0)
> +		ipaCount += ret;
>  
>  	if (!ipaCount)
>  		LOG(IPAManager, Warning)
> -			<< "No IPA found in '" IPA_MODULE_DIR "' and '"
> -			<< modulePaths << "'";
> +			<< "No IPA found in '" IPA_MODULE_DIR "'";

I like the warning messages better now. A message will be printed if
LIBCAMERA_IPA_MODULE_PATH is set and no IPA module is found there, which
is a sign of a potential issue, and then another error will be printed
if no IPA modules are found at all, mentioning IPA_MODULE_DIR.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  }
>  
>  IPAManager::~IPAManager()

-- 
Regards,

Laurent Pinchart