[libcamera-devel] [PATCH 2/3] pipeline: rkisp1: Fix validation when sensor max is larger than ISP input

[Paul Elder]

If the maximum sensor output size is larger than the maximum ISP input
size, the maximum sensor size could be selected and the pipeline would
fail with an EPIPE. Fix this by clamping the sensor size to the ISP
input size at validation time.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 17 ++++++++++--
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 26 +++++++++----------
 src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  4 ++-
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 586b46d64..fb67ba8f4 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1202,11 +1202,24 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 	if (param_->open() < 0)
 		return false;
 
+	/*
+	 * Retrieve ISP maximum input size for config validation in the path
+	 * classes
+	 */
+	Size ispMaxInSize = { 0, 0 };
+	V4L2Subdevice::Formats ispFormats = isp_->formats(0);
+	for (const auto &[mbus, sizes] : ispFormats) {
+		for (const auto &size : sizes) {
+			if (ispMaxInSize < size.max)
+				ispMaxInSize = size.max;
+		}
+	}
+
 	/* Locate and open the ISP main and self paths. */
-	if (!mainPath_.init(media_))
+	if (!mainPath_.init(media_, ispMaxInSize))
 		return false;
 
-	if (hasSelfPath_ && !selfPath_.init(media_))
+	if (hasSelfPath_ && !selfPath_.init(media_, ispMaxInSize))
 		return false;
 
 	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
index b62b645ca..eaab7e857 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
@@ -62,7 +62,7 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
 {
 }
 
-bool RkISP1Path::init(MediaDevice *media)
+bool RkISP1Path::init(MediaDevice *media, const Size &ispMaxInSize)
 {
 	std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
 	std::string video = std::string("rkisp1_") + name_ + "path";
@@ -76,6 +76,7 @@ bool RkISP1Path::init(MediaDevice *media)
 		return false;
 
 	populateFormats();
+	ispMaxInSize_ = ispMaxInSize;
 
 	link_ = media->link("rkisp1_isp", 2, resizer, 0);
 	if (!link_)
@@ -172,7 +173,9 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size,
 		/* Add all the RAW sizes the sensor can produce for this code. */
 		for (const auto &rawSize : sensor->sizes(mbusCode)) {
 			if (rawSize.width > maxResolution_.width ||
-			    rawSize.height > maxResolution_.height)
+			    rawSize.height > maxResolution_.height ||
+			    rawSize.width > ispMaxInSize_.width ||
+			    rawSize.height > ispMaxInSize_.height)
 				continue;
 
 			streamFormats[format].push_back({ rawSize, rawSize });
@@ -275,8 +278,9 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,
 	if (!found)
 		cfg->pixelFormat = isRaw ? rawFormat : formats::NV12;
 
-	Size minResolution;
-	Size maxResolution;
+	Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
+					   .boundedTo(resolution);
+	Size minResolution = minResolution_.expandedToAspectRatio(resolution);
 
 	if (isRaw) {
 		/*
@@ -287,16 +291,10 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,
 		V4L2SubdeviceFormat sensorFormat =
 			sensor->getFormat({ mbusCode }, cfg->size);
 
-		minResolution = sensorFormat.size;
-		maxResolution = sensorFormat.size;
-	} else {
-		/*
-		 * Adjust the size based on the sensor resolution and absolute
-		 * limits of the ISP.
-		 */
-		minResolution = minResolution_.expandedToAspectRatio(resolution);
-		maxResolution = maxResolution_.boundedToAspectRatio(resolution)
-					      .boundedTo(resolution);
+		if (!sensorFormat.size.isNull()) {
+			minResolution = sensorFormat.size.boundedTo(ispMaxInSize_);
+			maxResolution = sensorFormat.size.boundedTo(ispMaxInSize_);
+		}
 	}
 
 	cfg->size.boundTo(maxResolution);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
index cd77957ee..c96bd5557 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
@@ -35,7 +35,7 @@ public:
 	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
 		   const Size &minResolution, const Size &maxResolution);
 
-	bool init(MediaDevice *media);
+	bool init(MediaDevice *media, const Size &ispMaxInSize);
 
 	int setEnabled(bool enable) { return link_->setEnabled(enable); }
 	bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }
@@ -77,6 +77,8 @@ private:
 	std::unique_ptr<V4L2Subdevice> resizer_;
 	std::unique_ptr<V4L2VideoDevice> video_;
 	MediaLink *link_;
+
+	Size ispMaxInSize_;
 };
 
 class RkISP1MainPath : public RkISP1Path
-- 
2.39.2