| Message ID | 20240304181816.2361334-2-julien.vuillaumier@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Julien Vuillaumier (2024-03-04 18:18:16) > 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" > > Signed-off-by: Julien Vuillaumier <julien.vuillaumier@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. > + > + 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) { Does this need to be a lambda? is there a specific benefit, or can it be a function? I find lambda's hard to parse :-( but perhaps that's just me, so lets see what others say. > 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 s/devices/device/ > + * pipeline handler registered. > + */ > + const char *pipesLists = > + utils::secure_getenv("LIBCAMERA_PIPELINES_MATCH_LIST"); > + if (pipesLists) { > + for (const auto &pipeName : utils::split(pipesLists, ",")) { > + if (pipeName.empty()) > + continue; > + > + auto nameMatch = [pipeName](const PipelineHandlerFactoryBase *f) { > + return (f->name() == pipeName); > + }; > + > + auto iter = std::find_if(factories.begin(), > + factories.end(), > + nameMatch); If we do this - I'd probably add support to the PipelineHandlerFactory to support getting by name as a preceeding patch. > + > + 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() << "'"; > + /* > + * Try each pipeline handler until it exhausts > + * all pipelines it can provide. > + */ > + pipeMatch(factory); > } > } > > -- > 2.34.1 >
Hi 2024. március 4., hétfő 19:18 keltezéssel, Julien Vuillaumier <julien.vuillaumier@nxp.com> írta: > 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" > > Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com> > --- > [...] > + > + /* > + * 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. > + */ > + const char *pipesLists = > + utils::secure_getenv("LIBCAMERA_PIPELINES_MATCH_LIST"); > + if (pipesLists) { > + for (const auto &pipeName : utils::split(pipesLists, ",")) { > + if (pipeName.empty()) > + continue; > + > + auto nameMatch = [pipeName](const PipelineHandlerFactoryBase *f) { `[&pipeName]` to avoid the copy. > + return (f->name() == pipeName); > + }; > + > + auto iter = std::find_if(factories.begin(), > + factories.end(), > + nameMatch); > + > + 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() << "'"; > + /* > + * Try each pipeline handler until it exhausts > + * all pipelines it can provide. > + */ > + pipeMatch(factory); > } > } > > -- > 2.34.1 > Regards, Barnabás Pőcze
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@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 >
Hi Kieran, On 05/03/2024 07:54, Kieran Bingham wrote: > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button > > > Quoting Julien Vuillaumier (2024-03-04 18:18:16) >> 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" >> >> Signed-off-by: Julien Vuillaumier <julien.vuillaumier@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. >> + >> + 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) { > > Does this need to be a lambda? is there a specific benefit, or can it be > a function? I find lambda's hard to parse :-( but perhaps that's just > me, so lets see what others say. This lambda can be moved to a function, but this function would probably never be used from anywhere else - for that reason it was made local as a lambda. But if consensus is to limit usage of lambda, a function can be used instead. > >> 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 > > s/devices/device/ > I will update in v2 - thanks. >> + * pipeline handler registered. >> + */ >> + const char *pipesLists = >> + utils::secure_getenv("LIBCAMERA_PIPELINES_MATCH_LIST"); >> + if (pipesLists) { >> + for (const auto &pipeName : utils::split(pipesLists, ",")) { >> + if (pipeName.empty()) >> + continue; >> + >> + auto nameMatch = [pipeName](const PipelineHandlerFactoryBase *f) { >> + return (f->name() == pipeName); >> + }; >> + >> + auto iter = std::find_if(factories.begin(), >> + factories.end(), >> + nameMatch); > > If we do this - I'd probably add support to the PipelineHandlerFactory > to support getting by name as a preceeding patch. > Sure, I can do - will update in v2. >> + >> + 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() << "'"; >> + /* >> + * Try each pipeline handler until it exhausts >> + * all pipelines it can provide. >> + */ >> + pipeMatch(factory); >> } >> } >> >> -- >> 2.34.1 >>
Hi Barnabás, On 05/03/2024 17:42, Barnabás Pőcze wrote: > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button > > > Hi > > > 2024. március 4., hétfő 19:18 keltezéssel, Julien Vuillaumier <julien.vuillaumier@nxp.com> írta: > >> 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" >> >> Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com> >> --- >> [...] >> + >> + /* >> + * 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. >> + */ >> + const char *pipesLists = >> + utils::secure_getenv("LIBCAMERA_PIPELINES_MATCH_LIST"); >> + if (pipesLists) { >> + for (const auto &pipeName : utils::split(pipesLists, ",")) { >> + if (pipeName.empty()) >> + continue; >> + >> + auto nameMatch = [pipeName](const PipelineHandlerFactoryBase *f) { > > `[&pipeName]` to avoid the copy. I will update in v2 - thanks > >> + return (f->name() == pipeName); >> + }; >> + >> + auto iter = std::find_if(factories.begin(), >> + factories.end(), >> + nameMatch); >> + >> + 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() << "'"; >> + /* >> + * Try each pipeline handler until it exhausts >> + * all pipelines it can provide. >> + */ >> + pipeMatch(factory); >> } >> } >> >> -- >> 2.34.1 >> > > > Regards, > Barnabás Pőcze
Hi Jacopo, On 05/03/2024 18:38, Jacopo Mondi wrote: > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button > > > 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 :) > No worries, env variable name can be changed if needed :) >> >> Signed-off-by: Julien Vuillaumier <julien.vuillaumier@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' > > ? Sure, I will update for the v2 with: 'Define an ordered list of pipeline names to be used to match 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' > I will update the comment in v2 >> + */ >> + const char *pipesLists = > > nit: pipesList > I will update the variable name in v2 >> + 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 > Agreed, not sure empty() test is strictly necessary. Intent was presumably to catch early (odd) definitions like: var='pipe1,,pipe2' But things should work without. I will double check and remove in v2 if not needed. >> + >> + auto nameMatch = [pipeName](const PipelineHandlerFactoryBase *f) { >> + return (f->name() == pipeName); > > no need for () > I will remove the extra () in v2 >> + }; >> + >> + 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 > That was an attempt to adhere to coding style and stay within the 80 columns - but the lambda declaration failed to do so anyway... Thank you for the suggestion - I will see how that fits after moving this block to a getting by name method of the PipelineHandlerFactory, as Kieran suggested. >> + >> + 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 ? > The order of the pipelines listed in the env variable is relevant as it defines their respective priorities for the match. For that reason, if a list exists, the preceding 'for' block iterates in order through every pipeline name and attempts match with relevant pipeline. That second 'for' block is the default case when there is no list defined. It corresponds to the current implementation where match is attempted with every pipeline registered in PipelineHandlerFactoryBase, but without specific order. >> + /* >> + * Try each pipeline handler until it exhausts >> + * all pipelines it can provide. >> + */ >> + pipeMatch(factory); >> } >> } >> >> -- >> 2.34.1 >>
Quoting Julien Vuillaumier (2024-03-06 17:57:58) > Hi Kieran, > > On 05/03/2024 07:54, Kieran Bingham wrote: > > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button > > > > > > Quoting Julien Vuillaumier (2024-03-04 18:18:16) > >> 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" > >> > >> Signed-off-by: Julien Vuillaumier <julien.vuillaumier@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. > >> + > >> + 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) { > > > > Does this need to be a lambda? is there a specific benefit, or can it be > > a function? I find lambda's hard to parse :-( but perhaps that's just > > me, so lets see what others say. > > This lambda can be moved to a function, but this function would probably > never be used from anywhere else - for that reason it was made local as > a lambda. But if consensus is to limit usage of lambda, a function can > be used instead. Indeed, that makes sense. We've often used private class functions where needed though. I don't think what you've done is wrong, just not what I'm used to ;-) so I don't specifically mind if it stays, but just to ease readibility a block comment above the lambda might help. -- Kieran > > > > >> 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 > > > > s/devices/device/ > > > > I will update in v2 - thanks. > > >> + * pipeline handler registered. > >> + */ > >> + const char *pipesLists = > >> + utils::secure_getenv("LIBCAMERA_PIPELINES_MATCH_LIST"); > >> + if (pipesLists) { > >> + for (const auto &pipeName : utils::split(pipesLists, ",")) { > >> + if (pipeName.empty()) > >> + continue; > >> + > >> + auto nameMatch = [pipeName](const PipelineHandlerFactoryBase *f) { > >> + return (f->name() == pipeName); > >> + }; > >> + > >> + auto iter = std::find_if(factories.begin(), > >> + factories.end(), > >> + nameMatch); > > > > If we do this - I'd probably add support to the PipelineHandlerFactory > > to support getting by name as a preceeding patch. > > > > Sure, I can do - will update in v2. > > >> + > >> + 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() << "'"; > >> + /* > >> + * Try each pipeline handler until it exhausts > >> + * all pipelines it can provide. > >> + */ > >> + pipeMatch(factory); > >> } > >> } > >> > >> -- > >> 2.34.1 > >> >
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. + + 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. + */ + const char *pipesLists = + utils::secure_getenv("LIBCAMERA_PIPELINES_MATCH_LIST"); + if (pipesLists) { + for (const auto &pipeName : utils::split(pipesLists, ",")) { + if (pipeName.empty()) + continue; + + auto nameMatch = [pipeName](const PipelineHandlerFactoryBase *f) { + return (f->name() == pipeName); + }; + + auto iter = std::find_if(factories.begin(), + factories.end(), + nameMatch); + + 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() << "'"; + /* + * Try each pipeline handler until it exhausts + * all pipelines it can provide. + */ + pipeMatch(factory); } }
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" Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com> --- Documentation/environment_variables.rst | 5 +++ src/libcamera/camera_manager.cpp | 51 +++++++++++++++++++++---- 2 files changed, 48 insertions(+), 8 deletions(-)