[libcamera-devel] [PATCH v5 2/5] libcamera: Add SensorCropMaximum property

[Laurent Pinchart]

Hi Jacopo,
On Sat, Oct 24, 2020 at 09:17:15PM +0200, Jacopo Mondi wrote:
> On Sat, Oct 24, 2020 at 06:43:44PM +0200, Jacopo Mondi wrote:
> > On Fri, Oct 23, 2020 at 11:21:56AM +0100, David Plowman wrote:
> > > The SensorCropMaximum camera property reports the location of that
> >
> > s/that/the ?
> >
> > > 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>
> >
> > Nice work! Thank you and Laurent for pushing this forward!
> >
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> >
> > > ---
> > >  src/libcamera/property_ids.yaml | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > > index 7261263a..a306a422 100644
> > > --- a/src/libcamera/property_ids.yaml
> > > +++ b/src/libcamera/property_ids.yaml
> > > @@ -663,4 +663,18 @@ controls:
> > >          \todo Rename this property to ActiveAreas once we will have property
> > >                categories (i.e. Properties::PixelArray::ActiveAreas)
> > >
> > > +  - ScalerCropMaximum:
> > > +      type: Rectangle
> > > +      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.
> 
> Looking at the RPi implementation I had to get back here to check what
> the reference of the property is and found it only mentioned in the
> commit message. Should we mention it is relative to the full pixel
> array as it's done for ScalerCrop ?

I think it's important to document it clearly for the ScalerCrop
property. ScalerCropMaximum is then defined based on the ScalerCrop
definition, so I don't consider it necessary to duplicate the
information here. I don't mind much though (and this property is meant
to be temporary anyway).

> > > +
> > > +        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