[{"id":12625,"web_url":"https://patchwork.libcamera.org/comment/12625/","msgid":"<20200921130648.m7s2k4p3btqpijwq@uno.localdomain>","date":"2020-09-21T13:06:48","subject":"Re: [libcamera-devel] [RFC PATCH 2/4] libcamera: Add SensorCrop\n\tcontrol","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n  super nit: in Subject: \"libcamera: controls: Add SensorCrop\"\n\nOn Mon, Sep 07, 2020 at 05:44:48PM +0100, David Plowman wrote:\n> The SensorCrop control selects how much of the sensor's output image\n> will be scaled to form the output image. It can be used to implement\n> digital zoom.\n> ---\n>  src/libcamera/control_ids.yaml | 9 +++++++++\n>  1 file changed, 9 insertions(+)\n>\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index 3560d4a..cebaa25 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -273,4 +273,13 @@ controls:\n>          order in an array of 9 floating point values.\n>\n>        size: [3x3]\n> +\n> +  - SensorCrop:\n\nI wonder, is this a Sensor related property ? Roughly speaking that's\na selection on the input frame that is provided to the ISP for\nprocessing, if I think about the possibly forthcoming namespacing of\nthe controls IDs I don't see this being well placed as Sensor::Crop\n(but rather as ISP::Crop possibly ?)\n\nI recall you had 'PipelineCrop' in previous version, am I wrong ?\n\n> +      type: Rectangle\n> +      description: |\n> +        Sets the portion of the full sensor image, in pixels, that will be\n> +        scaled up to form the whole of the final output image. This control\n\nAs a suggestion, take whatever you like in:\n\n           Selection rectangle (in pixel units) that defines the\n           portion of the image that will be scaled up to form the\n           final output image. This control can be used to implement\n           digital zoom.\n\n> +        can be used to implement digital zoom. The size of the full sensor\n> +        image within which an application can crop is available from the\n> +        SensorOutputSize property.\n\n           The rectangle is defined in respect to the size of the\n           image which is processed by the ISP to produce the output\n           streams, whose size is reported by the SensorOutputSize\n           property.\n\n           \\sa properties::SensorOutputSize\n\nHow bad is it in your opinion that this will apply to all streams ? I\ndon't see many ways around and to me it's fine, but maybe we have\ndifferent expectations ?\n\nHow does it work if the Rectangle sizes are larger than the input\nframe sizes ? Should we enforce a behaviour like always clamping the\nSensorCrop size to the SensorOutputSize ones ?\n\nThanks\n  j\n\n\n>  ...\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7FC3CC3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Sep 2020 13:02:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 14F0562FD3;\n\tMon, 21 Sep 2020 15:02:57 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5734462B90\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Sep 2020 15:02:56 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id D659720013;\n\tMon, 21 Sep 2020 13:02:55 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 21 Sep 2020 15:06:48 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200921130648.m7s2k4p3btqpijwq@uno.localdomain>","References":"<20200907164450.13082-1-david.plowman@raspberrypi.com>\n\t<20200907164450.13082-3-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200907164450.13082-3-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 2/4] libcamera: Add SensorCrop\n\tcontrol","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12627,"web_url":"https://patchwork.libcamera.org/comment/12627/","msgid":"<CAHW6GY+fJQwSRbvRZmovXMED4YREMGKuPSVdbCiMpT2YKywcTw@mail.gmail.com>","date":"2020-09-21T13:37:02","subject":"Re: [libcamera-devel] [RFC PATCH 2/4] libcamera: Add SensorCrop\n\tcontrol","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for the reply.\n\nOn Mon, 21 Sep 2020 at 14:02, Jacopo Mondi <jacopo@jmondi.org> wrote:\n\n> Hi David,\n>   super nit: in Subject: \"libcamera: controls: Add SensorCrop\"\n>\n> On Mon, Sep 07, 2020 at 05:44:48PM +0100, David Plowman wrote:\n> > The SensorCrop control selects how much of the sensor's output image\n> > will be scaled to form the output image. It can be used to implement\n> > digital zoom.\n> > ---\n> >  src/libcamera/control_ids.yaml | 9 +++++++++\n> >  1 file changed, 9 insertions(+)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml\n> b/src/libcamera/control_ids.yaml\n> > index 3560d4a..cebaa25 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -273,4 +273,13 @@ controls:\n> >          order in an array of 9 floating point values.\n> >\n> >        size: [3x3]\n> > +\n> > +  - SensorCrop:\n>\n> I wonder, is this a Sensor related property ? Roughly speaking that's\n> a selection on the input frame that is provided to the ISP for\n> processing, if I think about the possibly forthcoming namespacing of\n> the controls IDs I don't see this being well placed as Sensor::Crop\n> (but rather as ISP::Crop possibly ?)\n>\n> I recall you had 'PipelineCrop' in previous version, am I wrong ?\n>\n\nHehe. Every time I come back to the topic of digital zoom it seems to\nchange its name! Yes, it was PipelineCrop before, now it's SensorCrop but I\ncan go with IspCrop (becoming ISP::Crop) too.  :)\n\n\n>\n> > +      type: Rectangle\n> > +      description: |\n> > +        Sets the portion of the full sensor image, in pixels, that will\n> be\n> > +        scaled up to form the whole of the final output image. This\n> control\n>\n> As a suggestion, take whatever you like in:\n>\n>            Selection rectangle (in pixel units) that defines the\n>            portion of the image that will be scaled up to form the\n>            final output image. This control can be used to implement\n>            digital zoom.\n>\n> > +        can be used to implement digital zoom. The size of the full\n> sensor\n> > +        image within which an application can crop is available from the\n> > +        SensorOutputSize property.\n>\n>            The rectangle is defined in respect to the size of the\n>            image which is processed by the ISP to produce the output\n>            streams, whose size is reported by the SensorOutputSize\n>            property.\n>\n>            \\sa properties::SensorOutputSize\n>\n> How bad is it in your opinion that this will apply to all streams ? I\n> don't see many ways around and to me it's fine, but maybe we have\n> different expectations ?\n>\n\nWell, I'm OK with it too, I think it makes life really quite complicated if\ndifferent output streams have different FOVs. I certainly know of ISPs that\ncan do this, but I don't think I've ever found myself working on a camera\napplication and thinking \"I really need this feature\". Generally speaking I\nthink you want the same image - just in a different resolution, or colour\nspace or format, something like that.\n\n\n>\n> How does it work if the Rectangle sizes are larger than the input\n> frame sizes ? Should we enforce a behaviour like always clamping the\n> SensorCrop size to the SensorOutputSize ones ?\n>\n\nIn the Raspberry Pi pipeline handler I always use the Rectangle::clamp\nmethod, so it forces it to fit whatever the application passes in. This\nseems fair behaviour to me - application gives you rubbish, coerce it to\nthe nearest reasonable thing and report what you did - and is, I think,\nmore helpful than reporting an error and failing.\n\nThanks also for the other suggestions!\n\nBest regards\nDavid\n\n\n>\n> Thanks\n>   j\n>\n>\n> >  ...\n> > --\n> > 2.20.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 45474BF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Sep 2020 13:37:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE84762FD3;\n\tMon, 21 Sep 2020 15:37:18 +0200 (CEST)","from mail-ot1-x341.google.com (mail-ot1-x341.google.com\n\t[IPv6:2607:f8b0:4864:20::341])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A1F3062B90\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Sep 2020 15:37:17 +0200 (CEST)","by mail-ot1-x341.google.com with SMTP id y5so12332581otg.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Sep 2020 06:37:17 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"e7o/nssX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=MKoFHef3OLICLTJRAl7v6X6m8qQj41btD1/uEOP3n2U=;\n\tb=e7o/nssX41ZtRQe4GtF0A4iXjZy5jYTtzv13h2RHSxCbW24G8xrT1ZwxzLz6BVI8Bc\n\twxvSjHLyJuJjGJc6aun22JodQJ9wVotprlmT2jrv5TfWznkMSG1NnIzP5omu8X9FePDd\n\tCvO0nz1vmNoPKVXsp9D/Crtxyt8K+BcioAS5/AK7O6YIHEEvx3PrpOx9BMa/Z39e9Ilx\n\t9jXWrMh1mNj0sskME46cxPuhKBrw0pZMrVVJC1eHvkOkptBTq4+MXY30dnRzlVyyNhnm\n\tcweYPkJXptiAuuQ+ZuJb/uRsqijHZZ3OwMLIDi3/86+D/h5QPD/Jnn6rJcEHv6Szm2T4\n\tNbwA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=MKoFHef3OLICLTJRAl7v6X6m8qQj41btD1/uEOP3n2U=;\n\tb=eSwy3ohh8ecH0VlteJeHUJPjiySyQKj7FJzE5eddT57l9ZyuweN9H/8+usDR0iXd1Y\n\t9yBFksU5R51DK4Ay8OFO9bspSr8eQSw8GcuCFNwIBqSUyCLGRHnPD/A+HMh2pI3fJbra\n\te8JVntRkx6iQgR5mKqBt6tuDc0x9IV1Mmka6EnzzQj5fxJWwh2E068sql/4jaFbNVXjd\n\tNiQf/zMzqFQWC53LvCgMI3slI1GXxdRRmL8Rd/brnNxAfGDhS65RSTG3guTYY1UYBlGf\n\t5piMhWs/Q1IWdPYzUCFWxydAf4NzigBfxIOrp/Y5tJL5rjwGINHdfCRmnn3D6JeZXVBH\n\thkmg==","X-Gm-Message-State":"AOAM531Ja2quVfU+DUj/+iqVFr35ffuAvJgPPvKzyzL9yF7f/S1G97w1\n\taMdpyUMnw8T0EdI4i7QLA+LX7ceiRVdzQLNI5Wqfms1GQ5Q=","X-Google-Smtp-Source":"ABdhPJyOxbdN+GtGZx03CFYWN2TrRmRqIb0s6BwhMNSpJ7ahn+T5MSL13TK7fB+xHOojubBNR+dTdqHda4D4Nf2Gtbo=","X-Received":"by 2002:a9d:7dd8:: with SMTP id\n\tk24mr32552980otn.160.1600695436286; \n\tMon, 21 Sep 2020 06:37:16 -0700 (PDT)","MIME-Version":"1.0","References":"<20200907164450.13082-1-david.plowman@raspberrypi.com>\n\t<20200907164450.13082-3-david.plowman@raspberrypi.com>\n\t<20200921130648.m7s2k4p3btqpijwq@uno.localdomain>","In-Reply-To":"<20200921130648.m7s2k4p3btqpijwq@uno.localdomain>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Mon, 21 Sep 2020 14:37:02 +0100","Message-ID":"<CAHW6GY+fJQwSRbvRZmovXMED4YREMGKuPSVdbCiMpT2YKywcTw@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [RFC PATCH 2/4] libcamera: Add SensorCrop\n\tcontrol","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============5161526751406003137==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]