[v3,15/17] pipeline: rkisp1: Fix config validation when dewarper is used
diff mbox series

Message ID 20241206101344.767170-16-stefan.klug@ideasonboard.com
State Accepted
Delegated to: Paul Elder
Headers show
Series
  • rkisp1: Fix aspect ratio and ScalerCrop
Related show

Commit Message

Stefan Klug Dec. 6, 2024, 10:13 a.m. UTC
When the dewarper is used, config->validate() needs to take the
restrictions of the dewarper into account. Add the corresponding checks.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 46 +++++++++++++++++++++---
 1 file changed, 41 insertions(+), 5 deletions(-)

Comments

Paul Elder Dec. 12, 2024, 12:12 p.m. UTC | #1
On Fri, Dec 06, 2024 at 11:13:37AM +0100, Stefan Klug wrote:
> When the dewarper is used, config->validate() needs to take the
> restrictions of the dewarper into account. Add the corresponding checks.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 46 +++++++++++++++++++++---
>  1 file changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index c582e164670c..2baff6da7cb7 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -497,6 +497,7 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)
>  
>  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  {
> +	const PipelineHandlerRkISP1 *pipe = data_->pipe();
>  	const CameraSensor *sensor = data_->sensor_.get();
>  	unsigned int pathCount = data_->selfPath_ ? 2 : 1;
>  	Status status;
> @@ -553,6 +554,18 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  		}
>  	}
>  
> +	bool useDewarper = false;
> +	if (pipe->dewarper_) {
> +		/*
> +		 * Platforms with dewarper support, such as i.MX8MP, support a

s/ a / only a /

> +		 * single stream. We can inspect config_[0] only here.
> +		 */
> +		bool isRaw = PixelFormatInfo::info(config_[0].pixelFormat).colourEncoding ==
> +			     PixelFormatInfo::ColourEncodingRAW;
> +		if (!isRaw)
> +			useDewarper = true;
> +	}
> +
>  	/*
>  	 * If there are more than one stream in the configuration figure out the
>  	 * order to evaluate the streams. The first stream has the highest
> @@ -565,12 +578,31 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  	if (config_.size() == 2 && fitsAllPaths(config_[0]))
>  		std::reverse(order.begin(), order.end());
>  
> +	/*
> +	 * Validate the configuration against the desired path and, if the
> +	 * platform supports it, the dewarper.

s/the dewarper/use the dewarper/

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> +	 */
>  	auto validateConfig = [&](StreamConfiguration &cfg, RkISP1Path *path,
>  				  Stream *stream, Status expectedStatus) {
>  		StreamConfiguration tryCfg = cfg;
> -		if (path->validate(sensor, sensorConfig, &tryCfg) != expectedStatus)
> +
> +		Status ret = path->validate(sensor, sensorConfig, &tryCfg);
> +		if (ret == Invalid)
> +			return false;
> +
> +		if (!useDewarper &&
> +		    (expectedStatus == Valid && ret == Adjusted))
>  			return false;
>  
> +		if (useDewarper) {
> +			bool adjusted;
> +
> +			pipe->dewarper_->validateOutput(&tryCfg, &adjusted,
> +							Converter::Alignment::Down);
> +			if (expectedStatus == Valid && adjusted)
> +				return false;
> +		}
> +
>  		cfg = tryCfg;
>  		cfg.setStream(stream);
>  		return true;
> @@ -820,6 +852,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	const PixelFormat &streamFormat = config->at(0).pixelFormat;
>  	const PixelFormatInfo &info = PixelFormatInfo::info(streamFormat);
>  	isRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> +	useDewarper_ = dewarper_ && !isRaw_;
>  
>  	/* YUYV8_2X8 is required on the ISP source path pad for YUV output. */
>  	if (!isRaw_)
> @@ -832,8 +865,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	if (media_->hwRevision() == RKISP1_V_IMX8MP) {
>  		/* imx8mp has only a single path. */
>  		const auto &cfg = config->at(0);
> -		Size ispCrop = format.size.boundedToAspectRatio(cfg.size)
> -					  .alignedUpTo(2, 2);
> +		Size ispCrop = format.size.boundedToAspectRatio(cfg.size);
> +		if (useDewarper_)
> +			ispCrop = dewarper_->adjustInputSize(cfg.pixelFormat,
> +							     ispCrop);
> +		else
> +			ispCrop.alignUpTo(2, 2);
> +
>  		outputCrop = ispCrop.centeredTo(Rectangle(format.size).center());
>  		format.size = ispCrop;
>  	}
> @@ -875,8 +913,6 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  				if (ret)
>  					return ret;
>  
> -				useDewarper_ = true;
> -
>  				/*
>  				 * Calculate the crop rectangle of the data
>  				 * flowing into the dewarper in sensor
> -- 
> 2.43.0
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index c582e164670c..2baff6da7cb7 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -497,6 +497,7 @@  bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)
 
 CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 {
+	const PipelineHandlerRkISP1 *pipe = data_->pipe();
 	const CameraSensor *sensor = data_->sensor_.get();
 	unsigned int pathCount = data_->selfPath_ ? 2 : 1;
 	Status status;
@@ -553,6 +554,18 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 		}
 	}
 
+	bool useDewarper = false;
+	if (pipe->dewarper_) {
+		/*
+		 * Platforms with dewarper support, such as i.MX8MP, support a
+		 * single stream. We can inspect config_[0] only here.
+		 */
+		bool isRaw = PixelFormatInfo::info(config_[0].pixelFormat).colourEncoding ==
+			     PixelFormatInfo::ColourEncodingRAW;
+		if (!isRaw)
+			useDewarper = true;
+	}
+
 	/*
 	 * If there are more than one stream in the configuration figure out the
 	 * order to evaluate the streams. The first stream has the highest
@@ -565,12 +578,31 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 	if (config_.size() == 2 && fitsAllPaths(config_[0]))
 		std::reverse(order.begin(), order.end());
 
+	/*
+	 * Validate the configuration against the desired path and, if the
+	 * platform supports it, the dewarper.
+	 */
 	auto validateConfig = [&](StreamConfiguration &cfg, RkISP1Path *path,
 				  Stream *stream, Status expectedStatus) {
 		StreamConfiguration tryCfg = cfg;
-		if (path->validate(sensor, sensorConfig, &tryCfg) != expectedStatus)
+
+		Status ret = path->validate(sensor, sensorConfig, &tryCfg);
+		if (ret == Invalid)
+			return false;
+
+		if (!useDewarper &&
+		    (expectedStatus == Valid && ret == Adjusted))
 			return false;
 
+		if (useDewarper) {
+			bool adjusted;
+
+			pipe->dewarper_->validateOutput(&tryCfg, &adjusted,
+							Converter::Alignment::Down);
+			if (expectedStatus == Valid && adjusted)
+				return false;
+		}
+
 		cfg = tryCfg;
 		cfg.setStream(stream);
 		return true;
@@ -820,6 +852,7 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	const PixelFormat &streamFormat = config->at(0).pixelFormat;
 	const PixelFormatInfo &info = PixelFormatInfo::info(streamFormat);
 	isRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
+	useDewarper_ = dewarper_ && !isRaw_;
 
 	/* YUYV8_2X8 is required on the ISP source path pad for YUV output. */
 	if (!isRaw_)
@@ -832,8 +865,13 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	if (media_->hwRevision() == RKISP1_V_IMX8MP) {
 		/* imx8mp has only a single path. */
 		const auto &cfg = config->at(0);
-		Size ispCrop = format.size.boundedToAspectRatio(cfg.size)
-					  .alignedUpTo(2, 2);
+		Size ispCrop = format.size.boundedToAspectRatio(cfg.size);
+		if (useDewarper_)
+			ispCrop = dewarper_->adjustInputSize(cfg.pixelFormat,
+							     ispCrop);
+		else
+			ispCrop.alignUpTo(2, 2);
+
 		outputCrop = ispCrop.centeredTo(Rectangle(format.size).center());
 		format.size = ispCrop;
 	}
@@ -875,8 +913,6 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 				if (ret)
 					return ret;
 
-				useDewarper_ = true;
-
 				/*
 				 * Calculate the crop rectangle of the data
 				 * flowing into the dewarper in sensor