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

26677 mbox series

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,

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
>

Hi Kieran,

Quoting Kieran Bingham (2026-05-08 00: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.

I wrote three versions of this reply arguing with myself and I think I've
finally reached a conclusion. I'll document all three of my thoughts for
posterity...

The first gut feeling is that this feels like a hack. I was going to reference
the ScalerCrop documentation but it only mentions what happens to it as a
control, and not what happens to it as metadata.

Regardless, it feels like this conflates ScalerCrop as "the metadata that
describes what scaling and cropping was applied by the pipeline" with "metadata
to describe the sensor crop". It's still useful information to report, but it
might be better in a different property.

Well, I suppose if ScalerCrop can be implemented completely by sensor scaling
or cropping then maybe this could work as-is... But hold on, the libcamera
ScalerCrop is defined as being relative to PixelArrayActiveAreas. So it's a
combination of all scaling and cropping done in the *entire pipeline*,
including the sensor.

If there are multiple points in the pipeline where scaling and cropping are
done, then they are all summed together into the final ScalerCrop metadata; in
the case of the control then it would be split into each portion of the
pipeline that does part of the scaler crop. And currently many sensors have a
crop in them already (depending on the "best resolution") and we have to take
into account the sensor crop to adjust the ScalerCrop control/metadata compared
to what is done by other components in the pipeline.

So in conclusion I think it is actually fair to use the ScalerCrop for this
purpose. It won't be available as a control as it's not controlable, but the
sensor crop is indeed *a part of the pipeline* in which "scaling or cropping"
is done relative to PixelArrayActiveAreas.

We'll need to update the control documentation to cover nuances in the metadata
case though.


Paul

> 
> 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,
> -- 
> --
> Kieran
>