[v3,2/2] libcamera: camera_manager: Add environment variable to order pipelines match
diff mbox series

Message ID 20240327172731.434048-3-julien.vuillaumier@nxp.com
State New
Headers show
Series
  • Add environment variable to order pipelines match
Related show

Commit Message

Julien Vuillaumier March 27, 2024, 5:27 p.m. UTC
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"

Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@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(-)

Comments

Kieran Bingham April 23, 2024, 9:09 a.m. UTC | #1
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@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@nxp.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@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
>
Milan Zamazal April 23, 2024, 10:34 a.m. UTC | #2
Kieran Bingham <kieran.bingham@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@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@nxp.com>
>> Reviewed-by: Jacopo Mondi <jacopo.mondi@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
>>
Julien Vuillaumier April 26, 2024, 3:13 p.m. UTC | #3
Hi Kieran,

On 23/04/2024 11:09, Kieran Bingham wrote:

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

Definitely, adding the option to configure the pipelines list using 
Milan's global configuration file would be useful.

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

If using the short names is the preferred way to go, then I suppose it 
is better adding that change first. It should work the same with the 
other patches.
So, if nobody objects, I will add to those patches the change in the 
pipeline registration macro, in order to assign to each pipeline the 
short name used in meson files.

Thanks,
Julien

> 
> --
> Kieran
> 
> 
>>
>> Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
>> Reviewed-by: Jacopo Mondi <jacopo.mondi@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
>>
Kieran Bingham April 26, 2024, 9:18 p.m. UTC | #4
Quoting Julien Vuillaumier (2024-04-26 16:13:43)
> Hi Kieran,
> 
> On 23/04/2024 11:09, Kieran Bingham wrote:
> 
> > 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.
> 
> Definitely, adding the option to configure the pipelines list using 
> Milan's global configuration file would be useful.
> 
> > 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@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 ?
> 
> If using the short names is the preferred way to go, then I suppose it 
> is better adding that change first. It should work the same with the 
> other patches.
> So, if nobody objects, I will add to those patches the change in the 
> pipeline registration macro, in order to assign to each pipeline the 
> short name used in meson files.

I think the short names are more likely to be the better reference for
users indeed. Maybe even something to use in the logging mechanism.

If you're happy to handle the update to the Pipeline Registration Macro,
that makes merging the topic easier I believe, so yes please!

--
Kieran


> 
> Thanks,
> Julien
> 
> > 
> > --
> > Kieran
> > 
> > 
> >>
> >> Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
> >> Reviewed-by: Jacopo Mondi <jacopo.mondi@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
> >>
>

Patch
diff mbox series

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";
 	}
 }