[libcamera-devel] [PATCH v2 03/20] libcamera: ipu3: Make sure the config is valid

[Laurent Pinchart]

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 at 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 at ideasonboard.com>

> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
> > > +		return {};
> > >
> > >  	return config;
> > >  }

-- 
Regards,

Laurent Pinchart