[libcamera-devel] [PATCH v3] libcamera: properties: Re-define ScalerCropMaximum

[David Plowman]

Hi Jacopo

Thanks for version 3 - just confirming that it all looks good to me now!

Best regards
David

On Tue, 1 Dec 2020 at 10:06, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Currently the properties::ScalerCropMaximum property reports the maximum
> valid Rectangle for the controls::ScalerCrop control, which can be
> combined with the sensor pixel array size in order to obtain the minimum
> available zoom factor.
>
> It is also useful to be able to calculate the maximum available zoom
> factor, and in order to do so the minimum valid crop rectangle has to
> be reported.
>
> Transform the ScalerCropMaximum property in ScalerCropLimits, which
> reports both the maximum and minimum crop limits and port the only user
> of such a property (Raspberry Pi) to use it.
>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> v2 -> v3:
> - Fix grammar issues pointed out by David
> - Add David's tag
>
> v1 -> v2:
> - Re-based on master
> - Specify that the top-left corner of the minimum rectangle is not relevant
> - RPi: scale ispMinCropSize_ in the sensor's pixel array matrix coordinates
>   Tested with 640x480 2x2 binned mode -> min = 128x128
>   Tested with 3280x2464 non binned mode -> min = 64x64
> ---
>  src/libcamera/control_ids.yaml                |  2 +-
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 17 +++++---
>  src/libcamera/property_ids.yaml               | 42 ++++++++++++++-----
>  3 files changed, 45 insertions(+), 16 deletions(-)
>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index a883e27e22e9..44c4bb8e86b6 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -283,7 +283,7 @@ controls:
>          a binning or skipping mode.
>
>          This control is only present when the pipeline supports scaling. Its
> -        maximum valid value is given by the properties::ScalerCropMaximum
> +        valid value limits are given by the properties::ScalerCropLimits
>          property, and the two can be used to implement digital zoom.
>
>    # ----------------------------------------------------------------------------
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 6fcdf557afcf..22d37e7f1ea1 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -708,13 +708,19 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>                 LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
>
>         /*
> -        * Update the ScalerCropMaximum to the correct value for this camera mode.
> -        * For us, it's the same as the "analogue crop".
> +        * Update the ScalerCropLimits to the correct values for this camera
> +        * mode. For us, it's the same as the "analogue crop" and pixel array
> +        * portion used to produce the ispMinCropSize_;
>          *
>          * \todo Make this property the ScalerCrop maximum value when dynamic
>          * controls are available and set it at validate() time
>          */
> -       data->properties_.set(properties::ScalerCropMaximum, data->sensorInfo_.analogCrop);
> +       Rectangle sensorMinCrop =
> +               Rectangle(data->ispMinCropSize_).scaledBy(
> +                               data->sensorInfo_.analogCrop.size(),
> +                               data->sensorInfo_.outputSize);
> +       data->properties_.set(properties::ScalerCropLimits,
> +                             { data->sensorInfo_.analogCrop, sensorMinCrop });
>
>         return ret;
>  }
> @@ -946,11 +952,12 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>         data->properties_ = data->sensor_->properties();
>
>         /*
> -        * Set a default value for the ScalerCropMaximum property to show
> +        * Set default values for the ScalerCropLimits property to show
>          * that we support its use, however, initialise it to zero because
>          * it's not meaningful until a camera mode has been chosen.
>          */
> -       data->properties_.set(properties::ScalerCropMaximum, Rectangle{});
> +       data->properties_.set(properties::ScalerCropLimits,
> +                             { Rectangle{}, Rectangle{} });
>
>         /*
>          * We cache three things about the sensor in relation to transforms
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 64e88f0361d6..76474bfc5c31 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -663,19 +663,41 @@ controls:
>          \todo Rename this property to ActiveAreas once we will have property
>                categories (i.e. Properties::PixelArray::ActiveAreas)
>
> -  - ScalerCropMaximum:
> +  - ScalerCropLimits:
>        type: Rectangle
> +      size: [2]
>        description: |
> -        The maximum valid rectangle for the controls::ScalerCrop control. This
> -        reflects the minimum mandatory cropping applied in the camera sensor and
> -        the rest of the pipeline. Just as the ScalerCrop control, it defines a
> -        rectangle taken from the sensor's active pixel array.
> -
> -        This property is valid only after the camera has been successfully
> -        configured and its value may change whenever a new configuration is
> +        The maximum and minimum valid rectangles for the controls::ScalerCrop
> +        control, specified in this order.
> +
> +        This two rectangles respectively reflect the minimum and maximum
> +        mandatory cropping applied in the camera sensor and the rest of the
> +        pipeline. Both Rectangles are defined relatively to the sensor's active
> +        pixel array (properties::PixelArrayActiveAreas).
> +
> +        Implementation details for the maximum valid crop rectangle.
> +        The maximum valid crop rectangle depends on the camera configuration as
> +        pipelines are free to adjust the frame size requested from the sensor
> +        and used to produce the final image streams. The largest possible crop
> +        rectangle is then limited by the size of the sensor's active pixel area
> +        portion used to produce the sensor output frame.
> +
> +        Implementation details for the minimum valid crop rectangle. If a
> +        pipeline cannot up-scale, the minimum valid crop rectangle is equal to
> +        the size of sensor pixel array portion used to produce the smallest
> +        available stream resolution, accounting for any applied pixel
> +        sub-sampling technique in use and including any border pixels required
> +        by the ISP for image processing. If a pipeline can up-scale, the minimum
> +        valid crop rectangle is equal to the pixel array portion that produces
> +        the smallest frame size accepted by the ISP input. The rectangle
> +        position, defined by its top-left corner, is not relevant and should be
> +        set to (0, 0) by pipelines.
> +
> +        The two rectangles are valid only after the camera has been successfully
> +        configured and their value may change whenever a new configuration is
>          applied.
>
> -        \todo Turn this property into a "maximum control value" for the
> -        ScalerCrop control once "dynamic" controls have been implemented.
> +        \todo Express this property as the limits for the ScalerCrop control
> +        once "dynamic" controls have been implemented.
>
>  ...
> --
> 2.29.1
>