[PATCH v3 2/2] libcamera: camera_manager: Add environment variable to order pipelines match
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Apr 26 23:18:21 CEST 2024
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 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 ?
>
> 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 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
> >>
>
More information about the libcamera-devel
mailing list