[v3,07/11] software-isp: Map input crops to native sensor coordinates
diff mbox series

Message ID 20261016202818.1405645-8-halfline@gmail.com
State Superseded
Headers show
Series
  • software-isp: Align crops, statistics and metadata
Related show

Commit Message

Ray Strode Oct. 16, 2026, 8:28 p.m. UTC 
From: Ray Strode <rstrode@redhat.com>

This commit adds the coordinate conversion needed to report a debayer crop
as ScalerCrop metadata.

Debayer crops use captured-buffer pixels. ScalerCrop uses native sensor
pixels relative to the active array, so sensor binning, analogue crop
offsets and sensor transforms must be accounted for.

The conversion reverses the sensor transform, maps the input rectangle into
the analogue crop and subtracts the active-array origin. Integer
coordinates and dimensions follow the truncation used by the existing
rectangle transformation.

Signed-off-by: Ray Strode <rstrode@redhat.com>
---
 .../internal/software_isp/debayer_geometry.h  | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Stefan Klug Oct. 29, 2026, 11:30 a.m. UTC | #1 
Hi Ray,

Thank you for the patch.

Quoting Ray Strode (2026-10-16 22:28:14)
> From: Ray Strode <rstrode@redhat.com>
> 
> This commit adds the coordinate conversion needed to report a debayer crop
> as ScalerCrop metadata.
> 
> Debayer crops use captured-buffer pixels. ScalerCrop uses native sensor
> pixels relative to the active array, so sensor binning, analogue crop
> offsets and sensor transforms must be accounted for.
> 
> The conversion reverses the sensor transform, maps the input rectangle into
> the analogue crop and subtracts the active-array origin. Integer
> coordinates and dimensions follow the truncation used by the existing
> rectangle transformation.
> 
> Signed-off-by: Ray Strode <rstrode@redhat.com>
> ---
>  .../internal/software_isp/debayer_geometry.h  | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/include/libcamera/internal/software_isp/debayer_geometry.h b/include/libcamera/internal/software_isp/debayer_geometry.h
> index 46721753..8409fd09 100644
> --- a/include/libcamera/internal/software_isp/debayer_geometry.h
> +++ b/include/libcamera/internal/software_isp/debayer_geometry.h
> @@ -9,8 +9,10 @@
>  
>  #include <algorithm>
>  #include <array>
> +#include <utility>
>  
>  #include <libcamera/geometry.h>
> +#include <libcamera/transform.h>
>  
>  namespace libcamera {
>  
> @@ -48,4 +50,29 @@ inline Rectangle debayerCrop(const Size &inputSize, const Size &nativeSize,
>         return visibleSize.centeredTo(Rectangle(inputSize).center());
>  }
>  
> +/*
> + * Map an image crop to native sensor pixels relative to the active array.
> + * The input must cover the whole analogue crop. Undo the sensor transform
> + * before accounting for binning and the analogue crop offset. Integer
> + * coordinates and dimensions are truncated as in transformedBetween().
> + */
> +inline Rectangle debayerCropToSensor(Rectangle crop, Size inputSize,
> +                                    const Rectangle &analogCrop,
> +                                    const Rectangle &activeArea,
> +                                    Transform transform)
> +{
> +       if (!!(transform & Transform::Transpose)) {
> +               std::swap(crop.x, crop.y);
> +               std::swap(crop.width, crop.height);
> +               std::swap(inputSize.width, inputSize.height);
> +       }
> +       if (!!(transform & Transform::HFlip))
> +               crop.x = inputSize.width - crop.x - crop.width;
> +       if (!!(transform & Transform::VFlip))
> +               crop.y = inputSize.height - crop.y - crop.height;
> +
> +       return crop.transformedBetween(Rectangle(inputSize), analogCrop)
> +               .translatedBy(-activeArea.topLeft());

I think this was correct but is no longer correct after the changes
applied here: https://patchwork.libcamera.org/patch/28335/

Basically we want to reference everything to the physical pixel array
aka PixelArrayArea. Unfortunately in this series the correction of the
ScalerCrop docs was missed, but the implementation is already like that.
There is also a related discussion about ScalerCrop here:
https://patchwork.libcamera.org/patch/26905/#40193

I'll prepare a patch that fixes the ScalerCrop/ScalerCropMaximum
documentation. So I think you can drop the translatedBy() and adjust the
docs accordingly.

Best regards,
Stefan

> +}
> +
>  } /* namespace libcamera */
> -- 
> 2.56.0
>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/software_isp/debayer_geometry.h b/include/libcamera/internal/software_isp/debayer_geometry.h
index 46721753..8409fd09 100644
--- a/include/libcamera/internal/software_isp/debayer_geometry.h
+++ b/include/libcamera/internal/software_isp/debayer_geometry.h
@@ -9,8 +9,10 @@ 
 
 #include <algorithm>
 #include <array>
+#include <utility>
 
 #include <libcamera/geometry.h>
+#include <libcamera/transform.h>
 
 namespace libcamera {
 
@@ -48,4 +50,29 @@  inline Rectangle debayerCrop(const Size &inputSize, const Size &nativeSize,
 	return visibleSize.centeredTo(Rectangle(inputSize).center());
 }
 
+/*
+ * Map an image crop to native sensor pixels relative to the active array.
+ * The input must cover the whole analogue crop. Undo the sensor transform
+ * before accounting for binning and the analogue crop offset. Integer
+ * coordinates and dimensions are truncated as in transformedBetween().
+ */
+inline Rectangle debayerCropToSensor(Rectangle crop, Size inputSize,
+				     const Rectangle &analogCrop,
+				     const Rectangle &activeArea,
+				     Transform transform)
+{
+	if (!!(transform & Transform::Transpose)) {
+		std::swap(crop.x, crop.y);
+		std::swap(crop.width, crop.height);
+		std::swap(inputSize.width, inputSize.height);
+	}
+	if (!!(transform & Transform::HFlip))
+		crop.x = inputSize.width - crop.x - crop.width;
+	if (!!(transform & Transform::VFlip))
+		crop.y = inputSize.height - crop.y - crop.height;
+
+	return crop.transformedBetween(Rectangle(inputSize), analogCrop)
+		.translatedBy(-activeArea.topLeft());
+}
+
 } /* namespace libcamera */