[0/2] pipeline: simple: Add ScalerCrop reporting
mbox series

Message ID 20260507-kbingham-simple-scaler-crop-v1-0-7a5af1948565@ideasonboard.com
Headers show
Series
  • pipeline: simple: Add ScalerCrop reporting
Related show

Message

Kieran Bingham May 7, 2026, 3:25 p.m. UTC
While the Simple Pipeline handler doesn't support ScalerCrop, it can be
beneficial to report the ScalerCropMaximum and the ScalerCrop metadata
anyway as applications can use this information to determine the
positioning of the pixels captured in respect to the active area.

This series reports the AnalogCrop as the ScalerCropMaximum in the
camera properties, and also as the the ScalerCrop in each completed
frame metadata.

I'm curious on opinions if we should have more specific names for
reporting this metadata when theres' not actually a ScalerCrop
involved... 

With this series, it's possible to view the bounding rectangles and crop
positions of the selected modes in camshark.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
Kieran Bingham (2):
      pipeline: simple: Report ScalerCropMaximum camera property
      pipeline: simple: Report the ScalerCrop

 src/libcamera/pipeline/simple/simple.cpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)
---
base-commit: 183e37362f57ff3ce7493abf0bc6f1b57b931f55
change-id: 20260507-kbingham-simple-scaler-crop-60b506e2bc4c

Best regards,

Comments

Stefan Klug June 2, 2026, 3:43 p.m. UTC | #1
Hi Kieran,

Quoting Kieran Bingham (2026-05-07 17:25:32)
> While the Simple Pipeline handler doesn't support ScalerCrop, it can be
> beneficial to report the ScalerCropMaximum and the ScalerCrop metadata
> anyway as applications can use this information to determine the
> positioning of the pixels captured in respect to the active area.
> 
> This series reports the AnalogCrop as the ScalerCropMaximum in the
> camera properties, and also as the the ScalerCrop in each completed
> frame metadata.
> 
> I'm curious on opinions if we should have more specific names for
> reporting this metadata when theres' not actually a ScalerCrop
> involved... 

I think that would be beneficial.

My proposal would be to add a PixelArray property of type Rectangle.
This would complement PixelArraySize [1] by also specifying the offset.

The documentation of PixelArrayActiveAreas [2] even mentions the
"PixelArraySize rectangle" which could then become the "PixelArray
rectangle".

This series uses the analog crop rectangle for the ScalerCrop. I fear
this is not according to documentation the same way as it is on rkisp1.
In both cases we report ScalerCrop relative to the physical pixel array
and not relative to the libcamera "PixelArraySize" (effectively we would
need to set the offset to 0 in this case to comply with the docs.) But
then we'd need the above mentioned PixelArray property to get the offset
in physical world.  I don't know how right or wrong other platforms are
(especially Rpi), but it ties into the rectangles discussion.

So question is: Merge this in even though it is as wrong as rkisp1 or
clarify the proper behavior first (I have a slight hope that other
platforms do the same mistake as rkisp1) and we could change the docs to
say ScalerCrop references the physical array...

[1] https://docs.libcamera.org/master/internal-api/namespacelibcamera_1_1properties.html#aac88aeb6478c1f8528cc08521486dae3 
[2] https://docs.libcamera.org/master/internal-api/namespacelibcamera_1_1properties.html#acec5675f79b6c456aca72c7532a263a4

> 
> With this series, it's possible to view the bounding rectangles and crop
> positions of the selected modes in camshark.

In my sklug/next branch I reworked the rectangle drawing. It might make
sense to double check with the current version. My expectation is that
the ScalerCrop is as big as PixelArraySize, but offset from it.

Best regards,
Stefan

> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
> Kieran Bingham (2):
>       pipeline: simple: Report ScalerCropMaximum camera property
>       pipeline: simple: Report the ScalerCrop
> 
>  src/libcamera/pipeline/simple/simple.cpp | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> ---
> base-commit: 183e37362f57ff3ce7493abf0bc6f1b57b931f55
> change-id: 20260507-kbingham-simple-scaler-crop-60b506e2bc4c
> 
> Best regards,
> -- 
> --
> Kieran
>