[libcamera-devel,3/5] ipa: ipa_manager: Print the loaded IPA modules path

2079 diff mbox series

Add debug message to report which IPA modules have been loaded and in
which order.

The loading order is particularly relevant for the test VIMC IPA, as the
same IPA is compiled with an open source license tag and a proprietary
one and they both match() against the VIMC pipeline handler. Being
informed about their loading order is helpful to understand which one of
the two is actually in use.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/ipa_manager.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Hi Jacopo,

Thank you for the patch.

On Thu, Oct 03, 2019 at 05:20:35PM +0200, Jacopo Mondi wrote:
> Add debug message to report which IPA modules have been loaded and in
> which order.
> 
> The loading order is particularly relevant for the test VIMC IPA, as the
> same IPA is compiled with an open source license tag and a proprietary
> one and they both match() against the VIMC pipeline handler. Being
> informed about their loading order is helpful to understand which one of
> the two is actually in use.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/ipa_manager.cpp | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index 708233e8a9c7..e990bdcdf84b 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -120,13 +120,16 @@ int IPAManager::addDir(const char *libDir)
>  		if (strcmp(&ent->d_name[offset], ".so"))
>  			continue;
>  
> -		IPAModule *ipaModule = new IPAModule(std::string(libDir) +
> -						     "/" + ent->d_name);
> +		std::string modulePath = std::string(libDir) + "/" +
> +					 ent->d_name;
> +		IPAModule *ipaModule = new IPAModule(modulePath);
>  		if (!ipaModule->isValid()) {
>  			delete ipaModule;
>  			continue;
>  		}
>  
> +		LOG(IPAManager, Debug) << "Loaded IPA module: " << modulePath;

I'd write

		LOG(IPAManager, Debug)
			<< "Loaded IPA module '" << modulePath << "'";

The rationale is that strings that could contain spaces or can be empty
(the former case only here) should be quoted. No need for a colon
either, the module path name is the continuation of the sentence (you
wouldn't write "I'll have dinner in a: restaurant" :-)).

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

> +
>  		modules_.push_back(ipaModule);
>  		count++;
>  	}

Hi Jacopo,

Thanks for your patch.

On 2019-10-03 17:20:35 +0200, Jacopo Mondi wrote:
> Add debug message to report which IPA modules have been loaded and in
> which order.
> 
> The loading order is particularly relevant for the test VIMC IPA, as the
> same IPA is compiled with an open source license tag and a proprietary
> one and they both match() against the VIMC pipeline handler. Being
> informed about their loading order is helpful to understand which one of
> the two is actually in use.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

With Laurent's comment addressed,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/ipa_manager.cpp | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index 708233e8a9c7..e990bdcdf84b 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -120,13 +120,16 @@ int IPAManager::addDir(const char *libDir)
>  		if (strcmp(&ent->d_name[offset], ".so"))
>  			continue;
>  
> -		IPAModule *ipaModule = new IPAModule(std::string(libDir) +
> -						     "/" + ent->d_name);
> +		std::string modulePath = std::string(libDir) + "/" +
> +					 ent->d_name;
> +		IPAModule *ipaModule = new IPAModule(modulePath);
>  		if (!ipaModule->isValid()) {
>  			delete ipaModule;
>  			continue;
>  		}
>  
> +		LOG(IPAManager, Debug) << "Loaded IPA module: " << modulePath;
> +
>  		modules_.push_back(ipaModule);
>  		count++;
>  	}
> -- 
> 2.23.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel