| Message ID | 20250715023013.16435-1-uajain@igalia.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Umang Jain <uajain@igalia.com> writes: > PipelineHandler::generateConfiguration() documentation states the > following: > > * \return A valid CameraConfiguration if the requested roles can be > * satisfied, or a null pointer otherwise. > */ > > Hence, always check the return status of config->validate(), during > generateConfiguration() call in each pipeline handler. > > Signed-off-by: Umang Jain <uajain@igalia.com> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 3 ++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- > src/libcamera/pipeline/simple/simple.cpp | 3 ++- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ++- > src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- > 5 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index f4014b95..c96fefa4 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -798,7 +798,8 @@ PipelineHandlerISI::generateConfiguration(Camera *camera, > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 675f0a74..fca1c74e 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -787,7 +787,8 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index efb07051..f994106e 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1322,7 +1322,8 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 4b5816df..9ed85f3b 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -250,7 +250,8 @@ PipelineHandlerUVC::generateConfiguration(Camera *camera, > > config->addConfiguration(cfg); > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index 07273bd2..56719fa3 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -245,7 +245,8 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera, > > config->addConfiguration(cfg); > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > }
Hi 2025. 07. 15. 4:30 keltezéssel, Umang Jain írta: > PipelineHandler::generateConfiguration() documentation states the > following: > > * \return A valid CameraConfiguration if the requested roles can be > * satisfied, or a null pointer otherwise. > */ > > Hence, always check the return status of config->validate(), during > generateConfiguration() call in each pipeline handler. This is already part of the ipu3 and mali-c55 pipeline handlers. So I guess consistency is good; any reason is it not part of `Camera::generateConfiguration()` ? Regards, Barnabás Pőcze > > Signed-off-by: Umang Jain <uajain@igalia.com> > --- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 3 ++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- > src/libcamera/pipeline/simple/simple.cpp | 3 ++- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ++- > src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- > 5 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index f4014b95..c96fefa4 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -798,7 +798,8 @@ PipelineHandlerISI::generateConfiguration(Camera *camera, > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 675f0a74..fca1c74e 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -787,7 +787,8 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index efb07051..f994106e 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1322,7 +1322,8 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 4b5816df..9ed85f3b 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -250,7 +250,8 @@ PipelineHandlerUVC::generateConfiguration(Camera *camera, > > config->addConfiguration(cfg); > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index 07273bd2..56719fa3 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -245,7 +245,8 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera, > > config->addConfiguration(cfg); > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > }
Hi Umang, Thank you for the patch. On Tue, Jul 15, 2025 at 08:00:13AM +0530, Umang Jain wrote: > PipelineHandler::generateConfiguration() documentation states the > following: > > * \return A valid CameraConfiguration if the requested roles can be > * satisfied, or a null pointer otherwise. > */ > > Hence, always check the return status of config->validate(), during > generateConfiguration() call in each pipeline handler. > > Signed-off-by: Umang Jain <uajain@igalia.com> > --- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 3 ++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- > src/libcamera/pipeline/simple/simple.cpp | 3 ++- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ++- > src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- > 5 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index f4014b95..c96fefa4 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -798,7 +798,8 @@ PipelineHandlerISI::generateConfiguration(Camera *camera, > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; Returning nullptr from generateConfiguration() is meant to cover cases where the requested roles really can't be met. For instance, this can be the case when requesting more streams than the hardware supports, or when one of the stream roles has an unsupported value. The call to validate() in generateConfiguration() was initially added in a pipeline handler that filled part of the configuration in generateConfiguration() and relied on validate() to fill the rest, instead of duplicating the same logic in both functions. It then got copied blindly to other pipeline handlers. The validate() call is not supposed to return Invalid here, that would be a bug in the pipeline handler. Blindly returning nullptr, and worse, returning it without any indication to the user as to what went wrong, doesn't seem desirable to me. > > return config; > } > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 675f0a74..fca1c74e 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -787,7 +787,8 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index efb07051..f994106e 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1322,7 +1322,8 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo > config->addConfiguration(cfg); > } > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 4b5816df..9ed85f3b 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -250,7 +250,8 @@ PipelineHandlerUVC::generateConfiguration(Camera *camera, > > config->addConfiguration(cfg); > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > } > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index 07273bd2..56719fa3 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -245,7 +245,8 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera, > > config->addConfiguration(cfg); > > - config->validate(); > + if (config->validate() == CameraConfiguration::Invalid) > + return nullptr; > > return config; > }
Hi Laurent, Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > Hi Umang, > > Thank you for the patch. > > On Tue, Jul 15, 2025 at 08:00:13AM +0530, Umang Jain wrote: >> PipelineHandler::generateConfiguration() documentation states the >> following: >> >> * \return A valid CameraConfiguration if the requested roles can be >> * satisfied, or a null pointer otherwise. >> */ >> >> Hence, always check the return status of config->validate(), during >> generateConfiguration() call in each pipeline handler. >> >> Signed-off-by: Umang Jain <uajain@igalia.com> >> --- >> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 3 ++- >> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- >> src/libcamera/pipeline/simple/simple.cpp | 3 ++- >> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ++- >> src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- >> 5 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >> index f4014b95..c96fefa4 100644 >> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >> @@ -798,7 +798,8 @@ PipelineHandlerISI::generateConfiguration(Camera *camera, >> config->addConfiguration(cfg); >> } >> >> - config->validate(); >> + if (config->validate() == CameraConfiguration::Invalid) >> + return nullptr; > > Returning nullptr from generateConfiguration() is meant to cover cases > where the requested roles really can't be met. For instance, this can be > the case when requesting more streams than the hardware supports, or > when one of the stream roles has an unsupported value. > > The call to validate() in generateConfiguration() was initially added in > a pipeline handler that filled part of the configuration in > generateConfiguration() and relied on validate() to fill the rest, > instead of duplicating the same logic in both functions. It then got > copied blindly to other pipeline handlers. The validate() call is not > supposed to return Invalid here, that would be a bug in the pipeline > handler. Do we want to check that then in a different way? At least by replacing the check above with an assertion? > Blindly returning nullptr, and worse, returning it without any > indication to the user as to what went wrong, doesn't seem desirable > to me. > >> >> return config; >> } >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> index 675f0a74..fca1c74e 100644 >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> @@ -787,7 +787,8 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, >> config->addConfiguration(cfg); >> } >> >> - config->validate(); >> + if (config->validate() == CameraConfiguration::Invalid) >> + return nullptr; >> >> return config; >> } >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> index efb07051..f994106e 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -1322,7 +1322,8 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo >> config->addConfiguration(cfg); >> } >> >> - config->validate(); >> + if (config->validate() == CameraConfiguration::Invalid) >> + return nullptr; >> >> return config; >> } >> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> index 4b5816df..9ed85f3b 100644 >> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> @@ -250,7 +250,8 @@ PipelineHandlerUVC::generateConfiguration(Camera *camera, >> >> config->addConfiguration(cfg); >> >> - config->validate(); >> + if (config->validate() == CameraConfiguration::Invalid) >> + return nullptr; >> >> return config; >> } >> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp >> index 07273bd2..56719fa3 100644 >> --- a/src/libcamera/pipeline/vimc/vimc.cpp >> +++ b/src/libcamera/pipeline/vimc/vimc.cpp >> @@ -245,7 +245,8 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera, >> >> config->addConfiguration(cfg); >> >> - config->validate(); >> + if (config->validate() == CameraConfiguration::Invalid) >> + return nullptr; >> >> return config; >> }
On Tue, Jul 15, 2025 at 12:10:16PM +0200, Milan Zamazal wrote: > Hi Laurent, > > Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > > > Hi Umang, > > > > Thank you for the patch. > > > > On Tue, Jul 15, 2025 at 08:00:13AM +0530, Umang Jain wrote: > >> PipelineHandler::generateConfiguration() documentation states the > >> following: > >> > >> * \return A valid CameraConfiguration if the requested roles can be > >> * satisfied, or a null pointer otherwise. > >> */ > >> > >> Hence, always check the return status of config->validate(), during > >> generateConfiguration() call in each pipeline handler. > >> > >> Signed-off-by: Umang Jain <uajain@igalia.com> > >> --- > >> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 3 ++- > >> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- > >> src/libcamera/pipeline/simple/simple.cpp | 3 ++- > >> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ++- > >> src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- > >> 5 files changed, 10 insertions(+), 5 deletions(-) > >> > >> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > >> index f4014b95..c96fefa4 100644 > >> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > >> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > >> @@ -798,7 +798,8 @@ PipelineHandlerISI::generateConfiguration(Camera *camera, > >> config->addConfiguration(cfg); > >> } > >> > >> - config->validate(); > >> + if (config->validate() == CameraConfiguration::Invalid) > >> + return nullptr; > > > > Returning nullptr from generateConfiguration() is meant to cover cases > > where the requested roles really can't be met. For instance, this can be > > the case when requesting more streams than the hardware supports, or > > when one of the stream roles has an unsupported value. > > > > The call to validate() in generateConfiguration() was initially added in > > a pipeline handler that filled part of the configuration in > > generateConfiguration() and relied on validate() to fill the rest, > > instead of duplicating the same logic in both functions. It then got > > copied blindly to other pipeline handlers. The validate() call is not > > supposed to return Invalid here, that would be a bug in the pipeline > > handler. > > Do we want to check that then in a different way? At least by replacing > the check above with an assertion? I second this, along with a documentation extension of stating the logic of using validate() in generateConfiguration(). > > > Blindly returning nullptr, and worse, returning it without any > > indication to the user as to what went wrong, doesn't seem desirable > > to me. > > > >> > >> return config; > >> } > >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > >> index 675f0a74..fca1c74e 100644 > >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > >> @@ -787,7 +787,8 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, > >> config->addConfiguration(cfg); > >> } > >> > >> - config->validate(); > >> + if (config->validate() == CameraConfiguration::Invalid) > >> + return nullptr; > >> > >> return config; > >> } > >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > >> index efb07051..f994106e 100644 > >> --- a/src/libcamera/pipeline/simple/simple.cpp > >> +++ b/src/libcamera/pipeline/simple/simple.cpp > >> @@ -1322,7 +1322,8 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo > >> config->addConfiguration(cfg); > >> } > >> > >> - config->validate(); > >> + if (config->validate() == CameraConfiguration::Invalid) > >> + return nullptr; > >> > >> return config; > >> } > >> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > >> index 4b5816df..9ed85f3b 100644 > >> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > >> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > >> @@ -250,7 +250,8 @@ PipelineHandlerUVC::generateConfiguration(Camera *camera, > >> > >> config->addConfiguration(cfg); > >> > >> - config->validate(); > >> + if (config->validate() == CameraConfiguration::Invalid) > >> + return nullptr; > >> > >> return config; > >> } > >> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > >> index 07273bd2..56719fa3 100644 > >> --- a/src/libcamera/pipeline/vimc/vimc.cpp > >> +++ b/src/libcamera/pipeline/vimc/vimc.cpp > >> @@ -245,7 +245,8 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera, > >> > >> config->addConfiguration(cfg); > >> > >> - config->validate(); > >> + if (config->validate() == CameraConfiguration::Invalid) > >> + return nullptr; > >> > >> return config; > >> } >
diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index f4014b95..c96fefa4 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -798,7 +798,8 @@ PipelineHandlerISI::generateConfiguration(Camera *camera, config->addConfiguration(cfg); } - config->validate(); + if (config->validate() == CameraConfiguration::Invalid) + return nullptr; return config; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 675f0a74..fca1c74e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -787,7 +787,8 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, config->addConfiguration(cfg); } - config->validate(); + if (config->validate() == CameraConfiguration::Invalid) + return nullptr; return config; } diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index efb07051..f994106e 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1322,7 +1322,8 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo config->addConfiguration(cfg); } - config->validate(); + if (config->validate() == CameraConfiguration::Invalid) + return nullptr; return config; } diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 4b5816df..9ed85f3b 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -250,7 +250,8 @@ PipelineHandlerUVC::generateConfiguration(Camera *camera, config->addConfiguration(cfg); - config->validate(); + if (config->validate() == CameraConfiguration::Invalid) + return nullptr; return config; } diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 07273bd2..56719fa3 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -245,7 +245,8 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera, config->addConfiguration(cfg); - config->validate(); + if (config->validate() == CameraConfiguration::Invalid) + return nullptr; return config; }
PipelineHandler::generateConfiguration() documentation states the following: * \return A valid CameraConfiguration if the requested roles can be * satisfied, or a null pointer otherwise. */ Hence, always check the return status of config->validate(), during generateConfiguration() call in each pipeline handler. Signed-off-by: Umang Jain <uajain@igalia.com> --- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 3 ++- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- src/libcamera/pipeline/simple/simple.cpp | 3 ++- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ++- src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- 5 files changed, 10 insertions(+), 5 deletions(-)