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

[Milan Zamazal]

Kieran Bingham <kieran.bingham at ideasonboard.com> writes:

> 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.

As for the configuration file, I just posted "Add global configuration file"
patch series.  I haven't tried to include these patches there but feel free to
comment on / use / ignore the configuration file patches.

Regards,
Milan

> 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
>>