[v2,2/3] libcamera: ipa: core: Use native coordinates for IPACameraSensorInfo::analogCrop

27848 diff mbox series

diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom
index bce797245829..d3375333baa0 100644
--- a/include/libcamera/ipa/core.mojom
+++ b/include/libcamera/ipa/core.mojom
@@ -153,17 +153,24 @@  module libcamera;
  */
 
 /**
- * \var IPACameraSensorInfo::activeAreaSize
- * \brief The size of the pixel array active area of the sensor
+ * \var IPACameraSensorInfo::activeArea
+ * \brief The active pixel area of the sensor, relative to the physical pixel
+ * array
+ *
+ * The active area rectangle is expressed relative to the top-left corner of the
+ * physical pixel array. It describes the region of the pixel array which
+ * produces valid image data, excluding non-readable and optical black pixels.
+ *
+ * \note New IPA implementations should *not* use this property and instead rely
+ * on coordinates relative to the physical sensor array
  */
 
 /**
  * \var IPACameraSensorInfo::analogCrop
- * \brief The portion of the pixel array active area which is read-out and
- * processed
+ * \brief The portion of the pixel array which is read-out and processed
  *
  * The analog crop rectangle top-left corner is defined as the displacement
- * from the top-left corner of the pixel array active area. The rectangle
+ * from the top-left corner of the physical pixel array. The rectangle
  * horizontal and vertical sizes define the portion of the pixel array which
  * is read-out and provided to the sensor's internal processing pipeline, before
  * any pixel sub-sampling method, such as pixel binning, skipping and averaging
@@ -243,7 +250,7 @@  struct IPACameraSensorInfo {
 	uint32 bitsPerPixel;
 	uint32 cfaPattern;
 
-	Size activeAreaSize;
+	Rectangle activeArea;
 	Rectangle analogCrop;
 	Size outputSize;
 
diff --git a/src/ipa/mali-c55/algorithms/lsc.cpp b/src/ipa/mali-c55/algorithms/lsc.cpp
index fff0dc7d0e64..6cd4e9020504 100644
--- a/src/ipa/mali-c55/algorithms/lsc.cpp
+++ b/src/ipa/mali-c55/algorithms/lsc.cpp
@@ -57,7 +57,7 @@  int Lsc::init(IPAContext &context, const ValueNode &tuningData)
 				.keys = { "r", "g", "b" },
 				.numHSamples = kMeshSize,
 				.numVSamples = kMeshSize,
-				.sensorSize = context.sensorInfo.activeAreaSize
+				.sensorSize = context.sensorInfo.activeArea.size()
 			     });
 }
 
diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
index f2858656023e..bb41dd3682bd 100644
--- a/src/ipa/rkisp1/algorithms/lsc.cpp
+++ b/src/ipa/rkisp1/algorithms/lsc.cpp
@@ -127,7 +127,7 @@  int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
 				.keys = { "r", "gr", "gb", "b" },
 				.numHSamples = RKISP1_CIF_ISP_LSC_SAMPLES_MAX,
 				.numVSamples = RKISP1_CIF_ISP_LSC_SAMPLES_MAX,
-				.sensorSize = context.sensorInfo.activeAreaSize
+				.sensorSize = context.sensorInfo.activeArea.size()
 			     });
 }
 
diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index 7e00c2799b4e..9b851007a124 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -561,10 +561,15 @@  void IpaBase::setMode(const IPACameraSensorInfo &sensorInfo)
 	mode_.bitdepth = sensorInfo.bitsPerPixel;
 	mode_.width = sensorInfo.outputSize.width;
 	mode_.height = sensorInfo.outputSize.height;
-	mode_.sensorWidth = sensorInfo.activeAreaSize.width;
-	mode_.sensorHeight = sensorInfo.activeAreaSize.height;
-	mode_.cropX = sensorInfo.analogCrop.x;
-	mode_.cropY = sensorInfo.analogCrop.y;
+	mode_.sensorWidth = sensorInfo.activeArea.width;
+	mode_.sensorHeight = sensorInfo.activeArea.height;
+	/*
+	 * CameraMode::cropX/Y are defined relative to the active pixel area,
+	 * whereas IPACameraSensorInfo::analogCrop is relative to the physical
+	 * pixel array. Rebase the crop origin onto the active area.
+	 */
+	mode_.cropX = sensorInfo.analogCrop.x - sensorInfo.activeArea.x;
+	mode_.cropY = sensorInfo.analogCrop.y - sensorInfo.activeArea.y;
 	mode_.pixelRate = sensorInfo.pixelRate;
 
 	/*
diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
index ef2bf656f845..9e32f798c2dd 100644
--- a/src/libcamera/sensor/camera_sensor_legacy.cpp
+++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
@@ -883,11 +883,10 @@  int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const
 	info->model = model();
 
 	/*
-	 * The active area size is a static property, while the crop
-	 * rectangle needs to be re-read as it depends on the sensor
-	 * configuration.
+	 * The active area is a static property, while the crop rectangle needs
+	 * to be re-read as it depends on the sensor configuration.
 	 */
-	info->activeAreaSize = { activeArea_.width, activeArea_.height };
+	info->activeArea = activeArea_;
 
 	/*
 	 * \todo Support for retreiving the crop rectangle is scheduled to
@@ -901,16 +900,6 @@  int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const
 			<< "The analogue crop rectangle has been defaulted to the active area size";
 	}
 
-	/*
-	 * IPACameraSensorInfo::analogCrop::x and IPACameraSensorInfo::analogCrop::y
-	 * are defined relatively to the active pixel area, while V4L2's
-	 * TGT_CROP target is defined in respect to the full pixel array.
-	 *
-	 * Compensate it by subtracting the active area offset.
-	 */
-	info->analogCrop.x -= activeArea_.x;
-	info->analogCrop.y -= activeArea_.y;
-
 	/* The bit depth and image size depend on the currently applied format. */
 	V4L2SubdeviceFormat format{};
 	ret = subdev_->getFormat(pad_, &format);
diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
index 544d0e52c20d..a1f48b5ff557 100644
--- a/src/libcamera/sensor/camera_sensor_raw.cpp
+++ b/src/libcamera/sensor/camera_sensor_raw.cpp
@@ -1009,27 +1009,16 @@  int CameraSensorRaw::sensorInfo(IPACameraSensorInfo *info) const
 	info->model = model();
 
 	/*
-	 * The active area size is a static property, while the crop
-	 * rectangle needs to be re-read as it depends on the sensor
-	 * configuration.
+	 * The active area is a static property, while the crop rectangle needs
+	 * to be re-read as it depends on the sensor configuration.
 	 */
-	info->activeAreaSize = { activeArea_.width, activeArea_.height };
+	info->activeArea = activeArea_;
 
 	int ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_CROP,
 					&info->analogCrop);
 	if (ret)
 		return ret;
 
-	/*
-	 * IPACameraSensorInfo::analogCrop::x and IPACameraSensorInfo::analogCrop::y
-	 * are defined relatively to the active pixel area, while V4L2's
-	 * TGT_CROP target is defined in respect to the full pixel array.
-	 *
-	 * Compensate it by subtracting the active area offset.
-	 */
-	info->analogCrop.x -= activeArea_.x;
-	info->analogCrop.y -= activeArea_.y;
-
 	/* The bit depth and image size depend on the currently applied format. */
 	V4L2SubdeviceFormat format{};
 	ret = subdev_->getFormat(streams_.image.source, &format);