[{"id":14508,"web_url":"https://patchwork.libcamera.org/comment/14508/","msgid":"","date":"2021-01-10T23:53:38","subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: ipu3: Handle\n\tScalerCrop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Jan 05, 2021 at 08:05:19PM +0100, Jacopo Mondi wrote:\n> Add support for caching the value of control::ScalerCrop to return\n> it in the request metadata.\n> \n> No cropping is currently applied on the input video device, the control\n> value is returned in the metadata pack as it is received.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 22 ++++++++++++++++++++--\n> 1 file changed, 20 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index f1329ffb0463..381524bb3499 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -66,6 +66,7 @@ public:\n> \tStream rawStream_;\n> \n> \tint32_t exposureTime_;\n> +\tRectangle scalerCrop_;\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -75,6 +76,7 @@ public:\n> \n> \tStatus validate() override;\n> \n> +\tconst Size &cio2Size() const { return cio2Configuration_.size; }\n\nDo we need this, can't we keep using config->cio2Format().size ?\n\n> \tconst StreamConfiguration &cio2Format() const { return cio2Configuration_; }\n> \tconst ImgUDevice::PipeConfig imguConfig() const { return pipeConfig_; }\n> \n> @@ -468,12 +470,19 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t * Pass the requested stream size to the CIO2 unit and get back the\n> \t * adjusted format to be propagated to the ImgU output devices.\n> \t */\n> -\tconst Size &sensorSize = config->cio2Format().size;\n> \tV4L2DeviceFormat cio2Format;\n> -\tret = cio2->configure(sensorSize, &cio2Format);\n> +\tret = cio2->configure(config->cio2Size(), &cio2Format);\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\t/* Initialize the scaler crop using the sensor's analogue crop. */\n> +\tCameraSensorInfo sensorInfo;\n> +\tret = cio2->sensor()->sensorInfo(&sensorInfo);\n> +\tif (ret)\n> +\t\t/* Use the requested CIO2 output size as fallback. */\n> +\t\tsensorInfo.analogCrop = Rectangle(config->cio2Size());\n> +\tdata->scalerCrop_ = sensorInfo.analogCrop;\n> +\n> \t/*\n> \t * If the ImgU gets configured, its driver seems to expect that\n> \t * buffers will be queued to its outputs, as otherwise the next\n> @@ -656,6 +665,14 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> \tIPU3CameraData *data = cameraData(camera);\n> \tint error = 0;\n> \n> +\tControlList &controls = request->controls();\n> +\tif (controls.contains(controls::ScalerCrop))\n> +\t\t/*\n> +\t\t * \\todo No scaling is applied. Just return the value in the\n> +\t\t * request metadata as it is.\n> +\t\t */\n> +\t\tdata->scalerCrop_ = controls.get(controls::ScalerCrop);\n\nThis means that request completing after the this request is queued will\nall have the new scaler crop rectangle. That's not right, it should be\nsynchronized with this request.\n\n> +\n> \t/*\n> \t * Queue a buffer on the CIO2, using the raw stream buffer provided in\n> \t * the request, if any, or a CIO2 internal buffer otherwise.\n> @@ -991,6 +1008,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> \t/* Mark the request as complete. */\n> \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> \trequest->metadata().set(controls::ExposureTime, exposureTime_);\n> +\trequest->metadata().set(controls::ScalerCrop, scalerCrop_);\n> \tpipe_->completeRequest(request);\n> }\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 709ABBD80C\n\tfor ;\n\tSun, 10 Jan 2021 23:53:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E079768086;\n\tMon, 11 Jan 2021 00:53:53 +0100 (CET)","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 179EA60317\n\tfor ;\n\tMon, 11 Jan 2021 00:53:53 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7B891EC;\n\tMon, 11 Jan 2021 00:53:52 +0100 (CET)"],"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=\"M6KR/dNi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610322832;\n\tbh=548dvwg3oKkHWLr0dNVIhzZpwnPcq8s0BcKS0GcvqSE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=M6KR/dNi0vhcadlVYIJO3NUq8Ur6kBiOvOfNf6mHaX+Fcb471NhI4Sl9a+CdlKaws\n\t2+rJkvK5x8UTQhRRwxyWss5D6+bjo9ZNNkzjoRXOemUWuaGeG/j88CDnEoHob0dDw6\n\tcdzUfzhZhOizQ2yxXfZRgsv65DSo9+MTxkiei6y4=","Date":"Mon, 11 Jan 2021 01:53:38 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210105190522.682324-1-jacopo@jmondi.org>\n\t<20210105190522.682324-10-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210105190522.682324-10-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: ipu3: Handle\n\tScalerCrop","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":14576,"web_url":"https://patchwork.libcamera.org/comment/14576/","msgid":"<20210118105533.wterffvu652wi6mn@uno.localdomain>","date":"2021-01-18T10:55:33","subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: ipu3: Handle\n\tScalerCrop","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Jan 11, 2021 at 01:53:38AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tue, Jan 05, 2021 at 08:05:19PM +0100, Jacopo Mondi wrote:\n> > Add support for caching the value of control::ScalerCrop to return\n> > it in the request metadata.\n> >\n> > No cropping is currently applied on the input video device, the control\n> > value is returned in the metadata pack as it is received.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 22 ++++++++++++++++++++--\n> > 1 file changed, 20 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index f1329ffb0463..381524bb3499 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -66,6 +66,7 @@ public:\n> > \tStream rawStream_;\n> >\n> > \tint32_t exposureTime_;\n> > +\tRectangle scalerCrop_;\n> > };\n> >\n> > class IPU3CameraConfiguration : public CameraConfiguration\n> > @@ -75,6 +76,7 @@ public:\n> >\n> > \tStatus validate() override;\n> >\n> > +\tconst Size &cio2Size() const { return cio2Configuration_.size; }\n>\n> Do we need this, can't we keep using config->cio2Format().size ?\n>\n> > \tconst StreamConfiguration &cio2Format() const { return cio2Configuration_; }\n> > \tconst ImgUDevice::PipeConfig imguConfig() const { return pipeConfig_; }\n> >\n> > @@ -468,12 +470,19 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \t * Pass the requested stream size to the CIO2 unit and get back the\n> > \t * adjusted format to be propagated to the ImgU output devices.\n> > \t */\n> > -\tconst Size &sensorSize = config->cio2Format().size;\n> > \tV4L2DeviceFormat cio2Format;\n> > -\tret = cio2->configure(sensorSize, &cio2Format);\n> > +\tret = cio2->configure(config->cio2Size(), &cio2Format);\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n> > +\t/* Initialize the scaler crop using the sensor's analogue crop. */\n> > +\tCameraSensorInfo sensorInfo;\n> > +\tret = cio2->sensor()->sensorInfo(&sensorInfo);\n> > +\tif (ret)\n> > +\t\t/* Use the requested CIO2 output size as fallback. */\n> > +\t\tsensorInfo.analogCrop = Rectangle(config->cio2Size());\n> > +\tdata->scalerCrop_ = sensorInfo.analogCrop;\n> > +\n> > \t/*\n> > \t * If the ImgU gets configured, its driver seems to expect that\n> > \t * buffers will be queued to its outputs, as otherwise the next\n> > @@ -656,6 +665,14 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> > \tIPU3CameraData *data = cameraData(camera);\n> > \tint error = 0;\n> >\n> > +\tControlList &controls = request->controls();\n> > +\tif (controls.contains(controls::ScalerCrop))\n> > +\t\t/*\n> > +\t\t * \\todo No scaling is applied. Just return the value in the\n> > +\t\t * request metadata as it is.\n> > +\t\t */\n> > +\t\tdata->scalerCrop_ = controls.get(controls::ScalerCrop);\n>\n> This means that request completing after the this request is queued will\n> all have the new scaler crop rectangle. That's not right, it should be\n> synchronized with this request.\n>\n\nCorrect -.-\n\nshould I rather take the scaler crop value to be set in metadata from\nthe Request's controls ?\n\n> > +\n> > \t/*\n> > \t * Queue a buffer on the CIO2, using the raw stream buffer provided in\n> > \t * the request, if any, or a CIO2 internal buffer otherwise.\n> > @@ -991,6 +1008,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> > \t/* Mark the request as complete. */\n> > \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> > \trequest->metadata().set(controls::ExposureTime, exposureTime_);\n> > +\trequest->metadata().set(controls::ScalerCrop, scalerCrop_);\n> > \tpipe_->completeRequest(request);\n> > }\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 A1CE7C0F1C\n\tfor ;\n\tMon, 18 Jan 2021 10:55:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EDB116811E;\n\tMon, 18 Jan 2021 11:55:15 +0100 (CET)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5730D60314\n\tfor ;\n\tMon, 18 Jan 2021 11:55:14 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id E133F240005;\n\tMon, 18 Jan 2021 10:55:13 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 18 Jan 2021 11:55:33 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210118105533.wterffvu652wi6mn@uno.localdomain>","References":"<20210105190522.682324-1-jacopo@jmondi.org>\n\t<20210105190522.682324-10-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: ipu3: Handle\n\tScalerCrop","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":14590,"web_url":"https://patchwork.libcamera.org/comment/14590/","msgid":"","date":"2021-01-19T05:59:44","subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: ipu3: Handle\n\tScalerCrop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Jan 18, 2021 at 11:55:33AM +0100, Jacopo Mondi wrote:\n> On Mon, Jan 11, 2021 at 01:53:38AM +0200, Laurent Pinchart wrote:\n> > On Tue, Jan 05, 2021 at 08:05:19PM +0100, Jacopo Mondi wrote:\n> > > Add support for caching the value of control::ScalerCrop to return\n> > > it in the request metadata.\n> > >\n> > > No cropping is currently applied on the input video device, the control\n> > > value is returned in the metadata pack as it is received.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 22 ++++++++++++++++++++--\n> > > 1 file changed, 20 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index f1329ffb0463..381524bb3499 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -66,6 +66,7 @@ public:\n> > > \tStream rawStream_;\n> > >\n> > > \tint32_t exposureTime_;\n> > > +\tRectangle scalerCrop_;\n> > > };\n> > >\n> > > class IPU3CameraConfiguration : public CameraConfiguration\n> > > @@ -75,6 +76,7 @@ public:\n> > >\n> > > \tStatus validate() override;\n> > >\n> > > +\tconst Size &cio2Size() const { return cio2Configuration_.size; }\n> >\n> > Do we need this, can't we keep using config->cio2Format().size ?\n> >\n> > > \tconst StreamConfiguration &cio2Format() const { return cio2Configuration_; }\n> > > \tconst ImgUDevice::PipeConfig imguConfig() const { return pipeConfig_; }\n> > >\n> > > @@ -468,12 +470,19 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > > \t * Pass the requested stream size to the CIO2 unit and get back the\n> > > \t * adjusted format to be propagated to the ImgU output devices.\n> > > \t */\n> > > -\tconst Size &sensorSize = config->cio2Format().size;\n> > > \tV4L2DeviceFormat cio2Format;\n> > > -\tret = cio2->configure(sensorSize, &cio2Format);\n> > > +\tret = cio2->configure(config->cio2Size(), &cio2Format);\n> > > \tif (ret)\n> > > \t\treturn ret;\n> > >\n> > > +\t/* Initialize the scaler crop using the sensor's analogue crop. */\n> > > +\tCameraSensorInfo sensorInfo;\n> > > +\tret = cio2->sensor()->sensorInfo(&sensorInfo);\n> > > +\tif (ret)\n> > > +\t\t/* Use the requested CIO2 output size as fallback. */\n> > > +\t\tsensorInfo.analogCrop = Rectangle(config->cio2Size());\n> > > +\tdata->scalerCrop_ = sensorInfo.analogCrop;\n> > > +\n> > > \t/*\n> > > \t * If the ImgU gets configured, its driver seems to expect that\n> > > \t * buffers will be queued to its outputs, as otherwise the next\n> > > @@ -656,6 +665,14 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> > > \tIPU3CameraData *data = cameraData(camera);\n> > > \tint error = 0;\n> > >\n> > > +\tControlList &controls = request->controls();\n> > > +\tif (controls.contains(controls::ScalerCrop))\n> > > +\t\t/*\n> > > +\t\t * \\todo No scaling is applied. Just return the value in the\n> > > +\t\t * request metadata as it is.\n> > > +\t\t */\n> > > +\t\tdata->scalerCrop_ = controls.get(controls::ScalerCrop);\n> >\n> > This means that request completing after the this request is queued will\n> > all have the new scaler crop rectangle. That's not right, it should be\n> > synchronized with this request.\n> \n> Correct -.-\n> \n> should I rather take the scaler crop value to be set in metadata from\n> the Request's controls ?\n\nYes that should fix the issue.\n\n> > > +\n> > > \t/*\n> > > \t * Queue a buffer on the CIO2, using the raw stream buffer provided in\n> > > \t * the request, if any, or a CIO2 internal buffer otherwise.\n> > > @@ -991,6 +1008,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> > > \t/* Mark the request as complete. */\n> > > \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> > > \trequest->metadata().set(controls::ExposureTime, exposureTime_);\n> > > +\trequest->metadata().set(controls::ScalerCrop, scalerCrop_);\n> > > \tpipe_->completeRequest(request);\n> > > }\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 938ACC0F1C\n\tfor ;\n\tTue, 19 Jan 2021 06:00:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6164868135;\n\tTue, 19 Jan 2021 07:00:03 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90A2760310\n\tfor ;\n\tTue, 19 Jan 2021 07:00:02 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 573E54FB;\n\tTue, 19 Jan 2021 07:00:01 +0100 (CET)"],"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=\"wISxCDd5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611036001;\n\tbh=6GIkqa+726vyGrlYi3FHgwkats03qT8KQBzVNhmxAlA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wISxCDd5aZe6lptu1HmywDFXlcziUzTibWghb25hl6bVFy74A4hXl40NlZt4lzRAv\n\tDC+3jHYrm+B5ht1xO0vbk8a0WYoT/X+VdWe9OTav9R4IPeEN7gxLU91jEUA258Dtzy\n\tTtuMuRrvcAYgX5080Z4yqha/fb6mF8RqouPtX1fk=","Date":"Tue, 19 Jan 2021 07:59:44 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210105190522.682324-1-jacopo@jmondi.org>\n\t<20210105190522.682324-10-jacopo@jmondi.org>\n\t\n\t<20210118105533.wterffvu652wi6mn@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210118105533.wterffvu652wi6mn@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: ipu3: Handle\n\tScalerCrop","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\" "}}]