[libcamera-devel] [PATCH v2 06/11] libcamera: ipu3: Register ScalerCrop control

[Jacopo Mondi]

Hi Laurent,

On Mon, Jan 25, 2021 at 01:31:09PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> > +	/*
> > +	 * Re-scale in the sensor's native coordinates. Report (0,0) as
> > +	 * top-left corner as we allow application to feely pan the crop area.
>
> s/feely/freely/
>
> > +	 */
> > +	Rectangle minCrop(minSize);
> > +	minCrop.scaledBy(analogueCrop.size(), sensorInfo.outputSize);
>
> scaledBy() is a const function that returns a new rectangle. You
> probably want scaleBy() here (or see below for an alternative).
>
> > +	minCrop.x = 0;
> > +	minCrop.y = 0;
>
> (x,y) are set to 0 by the constructor, so you could write
>
> 	Rectangle minCrop = minSize.scaledBy(analogueCrop.size(),
> 					     sensorInfo.outputSize);

I went for the minCrop initialization as 'scaledBy' is not a Size
class method.

In facts, if preferred, I could work around it with
       Rectangle minCrop = Rectangle(minSize).scaledBy(analogueCrop.size(),
                                              sensorInfo.outputSize);

Which I don't mind

>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > +
> > +	controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);
> > +
> >  	data->controlInfo_ = std::move(controls);
> >
> >  	return 0;
>
> --
> Regards,
>
> Laurent Pinchart