[libcamera-devel] [PATCH v4 1/5] libcamera: Add SensorCropMaximum property
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Oct 22 08:17:52 CEST 2020
Hi David,
An additional comment.
On Thu, Oct 22, 2020 at 09:04:28AM +0300, Laurent Pinchart wrote:
> Hi David,
>
> Thank you for the patch.
>
> On Mon, Oct 19, 2020 at 01:51:52PM +0100, David Plowman wrote:
> > The SensorCropMaximum camera property reports the location of that
> > part of the image sensor array that is scaled to produce the output
> > images, given in native sensor pixels. It will normally change when a
> > new camera mode is selected, and can be used to implement digital
> > zoom.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> > src/libcamera/property_ids.yaml | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index 7261263a..022cf65d 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -663,4 +663,24 @@ controls:
> > \todo Rename this property to ActiveAreas once we will have property
> > categories (i.e. Properties::PixelArray::ActiveAreas)
> >
> > + - ScalerCropMaximum:
> > + type: Rectangle
> > + description: |
> > + The size and location, in native sensor pixels, of the part of the
> > + sensor that is rescaled to produce the output images. Note that the
> > + units remain native sensor pixels, even if the sensor is being used in
> > + a binning skipping or scaling mode.
>
> Now that I've reviewed 5/5 I think we agree on the meaning of this
> property, but the text here doesn't seem to correctly match what I
> understand is our agreement :-) The above paragraph is actually what I
> was expecting for the ScalerCrop control, not the ScalerCropMaximum
> property.
>
> How about simply defining this property as the maximum value for
> ScalerCrop ? It could be written as just
>
> The maximum valid rectangle for the controls::ScalerCrop control.
This should be expanded a little bit, as, after reviewing the ScalerCrop
control, the second paragraph of this property doesn't fit there.
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.
(I wouldn't mention CSI-2 receiver explicitly as we may be dealing with
a parallel sensor.)
> with the last two paragraphs kept. The rest would then be moved to the
> documentation of ScalerCrop (if they're not there already, I'll review
> that patch next).
>
> If you agree with this, and with the commit message updated too,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > +
> > + The (x,y) location of this rectangle is relative to the
> > + PixelArrayActiveArea that is being used. The property also takes into
> > + account any further cropping being done by the CSI-2 receiver or
> > + elsewhere.
> > +
> > + This property is valid only after the Camera has been successfully
> > + configured and its value changes 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.
> > +
> > ...
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list