[v3,12/29] libcamera: rkisp1: Scale down in dewarper instead of resizer
diff mbox series

Message ID 20251125162851.2301793-13-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • Full dewarper support on imx8mp
Related show

Commit Message

Stefan Klug Nov. 25, 2025, 4:28 p.m. UTC
In order to allow digital zooming, scale down in the dewarper instead of
the resizer. That means forwarding the full sensor size data to the
dewarper. The ScalerCrop rectangle will also be applied at the dewarper.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---

Changes in v3:
- Collected tag
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 43 +++++++++++++++++++-----
 1 file changed, 35 insertions(+), 8 deletions(-)

Comments

Kieran Bingham Nov. 25, 2025, 5:08 p.m. UTC | #1
Quoting Stefan Klug (2025-11-25 16:28:24)
> In order to allow digital zooming, scale down in the dewarper instead of
> the resizer. That means forwarding the full sensor size data to the
> dewarper. The ScalerCrop rectangle will also be applied at the dewarper.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
> 
> Changes in v3:
> - Collected tag
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 43 +++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index a3b92804c69f..2d89b0b41958 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -883,16 +883,31 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>         /*
>          * On devices without DUAL_CROP (like the imx8mp) cropping needs to be
>          * done on the ISP/IS output.
> +        *
> +        * If the dewarper is used, the cropping shall be done by the dewarper.
>          */
>         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);
> +               /*
> +                * If the dewarper is used, all cropping including aspect ratio
> +                * preservation shall be done there. To ensure that the output
> +                * format provided by the ISP is supported by the dewarper, a
> +                * minimal crop still needs to be applied on the ISP output.
> +                *
> +                * \todo It might be possible to allocate bigger buffers
> +                * (aligned to 8 pixels) with a stride matching format.size for
> +                * the ISP. The not-filled border could later be ignored by the
> +                * dewarper. This way we could skip the minimal crop here and
> +                * the MaximumScalerCrop would always match the isp output.
> +                */
> +               Size ispCrop;
>                 if (data->usesDewarper_)
>                         ispCrop = dewarper_->adjustInputSize(cfg.pixelFormat,
> -                                                            ispCrop);
> +                                                            format.size);
>                 else
> -                       ispCrop.alignUpTo(2, 2);
> +                       ispCrop = format.size.boundedToAspectRatio(cfg.size)
> +                                         .alignedUpTo(2, 2);
>  
>                 outputCrop = ispCrop.centeredTo(Rectangle(format.size).center());
>                 format.size = ispCrop;
> @@ -925,13 +940,21 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>         for (const StreamConfiguration &cfg : *config) {
>                 if (cfg.stream() == &data->mainPathStream_) {
> -                       ret = mainPath_.configure(cfg, format);
> -                       streamConfig[0] = IPAStream(cfg.pixelFormat,
> -                                                   cfg.size);
> -                       /* Configure dewarp */
> +                       /*
> +                        * To allow for digital zoom, scaling down should happen
> +                        * in the dewarper, instead of the resizer. Configure
> +                        * the isp output to the same size as the sensor output.
> +                        */
> +                       StreamConfiguration ispCfg = cfg;
>                         if (data->usesDewarper_) {
>                                 outputCfgs.push_back(const_cast<StreamConfiguration &>(cfg));
> -                               ret = dewarper_->configure(cfg, outputCfgs);
> +
> +                               ispCfg.size = format.size;
> +                               ispCfg.stride =
> +                                       PixelFormatInfo::info(ispCfg.pixelFormat)
> +                                               .stride(ispCfg.size.width, 0);
> +
> +                               ret = dewarper_->configure(ispCfg, outputCfgs);
>                                 if (ret)
>                                         return ret;
>  
> @@ -944,6 +967,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>                                         outputCrop.transformedBetween(inputCrop,
>                                                                       sensorInfo.analogCrop);
>                         }
> +
> +                       ret = mainPath_.configure(ispCfg, format);
> +                       streamConfig[0] = IPAStream(cfg.pixelFormat,
> +                                                   cfg.size);
>                 } else if (hasSelfPath_) {
>                         ret = selfPath_.configure(cfg, format);
>                         streamConfig[1] = IPAStream(cfg.pixelFormat,
> -- 
> 2.51.0
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index a3b92804c69f..2d89b0b41958 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -883,16 +883,31 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	/*
 	 * On devices without DUAL_CROP (like the imx8mp) cropping needs to be
 	 * done on the ISP/IS output.
+	 *
+	 * If the dewarper is used, the cropping shall be done by the dewarper.
 	 */
 	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);
+		/*
+		 * If the dewarper is used, all cropping including aspect ratio
+		 * preservation shall be done there. To ensure that the output
+		 * format provided by the ISP is supported by the dewarper, a
+		 * minimal crop still needs to be applied on the ISP output.
+		 *
+		 * \todo It might be possible to allocate bigger buffers
+		 * (aligned to 8 pixels) with a stride matching format.size for
+		 * the ISP. The not-filled border could later be ignored by the
+		 * dewarper. This way we could skip the minimal crop here and
+		 * the MaximumScalerCrop would always match the isp output.
+		 */
+		Size ispCrop;
 		if (data->usesDewarper_)
 			ispCrop = dewarper_->adjustInputSize(cfg.pixelFormat,
-							     ispCrop);
+							     format.size);
 		else
-			ispCrop.alignUpTo(2, 2);
+			ispCrop = format.size.boundedToAspectRatio(cfg.size)
+					  .alignedUpTo(2, 2);
 
 		outputCrop = ispCrop.centeredTo(Rectangle(format.size).center());
 		format.size = ispCrop;
@@ -925,13 +940,21 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 
 	for (const StreamConfiguration &cfg : *config) {
 		if (cfg.stream() == &data->mainPathStream_) {
-			ret = mainPath_.configure(cfg, format);
-			streamConfig[0] = IPAStream(cfg.pixelFormat,
-						    cfg.size);
-			/* Configure dewarp */
+			/*
+			 * To allow for digital zoom, scaling down should happen
+			 * in the dewarper, instead of the resizer. Configure
+			 * the isp output to the same size as the sensor output.
+			 */
+			StreamConfiguration ispCfg = cfg;
 			if (data->usesDewarper_) {
 				outputCfgs.push_back(const_cast<StreamConfiguration &>(cfg));
-				ret = dewarper_->configure(cfg, outputCfgs);
+
+				ispCfg.size = format.size;
+				ispCfg.stride =
+					PixelFormatInfo::info(ispCfg.pixelFormat)
+						.stride(ispCfg.size.width, 0);
+
+				ret = dewarper_->configure(ispCfg, outputCfgs);
 				if (ret)
 					return ret;
 
@@ -944,6 +967,10 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 					outputCrop.transformedBetween(inputCrop,
 								      sensorInfo.analogCrop);
 			}
+
+			ret = mainPath_.configure(ispCfg, format);
+			streamConfig[0] = IPAStream(cfg.pixelFormat,
+						    cfg.size);
 		} else if (hasSelfPath_) {
 			ret = selfPath_.configure(cfg, format);
 			streamConfig[1] = IPAStream(cfg.pixelFormat,