[libcamera-devel] [PATCH 06/15] libcamera: ipu3: Rework stream validation

[Jacopo Mondi]

With the goal of moving stream assignement to configuration from
validate() to configure(), rework the validate() function to not
inspect the stream assigned to a configuration to identify it.

Re-work the validation logic to follow this simpler flow:
- if a stream is a raw stream use the sensor configuration
- if a stream is a processed one, make sure it respects the ImgU
  output size and alignment constraints.

Remove the 'adjustStream()' function as it depends on stream assignment
and was built on the assumption the main output should always work at
the maximum available resolution, and it addressed the case where no
width or height where supplied for a viewfinder stream, which should
only be validated against the ImgU output constraints, not defaulted to a
sane value.

Retain the call to assignStreams() in validate() for the moment in order
not to break capture operations, but while at it clean up the code a bit
by removing a rogue empty line and make a conditional check fit on a
single line.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 85 ++++++++++------------------
 1 file changed, 31 insertions(+), 54 deletions(-)

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index ed2360347fb4..daa6d71dae72 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -68,7 +68,6 @@ public:
 
 private:
 	void assignStreams();
-	void adjustStream(StreamConfiguration &cfg, bool scale);
 
 	/*
 	 * The IPU3CameraData instance is guaranteed to be valid as long as the
@@ -178,52 +177,6 @@ void IPU3CameraConfiguration::assignStreams()
 	}
 }
 
-void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
-{
-	/* The only pixel format the driver supports is NV12. */
-	cfg.pixelFormat = formats::NV12;
-
-	if (scale) {
-		/*
-		 * Provide a suitable default that matches the sensor aspect
-		 * ratio.
-		 */
-		if (!cfg.size.width || !cfg.size.height) {
-			cfg.size.width = 1280;
-			cfg.size.height = 1280 * cio2Configuration_.size.height
-					/ cio2Configuration_.size.width;
-		}
-
-		/*
-		 * \todo: Clamp the size to the hardware bounds when we will
-		 * figure them out.
-		 *
-		 * \todo: Handle the scaler (BDS) restrictions. The BDS can
-		 * only scale with the same factor in both directions, and the
-		 * scaling factor is limited to a multiple of 1/32. At the
-		 * moment the ImgU driver hides these constraints by applying
-		 * additional cropping, this should be fixed on the driver
-		 * side, and cropping should be exposed to us.
-		 */
-	} else {
-		/*
-		 * \todo: Properly support cropping when the ImgU driver
-		 * interface will be cleaned up.
-		 */
-		cfg.size = cio2Configuration_.size;
-	}
-
-	/*
-	 * Clamp the size to match the ImgU alignment constraints. The width
-	 * shall be a multiple of 8 pixels and the height a multiple of 4
-	 * pixels.
-	 */
-	if (cfg.size.width % 8 || cfg.size.height % 4) {
-		cfg.size.width &= ~7;
-		cfg.size.height &= ~3;
-	}
-}
-
 CameraConfiguration::Status IPU3CameraConfiguration::validate()
 {
 	Status status = Valid;
@@ -244,7 +197,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
 	 * resolution is large enough, pick the largest one.
 	 */
 	Size size = {};
-
 	for (const StreamConfiguration &cfg : config_) {
 		if (cfg.size.width > size.width)
 			size.width = cfg.size.width;
@@ -264,20 +216,45 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
 	for (unsigned int i = 0; i < config_.size(); ++i) {
 		StreamConfiguration &cfg = config_[i];
 		const StreamConfiguration oldCfg = cfg;
-		const Stream *stream = streams_[i];
+		const PixelFormatInfo &info =
+			PixelFormatInfo::info(cfg.pixelFormat);
 
-		if (stream == &data_->rawStream_) {
+		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
 			cfg.size = cio2Configuration_.size;
 			cfg.pixelFormat = cio2Configuration_.pixelFormat;
 			cfg.bufferCount = cio2Configuration_.bufferCount;
 		} else {
-			bool scale = stream == &data_->vfStream_;
-			adjustStream(config_[i], scale);
+			/*
+			 * Clamp the size to match the ImgU alignment
+			 * constraints.
+			 */
+			size.width = std::min(cfg.size.width,
+					      IPU3_OUTPUT_MAX_WIDTH);
+			size.height = std::min(cfg.size.height,
+					       IPU3_OUTPUT_MAX_HEIGHT);
+			size.width = std::max(cfg.size.width,
+					      minIPU3OutputSize.width);
+			size.height = std::max(cfg.size.height,
+					       minIPU3OutputSize.height);
+			if (cfg.size.width % 8 || cfg.size.height % 4) {
+				cfg.size.width &= ~7;
+				cfg.size.height &= ~3;
+			}
+			cfg.pixelFormat = formats::NV12;
 			cfg.bufferCount = IPU3_BUFFER_COUNT;
+
+			/*
+			 * \todo: Handle the scaler (BDS) restrictions. The BDS
+			 * can only scale with the same factor in both
+			 * directions, and the scaling factor is limited to a
+			 * multiple of 1/32. At the moment the ImgU driver hides
+			 * these constraints by applying additional cropping,
+			 * this should be fixed on the driver side, and cropping
+			 * should be exposed to us.
+			 */
 		}
 
-		if (cfg.pixelFormat != oldCfg.pixelFormat ||
-		    cfg.size != oldCfg.size) {
+		if (cfg.pixelFormat != oldCfg.pixelFormat || cfg.size != oldCfg.size) {
 			LOG(IPU3, Debug)
 				<< "Stream " << i << " configuration adjusted to "
 				<< cfg.toString();
-- 
2.27.0