[{"id":11141,"web_url":"https://patchwork.libcamera.org/comment/11141/","msgid":"<20200703104303.GD5963@pendragon.ideasonboard.com>","date":"2020-07-03T10:43:03","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Implement digital zoom","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Thu, Jul 02, 2020 at 11:53:36AM +0100, David Plowman wrote:\n> These changes add a Digital Zoom control, tacking a rectangle as its\n> argument, indicating the region of the sensor output that will be\n> zoomed up to the final output size.\n> \n> Additionally, we need have a method returning the \"sensorCrop\" which\n> gives the dimensions of the sensor output within which we can pan and\n> zoom.\n\nThis is a bit of a ad-hoc function. Jacopo has submitted \"[PATCH v6]\nlibcamera: properties: Define pixel array properties\" some time ago that\naims at reporting the same information, and more, through properties. It\nhasn't been merged yet as we're still trying to make sure all the\ndetails are correct. Could you maybe review the properties in that\npatch, to see if they would suit your need, and if you think they\ndescribe the necessary information about the sensor in a good way ?\n\n> ---\n> include/libcamera/camera.h | 2 ++\n> include/libcamera/internal/pipeline_handler.h | 4 +++\n> src/libcamera/camera.cpp | 26 +++++++++++++++++++\n> src/libcamera/control_ids.yaml | 10 +++++++\n> 4 files changed, 42 insertions(+)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 9c0e58f..d57b606 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -92,6 +92,8 @@ public:\n> \tstd::unique_ptr generateConfiguration(const StreamRoles &roles);\n> \tint configure(CameraConfiguration *config);\n> \n> +\tSize const &getSensorCrop() const;\n> +\n> \tRequest *createRequest(uint64_t cookie = 0);\n> \tint queueRequest(Request *request);\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 22e629a..37e069a 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -89,6 +89,8 @@ public:\n> \n> \tconst char *name() const { return name_; }\n> \n> +\tSize const &getSensorCrop() const { return sensorCrop_; }\n> +\n> protected:\n> \tvoid registerCamera(std::shared_ptr camera,\n> \t\t\t std::unique_ptr data);\n> @@ -100,6 +102,8 @@ protected:\n> \n> \tCameraManager *manager_;\n> \n> +\tSize sensorCrop_;\n> +\n> private:\n> \tvoid mediaDeviceDisconnected(MediaDevice *media);\n> \tvirtual void disconnect();\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 69a1b44..6614c93 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -793,6 +793,32 @@ int Camera::configure(CameraConfiguration *config)\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Return the size of the sensor image being used to form the output\n> + *\n> + * This method returns the size, in pixels, of the raw image read from the\n> + * sensor and which is used to form the output image(s). Note that these\n> + * values take account of any cropping performed on the sensor output so\n> + * as to produce the correct aspect ratio. It would normally be necessary\n> + * to retrieve these values in order to calculate correct parameters for\n> + * digital zoom.\n> + *\n> + * Example: a sensor mode may produce a 1920x1440 output image. But if an\n> + * application has requested a 16:9 image, the values returned here would\n> + * be 1920x1080 - the largest portion of the sensor output that provides\n> + * the correct aspect ratio.\n> + *\n> + * \\context This function is \\threadsafe. It will only return valid\n> + * (non-zero) values when the camera has been configured.\n> + *\n> + * \\return The dimensions of the sensor image in use.\n> + */\n> +\n> +Size const &Camera::getSensorCrop() const\n> +{\n> +\treturn p_->pipe_->getSensorCrop();\n> +}\n> +\n> /**\n> * \\brief Create a request object for the camera\n> * \\param[in] cookie Opaque cookie for application use\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index 8c3e4c7..ac72e2a 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -251,4 +251,14 @@ controls:\n> higher than anyone could reasonably want. Negative values are\n> not allowed. Note also that sharpening is not applied to raw\n> streams.\n> +\n> + - DigitalZoom:\n> + type: Rectangle\n> + description: |\n> + Sets the portion of the full sensor image, in pixels, that will be\n> + used for digital zoom. That is, this part of the sensor output will\n> + be scaled up to make the full size output image (and everything else\n> + discarded). To obtain the \"full sensor image\" that is available, the\n> + method Camera::getOutputCrop() should be called once the camera is\n> + configured. An application may pan and zoom within this rectangle.\n> ...","headers":{"Return-Path":"","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 ABC83BFFE2\n\tfor ;\n\tFri, 3 Jul 2020 10:43:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 30A3560C58;\n\tFri, 3 Jul 2020 12:43:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D67EF603AE\n\tfor ;\n\tFri, 3 Jul 2020 12:43:07 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 59C8A51B;\n\tFri, 3 Jul 2020 12:43:07 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ru5mTW3r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593772987;\n\tbh=BXPH1fhZ2dLs1wGUXjCe5wAEr7wBoOHV4oEd6AeKn1M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ru5mTW3r+SU1QMFLFb2XDD5uFf/OobYXPv2FDSxsaghsR/OYzoz/8LVLqLkTUUyuB\n\tZd7jPVt/tUF9wC7Q4i2m76usvR0tQhJryA62JKgh0SfO9zUwXS+pLftNwNBu/dSCoP\n\tIKQ4ys9rKGUHtV1dHP+g+Q1Jn5Hk5EShCzbmP+cI=","Date":"Fri, 3 Jul 2020 13:43:03 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20200703104303.GD5963@pendragon.ideasonboard.com>","References":"<20200702105337.31161-1-david.plowman@raspberrypi.com>\n\t<20200702105337.31161-2-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200702105337.31161-2-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Implement digital zoom","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","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\" "}},{"id":11160,"web_url":"https://patchwork.libcamera.org/comment/11160/","msgid":"","date":"2020-07-03T15:01:54","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Implement digital zoom","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nThanks for pointing this out.\n\nOn Fri, 3 Jul 2020 at 11:43, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> Thank you for the patch.\n>\n> On Thu, Jul 02, 2020 at 11:53:36AM +0100, David Plowman wrote:\n> > These changes add a Digital Zoom control, tacking a rectangle as its\n> > argument, indicating the region of the sensor output that will be\n> > zoomed up to the final output size.\n> >\n> > Additionally, we need have a method returning the \"sensorCrop\" which\n> > gives the dimensions of the sensor output within which we can pan and\n> > zoom.\n>\n> This is a bit of a ad-hoc function. Jacopo has submitted \"[PATCH v6]\n> libcamera: properties: Define pixel array properties\" some time ago that\n> aims at reporting the same information, and more, through properties. It\n> hasn't been merged yet as we're still trying to make sure all the\n> details are correct. Could you maybe review the properties in that\n> patch, to see if they would suit your need, and if you think they\n> describe the necessary information about the sensor in a good way ?\n\nYes, this is a bit awkward. The pixel array properties define things\nthat are only a property of the pixel array, but here we have\nsomething that depends on the pixel array, the camera modes defined\nfrom it, and some choices made by the pipeline hander. To recap an\nearlier example (from the imx477).\n\nThe pixel array actually has 4056x3040 usable pixels, and let's\nsuppose we're asking for 1080p output.\n\nWe've defined a 2x2 binned 2028x1520 camera mode, and our pipeline\nhandler should choose this. It then has to pick a 16:9 rectangle to\nmatch the output aspect ratio, and this represents the \"unzoomed\"\nfield of view.\n\nIt might choose the largest field of view that it can, therefore a\n2028x1141 rectangle taken from offset (0,190) in the sensor's\n2028x1520 output.\n\nOr, if we're feeling mathematically anxious, a pipeline handler might\nprefer 1:1 pixels, and therefore give us a 1920x1080 rectangle, from\noffset (54,220).\n\nIn the first case, the \"sensorCrop\" returns 2028x1141, and in the\nsecond, 1920x1080.\n\nIt seems to me this number is the culmination of quite a few different\nproperties and decisions, and only the pipeline handler can work it\nout - I can't see that a fixed set of \"pixel array properties\" would\nbe enough. But I agree, a \"less arbitrary\" way to signal this would be\nnicer.\n\nPlan B was of course always to use ratios, rather than pixels. In this\nscheme you simply don't need this function, though you don't get\n\"precise pixel-level control\" if that's what you want.\n\nIndeed, suggestions would be appreciated...\n\nBest regards\nDavid\n\n>\n> > ---\n> > include/libcamera/camera.h | 2 ++\n> > include/libcamera/internal/pipeline_handler.h | 4 +++\n> > src/libcamera/camera.cpp | 26 +++++++++++++++++++\n> > src/libcamera/control_ids.yaml | 10 +++++++\n> > 4 files changed, 42 insertions(+)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 9c0e58f..d57b606 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -92,6 +92,8 @@ public:\n> > std::unique_ptr generateConfiguration(const StreamRoles &roles);\n> > int configure(CameraConfiguration *config);\n> >\n> > + Size const &getSensorCrop() const;\n> > +\n> > Request *createRequest(uint64_t cookie = 0);\n> > int queueRequest(Request *request);\n> >\n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index 22e629a..37e069a 100644\n> > --- a/include/libcamera/internal/pipeline_handler.h\n> > +++ b/include/libcamera/internal/pipeline_handler.h\n> > @@ -89,6 +89,8 @@ public:\n> >\n> > const char *name() const { return name_; }\n> >\n> > + Size const &getSensorCrop() const { return sensorCrop_; }\n> > +\n> > protected:\n> > void registerCamera(std::shared_ptr camera,\n> > std::unique_ptr data);\n> > @@ -100,6 +102,8 @@ protected:\n> >\n> > CameraManager *manager_;\n> >\n> > + Size sensorCrop_;\n> > +\n> > private:\n> > void mediaDeviceDisconnected(MediaDevice *media);\n> > virtual void disconnect();\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 69a1b44..6614c93 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -793,6 +793,32 @@ int Camera::configure(CameraConfiguration *config)\n> > return 0;\n> > }\n> >\n> > +/**\n> > + * \\brief Return the size of the sensor image being used to form the output\n> > + *\n> > + * This method returns the size, in pixels, of the raw image read from the\n> > + * sensor and which is used to form the output image(s). Note that these\n> > + * values take account of any cropping performed on the sensor output so\n> > + * as to produce the correct aspect ratio. It would normally be necessary\n> > + * to retrieve these values in order to calculate correct parameters for\n> > + * digital zoom.\n> > + *\n> > + * Example: a sensor mode may produce a 1920x1440 output image. But if an\n> > + * application has requested a 16:9 image, the values returned here would\n> > + * be 1920x1080 - the largest portion of the sensor output that provides\n> > + * the correct aspect ratio.\n> > + *\n> > + * \\context This function is \\threadsafe. It will only return valid\n> > + * (non-zero) values when the camera has been configured.\n> > + *\n> > + * \\return The dimensions of the sensor image in use.\n> > + */\n> > +\n> > +Size const &Camera::getSensorCrop() const\n> > +{\n> > + return p_->pipe_->getSensorCrop();\n> > +}\n> > +\n> > /**\n> > * \\brief Create a request object for the camera\n> > * \\param[in] cookie Opaque cookie for application use\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index 8c3e4c7..ac72e2a 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -251,4 +251,14 @@ controls:\n> > higher than anyone could reasonably want. Negative values are\n> > not allowed. Note also that sharpening is not applied to raw\n> > streams.\n> > +\n> > + - DigitalZoom:\n> > + type: Rectangle\n> > + description: |\n> > + Sets the portion of the full sensor image, in pixels, that will be\n> > + used for digital zoom. That is, this part of the sensor output will\n> > + be scaled up to make the full size output image (and everything else\n> > + discarded). To obtain the \"full sensor image\" that is available, the\n> > + method Camera::getOutputCrop() should be called once the camera is\n> > + configured. An application may pan and zoom within this rectangle.\n> > ...\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","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 55735BFFE2\n\tfor ;\n\tFri, 3 Jul 2020 15:02:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C341160C5C;\n\tFri, 3 Jul 2020 17:02:07 +0200 (CEST)","from mail-ot1-x342.google.com (mail-ot1-x342.google.com\n\t[IPv6:2607:f8b0:4864:20::342])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D95C3603AD\n\tfor ;\n\tFri, 3 Jul 2020 17:02:05 +0200 (CEST)","by mail-ot1-x342.google.com with SMTP id d4so26915140otk.2\n\tfor ;\n\tFri, 03 Jul 2020 08:02:05 -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=\"PrFTdY3V\"; 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=wTMMhUQS/mGOWkJgET4nSdAbpKXuBChgvbeqjDWwB7c=;\n\tb=PrFTdY3V2SLP7ACiIGgIDezZWVKtVdpoDgwVDgjOHrZerk1PaZMJ3iaRRkZSZuHm5L\n\tgFt8D+yXphXsN5ji2NXKU4y8z7kzIQJM3hI2ygU41zSESU/TZu0f1wq4TkeXeP7OnxPs\n\tNVcXfHLEIkHPYhy/9sNuVpv0CpD4begNMoeE4GC2icBY80CaBghq4cwAq8QDP/oWUOm+\n\tJF7JvTDCT486n+WQpZq2McExWRCGsd9l7ugi7eEb0WIfijSvvG6/7mhB2DspPlXHxVcv\n\tUJoujL2FxVaB3XCzLajiZvlzvuTozZ++fAahTTVm1Xv+NuL5NekCTnmtQ407l2onMI81\n\tzgYg==","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=wTMMhUQS/mGOWkJgET4nSdAbpKXuBChgvbeqjDWwB7c=;\n\tb=CzN6dmCquCX7PGNYSBJOHTAkS5iXugJbPyyym/dB/uG2K3wL0csxguuSbebpr19WAv\n\tvkDfA4MamEZSwgrH2Ta0yHPiTBCPyzhrwfHfOXOvadQfoLXwlUKLYzut0kgCwpsoh5M+\n\tWRVcPAWkRO+qYwxKsP2yGdZK34AgBStLwx8sfd/jtcCF2ZI5hJWV4mobsqrczQxc/pYL\n\tmboJTcPQLPwqTzReOk0E9gccCOeqrny0qoP0CJmDCDiodB6rhXkZj/0DaUWMqiGBFm4n\n\tO8/8zQgMVne9Xcwc3Pg79Gd0MhmL06MlQ1C3vHQMIivIl925FqBMTe7ISfFBzBGUUd3W\n\tpjIQ==","X-Gm-Message-State":"AOAM532KVzKquLxGC6iDtWjxqfbCEdFT9h0aoTgwQr5BhCyKlPEG1TQq\n\t0Oa6fVmFCUmhCLdiX0EgVLbCwIPwGgG/9p3SX1DizQ==","X-Google-Smtp-Source":"ABdhPJxh6yLvv2TGOVmUhDTgvrhL1st7eXG04c0ddxviDDYhSZSeofRdATf/ggF0kD3MIY+lJDuTDl+FScFKIIzepEQ=","X-Received":"by 2002:a9d:6b19:: with SMTP id\n\tg25mr32551184otp.160.1593788524349; \n\tFri, 03 Jul 2020 08:02:04 -0700 (PDT)","MIME-Version":"1.0","References":"<20200702105337.31161-1-david.plowman@raspberrypi.com>\n\t<20200702105337.31161-2-david.plowman@raspberrypi.com>\n\t<20200703104303.GD5963@pendragon.ideasonboard.com>","In-Reply-To":"<20200703104303.GD5963@pendragon.ideasonboard.com>","From":"David Plowman ","Date":"Fri, 3 Jul 2020 16:01:54 +0100","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Implement digital zoom","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","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\" "}}]