[{"id":11337,"web_url":"https://patchwork.libcamera.org/comment/11337/","msgid":"","date":"2020-07-10T13:40:51","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: raspberrypi:\n\tImplement digital zoom","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi David,\n\nI've just been chatting briefly with Laurent and he has mentioned that\nthis topic should be integrated with the work that Jacopo has done on\nPixel Array properties.\n\nI haven't looked at that work yet, so I'm not (yet) in a position to\ncomment on that directly, but as I'd already written some generic\n(libcamera) framework comments about the Rectangle class below, I\nthought I should send this now...\n\n\n\nOn 09/07/2020 10:15, David Plowman wrote:\n> These changes implement digital zoom for the Raspberry Pi. We detect\n> the digital zoom control and update the V4L2 \"selection\" before\n> starting the ISP. We also update the value in the control that we send\n> to the IPA, so that it has the correct value.\n> ---\n> include/libcamera/ipa/raspberrypi.h | 1 +\n> src/ipa/raspberrypi/raspberrypi.cpp | 10 ++++\n> .../pipeline/raspberrypi/raspberrypi.cpp | 56 ++++++++++++++++++-\n> 3 files changed, 66 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index a18ce9a..e66402e 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -52,6 +52,7 @@ static const ControlInfoMap RPiControls = {\n> \t{ &controls::Contrast, ControlInfo(0.0f, 32.0f) },\n> \t{ &controls::Saturation, ControlInfo(0.0f, 32.0f) },\n> \t{ &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> +\t{ &controls::DigitalZoom, ControlInfo(Rectangle{ 0, 0, 0, 0 }, Rectangle{ 65535, 65535, 65535, 65535 }, Rectangle{ 0, 0, 0, 0 }) },\n\nI wonder if we should have some constructor helpers to make that easier.\n (Rectangle(0), Rectangle(65535), Rectangle(0))\n\n\nOddly, (and this is not a fault of this patch), the idea of a Rectangle\nwhich starts at x,y = {65535,65535}, and is of size {65535,65535} seems\nwrong, but of course this is jsut storage of maximum values.\n\nI can't currently think of a better way of storing that, and of course\nthe control has to store the max value like that anyway, so it's not an\nissue I don't think - just a quirk we might have to be aware of.\n\n\n\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index b1f2786..3c68078 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -658,6 +658,16 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\tbreak;\n> \t\t}\n> \n> +\t\tcase controls::DIGITAL_ZOOM: {\n> +\t\t\t/*\n> +\t\t\t * We send the zoom info back in the metadata where the pipeline\n> +\t\t\t * handler will update it to the values actually used.\n> +\t\t\t */\n> +\t\t\tRectangle crop = ctrl.second.get();\n> +\t\t\tlibcameraMetadata_.set(controls::DigitalZoom, crop);\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> \t\tdefault:\n> \t\t\tLOG(IPARPI, Warning)\n> \t\t\t\t<< \"Ctrl \" << controls::controls.at(ctrl.first)->name()\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index f4966f8..55db11d 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -287,7 +287,8 @@ class RPiCameraData : public CameraData\n> public:\n> \tRPiCameraData(PipelineHandler *pipe)\n> \t\t: CameraData(pipe), sensor_(nullptr), lsTable_(nullptr),\n> -\t\t state_(State::Stopped), dropFrame_(false), ispOutputCount_(0)\n> +\t\t state_(State::Stopped), dropFrame_(false), ispOutputCount_(0),\n> +\t\t ispMinSize_(0)\n> \t{\n> \t}\n> \n> @@ -322,6 +323,14 @@ public:\n> \tvoid handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);\n> \tvoid handleState();\n> \n> +\tvoid initSensorCrop(Rectangle const &crop, unsigned int ispMinSize)\n> +\t{\n> +\t\t/* The initial zoom region is the whole of the sensorCrop_. */\n> +\t\tsensorCrop_ = crop;\n> +\t\tzoomRect_ = Rectangle{ 0, 0, crop.width, crop.height };\n> +\t\tispMinSize_ = ispMinSize;\n> +\t}\n> +\n> \tCameraSensor *sensor_;\n> \t/* Array of Unicam and ISP device streams and associated buffers/streams. */\n> \tRPiDevice unicam_;\n> @@ -358,6 +367,15 @@ private:\n> \n> \tbool dropFrame_;\n> \tint ispOutputCount_;\n> +\t/*\n> +\t * sensorCrop_ is the largest region of the full sensor output that matches\n> +\t * the desired aspect ratio, and therefore represents the area within\n> +\t * which we can pan and zoom. zoomRect_ is the portion from within the\n> +\t * sensorCrop_ that pan/zoom is currently using.\n> +\t */\n> +\tRectangle sensorCrop_;\n> +\tRectangle zoomRect_;\n> +\tunsigned int ispMinSize_;\n> };\n> \n> class RPiCameraConfiguration : public CameraConfiguration\n> @@ -744,6 +762,10 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t\treturn ret;\n> \t}\n> \n> +\tRectangle testCrop = { 0, 0, 1, 1 };\n> +\tdata->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);\n> +\tconst unsigned int ispMinSize = testCrop.width;\n> +\n> \t/* Adjust aspect ratio by providing crops on the input image. */\n> \tRectangle crop = {\n> \t\t.x = 0,\n> @@ -763,8 +785,12 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \n> \tcrop.x = (sensorFormat.size.width - crop.width) >> 1;\n> \tcrop.y = (sensorFormat.size.height - crop.height) >> 1;\n> +\n> \tdata->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);\n> \n> +\tsensorCrop_ = Size(crop.width, crop.height);\n> +\tdata->initSensorCrop(crop, ispMinSize);\n> +\n> \tret = configureIPA(camera);\n> \tif (ret)\n> \t\tLOG(RPI, Error) << \"Failed to configure the IPA: \" << ret;\n> @@ -1553,6 +1579,34 @@ void RPiCameraData::tryRunPipeline()\n> \t */\n> \tRequest *request = requestQueue_.front();\n> \n> +\tif (request->controls().contains(controls::DigitalZoom)) {\n> +\t\tRectangle rect = request->controls().get(controls::DigitalZoom);\n> +\t\t/*\n> +\t\t * If a new digital zoom value was given, check that it lies within the\n> +\t\t * available sensorCrop_, coercing it if necessary.\n> +\t\t */\n> +\t\tif (rect.width && rect.height) {\n\nLaurent has very recently added a helper for the Size class as isNull.\n\nI suspect we could add in a Rectangle.isNull() to make that statement\n\t\tif (!rect.isNull())\n\nOf course, there could be intricacies of is a rectangle null only if the\nwidth/height are 0, but position could be set or does the position have\nto also be 0,0...\n\n\n\n> +\t\t\tzoomRect_ = rect;\n> +\t\t\tif (zoomRect_.width < ispMinSize_)\n> +\t\t\t\tzoomRect_.width = ispMinSize_;\n> +\t\t\telse if (zoomRect_.width > sensorCrop_.width)\n> +\t\t\t\tzoomRect_.width = sensorCrop_.width;\n> +\t\t\tif (zoomRect_.height < ispMinSize_)\n> +\t\t\t\tzoomRect_.height = ispMinSize_;\n> +\t\t\telse if (zoomRect_.height > sensorCrop_.height)\n> +\t\t\t\tzoomRect_.height = sensorCrop_.height;\n> +\t\t\tif (zoomRect_.x + zoomRect_.width > sensorCrop_.width)\n> +\t\t\t\tzoomRect_.x = sensorCrop_.width - zoomRect_.width;\n> +\t\t\tif (zoomRect_.y + zoomRect_.height > sensorCrop_.height)\n> +\t\t\t\tzoomRect_.y = sensorCrop_.height - zoomRect_.height;\n\nAyeee, and that is begging for some rectangle .clip(rect2) or\n.clamp(rect2) helpers to be added to include/libcamera/geometry.h ...\n\n\n> +\t\t}\n> +\t\tRectangle sensorRect = zoomRect_;\n> +\t\tsensorRect.x += sensorCrop_.x;\n> +\t\tsensorRect.y += sensorCrop_.y;\n> +\t\tisp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &sensorRect);\n> +\t\trequest->controls().set(controls::DigitalZoom, zoomRect_);\n> +\t}\n> +\n> \t/*\n> \t * Process all the user controls by the IPA. Once this is complete, we\n> \t * queue the ISP output buffer listed in the request to start the HW\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 57761BD790\n\tfor ;\n\tFri, 10 Jul 2020 13:40:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D610060485;\n\tFri, 10 Jul 2020 15:40:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CF56160484\n\tfor ;\n\tFri, 10 Jul 2020 15:40:54 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C8DE2C0;\n\tFri, 10 Jul 2020 15:40:54 +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=\"RI212jgH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594388454;\n\tbh=YMPtVm9Gg+D2zJwKE4H3RHUg3gTOHg4ZH/8GRrNSfLE=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=RI212jgHGtKl5EG2vdfc6jSrbj+nczqBrzyQR4MCoy9Xx+qPODV6DBiB9OSpc2lMD\n\tdyVcLQujucppciVrdL48zMxSbQQeWzwxT4qupYesOdC3WniZo4qW6ZzsehD/dT3p2k\n\tJwJD1RL1MpI0x5qXTEqMjhAYHjlQLSapAu9fW/qc=","To":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200709091555.1617-1-david.plowman@raspberrypi.com>\n\t<20200709091555.1617-3-david.plowman@raspberrypi.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Fri, 10 Jul 2020 14:40:51 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200709091555.1617-3-david.plowman@raspberrypi.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: raspberrypi:\n\tImplement 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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11447,"web_url":"https://patchwork.libcamera.org/comment/11447/","msgid":"","date":"2020-07-21T10:35:28","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: raspberrypi:\n\tImplement digital zoom","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Kieran\n\nThanks for the Rectangle constructors that I've seen appear in the\ncode, that's obviously helpful. Also the suggestion on adding a clip\nfunction to the Rectangle class is clearly sensible - so I'll include\nthat in the next version of patches!\n\nBest regards\nDavid\n\nOn Fri, 10 Jul 2020 at 14:40, Kieran Bingham\n wrote:\n>\n> Hi David,\n>\n> I've just been chatting briefly with Laurent and he has mentioned that\n> this topic should be integrated with the work that Jacopo has done on\n> Pixel Array properties.\n>\n> I haven't looked at that work yet, so I'm not (yet) in a position to\n> comment on that directly, but as I'd already written some generic\n> (libcamera) framework comments about the Rectangle class below, I\n> thought I should send this now...\n>\n>\n>\n> On 09/07/2020 10:15, David Plowman wrote:\n> > These changes implement digital zoom for the Raspberry Pi. We detect\n> > the digital zoom control and update the V4L2 \"selection\" before\n> > starting the ISP. We also update the value in the control that we send\n> > to the IPA, so that it has the correct value.\n> > ---\n> > include/libcamera/ipa/raspberrypi.h | 1 +\n> > src/ipa/raspberrypi/raspberrypi.cpp | 10 ++++\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 56 ++++++++++++++++++-\n> > 3 files changed, 66 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> > index a18ce9a..e66402e 100644\n> > --- a/include/libcamera/ipa/raspberrypi.h\n> > +++ b/include/libcamera/ipa/raspberrypi.h\n> > @@ -52,6 +52,7 @@ static const ControlInfoMap RPiControls = {\n> > { &controls::Contrast, ControlInfo(0.0f, 32.0f) },\n> > { &controls::Saturation, ControlInfo(0.0f, 32.0f) },\n> > { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> > + { &controls::DigitalZoom, ControlInfo(Rectangle{ 0, 0, 0, 0 }, Rectangle{ 65535, 65535, 65535, 65535 }, Rectangle{ 0, 0, 0, 0 }) },\n>\n> I wonder if we should have some constructor helpers to make that easier.\n> (Rectangle(0), Rectangle(65535), Rectangle(0))\n>\n>\n> Oddly, (and this is not a fault of this patch), the idea of a Rectangle\n> which starts at x,y = {65535,65535}, and is of size {65535,65535} seems\n> wrong, but of course this is jsut storage of maximum values.\n>\n> I can't currently think of a better way of storing that, and of course\n> the control has to store the max value like that anyway, so it's not an\n> issue I don't think - just a quirk we might have to be aware of.\n>\n>\n>\n> > };\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index b1f2786..3c68078 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -658,6 +658,16 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > break;\n> > }\n> >\n> > + case controls::DIGITAL_ZOOM: {\n> > + /*\n> > + * We send the zoom info back in the metadata where the pipeline\n> > + * handler will update it to the values actually used.\n> > + */\n> > + Rectangle crop = ctrl.second.get();\n> > + libcameraMetadata_.set(controls::DigitalZoom, crop);\n> > + break;\n> > + }\n> > +\n> > default:\n> > LOG(IPARPI, Warning)\n> > << \"Ctrl \" << controls::controls.at(ctrl.first)->name()\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index f4966f8..55db11d 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -287,7 +287,8 @@ class RPiCameraData : public CameraData\n> > public:\n> > RPiCameraData(PipelineHandler *pipe)\n> > : CameraData(pipe), sensor_(nullptr), lsTable_(nullptr),\n> > - state_(State::Stopped), dropFrame_(false), ispOutputCount_(0)\n> > + state_(State::Stopped), dropFrame_(false), ispOutputCount_(0),\n> > + ispMinSize_(0)\n> > {\n> > }\n> >\n> > @@ -322,6 +323,14 @@ public:\n> > void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);\n> > void handleState();\n> >\n> > + void initSensorCrop(Rectangle const &crop, unsigned int ispMinSize)\n> > + {\n> > + /* The initial zoom region is the whole of the sensorCrop_. */\n> > + sensorCrop_ = crop;\n> > + zoomRect_ = Rectangle{ 0, 0, crop.width, crop.height };\n> > + ispMinSize_ = ispMinSize;\n> > + }\n> > +\n> > CameraSensor *sensor_;\n> > /* Array of Unicam and ISP device streams and associated buffers/streams. */\n> > RPiDevice unicam_;\n> > @@ -358,6 +367,15 @@ private:\n> >\n> > bool dropFrame_;\n> > int ispOutputCount_;\n> > + /*\n> > + * sensorCrop_ is the largest region of the full sensor output that matches\n> > + * the desired aspect ratio, and therefore represents the area within\n> > + * which we can pan and zoom. zoomRect_ is the portion from within the\n> > + * sensorCrop_ that pan/zoom is currently using.\n> > + */\n> > + Rectangle sensorCrop_;\n> > + Rectangle zoomRect_;\n> > + unsigned int ispMinSize_;\n> > };\n> >\n> > class RPiCameraConfiguration : public CameraConfiguration\n> > @@ -744,6 +762,10 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > return ret;\n> > }\n> >\n> > + Rectangle testCrop = { 0, 0, 1, 1 };\n> > + data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);\n> > + const unsigned int ispMinSize = testCrop.width;\n> > +\n> > /* Adjust aspect ratio by providing crops on the input image. */\n> > Rectangle crop = {\n> > .x = 0,\n> > @@ -763,8 +785,12 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> >\n> > crop.x = (sensorFormat.size.width - crop.width) >> 1;\n> > crop.y = (sensorFormat.size.height - crop.height) >> 1;\n> > +\n> > data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);\n> >\n> > + sensorCrop_ = Size(crop.width, crop.height);\n> > + data->initSensorCrop(crop, ispMinSize);\n> > +\n> > ret = configureIPA(camera);\n> > if (ret)\n> > LOG(RPI, Error) << \"Failed to configure the IPA: \" << ret;\n> > @@ -1553,6 +1579,34 @@ void RPiCameraData::tryRunPipeline()\n> > */\n> > Request *request = requestQueue_.front();\n> >\n> > + if (request->controls().contains(controls::DigitalZoom)) {\n> > + Rectangle rect = request->controls().get(controls::DigitalZoom);\n> > + /*\n> > + * If a new digital zoom value was given, check that it lies within the\n> > + * available sensorCrop_, coercing it if necessary.\n> > + */\n> > + if (rect.width && rect.height) {\n>\n> Laurent has very recently added a helper for the Size class as isNull.\n>\n> I suspect we could add in a Rectangle.isNull() to make that statement\n> if (!rect.isNull())\n>\n> Of course, there could be intricacies of is a rectangle null only if the\n> width/height are 0, but position could be set or does the position have\n> to also be 0,0...\n>\n>\n>\n> > + zoomRect_ = rect;\n> > + if (zoomRect_.width < ispMinSize_)\n> > + zoomRect_.width = ispMinSize_;\n> > + else if (zoomRect_.width > sensorCrop_.width)\n> > + zoomRect_.width = sensorCrop_.width;\n> > + if (zoomRect_.height < ispMinSize_)\n> > + zoomRect_.height = ispMinSize_;\n> > + else if (zoomRect_.height > sensorCrop_.height)\n> > + zoomRect_.height = sensorCrop_.height;\n> > + if (zoomRect_.x + zoomRect_.width > sensorCrop_.width)\n> > + zoomRect_.x = sensorCrop_.width - zoomRect_.width;\n> > + if (zoomRect_.y + zoomRect_.height > sensorCrop_.height)\n> > + zoomRect_.y = sensorCrop_.height - zoomRect_.height;\n>\n> Ayeee, and that is begging for some rectangle .clip(rect2) or\n> .clamp(rect2) helpers to be added to include/libcamera/geometry.h ...\n>\n>\n> > + }\n> > + Rectangle sensorRect = zoomRect_;\n> > + sensorRect.x += sensorCrop_.x;\n> > + sensorRect.y += sensorCrop_.y;\n> > + isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &sensorRect);\n> > + request->controls().set(controls::DigitalZoom, zoomRect_);\n> > + }\n> > +\n> > /*\n> > * Process all the user controls by the IPA. Once this is complete, we\n> > * queue the ISP output buffer listed in the request to start the HW\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 47B0FBD792\n\tfor ;\n\tTue, 21 Jul 2020 10:35:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92B9560832;\n\tTue, 21 Jul 2020 12:35:43 +0200 (CEST)","from mail-ot1-x331.google.com (mail-ot1-x331.google.com\n\t[IPv6:2607:f8b0:4864:20::331])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DC6C560496\n\tfor ;\n\tTue, 21 Jul 2020 12:35:41 +0200 (CEST)","by mail-ot1-x331.google.com with SMTP id t18so14701680otq.5\n\tfor ;\n\tTue, 21 Jul 2020 03:35:41 -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=\"cJufq63w\"; 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=JnpV87U+ioRb4mp6jrgAx7FrbALriKcBf1FwoyznkmI=;\n\tb=cJufq63whBGOEHJHcrZLEvdZscs+aoDoLyFJd/DAqP3ZFSPVRIl5nhCuCfXGX06CGe\n\tvOLS346ICA2OzWc4YlZupdnYxOoiou5SS3yaw1bOvnZHAXaqaQ/eZ8ETIifz+iiyp9za\n\tCts9+SvuHrJPti+XnPvynIliHDdCb2VyOzrdmkzfM7kGT/+DvMOmKLaWTBu4SKa1hW/p\n\tp8VPrbiMz0wiWn5YlHx753E31IJQ8GfGcywVibINWRIzHx5MYxX9nhe3CaGelpZdHALI\n\t5fcCFk2wP3/sI5qjU6J1ShHkt0ylzzP0iIFgn2zzKsXijjUQIcwyLjnf64gB0CTcrVV0\n\t8jpA==","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=JnpV87U+ioRb4mp6jrgAx7FrbALriKcBf1FwoyznkmI=;\n\tb=uUjhMwGgX9YHV7jPMh8+I9Vu53SGPj2Oh4fRPVzEn/oU95Z97AVjHqTMEVmmVKVtsu\n\tISNii5Hj3PlR8Jm0SbvwejepASmydQpMjg6chGZCSGkyy4nleRdMbD1BKLmsbyy17fwT\n\tQLuDpqAKbuObFvxSBjHRl0JUStazRnYAwpesvXPJDgbj/Fasu+cv8oGSozVjWSjB5nKY\n\t2bhAhzYzwtyi8aW4P6xUKPREyYnHRcalo/4KHJZtME7ewxCtS4RO7qPfI4M9vLaNGPxe\n\tj+4UawIhqgDQLxbv7XEBAJhN+71vkO7Ktkwe46EEANfvz26QV5Y7vrXJfioomOt01Gru\n\tfKmg==","X-Gm-Message-State":"AOAM531J8BCq19CuFp08lmSCDxSLTySXeAKHCiD3tgMKXXKs7OJ4CMbh\n\tC6ELjLkTCpkm1OKNB29HjZGdl1F5H6gEkIGiBkYcmw==","X-Google-Smtp-Source":"ABdhPJw78oGVSUzYNdPjZ97ARJYLYRNiRcLn4qdybqvySbF3hvu3uCvQWjEv9B7lKwtOjDPnvGRhbii9hNcds7CDpJs=","X-Received":"by 2002:a9d:6b19:: with SMTP id\n\tg25mr24705224otp.160.1595327740562; \n\tTue, 21 Jul 2020 03:35:40 -0700 (PDT)","MIME-Version":"1.0","References":"<20200709091555.1617-1-david.plowman@raspberrypi.com>\n\t<20200709091555.1617-3-david.plowman@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Tue, 21 Jul 2020 11:35:28 +0100","Message-ID":"","To":"Kieran Bingham ","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: raspberrypi:\n\tImplement 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\" "}}]