[{"id":18633,"web_url":"https://patchwork.libcamera.org/comment/18633/","msgid":"<20210809161042.ecvevz7xufeajkvu@uno.localdomain>","date":"2021-08-09T16:10:42","subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: cio2: Tweak sensor size\n\tselection policy","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Umang,\n\nOn Tue, Aug 03, 2021 at 07:02:05PM +0530, Umang Jain wrote:\n> Do not compare higher precision of the ratios, as it might lead to\n> absurd selection of sensor size for a relatively low requested\n> resolution size.\n>\n> For example:\n> The imx258 driver supports the following sensor resolutions:\n>\n> - 4208x3118 = 1.349583066\n> - 2104x1560 = 1.348717949\n> - 1048x780 = 1.343589744\n>\n> It can be inferred that, that the aspect ratio only differs by a small\n> factor with each other. It does not makes sense to select a 4208x3118\n> for a requested size of say 640x480 or 1280x720, which is what is\n> happening currently.\n> ($) cam -c1 -swidth=640,height=480,role=raw\n> - CIO2 configuration: 4208x3118-SGRBG10_IPU3\n>\n> In order to address this constraint, only compare the ratio with single\n> precision to make a better decision on the sensor resolution policy\n> selection.\n>\n> ($) cam -c1 -srole=raw,width=640,height=480\n> - CIO2 configuration: 1048x780-SGRBG10_IPU3\n>\n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/pipeline/ipu3/cio2.cpp | 7 +++++++\n> 1 file changed, 7 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 9980b8bd..887b850d 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -281,6 +281,13 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector\n> \t\t\t\tcontinue;\n>\n> \t\t\tfloat ratio = static_cast(sz.width) / sz.height;\n> +\t\t\t/*\n> +\t\t\t * Comparing ratios with a single precision is enough.\n> +\t\t\t * This avoids the selection of a frame size which is\n> +\t\t\t * 2x or 3x than the requested size, having a slightly\n> +\t\t\t * better ratio w.r.t native resolution ratio.\n\nWell, the fact the 2x or 3x sizes selection happens is Nautilus\nspecific. I would stay simple and just\n\n /*\n * Comparing rations with a single precision\n * digit is enough.\n */\n\nBe aware that this actually truncates instead of rounding.. is this\nthe correct choice ? Also do we need to round the desired ratio too ?\n\nThanks\n j\n\n> +\t\t\t */\n> +\t\t\tratio = static_cast(ratio * 10) / 10.0;\n> \t\t\tfloat ratioDiff = fabsf(ratio - desiredRatio);\n> \t\t\tunsigned int area = sz.width * sz.height;\n> \t\t\tunsigned int areaDiff = area - desiredArea;\n> --\n> 2.31.0\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 F1C07BD87D\n\tfor ;\n\tMon, 9 Aug 2021 16:09:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 16EEB68826;\n\tMon, 9 Aug 2021 18:09:55 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B145E60269\n\tfor ;\n\tMon, 9 Aug 2021 18:09:54 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 422A060004;\n\tMon, 9 Aug 2021 16:09:53 +0000 (UTC)"],"Date":"Mon, 9 Aug 2021 18:10:42 +0200","From":"Jacopo Mondi ","To":"Umang Jain ","Message-ID":"<20210809161042.ecvevz7xufeajkvu@uno.localdomain>","References":"<20210803133205.6599-1-umang.jain@ideasonboard.com>\n\t<20210803133205.6599-5-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210803133205.6599-5-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: cio2: Tweak sensor size\n\tselection policy","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18660,"web_url":"https://patchwork.libcamera.org/comment/18660/","msgid":"<429d6363-0db9-45be-0b97-f4ca2b5149bc@ideasonboard.com>","date":"2021-08-10T07:48:43","subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: cio2: Tweak sensor size\n\tselection policy","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 8/9/21 9:40 PM, Jacopo Mondi wrote:\n> Hi Umang,\n>\n> On Tue, Aug 03, 2021 at 07:02:05PM +0530, Umang Jain wrote:\n>> Do not compare higher precision of the ratios, as it might lead to\n>> absurd selection of sensor size for a relatively low requested\n>> resolution size.\n>>\n>> For example:\n>> The imx258 driver supports the following sensor resolutions:\n>>\n>> - 4208x3118 = 1.349583066\n>> - 2104x1560 = 1.348717949\n>> - 1048x780 = 1.343589744\n>>\n>> It can be inferred that, that the aspect ratio only differs by a small\n>> factor with each other. It does not makes sense to select a 4208x3118\n>> for a requested size of say 640x480 or 1280x720, which is what is\n>> happening currently.\n>> ($) cam -c1 -swidth=640,height=480,role=raw\n>> - CIO2 configuration: 4208x3118-SGRBG10_IPU3\n>>\n>> In order to address this constraint, only compare the ratio with single\n>> precision to make a better decision on the sensor resolution policy\n>> selection.\n>>\n>> ($) cam -c1 -srole=raw,width=640,height=480\n>> - CIO2 configuration: 1048x780-SGRBG10_IPU3\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/libcamera/pipeline/ipu3/cio2.cpp | 7 +++++++\n>> 1 file changed, 7 insertions(+)\n>>\n>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n>> index 9980b8bd..887b850d 100644\n>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n>> @@ -281,6 +281,13 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector\n>> \t\t\t\tcontinue;\n>>\n>> \t\t\tfloat ratio = static_cast(sz.width) / sz.height;\n>> +\t\t\t/*\n>> +\t\t\t * Comparing ratios with a single precision is enough.\n>> +\t\t\t * This avoids the selection of a frame size which is\n>> +\t\t\t * 2x or 3x than the requested size, having a slightly\n>> +\t\t\t * better ratio w.r.t native resolution ratio.\n> Well, the fact the 2x or 3x sizes selection happens is Nautilus\n> specific. I would stay simple and just\n>\n> /*\n> * Comparing rations with a single precision\n> * digit is enough.\n> */\n>\n> Be aware that this actually truncates instead of rounding.. is this\n> the correct choice ? Also do we need to round the desired ratio too ?\n\nI think it's the correct choice of truncating for now. No, desired ratio \ncan stay as is, as it's constant. We need to nuke the subtle differences \nbetween \"competing\" sizes and either respective ratios\n\n.\n\n>\n> Thanks\n> j\n>\n>> +\t\t\t */\n>> +\t\t\tratio = static_cast(ratio * 10) / 10.0;\n>> \t\t\tfloat ratioDiff = fabsf(ratio - desiredRatio);\n>> \t\t\tunsigned int area = sz.width * sz.height;\n>> \t\t\tunsigned int areaDiff = area - desiredArea;\n>> --\n>> 2.31.0\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 63B9EC3240\n\tfor ;\n\tTue, 10 Aug 2021 07:48:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B4F9268826;\n\tTue, 10 Aug 2021 09:48:50 +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 DF64A687DE\n\tfor ;\n\tTue, 10 Aug 2021 09:48:49 +0200 (CEST)","from [192.168.1.104] (unknown [103.238.109.8])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 08B68EE;\n\tTue, 10 Aug 2021 09:48:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"o3WqT3Av\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628581729;\n\tbh=lZTdqj2Waxz114uPn4Ol4ycttj339UgClGF4NYqp3Hg=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=o3WqT3Avek6RXFtwAXEadVxrcwIsym0+s95756h0XKUWsglj81Fsu6C5SuE+reDDM\n\tOvVt2XL5hqhF2MWQqpedBQvj8RH+JMqOx4hj9/yAWOPHmlDPffwXWPEKKiZvwYTdSj\n\tqp0+l+5pis7V2Tvp5naXFTNDwLzpQqeELRSAHj8M=","To":"Jacopo Mondi ","References":"<20210803133205.6599-1-umang.jain@ideasonboard.com>\n\t<20210803133205.6599-5-umang.jain@ideasonboard.com>\n\t<20210809161042.ecvevz7xufeajkvu@uno.localdomain>","From":"Umang Jain ","Message-ID":"<429d6363-0db9-45be-0b97-f4ca2b5149bc@ideasonboard.com>","Date":"Tue, 10 Aug 2021 13:18:43 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210809161042.ecvevz7xufeajkvu@uno.localdomain>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: cio2: Tweak sensor size\n\tselection policy","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]