[libcamera-devel] [PATCH v2 08/20] libcamera: ipu3: Validate the stream combination

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Jul 10, 2020 at 09:09:34AM +0200, Jacopo Mondi wrote:
> On Thu, Jul 09, 2020 at 03:19:45PM +0200, Niklas Söderlund wrote:
> > On 2020-07-09 10:41:16 +0200, Jacopo Mondi wrote:
> > > The IPU3 pipeline handler supports 2 processed RGB/YUV streams
> > > and one RAW stream. Validate that the requested stream combination is
> > > supported in the pipeline handler validate() implementation and return
> > > an error in case it's not.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++++++++++----
> > >  1 file changed, 22 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 05e10ebb1a7d..9128e42d42ed 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -240,19 +240,37 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> > >  	}
> > >
> > >  	/*
> > > -	 * Select the sensor format by collecting the maximum width and height
> > > -	 * and picking the closest larger match, as the IPU3 can downscale
> > > -	 * only. If no resolution is requested for any stream, or if no sensor
> > > -	 * resolution is large enough, pick the largest one.
> > > +	 * Validate the requested stream configuration and select the sensor
> > > +	 * format by collecting the maximum width and height and picking the
> > > +	 * closest larger match, as the IPU3 can downscale only. If no
> > > +	 * resolution is requested for any stream, or if no sensor resolution is
> > > +	 * large enough, pick the largest one.
> > >  	 */
> > > +	unsigned int rawCount = 0;
> > > +	unsigned int outCount = 0;
> > >  	Size size;
> > >
> > >  	for (const StreamConfiguration &cfg : config_) {
> > > +		const PixelFormatInfo &info =
> > > +			PixelFormatInfo::info(cfg.pixelFormat);
> > > +
> > > +		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > > +			rawCount++;
> > > +		else
> > > +			outCount++;
> > > +
> > >  		if (cfg.size.width > size.width)
> > >  			size.width = cfg.size.width;
> > >  		if (cfg.size.height > size.height)
> > >  			size.height = cfg.size.height;
> > >  	}
> > > +	if (rawCount > 1 || outCount > 2) {
> > > +		LOG(IPU3, Error)
> > > +			<< "Camera configuration not supported: "
> > > +			<< "the platform supports up to one raw stream and "
> > > +			<< "two processed ones.";
> >
> > nit: I would not split the string literal here as it makes it harder to
> > grep for.
> 
> Yeah, we usually don't do that.
> 
> I'll find a shorter message

I would also make it a Debug message. It can occur during normal
operation.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
> > > +		return Invalid;
> > > +	}
> > >
> > >  	/* Generate raw configuration from CIO2. */
> > >  	cio2Configuration_ = data_->cio2_.generateConfiguration(size);

-- 
Regards,

Laurent Pinchart