[{"id":14755,"web_url":"https://patchwork.libcamera.org/comment/14755/","msgid":"","date":"2021-01-25T11:31:09","subject":"Re: [libcamera-devel] [PATCH v2 06/11] libcamera: ipu3: Register\n\tScalerCrop control","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 19, 2021 at 03:37:06PM +0100, Jacopo Mondi wrote:\n> Register the ScalerCrop control in the ImgU pipeline handler computed\n> by using the Viewfinder [1280x720] pipeline output configuration and\n> the sensor resolution as parameters.\n> \n> The ScalerCrop control limits should be updated everytime a new\n\ns/everytime/every time/\n\n> configuration is applied to the sensor.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++++++++++-\n> 1 file changed, 71 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index f928af4d92a2..fc5592f33032 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -40,6 +40,7 @@ static constexpr unsigned int IMGU_OUTPUT_WIDTH_ALIGN = 64;\n> static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4;\n> static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64;\n> static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32;\n> +static constexpr Size IPU3ViewfinderSize(1280, 720);\n> \n> static const ControlInfoMap::Map IPU3Controls = {\n> \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> @@ -376,7 +377,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,\n> \t\t\t * capped to the maximum sensor resolution and aligned\n> \t\t\t * to the ImgU output constraints.\n> \t\t\t */\n> -\t\t\tsize = sensorResolution.boundedTo({ 1280, 720 })\n> +\t\t\tsize = sensorResolution.boundedTo(IPU3ViewfinderSize)\n> \t\t\t\t\t .alignedDownTo(IMGU_OUTPUT_WIDTH_ALIGN,\n> \t\t\t\t\t\t\t IMGU_OUTPUT_HEIGHT_ALIGN);\n> \t\t\tpixelFormat = formats::NV12;\n> @@ -745,7 +746,7 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> */\n> int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n> {\n> -\tconst CameraSensor *sensor = data->cio2_.sensor();\n> +\tCameraSensor *sensor = data->cio2_.sensor();\n> \tCameraSensorInfo sensorInfo{};\n> \n> \tint ret = sensor->sensorInfo(&sensorInfo);\n> @@ -780,6 +781,74 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n> \tcontrols[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n> \t\t\t\t\t\t\tdefExposure);\n> \n> +\t/*\n> +\t * Compute the scaler crop limits.\n> +\t *\n> +\t * \\todo The scaler crop limits depend on the sensor configuration. It\n> +\t * should be updated when a new configuration is applied. To initialize\n> +\t * the control use the 'Viewfinder' configuration (1280x720) as the\n> +\t * pipeline output resolution and the full sensor size as input frame\n> +\t * (see the todo note in the validate() function about the usage of the\n> +\t * sensor's full frame as ImgU input).\n> +\t */\n> +\n> +\t/* Re-fetch the sensor info updated to use the largest resolution. */\n> +\tV4L2SubdeviceFormat sensorFormat;\n> +\tsensorFormat.size = sensor->resolution();\n> +\tret = sensor->setFormat(&sensorFormat);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = sensor->sensorInfo(&sensorInfo);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/*\n> +\t * The maximum scaler crop rectangle is the analogue crop used to\n> +\t * produce the maximum frame size.\n> +\t */\n> +\tconst Rectangle &analogueCrop = sensorInfo.analogCrop;\n> +\tRectangle maxCrop = analogueCrop;\n> +\n> +\t/*\n> +\t * As the ImgU cannot up-scale, the minimum selection rectangle has to\n> +\t * be as large as the pipeline output size. Use the default viewfinder\n> +\t * configuration as the desired output size and calculate the minimum\n> +\t * rectangle required to satisfy the ImgU processing margins, unless the\n> +\t * sensor resolution is smaller.\n> +\t *\n> +\t * \\todo This implementation is based on the same assumptions about the\n> +\t * ImgU pipeline configuration described in then viewfinder and main\n> +\t * output sizes calculation in the validate() function.\n> +\t */\n> +\n> +\t/* The strictly smaller size than the sensor resolution, aligned to margins. */\n> +\tSize minSize = Size(sensor->resolution().width - 1,\n> +\t\t\t sensor->resolution().height - 1)\n> +\t\t .alignedDownTo(IMGU_OUTPUT_WIDTH_MARGIN,\n> +\t\t\t\t IMGU_OUTPUT_HEIGHT_MARGIN);\n> +\n> +\t/*\n> +\t * Either the smallest margin-aligned size larger than the viewfinder\n> +\t * size or the adjusted sensor resolution.\n> +\t */\n> +\tminSize = Size(IPU3ViewfinderSize.width + 1,\n> +\t\t IPU3ViewfinderSize.height + 1)\n> +\t\t .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,\n> +\t\t\t IMGU_OUTPUT_HEIGHT_MARGIN)\n> +\t\t .boundedTo(minSize);\n> +\n> +\t/*\n> +\t * Re-scale in the sensor's native coordinates. Report (0,0) as\n> +\t * top-left corner as we allow application to feely pan the crop area.\n\ns/feely/freely/\n\n> +\t */\n> +\tRectangle minCrop(minSize);\n> +\tminCrop.scaledBy(analogueCrop.size(), sensorInfo.outputSize);\n\nscaledBy() is a const function that returns a new rectangle. You\nprobably want scaleBy() here (or see below for an alternative).\n\n> +\tminCrop.x = 0;\n> +\tminCrop.y = 0;\n\n(x,y) are set to 0 by the constructor, so you could write\n\n\tRectangle minCrop = minSize.scaledBy(analogueCrop.size(),\n\t\t\t\t\t sensorInfo.outputSize);\n\nReviewed-by: Laurent Pinchart \n\n> +\n> +\tcontrols[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n> +\n> \tdata->controlInfo_ = std::move(controls);\n> \n> \treturn 0;","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 2F7F2BD808\n\tfor ;\n\tMon, 25 Jan 2021 11:31:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 00060682C1;\n\tMon, 25 Jan 2021 12:31:29 +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 0FBC3682BB\n\tfor ;\n\tMon, 25 Jan 2021 12:31:29 +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 8C8FB331;\n\tMon, 25 Jan 2021 12:31:28 +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=\"ayH0HGrU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611574288;\n\tbh=JktG7KaRuiD0CfcJkijs83gwBaYYUSmmyuCZPnLiO1I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ayH0HGrUoGLZ2mqvpnk1ZLtqaO3sZWDkI2E4zb4D2lnsFUv4IIHf9OyJ3HgJTum/J\n\teKN3K9AH4EGdJk7HdJjFaw580j17Ywd4TccSdACfUzcBwy+uGX3f3yHcJYk2AJ1FJp\n\tuOJks0kkVU+DDUNytsBnu/8nFoBQBupvzQd1OLqI=","Date":"Mon, 25 Jan 2021 13:31:09 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210119143711.153517-1-jacopo@jmondi.org>\n\t<20210119143711.153517-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210119143711.153517-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 06/11] libcamera: ipu3: Register\n\tScalerCrop control","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":14765,"web_url":"https://patchwork.libcamera.org/comment/14765/","msgid":"<20210125141921.ej5uihqevvih7hdp@uno.localdomain>","date":"2021-01-25T14:19:21","subject":"Re: [libcamera-devel] [PATCH v2 06/11] libcamera: ipu3: Register\n\tScalerCrop control","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 25, 2021 at 01:31:09PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> > +\t/*\n> > +\t * Re-scale in the sensor's native coordinates. Report (0,0) as\n> > +\t * top-left corner as we allow application to feely pan the crop area.\n>\n> s/feely/freely/\n>\n> > +\t */\n> > +\tRectangle minCrop(minSize);\n> > +\tminCrop.scaledBy(analogueCrop.size(), sensorInfo.outputSize);\n>\n> scaledBy() is a const function that returns a new rectangle. You\n> probably want scaleBy() here (or see below for an alternative).\n>\n> > +\tminCrop.x = 0;\n> > +\tminCrop.y = 0;\n>\n> (x,y) are set to 0 by the constructor, so you could write\n>\n> \tRectangle minCrop = minSize.scaledBy(analogueCrop.size(),\n> \t\t\t\t\t sensorInfo.outputSize);\n\nI went for the minCrop initialization as 'scaledBy' is not a Size\nclass method.\n\nIn facts, if preferred, I could work around it with\n Rectangle minCrop = Rectangle(minSize).scaledBy(analogueCrop.size(),\n sensorInfo.outputSize);\n\nWhich I don't mind\n\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > +\n> > +\tcontrols[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n> > +\n> > \tdata->controlInfo_ = std::move(controls);\n> >\n> > \treturn 0;\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 923C8C0F2B\n\tfor ;\n\tMon, 25 Jan 2021 14:19:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E0F7A682D0;\n\tMon, 25 Jan 2021 15:19:02 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 67EF9682CB\n\tfor ;\n\tMon, 25 Jan 2021 15:19:01 +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 relay6-d.mail.gandi.net (Postfix) with ESMTPSA id E36C3C0008;\n\tMon, 25 Jan 2021 14:19:00 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 25 Jan 2021 15:19:21 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210125141921.ej5uihqevvih7hdp@uno.localdomain>","References":"<20210119143711.153517-1-jacopo@jmondi.org>\n\t<20210119143711.153517-7-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 06/11] libcamera: ipu3: Register\n\tScalerCrop control","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":14766,"web_url":"https://patchwork.libcamera.org/comment/14766/","msgid":"","date":"2021-01-25T14:24:18","subject":"Re: [libcamera-devel] [PATCH v2 06/11] libcamera: ipu3: Register\n\tScalerCrop control","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 25, 2021 at 03:19:21PM +0100, Jacopo Mondi wrote:\n> On Mon, Jan 25, 2021 at 01:31:09PM +0200, Laurent Pinchart wrote:\n> > > +\t/*\n> > > +\t * Re-scale in the sensor's native coordinates. Report (0,0) as\n> > > +\t * top-left corner as we allow application to feely pan the crop area.\n> >\n> > s/feely/freely/\n> >\n> > > +\t */\n> > > +\tRectangle minCrop(minSize);\n> > > +\tminCrop.scaledBy(analogueCrop.size(), sensorInfo.outputSize);\n> >\n> > scaledBy() is a const function that returns a new rectangle. You\n> > probably want scaleBy() here (or see below for an alternative).\n> >\n> > > +\tminCrop.x = 0;\n> > > +\tminCrop.y = 0;\n> >\n> > (x,y) are set to 0 by the constructor, so you could write\n> >\n> > \tRectangle minCrop = minSize.scaledBy(analogueCrop.size(),\n> > \t\t\t\t\t sensorInfo.outputSize);\n> \n> I went for the minCrop initialization as 'scaledBy' is not a Size\n> class method.\n\nOops, indeed.\n\n> In facts, if preferred, I could work around it with\n> Rectangle minCrop = Rectangle(minSize).scaledBy(analogueCrop.size(),\n> sensorInfo.outputSize);\n> \n> Which I don't mind\n\nUp to you. If you prefer keeping the existing code, just remember to\ns/scaledBy/scaleBy/\n\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > +\n> > > +\tcontrols[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n> > > +\n> > > \tdata->controlInfo_ = std::move(controls);\n> > >\n> > > \treturn 0;","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 583FABD808\n\tfor ;\n\tMon, 25 Jan 2021 14:24:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E1A8C682D0;\n\tMon, 25 Jan 2021 15:24:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5361F682CB\n\tfor ;\n\tMon, 25 Jan 2021 15:24:38 +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 BAF37331;\n\tMon, 25 Jan 2021 15:24:37 +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=\"UxNFsIGz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611584677;\n\tbh=9kesMq6XVu93dSp9ba59foPJo9RAGoRy4r4kk6JYQsk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UxNFsIGzKx16mzPF2GawAIJB3G7tX/bh/z8gBjT18/DkEBJKi3Pu5va1mVsaJJFr/\n\tti6FJQsKZAu2SPh8JM4XxCCRTrRJi/LRxKjGcIHJAqu802yu3dbukAzwnv46dvvnBq\n\t4NU+ll8bxZFky9bYMreLUrKN9BsE7+UTHSr9ChSU=","Date":"Mon, 25 Jan 2021 16:24:18 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210119143711.153517-1-jacopo@jmondi.org>\n\t<20210119143711.153517-7-jacopo@jmondi.org>\n\t\n\t<20210125141921.ej5uihqevvih7hdp@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210125141921.ej5uihqevvih7hdp@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 06/11] libcamera: ipu3: Register\n\tScalerCrop control","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\" "}}]