[v2,1/3] libcamera: Clarify meaning of PixelArraySize and other rectangles
diff mbox series

Message ID 20260819141541.221617-2-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Clarify rectangle behavior
Related show

Commit Message

Stefan Klug Aug. 19, 2026, 2:15 p.m. UTC
Current documented behavior for PixelArraySize is to report the readable
array size of the sensor data. If the readable array is offset relative
to the physical sensor array, all related rectangles (ScalerCrop,
PixelArrayActiveAreas) shall be reported relative to PixelArraySize.

However no known pipeline and IPA implements it that way. All
pipelines/IPAs report PixelArrayActiveAreas relative to the physical
size resulting in an inconsistent behavior where the right border of the
active area is outside of PixelArraySize.

Adjust the documentation to fit the actual implementations. Add another
property named PixelArrayReadableArea to convey the information that
PixelArraySize was intended for.

This has the added benefit that all rectangles are relative to the
physical sensor array, which helps in aligning tuning data like lens
shading correction. Using the active area to align tuning data had the
downside that the offset of the active area relative to the physical
pixel array was nowhere reported by libcamera and is a property that can
change in different versions of a sensor driver. Relying on physical
coordinates is expected to provide long-term stable absolute
coordinates.

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

---

Changes in v2:
- Fixed typos reported by Paul
- Collected tag
- Improved commit message
- Updated docs based on Jacpos review
- Added PixelArrayReadableArea to CameraSensorRaw
- Improved PixelArraySize in CameraSensorRaw but left a todo
---
 src/libcamera/property_ids_core.yaml          | 123 ++++++++++--------
 src/libcamera/sensor/camera_sensor_legacy.cpp |  15 ++-
 src/libcamera/sensor/camera_sensor_raw.cpp    |  12 ++
 3 files changed, 97 insertions(+), 53 deletions(-)

Patch
diff mbox series

diff --git a/src/libcamera/property_ids_core.yaml b/src/libcamera/property_ids_core.yaml
index d5b3d309c055..fd9dcbaa05a4 100644
--- a/src/libcamera/property_ids_core.yaml
+++ b/src/libcamera/property_ids_core.yaml
@@ -427,13 +427,31 @@  controls:
   - PixelArraySize:
       type: Size
       description: |
-        The camera sensor pixel array readable area vertical and horizontal
-        sizes, in pixels.
+        The camera sensor pixel array vertical and horizontal sizes, in pixels.
 
-        The PixelArraySize property defines the size in pixel units of the
-        readable part of full pixel array matrix, including optical black
+        The PixelArraySize property defines the size in pixel units of the full
+        pixel array matrix including readable and non-readable pixels. For the
+        largest readable area \see PixelArrayReadableArea.
+
+        This property defines the horizontal and vertical sizes of a rectangle
+        whose top-left corner is placed in position (0, 0). All the other
+        properties that describe portions of the pixel array such as
+        PixelArrayActiveAreas, PixelArrayReadableArea and
+        PixelArrayOpticalBlackRectangles are defined relative to this rectangle.
+
+        \todo Rename this property to Size once we will have property
+              categories (i.e. Properties::PixelArray::Size)
+
+  - PixelArrayReadableArea:
+      type: Rectangle
+      description: |
+        The camera sensor pixel array readable area.
+
+        The PixelArrayReadableArea property defines the rectangle in pixel units
+        of the readable part of full pixel array matrix, including optical black
         pixels used for calibration, pixels which are not considered valid for
-        capture and active pixels containing valid image data.
+        capture and active pixels containing valid image data. It is defined
+        relative to the PixelArraySize rectangle.
 
         The property describes the maximum size of the raw data captured by the
         camera, which might not correspond to the physical size of the sensor
@@ -442,23 +460,25 @@  controls:
 
         For example, let's consider a pixel array matrix assembled as follows
 
-             +--------------------------------------------------+
-             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx|
-             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx|
-             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx|
-             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx|
-             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx|
-             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx|
-             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx|
-             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx|
+                          PixelArraySize.width
+             /--------------------------------------------------/
+             +--------------------------------------------------+ /
+             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx| |
+             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| | PixelArraySize.height
              ...          ...           ...      ...          ...
 
              ...          ...           ...      ...          ...
-             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx|
-             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx|
-             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx|
-             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx|
-             +--------------------------------------------------+
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx| |
+             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx| |
+             +--------------------------------------------------+ /
 
         starting with two lines of non-readable pixels (x), followed by N lines
         of readable data (D) surrounded by two columns of non-readable pixels on
@@ -467,35 +487,32 @@  controls:
         sizes of the largest possible buffer of raw data that can be presented
         to applications.
 
-                             PixelArraySize.width
-               /----------------------------------------------/
-               +----------------------------------------------+ /
-               |DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD| |
-               |DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD| |
-               |DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD| |
-               |DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD| |
-               |DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD| |
-               |DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD| | PixelArraySize.height
-               ...        ...           ...      ...        ...
-               ...        ...           ...      ...        ...
-               |DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD| |
-               |DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD| |
-               +----------------------------------------------+ /
+                          PixelArrayReadableArea.width
+                /--------------------------------------------/
+                PixelArrayReadableArea.x
+             +--o-----------------------------------------------+
+             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx|
+             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx|
+             oxxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| / PixelArrayReadableArea.y
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| | PixelArrayReadableArea.height
+             ...          ...           ...      ...          ...
 
-        This defines a rectangle whose top-left corner is placed in position (0,
-        0) and whose vertical and horizontal sizes are defined by this property.
-        All other rectangles that describe portions of the pixel array, such as
-        the optical black pixels rectangles and active pixel areas, are defined
-        relatively to this rectangle.
+             ...          ...           ...      ...          ...
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| |
+             |xxDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDxx| /
+             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx|
+             |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx|
+             +--------------------------------------------------+
 
         All the coordinates are expressed relative to the default sensor readout
         direction, without any transformation (such as horizontal and vertical
         flipping) applied. When mapping them to the raw pixel buffer,
         applications shall take any configured transformation into account.
 
-        \todo Rename this property to Size once we will have property
-              categories (i.e. Properties::PixelArray::Size)
-
   - PixelArrayOpticalBlackRectangles:
       type: Rectangle
       size: [n]
@@ -505,7 +522,8 @@  controls:
 
         This property describes the position and size of optical black pixel
         regions in the raw data buffer as stored in memory, which might differ
-        from their actual physical location in the pixel array matrix.
+        from their actual physical location in the pixel array matrix. It is
+        defined relative to the PixelArraySize rectangle.
 
         It is important to note, in fact, that camera sensors might
         automatically reorder or skip portions of their pixels array matrix when
@@ -519,7 +537,7 @@  controls:
         they could be positioned too close to the chip margins or to the optical
         black shielding placed on top of optical black pixels.
 
-                             PixelArraySize.width
+                             PixelArrayReadableArea.width
                /----------------------------------------------/
                   x1                                       x2
                +--o---------------------------------------o---+ /
@@ -532,7 +550,7 @@  controls:
                |IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII| |
                |IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII| |
             y3 |IIOOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOOII| |
-               |IIOOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOOII| | PixelArraySize.height
+               |IIOOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOOII| | PixelArrayReadableArea.height
                |IIOOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOOII| |
                ...          ...           ...     ...       ...
                ...          ...           ...     ...       ...
@@ -560,7 +578,7 @@  controls:
         skips the invalid lines and columns, producing the following data
         buffer, when captured to memory
 
-                             PixelArraySize.width
+                             PixelArrayReadableArea.width
                /----------------------------------------------/
                                                            x1
                +--------------------------------------------o-+ /
@@ -570,7 +588,7 @@  controls:
                |OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO| |
             y1 oOOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOO| |
                |OOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOO| |
-               |OOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOO| | PixelArraySize.height
+               |OOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOO| | PixelArrayReadableArea.height
                ...       ...          ...       ...         ... |
                ...       ...          ...       ...         ... |
                |OOPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOO| |
@@ -578,14 +596,17 @@  controls:
                +----------------------------------------------+ /
 
         then the invalid lines and columns should not be reported as part of the
-        PixelArraySize property in first place.
+        PixelArrayReadableArea property in first place. As the coordinates of
+        PixelArrayReadableArea are relative to the physical pixel array, care
+        must be taken to report it in a way that the valid pixel data maps to
+        the correct physical position.
 
         In this case, the position of the black pixel rectangles will be
 
             PixelArrayOpticalBlackRectangles = {
-               { 0, 0, y1 + 1, PixelArraySize[0] },
-               { 0, y1, 2, PixelArraySize[1] - y1 + 1 },
-               { x1, y1, 2, PixelArraySize[1] - y1 + 1 },
+               { 0, 0, PixelArrayReadableArea.width, y1 + 1 },
+               { 0, y1, 2, PixelArrayReadableArea.height - y1 + 1 },
+               { x1, y1, 2, PixelArrayReadableArea.height - y1 + 1 },
             };
 
         \todo Rename this property to Size once we will have property
@@ -602,7 +623,7 @@  controls:
         This property describes an arbitrary number of overlapping rectangles,
         with each rectangle representing the maximum image size that the camera
         sensor can produce for a particular aspect ratio. They are defined
-        relatively to the PixelArraySize rectangle.
+        relative to the PixelArraySize rectangle.
 
         When multiple rectangles are reported, they shall be ordered from the
         tallest to the shortest.
diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
index 4338abc523a8..ef2bf656f845 100644
--- a/src/libcamera/sensor/camera_sensor_legacy.cpp
+++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
@@ -131,6 +131,7 @@  private:
 	controls::draft::TestPatternModeEnum testPatternMode_;
 
 	Size pixelArraySize_;
+	Rectangle readableArea_;
 	Rectangle activeArea_;
 	const BayerFormat *bayerFormat_;
 	bool supportHFlips_;
@@ -396,7 +397,7 @@  int CameraSensorLegacy::validateSensorDriver()
 	 * test platforms have been updated.
 	 */
 	Rectangle rect;
-	int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect);
+	int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_NATIVE_SIZE, &rect);
 	if (ret) {
 		/*
 		 * Default the pixel array size to the largest size supported
@@ -413,9 +414,18 @@  int CameraSensorLegacy::validateSensorDriver()
 		pixelArraySize_ = rect.size();
 	}
 
+	ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &readableArea_);
+	if (ret) {
+		readableArea_ = Rectangle(pixelArraySize_);
+		LOG(CameraSensor, Warning)
+			<< "The PixelArrayReadableArea property has been defaulted to "
+			<< readableArea_;
+		err = -EINVAL;
+	}
+
 	ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_);
 	if (ret) {
-		activeArea_ = Rectangle(pixelArraySize_);
+		activeArea_ = readableArea_;
 		LOG(CameraSensor, Warning)
 			<< "The PixelArrayActiveAreas property has been defaulted to "
 			<< activeArea_;
@@ -627,6 +637,7 @@  int CameraSensorLegacy::initProperties()
 	}
 
 	properties_.set(properties::PixelArraySize, pixelArraySize_);
+	properties_.set(properties::PixelArrayReadableArea, readableArea_);
 	properties_.set(properties::PixelArrayActiveAreas, { activeArea_ });
 
 	/* Color filter array pattern, register only for RAW sensors. */
diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
index 849423dd20cf..544d0e52c20d 100644
--- a/src/libcamera/sensor/camera_sensor_raw.cpp
+++ b/src/libcamera/sensor/camera_sensor_raw.cpp
@@ -142,6 +142,7 @@  private:
 	controls::draft::TestPatternModeEnum testPatternMode_;
 
 	Size pixelArraySize_;
+	Rectangle readableArea_;
 	Rectangle activeArea_;
 	BayerFormat::Order cfaPattern_;
 	bool supportHFlips_;
@@ -431,7 +432,17 @@  std::optional<int> CameraSensorRaw::init()
 		return { ret };
 	}
 
+	/*
+	 * \todo Implement querying the physical sensor size based on pad 1,
+	 * stream 0. See
+	 * https://lore.kernel.org/linux-media/20260409201501.975242-23-sakari.ailus@linux.intel.com/
+	 */
+	LOG(CameraSensor, Warning) << "PixelArraySize might be incorrect";
 	pixelArraySize_ = rect.size();
+	pixelArraySize_.width += rect.x;
+	pixelArraySize_.height += rect.y;
+
+	readableArea_ = rect;
 
 	ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_CROP_DEFAULT,
 				    &activeArea_);
@@ -635,6 +646,7 @@  int CameraSensorRaw::initProperties()
 	}
 
 	properties_.set(properties::PixelArraySize, pixelArraySize_);
+	properties_.set(properties::PixelArrayReadableArea, readableArea_);
 	properties_.set(properties::PixelArrayActiveAreas, { activeArea_ });
 
 	/* Color filter array pattern. */