[libcamera-devel] [PATCH v2] libcamera: properties: Re-define ScalerCropMaximum
Jacopo Mondi
jacopo at jmondi.org
Tue Dec 1 10:35:12 CET 2020
Hi David,
On Mon, Nov 30, 2020 at 02:34:03PM +0000, David Plowman wrote:
> Hi Jacopo
>
> Thanks for the updated patch!
>
> On Mon, 30 Nov 2020 at 12:44, 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.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > 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
>
> Perfect!
>
> >
> > ---
> > 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, they're 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 });
> >
>
> Looks good to me!
>
> > 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..97904a666c0d 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 produced the final image streams. The largest possible crop
>
> s/produced/produce/
>
> > + 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
>
> Still seem to be missing my comma after up-scale!
Ouch!
>
> > + the size of sensor pixel array portion used to produce the smallest
> > + available stream resolution, accounting any applied pixel sub-sampling
>
> s/accounting/accounting for/
>
> > + 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 pixel array portion that produces the smallest
>
> s/to/to the/
>
> Apart from the little grammar things:
>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
Thanks, will collect your suggestions on v3.
Thanks
j
>
> Thanks and best regards
> David
>
> > + 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
> >
More information about the libcamera-devel
mailing list