[PATCH v3 2/2] libcamera: camera_manager: Add environment variable to order pipelines match

[Kieran Bingham]

Quoting Julien Vuillaumier (2024-03-27 17:27:31)
> To match the enumerated media devices, each registered pipeline handler
> 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 to give the option to define an ordered list of pipelines to
> match on.
> 
> LIBCAMERA_PIPELINES_MATCH_LIST="<name1>[,<name2>[,<name3>...]]]"
> 
> Example:
> LIBCAMERA_PIPELINES_MATCH_LIST="PipelineHandlerRkISP1,SimplePipelineHandler"

I still think this env variable (and presumably also possible to set in
a configuration file with Milan's configuration file work) makes sense.

I do wish the match list used the short names that are equivalent to how
the meson configuration would name them though. Perhaps as you suggested
by using a parameter to the pipeline handler registration instead so
that a more suitable name is used for the references.

That would still then work with the previous patch I believe.

So ... I think it's worth making the naming easier for users if we set
this ... but I also think this patch is ok.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

So the question is - if we merge this - does it become some sort of
expected ABI and mean we /shouldn't/ then change the name to the more
user-friendly ones? And therefore we should do that first? or just do it
on top ?

--
Kieran


> 
> Signed-off-by: Julien Vuillaumier <julien.vuillaumier at nxp.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
>  Documentation/environment_variables.rst     |  8 ++++
>  include/libcamera/internal/camera_manager.h |  1 +
>  src/libcamera/camera_manager.cpp            | 53 ++++++++++++++++-----
>  3 files changed, 50 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst
> index a9b230bc..c763435c 100644
> --- a/Documentation/environment_variables.rst
> +++ b/Documentation/environment_variables.rst
> @@ -37,6 +37,14 @@ LIBCAMERA_IPA_MODULE_PATH
>  
>     Example value: ``${HOME}/.libcamera/lib:/opt/libcamera/vendor/lib``
>  
> +LIBCAMERA_PIPELINES_MATCH_LIST
> +   Define an ordered list of pipeline names to be used to match the media
> +   devices in the system. The pipeline handler names used to populate the
> +   variable are the ones passed to the REGISTER_PIPELINE_HANDLER() macro in the
> +   source code.
> +
> +   Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``
> +
>  LIBCAMERA_RPI_CONFIG_FILE
>     Define a custom configuration file to use in the Raspberry Pi pipeline handler.
>  
> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h
> index 33ebe069..5694d40d 100644
> --- a/include/libcamera/internal/camera_manager.h
> +++ b/include/libcamera/internal/camera_manager.h
> @@ -44,6 +44,7 @@ protected:
>  private:
>         int init();
>         void createPipelineHandlers();
> +       void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);
>         void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);
>  
>         /*
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 355f3ada..ce6607e1 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -99,16 +99,37 @@ int CameraManager::Private::init()
>  
>  void CameraManager::Private::createPipelineHandlers()
>  {
> -       CameraManager *const o = LIBCAMERA_O_PTR();
> -
>         /*
>          * \todo Try to read handlers and order from configuration
> -        * file and only fallback on all handlers if there is no
> -        * configuration file.
> +        * file and only fallback on environment variable or all handlers, if
> +        * there is no configuration file.
>          */
> +       const char *pipesList =
> +               utils::secure_getenv("LIBCAMERA_PIPELINES_MATCH_LIST");
> +       if (pipesList) {
> +               /*
> +                * When a list of preferred pipelines is defined, iterate
> +                * through the ordered list to match the enumerated devices.
> +                */
> +               for (const auto &pipeName : utils::split(pipesList, ",")) {
> +                       const PipelineHandlerFactoryBase *factory;
> +                       factory = PipelineHandlerFactoryBase::getFactoryByName(pipeName);
> +                       if (!factory)
> +                               continue;
> +
> +                       LOG(Camera, Debug)
> +                               << "Found listed pipeline handler '"
> +                               << pipeName << "'";
> +                       pipelineFactoryMatch(factory);
> +               }
> +
> +               return;
> +       }
> +
>         const std::vector<PipelineHandlerFactoryBase *> &factories =
>                 PipelineHandlerFactoryBase::factories();
>  
> +       /* Match all the registered pipeline handlers. */
>         for (const PipelineHandlerFactoryBase *factory : factories) {
>                 LOG(Camera, Debug)
>                         << "Found registered pipeline handler '"
> @@ -117,15 +138,23 @@ void CameraManager::Private::createPipelineHandlers()
>                  * Try each pipeline handler until it exhaust
>                  * all pipelines it can provide.
>                  */
> -               while (1) {
> -                       std::shared_ptr<PipelineHandler> pipe = factory->create(o);
> -                       if (!pipe->match(enumerator_.get()))
> -                               break;
> +               pipelineFactoryMatch(factory);
> +       }
> +}
>  
> -                       LOG(Camera, Debug)
> -                               << "Pipeline handler \"" << factory->name()
> -                               << "\" matched";
> -               }
> +void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory)
> +{
> +       CameraManager *const o = LIBCAMERA_O_PTR();
> +
> +       /* Provide as many matching pipelines as possible. */
> +       while (1) {
> +               std::shared_ptr<PipelineHandler> pipe = factory->create(o);
> +               if (!pipe->match(enumerator_.get()))
> +                       break;
> +
> +               LOG(Camera, Debug)
> +                       << "Pipeline handler \"" << factory->name()
> +                       << "\" matched";
>         }
>  }
>  
> -- 
> 2.34.1
>