| Message ID | 20200709084128.5316-9-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your work. 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@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. Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > + return Invalid; > + } > > /* Generate raw configuration from CIO2. */ > cio2Configuration_ = data_->cio2_.generateConfiguration(size); > -- > 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:19:45PM +0200, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your work. > > 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@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 > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > + return Invalid; > > + } > > > > /* Generate raw configuration from CIO2. */ > > cio2Configuration_ = data_->cio2_.generateConfiguration(size); > > -- > > 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 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@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@ideasonboard.com> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > > + return Invalid; > > > + } > > > > > > /* Generate raw configuration from CIO2. */ > > > cio2Configuration_ = data_->cio2_.generateConfiguration(size);
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."; + return Invalid; + } /* Generate raw configuration from CIO2. */ cio2Configuration_ = data_->cio2_.generateConfiguration(size);
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@jmondi.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)