<div dir="ltr"><div>Hi Jacopo</div><div><br></div><div>Thanks for the reply.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 21 Sep 2020 at 14:02, Jacopo Mondi <<a href="mailto:jacopo@jmondi.org">jacopo@jmondi.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi David,<br>
  super nit: in Subject: "libcamera: controls: Add SensorCrop"<br>
<br>
On Mon, Sep 07, 2020 at 05:44:48PM +0100, David Plowman wrote:<br>
> The SensorCrop control selects how much of the sensor's output image<br>
> will be scaled to form the output image. It can be used to implement<br>
> digital zoom.<br>
> ---<br>
>  src/libcamera/control_ids.yaml | 9 +++++++++<br>
>  1 file changed, 9 insertions(+)<br>
><br>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml<br>
> index 3560d4a..cebaa25 100644<br>
> --- a/src/libcamera/control_ids.yaml<br>
> +++ b/src/libcamera/control_ids.yaml<br>
> @@ -273,4 +273,13 @@ controls:<br>
>          order in an array of 9 floating point values.<br>
><br>
>        size: [3x3]<br>
> +<br>
> +  - SensorCrop:<br>
<br>
I wonder, is this a Sensor related property ? Roughly speaking that's<br>
a selection on the input frame that is provided to the ISP for<br>
processing, if I think about the possibly forthcoming namespacing of<br>
the controls IDs I don't see this being well placed as Sensor::Crop<br>
(but rather as ISP::Crop possibly ?)<br>
<br>
I recall you had 'PipelineCrop' in previous version, am I wrong ?<br></blockquote><div><br></div><div>Hehe. Every time I come back to the topic of digital zoom it seems to change its name! Yes, it was PipelineCrop before, now it's SensorCrop but I can go with IspCrop (becoming ISP::Crop) too.  :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +      type: Rectangle<br>
> +      description: |<br>
> +        Sets the portion of the full sensor image, in pixels, that will be<br>
> +        scaled up to form the whole of the final output image. This control<br>
<br>
As a suggestion, take whatever you like in:<br>
<br>
           Selection rectangle (in pixel units) that defines the<br>
           portion of the image that will be scaled up to form the<br>
           final output image. This control can be used to implement<br>
           digital zoom.<br>
<br>
> +        can be used to implement digital zoom. The size of the full sensor<br>
> +        image within which an application can crop is available from the<br>
> +        SensorOutputSize property.<br>
<br>
           The rectangle is defined in respect to the size of the<br>
           image which is processed by the ISP to produce the output<br>
           streams, whose size is reported by the SensorOutputSize<br>
           property.<br>
<br>
           \sa properties::SensorOutputSize<br>
<br>
How bad is it in your opinion that this will apply to all streams ? I<br>
don't see many ways around and to me it's fine, but maybe we have<br>
different expectations ?<br></blockquote><div><br></div><div>Well, I'm OK with it too, I think it makes life really quite complicated if different output streams have different FOVs. I certainly know of ISPs that can do this, but I don't think I've ever found myself working on a camera application and thinking "I really need this feature". Generally speaking I think you want the same image - just in a different resolution, or colour space or format, something like that.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
How does it work if the Rectangle sizes are larger than the input<br>
frame sizes ? Should we enforce a behaviour like always clamping the<br>
SensorCrop size to the SensorOutputSize ones ?<br></blockquote><div><br></div><div>In the Raspberry Pi pipeline handler I always use the Rectangle::clamp method, so it forces it to fit whatever the application passes in. This seems fair behaviour to me - application gives you rubbish, coerce it to the nearest reasonable thing and report what you did - and is, I think, more helpful than reporting an error and failing.</div><div><br></div><div>Thanks also for the other suggestions!</div><div><br></div><div>Best regards</div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks<br>
  j<br>
<br>
<br>
>  ...<br>
> --<br>
> 2.20.1<br>
><br>
> _______________________________________________<br>
> libcamera-devel mailing list<br>
> <a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a><br>
> <a href="https://lists.libcamera.org/listinfo/libcamera-devel" rel="noreferrer" target="_blank">https://lists.libcamera.org/listinfo/libcamera-devel</a><br>
</blockquote></div></div>