[PATCH v1 1/1] libcamera: camera_manager: Add environment variable to order pipelines match

[Jacopo Mondi]

Hi Julien

On Mon, Mar 04, 2024 at 07:18:16PM +0100, Julien Vuillaumier wrote:
> To match the enumerated media devices, each pipeline handler registered
> is used in no specific order. It is a limitation when several pipelines
> can match the devices, and user has to select a specific pipeline.
>
> For this purpose, environment variable LIBCAMERA_PIPELINES_MATCH_LIST is
> created that gives the option to define an ordered list of pipelines
> to invoke during the match process.
>
> LIBCAMERA_PIPELINES_MATCH_LIST="<name1>[,<name2>[,<name3>...]]]"
>
> Example:
> LIBCAMERA_PIPELINES_MATCH_LIST="PipelineHandlerRkISP1,SimplePipelineHandler"

I think that's a good idea. I expect some bikeshedding on the env
variable name :)

>
> Signed-off-by: Julien Vuillaumier <julien.vuillaumier at nxp.com>
> ---
>  Documentation/environment_variables.rst |  5 +++
>  src/libcamera/camera_manager.cpp        | 51 +++++++++++++++++++++----
>  2 files changed, 48 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst
> index a9b230bc..f3a0d431 100644
> --- a/Documentation/environment_variables.rst
> +++ b/Documentation/environment_variables.rst
> @@ -37,6 +37,11 @@ LIBCAMERA_IPA_MODULE_PATH
>
>     Example value: ``${HOME}/.libcamera/lib:/opt/libcamera/vendor/lib``
>
> +LIBCAMERA_PIPELINES_MATCH_LIST
> +   Define ordered list of pipelines to be used to match the media devices.

nit:

'an ordered list of pipeline names'
'the media devices in the system'

?

> +
> +   Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``
> +
>  LIBCAMERA_RPI_CONFIG_FILE
>     Define a custom configuration file to use in the Raspberry Pi pipeline handler.
>
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 355f3ada..9568801e 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -109,14 +109,7 @@ void CameraManager::Private::createPipelineHandlers()
>  	const std::vector<PipelineHandlerFactoryBase *> &factories =
>  		PipelineHandlerFactoryBase::factories();
>
> -	for (const PipelineHandlerFactoryBase *factory : factories) {
> -		LOG(Camera, Debug)
> -			<< "Found registered pipeline handler '"
> -			<< factory->name() << "'";
> -		/*
> -		 * Try each pipeline handler until it exhaust
> -		 * all pipelines it can provide.
> -		 */
> +	auto pipeMatch = [&](const PipelineHandlerFactoryBase *factory) {
>  		while (1) {
>  			std::shared_ptr<PipelineHandler> pipe = factory->create(o);
>  			if (!pipe->match(enumerator_.get()))
> @@ -126,6 +119,48 @@ void CameraManager::Private::createPipelineHandlers()
>  				<< "Pipeline handler \"" << factory->name()
>  				<< "\" matched";
>  		}
> +	};
> +
> +	/*
> +	 * When a list of preferred pipelines is defined, iterate through the
> +	 * ordered list to match the devices enumerated.
> +	 * Otherwise, devices matching is done in no specific order with each
> +	 * pipeline handler registered.

nit: 'registered pipeline handler'

> +	 */
> +	const char *pipesLists =

nit: pipesList

> +		utils::secure_getenv("LIBCAMERA_PIPELINES_MATCH_LIST");
> +	if (pipesLists) {
> +		for (const auto &pipeName : utils::split(pipesLists, ",")) {
> +			if (pipeName.empty())
> +				continue;

Not sure if this can be empty, but better safe than sorry

> +
> +			auto nameMatch = [pipeName](const PipelineHandlerFactoryBase *f) {
> +				return (f->name() == pipeName);

no need for ()

> +			};
> +
> +			auto iter = std::find_if(factories.begin(),
> +						 factories.end(),
> +						 nameMatch);

You could also inline the above here

			auto iter = std::find_if(factories.begin(),
						 factories.end(),
						 [&pipeName](const PipelineHandlerFactoryBase *f)
                                                 {
                                                        return f->name() == pipeName;
                                                 });

Up to you

> +
> +			if (iter != factories.end()) {
> +				LOG(Camera, Debug)
> +					<< "Found pipeline handler from list '"
> +					<< (*iter)->name() << "'";
> +				pipeMatch(*iter);
> +			}
> +		}
> +		return;
> +	}
> +
> +	for (const PipelineHandlerFactoryBase *factory : factories) {
> +		LOG(Camera, Debug)
> +			<< "Found registered pipeline handler '"
> +			<< factory->name() << "'";

If order was not relevant, you could have simply searched
factory->name() in pipesLists here, but I presume the order in which
pipelines are specified in the env variable and matched is relevant,
right ?

> +		/*
> +		 * Try each pipeline handler until it exhausts
> +		 * all pipelines it can provide.
> +		 */
> +		pipeMatch(factory);
>  	}
>  }
>
> --
> 2.34.1
>