| Message ID | 20200709084128.5316-4-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your patch. On 2020-07-09 10:41:11 +0200, Jacopo Mondi wrote: > Inspect the return status of validate() in the IPU3 pipeline handler > generateConfigurtion() implementation. If the generated configuration is > not valid, return a an empty configuration to the application. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 85d21b4db046..e62a5d5b3517 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -357,7 +357,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, > config->addConfiguration(cfg); > } > > - config->validate(); > + if (CameraConfiguration::Invalid == config->validate()) nit: This style where the value comes first in an if-statement does not seem to match the rest of the file. Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > + return {}; > > return config; > } > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On Thu, Jul 09, 2020 at 03:02:50PM +0200, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your patch. > > On 2020-07-09 10:41:11 +0200, Jacopo Mondi wrote: > > Inspect the return status of validate() in the IPU3 pipeline handler > > generateConfigurtion() implementation. If the generated configuration is > > not valid, return a an empty configuration to the application. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 85d21b4db046..e62a5d5b3517 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -357,7 +357,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, > > config->addConfiguration(cfg); > > } > > > > - config->validate(); > > + if (CameraConfiguration::Invalid == config->validate()) > > nit: This style where the value comes first in an if-statement does not > seem to match the rest of the file. Not sure why, but this looked nicer to me, but I can change it back. Thanks j > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > + return {}; > > > > return config; > > } > > -- > > 2.27.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards, > Niklas Söderlund
Hi Jacopo, Thank you for the patch. On Fri, Jul 10, 2020 at 08:59:45AM +0200, Jacopo Mondi wrote: > On Thu, Jul 09, 2020 at 03:02:50PM +0200, Niklas Söderlund wrote: > > On 2020-07-09 10:41:11 +0200, Jacopo Mondi wrote: > > > Inspect the return status of validate() in the IPU3 pipeline handler > > > generateConfigurtion() implementation. If the generated configuration is s/generateConfigurtion/generateConfiguration/ > > > not valid, return a an empty configuration to the application. s/a an/an/ > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > index 85d21b4db046..e62a5d5b3517 100644 > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > @@ -357,7 +357,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, > > > config->addConfiguration(cfg); > > > } > > > > > > - config->validate(); > > > + if (CameraConfiguration::Invalid == config->validate()) > > > > nit: This style where the value comes first in an if-statement does not > > seem to match the rest of the file. > > Not sure why, but this looked nicer to me, but I can change it back. I agree with Niklas, we usually do it the other way around. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > > + return {}; > > > > > > return config; > > > }
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 85d21b4db046..e62a5d5b3517 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -357,7 +357,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, config->addConfiguration(cfg); } - config->validate(); + if (CameraConfiguration::Invalid == config->validate()) + return {}; return config; }
Inspect the return status of validate() in the IPU3 pipeline handler generateConfigurtion() implementation. If the generated configuration is not valid, return a an empty configuration to the application. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)