[{"id":19163,"web_url":"https://patchwork.libcamera.org/comment/19163/","msgid":"<5ee44de3-af95-21e3-d69e-8a8050875f0f@ideasonboard.com>","date":"2021-08-30T08:32:27","subject":"Re: [libcamera-devel] [PATCH v3 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 Me,\n\nOn 8/30/21 1:51 PM, 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\nThis cam command is invalid as of today. It was fine when I wrote the \nseries.\n\nThe invalidation comes from 4fec25a4b (\"ipu3: Disallow raw only camera \nconfiguration\"). I am tempted to remove this cam command (since it's not \nvalid today), but I don't think I would have a better explanation than \nthis. All I can do is to replace it with a block of text (essentially \nmaking the commit message still longer)\n\nAny ideas anyone?\n\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> Tested-by: Jacopo Mondi \n> Reviewed-by: Jacopo Mondi \n> Reviewed-by: Kieran Bingham \n> ---\n> src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index ceaf665e..9cedcb5b 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -295,6 +295,14 @@ 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 * Ratios can differ by small mantissa difference which\n> +\t\t\t * can affect the selection of the sensor output size\n> +\t\t\t * wildly. We are interested in selection of the closest\n> +\t\t\t * size with respect to the desired output size, hence\n> +\t\t\t * comparing it with a single precision digit is enough.\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;","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 EDF12BDC71\n\tfor ;\n\tMon, 30 Aug 2021 08:32:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EDA7C6916C;\n\tMon, 30 Aug 2021 10:32:34 +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 B599868934\n\tfor ;\n\tMon, 30 Aug 2021 10:32:33 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8CB0B5A7;\n\tMon, 30 Aug 2021 10:32:32 +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=\"fPwPXJXF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630312353;\n\tbh=R2g1ASu/p7zrm5lbqu88d46cqw84zFtrBFHNCBiWino=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=fPwPXJXFtzt60ByxT+WihSxpWPZHlMW3dKbaeGNw45M/MQ//VKyRl/G9AzePBJq/H\n\tbsbUZ2WK5CmH4LpXjmVGZ8zW0QCpqVAjhV6/0hhGar8rcapEihZ2jg8tBdGmpptK1u\n\tn3x65zZ0AMBhNEFmiz/ETF4tJbK6ao+EVKkdRzDs=","To":"libcamera-devel@lists.libcamera.org","References":"<20210830082150.42514-1-umang.jain@ideasonboard.com>\n\t<20210830082150.42514-5-umang.jain@ideasonboard.com>","From":"Umang Jain ","Message-ID":"<5ee44de3-af95-21e3-d69e-8a8050875f0f@ideasonboard.com>","Date":"Mon, 30 Aug 2021 14:02:27 +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":"<20210830082150.42514-5-umang.jain@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19164,"web_url":"https://patchwork.libcamera.org/comment/19164/","msgid":"<20210830085249.ocg2f4glzdegq3dh@uno.localdomain>","date":"2021-08-30T08:52:49","subject":"Re: [libcamera-devel] [PATCH v3 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":"On Mon, Aug 30, 2021 at 02:02:27PM +0530, Umang Jain wrote:\n> Hi Me,\n>\n> On 8/30/21 1:51 PM, 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>\n> This cam command is invalid as of today. It was fine when I wrote the\n> series.\n>\n> The invalidation comes from 4fec25a4b (\"ipu3: Disallow raw only camera\n> configuration\"). I am tempted to remove this cam command (since it's not\n> valid today), but I don't think I would have a better explanation than this.\n> All I can do is to replace it with a block of text (essentially making the\n> commit message still longer)\n>\n> Any ideas anyone?\n>\n\nWhat about adding a YUV stream of the same size to the command ?\n\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> > Tested-by: Jacopo Mondi \n> > Reviewed-by: Jacopo Mondi \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > 1 file changed, 8 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index ceaf665e..9cedcb5b 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > @@ -295,6 +295,14 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector\n> > \t\t\t\tcontinue;\n> > \t\t\tfloat ratio = static_cast(sz.width) / sz.height;\n> > +\t\t\t/*\n> > +\t\t\t * Ratios can differ by small mantissa difference which\n> > +\t\t\t * can affect the selection of the sensor output size\n> > +\t\t\t * wildly. We are interested in selection of the closest\n> > +\t\t\t * size with respect to the desired output size, hence\n> > +\t\t\t * comparing it with a single precision digit is enough.\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;","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 B5EDEBDC71\n\tfor ;\n\tMon, 30 Aug 2021 08:52:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3FEF16916A;\n\tMon, 30 Aug 2021 10:52:02 +0200 (CEST)","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 0A3D168934\n\tfor ;\n\tMon, 30 Aug 2021 10:52:01 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 4F40BC000E;\n\tMon, 30 Aug 2021 08:52:00 +0000 (UTC)"],"Date":"Mon, 30 Aug 2021 10:52:49 +0200","From":"Jacopo Mondi ","To":"Umang Jain ","Message-ID":"<20210830085249.ocg2f4glzdegq3dh@uno.localdomain>","References":"<20210830082150.42514-1-umang.jain@ideasonboard.com>\n\t<20210830082150.42514-5-umang.jain@ideasonboard.com>\n\t<5ee44de3-af95-21e3-d69e-8a8050875f0f@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<5ee44de3-af95-21e3-d69e-8a8050875f0f@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 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":19165,"web_url":"https://patchwork.libcamera.org/comment/19165/","msgid":"","date":"2021-08-30T10:08:44","subject":"Re: [libcamera-devel] [PATCH v3 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/30/21 2:22 PM, Jacopo Mondi wrote:\n> On Mon, Aug 30, 2021 at 02:02:27PM +0530, Umang Jain wrote:\n>> Hi Me,\n>>\n>> On 8/30/21 1:51 PM, 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>> This cam command is invalid as of today. It was fine when I wrote the\n>> series.\n>>\n>> The invalidation comes from 4fec25a4b (\"ipu3: Disallow raw only camera\n>> configuration\"). I am tempted to remove this cam command (since it's not\n>> valid today), but I don't think I would have a better explanation than this.\n>> All I can do is to replace it with a block of text (essentially making the\n>> commit message still longer)\n>>\n>> Any ideas anyone?\n>>\n> What about adding a YUV stream of the same size to the command ?\n\n\nAs of master, 7208e70211a6 (\"libcamera: ipu3: Always use sensor full \nframe size\") is in effect  right? So it's always selecting the highest \nsensor format resolution.\n\nI cherry-picked your \"DNI: libcamera: ipu3: Use the optimal sensor side\" \nwhich generates the same results as the commit message, to double-check.\n\nSo I don't adding YUV stream helps. Probably I'll just need to drop \nthese commands and explain everything in a text.\n\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>>> Tested-by: Jacopo Mondi \n>>> Reviewed-by: Jacopo Mondi \n>>> Reviewed-by: Kieran Bingham \n>>> ---\n>>> src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n>>> 1 file changed, 8 insertions(+)\n>>>\n>>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> index ceaf665e..9cedcb5b 100644\n>>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> @@ -295,6 +295,14 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector\n>>> \t\t\t\tcontinue;\n>>> \t\t\tfloat ratio = static_cast(sz.width) / sz.height;\n>>> +\t\t\t/*\n>>> +\t\t\t * Ratios can differ by small mantissa difference which\n>>> +\t\t\t * can affect the selection of the sensor output size\n>>> +\t\t\t * wildly. We are interested in selection of the closest\n>>> +\t\t\t * size with respect to the desired output size, hence\n>>> +\t\t\t * comparing it with a single precision digit is enough.\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;","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 45773BD87D\n\tfor ;\n\tMon, 30 Aug 2021 10:08:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4E1436916A;\n\tMon, 30 Aug 2021 12:08:51 +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 3A35660258\n\tfor ;\n\tMon, 30 Aug 2021 12:08:50 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A74625A7;\n\tMon, 30 Aug 2021 12:08:48 +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=\"wTpJPkzM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630318129;\n\tbh=1dBT4IUAfODrkeZ5LQcE6f2O7098wgnQmnkj/uFtljM=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=wTpJPkzMd/4e9MAZxLKHUseKiiYh+ObHrC7kjyVLZjtOpaWtgRiLw7zpRgHw3JepI\n\tJnzYaOLJwt5MbqtNcJ3tG/4Uv85N1drFhZSbrFSL8uN+8IuRxs/xpeR9QhfHME8qP3\n\trcaNa/rCLtWGAl8JA37+6igvt6oSuCh6zILj5Qpg=","To":"Jacopo Mondi ","References":"<20210830082150.42514-1-umang.jain@ideasonboard.com>\n\t<20210830082150.42514-5-umang.jain@ideasonboard.com>\n\t<5ee44de3-af95-21e3-d69e-8a8050875f0f@ideasonboard.com>\n\t<20210830085249.ocg2f4glzdegq3dh@uno.localdomain>","From":"Umang Jain ","Message-ID":"","Date":"Mon, 30 Aug 2021 15:38:44 +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":"<20210830085249.ocg2f4glzdegq3dh@uno.localdomain>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 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":19167,"web_url":"https://patchwork.libcamera.org/comment/19167/","msgid":"<20210830102338.ix3zbwuy5pywbhah@uno.localdomain>","date":"2021-08-30T10:23:38","subject":"Re: [libcamera-devel] [PATCH v3 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":"Hello Umang,\n\nOn Mon, Aug 30, 2021 at 03:38:44PM +0530, Umang Jain wrote:\n> Hi Jacopo\n>\n> On 8/30/21 2:22 PM, Jacopo Mondi wrote:\n> > On Mon, Aug 30, 2021 at 02:02:27PM +0530, Umang Jain wrote:\n> > > Hi Me,\n> > >\n> > > On 8/30/21 1:51 PM, 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> > > This cam command is invalid as of today. It was fine when I wrote the\n> > > series.\n> > >\n> > > The invalidation comes from 4fec25a4b (\"ipu3: Disallow raw only camera\n> > > configuration\"). I am tempted to remove this cam command (since it's not\n> > > valid today), but I don't think I would have a better explanation than this.\n> > > All I can do is to replace it with a block of text (essentially making the\n> > > commit message still longer)\n> > >\n> > > Any ideas anyone?\n> > >\n> > What about adding a YUV stream of the same size to the command ?\n>\n>\n> As of master, 7208e70211a6 (\"libcamera: ipu3: Always use sensor full frame\n> size\") is in effect  right? So it's always selecting the highest sensor\n> format resolution.\n\nAh ups, I'm running with my branch :)\n\n>\n> I cherry-picked your \"DNI: libcamera: ipu3: Use the optimal sensor side\"\n> which generates the same results as the commit message, to double-check.\n>\n> So I don't adding YUV stream helps. Probably I'll just need to drop these\n> commands and explain everything in a text.\n\nYou're right!\nOr maybe, for the sake of the example, just revert the commit that\nblocks the single RAW stream ?\n\nAnyway, up to you :)\n\n>\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> > > > Tested-by: Jacopo Mondi \n> > > > Reviewed-by: Jacopo Mondi \n> > > > Reviewed-by: Kieran Bingham \n> > > > ---\n> > > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > > 1 file changed, 8 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > index ceaf665e..9cedcb5b 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > @@ -295,6 +295,14 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector\n> > > > \t\t\t\tcontinue;\n> > > > \t\t\tfloat ratio = static_cast(sz.width) / sz.height;\n> > > > +\t\t\t/*\n> > > > +\t\t\t * Ratios can differ by small mantissa difference which\n> > > > +\t\t\t * can affect the selection of the sensor output size\n> > > > +\t\t\t * wildly. We are interested in selection of the closest\n> > > > +\t\t\t * size with respect to the desired output size, hence\n> > > > +\t\t\t * comparing it with a single precision digit is enough.\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;","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 AC1A9BDC71\n\tfor ;\n\tMon, 30 Aug 2021 10:22:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7682269166;\n\tMon, 30 Aug 2021 12:22:51 +0200 (CEST)","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 90C4160258\n\tfor ;\n\tMon, 30 Aug 2021 12:22:49 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id CCEA6C0019;\n\tMon, 30 Aug 2021 10:22:48 +0000 (UTC)"],"Date":"Mon, 30 Aug 2021 12:23:38 +0200","From":"Jacopo Mondi ","To":"Umang Jain ","Message-ID":"<20210830102338.ix3zbwuy5pywbhah@uno.localdomain>","References":"<20210830082150.42514-1-umang.jain@ideasonboard.com>\n\t<20210830082150.42514-5-umang.jain@ideasonboard.com>\n\t<5ee44de3-af95-21e3-d69e-8a8050875f0f@ideasonboard.com>\n\t<20210830085249.ocg2f4glzdegq3dh@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 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":19168,"web_url":"https://patchwork.libcamera.org/comment/19168/","msgid":"","date":"2021-08-30T10:32:41","subject":"Re: [libcamera-devel] [PATCH v3 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/30/21 3:53 PM, Jacopo Mondi wrote:\n> Hello Umang,\n>\n> On Mon, Aug 30, 2021 at 03:38:44PM +0530, Umang Jain wrote:\n>> Hi Jacopo\n>>\n>> On 8/30/21 2:22 PM, Jacopo Mondi wrote:\n>>> On Mon, Aug 30, 2021 at 02:02:27PM +0530, Umang Jain wrote:\n>>>> Hi Me,\n>>>>\n>>>> On 8/30/21 1:51 PM, 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>>>> This cam command is invalid as of today. It was fine when I wrote the\n>>>> series.\n>>>>\n>>>> The invalidation comes from 4fec25a4b (\"ipu3: Disallow raw only camera\n\nops, wrong commit message here, it should have been 0536a9aa7189f75c898\n\n\n>>>> configuration\"). I am tempted to remove this cam command (since it's not\n>>>> valid today), but I don't think I would have a better explanation than this.\n>>>> All I can do is to replace it with a block of text (essentially making the\n>>>> commit message still longer)\n>>>>\n>>>> Any ideas anyone?\n>>>>\n>>> What about adding a YUV stream of the same size to the command ?\n>>\n>> As of master, 7208e70211a6 (\"libcamera: ipu3: Always use sensor full frame\n>> size\") is in effect  right? So it's always selecting the highest sensor\n>> format resolution.\n> Ah ups, I'm running with my branch :)\n>\n>> I cherry-picked your \"DNI: libcamera: ipu3: Use the optimal sensor side\"\n>> which generates the same results as the commit message, to double-check.\n>>\n>> So I don't adding YUV stream helps. Probably I'll just need to drop these\n>> commands and explain everything in a text.\n> You're right!\n> Or maybe, for the sake of the example, just revert the commit that\n> blocks the single RAW stream ?\n\n\nYes, I can probably mention that with a '*' on the command.\n\n     * Please revert 0536a9aa718(\" ipu3: Disallow raw only camera \nconfiguration \") if the configuration is reported invalid\n\nIt seems we will have raw-only configs in the future (it's a acceptable \ncase to have, but just not sure when/priority), so at the time those \ncommand will be fine.\n\n\n>\n> Anyway, up to you :)\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>>>>> Tested-by: Jacopo Mondi \n>>>>> Reviewed-by: Jacopo Mondi \n>>>>> Reviewed-by: Kieran Bingham \n>>>>> ---\n>>>>> src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n>>>>> 1 file changed, 8 insertions(+)\n>>>>>\n>>>>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>>>> index ceaf665e..9cedcb5b 100644\n>>>>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n>>>>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>>>> @@ -295,6 +295,14 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector\n>>>>> \t\t\t\tcontinue;\n>>>>> \t\t\tfloat ratio = static_cast(sz.width) / sz.height;\n>>>>> +\t\t\t/*\n>>>>> +\t\t\t * Ratios can differ by small mantissa difference which\n>>>>> +\t\t\t * can affect the selection of the sensor output size\n>>>>> +\t\t\t * wildly. We are interested in selection of the closest\n>>>>> +\t\t\t * size with respect to the desired output size, hence\n>>>>> +\t\t\t * comparing it with a single precision digit is enough.\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;","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 0DDE5BD87D\n\tfor ;\n\tMon, 30 Aug 2021 10:32:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F4CE6916A;\n\tMon, 30 Aug 2021 12:32:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E50C460258\n\tfor ;\n\tMon, 30 Aug 2021 12:32:46 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DFE865A7;\n\tMon, 30 Aug 2021 12:32:45 +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=\"FYjXsxp7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630319566;\n\tbh=y7Ne0UQYfzS3nY/5vRzb9n8X1DXO0MJtFZnh2cnFmfI=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=FYjXsxp7AMHvyc7HUQ0cSBa2SHPAa1jUlKUW+HIZ18zXLAysNyYwDKucSSnbwgjFU\n\tmsibLR2j8ATIklPgeELOmd0eE6t7CFy8JCCFIFT91onPhsxRqnYpy6vkwDFxl9u3DZ\n\trD12Ytc2u6wXh1UWrrv84R/ZwK01H0ljaNNbLYBk=","To":"Jacopo Mondi ","References":"<20210830082150.42514-1-umang.jain@ideasonboard.com>\n\t<20210830082150.42514-5-umang.jain@ideasonboard.com>\n\t<5ee44de3-af95-21e3-d69e-8a8050875f0f@ideasonboard.com>\n\t<20210830085249.ocg2f4glzdegq3dh@uno.localdomain>\n\t\n\t<20210830102338.ix3zbwuy5pywbhah@uno.localdomain>","From":"Umang Jain ","Message-ID":"","Date":"Mon, 30 Aug 2021 16:02:41 +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":"<20210830102338.ix3zbwuy5pywbhah@uno.localdomain>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 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\" "}}]