[{"id":17369,"web_url":"https://patchwork.libcamera.org/comment/17369/","msgid":"","date":"2021-06-02T01:08:36","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n> We report a hard-coded frame duration value for CTS. However, when the\n> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n> duration was found to be out-of-range. Since different sensors have\n> different frame durations range-limits, we need to bump up our value\n> to match the lower end of the 'imx258' frame duration range-limit\n> (rounded up to the nearest micro-second).\n> \n> This allows the following CTS test to pass on nautilus platform:\n> - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/ipa/ipu3/ipu3.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 700a5660..e51b0401 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> \n> \t/* \\todo Populate this with real values */\n> \tctrls.set(controls::FrameDuration,\n> -\t\t static_cast(33334));\n> +\t\t static_cast(34483));\n\nI think it's time to fix this properly by calculating the actual frame\nduration. It should hopefully not be more complicated than\n\n\tduration = (width + hblank) * (height + vblank) / pixel_rate;\n\n> \n> \tIPU3Action op;\n> \top.op = ActionMetadataReady;","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 F3A8CC3206\n\tfor ;\n\tWed, 2 Jun 2021 01:08:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 72DB268922;\n\tWed, 2 Jun 2021 03:08:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 633DE602A8\n\tfor ;\n\tWed, 2 Jun 2021 03:08:47 +0200 (CEST)","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 DE9F550E;\n\tWed, 2 Jun 2021 03:08:46 +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=\"cJjs9YuY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622596127;\n\tbh=VbZ9wNldboKVR6O/JPeZywbxIrePQiwIlTbidfw0dik=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cJjs9YuYJB/P7mogG0IkH7rTtXN37/1zN5KI2z+XPTQg0CZL9heRPdqQuKqDXJknC\n\t1lG8kt6KElcwAD2NsO4fOUHzNXchwZ+BDyCLE5pCq9kqyGtimb+9KEV/FTildEqwEr\n\tQIEFrv3YL9q5F+CVEK8TV59b1/7ruDpuYjmK1KRk=","Date":"Wed, 2 Jun 2021 04:08:36 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210601112456.118755-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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":17383,"web_url":"https://patchwork.libcamera.org/comment/17383/","msgid":"","date":"2021-06-02T10:27:37","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 6/2/21 6:38 AM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n>> We report a hard-coded frame duration value for CTS. However, when the\n>> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n>> duration was found to be out-of-range. Since different sensors have\n>> different frame durations range-limits, we need to bump up our value\n>> to match the lower end of the 'imx258' frame duration range-limit\n>> (rounded up to the nearest micro-second).\n>>\n>> This allows the following CTS test to pass on nautilus platform:\n>> - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/ipa/ipu3/ipu3.cpp | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>> index 700a5660..e51b0401 100644\n>> --- a/src/ipa/ipu3/ipu3.cpp\n>> +++ b/src/ipa/ipu3/ipu3.cpp\n>> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n>> \n>> \t/* \\todo Populate this with real values */\n>> \tctrls.set(controls::FrameDuration,\n>> -\t\t static_cast(33334));\n>> +\t\t static_cast(34483));\n> I think it's time to fix this properly by calculating the actual frame\n> duration. It should hopefully not be more complicated than\n>\n> \tduration = (width + hblank) * (height + vblank) / pixel_rate;\nWe need to get vblank, for each frame's exposure I think. We need to \nplumb this into IPAIPU3 and JM seem to have the base helper class \nin-progress. For now, I have slightly made an improvement by using the \nminVBlank_ value instead. It doesn't makes CTS on nautilus pass, but \nanyway is an improvement over hard-coded value. I'll check with Paul, to \nsee if it doesn't regress CTS on soraka. Let me know you thoughts!\n>\n>> \n>> \tIPU3Action op;\n>> \top.op = ActionMetadataReady;","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 12AACC3206\n\tfor ;\n\tWed, 2 Jun 2021 10:27:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7BB886050E;\n\tWed, 2 Jun 2021 12:27:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E71D602A4\n\tfor ;\n\tWed, 2 Jun 2021 12:27:43 +0200 (CEST)","from [192.168.0.105] (unknown [103.251.226.189])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 631DB4A5;\n\tWed, 2 Jun 2021 12:27:42 +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=\"cYg67E1Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622629663;\n\tbh=cfNBB5a6hVjKuTvsHuLgHYGtJXXZ/TLd3oJalk61PpE=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=cYg67E1Z/jSanUf7MaovvsRiyadRUj37dqzBOaDPfqqQn1LxakZUx7EfqSx9Z+WGp\n\t8gdlsiHNzAbUUaA8MqS2d2KLHOiPD8bQREyySvkhclOG8qoapn4CwnI9Sxh5vz1rAH\n\tpBQdUXdqtG7C9M7SQN8tbcsQGOqvFKvI0n5H6Vtg=","To":"Laurent Pinchart ","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>\n\t","From":"Umang Jain ","Message-ID":"","Date":"Wed, 2 Jun 2021 15:57:37 +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":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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":17389,"web_url":"https://patchwork.libcamera.org/comment/17389/","msgid":"","date":"2021-06-02T18:10:30","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:\n> On 6/2/21 6:38 AM, Laurent Pinchart wrote:\n> > On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n> >> We report a hard-coded frame duration value for CTS. However, when the\n> >> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n> >> duration was found to be out-of-range. Since different sensors have\n> >> different frame durations range-limits, we need to bump up our value\n> >> to match the lower end of the 'imx258' frame duration range-limit\n> >> (rounded up to the nearest micro-second).\n> >>\n> >> This allows the following CTS test to pass on nautilus platform:\n> >> - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n> >>\n> >> Signed-off-by: Umang Jain \n> >> ---\n> >> src/ipa/ipu3/ipu3.cpp | 2 +-\n> >> 1 file changed, 1 insertion(+), 1 deletion(-)\n> >>\n> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> >> index 700a5660..e51b0401 100644\n> >> --- a/src/ipa/ipu3/ipu3.cpp\n> >> +++ b/src/ipa/ipu3/ipu3.cpp\n> >> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> >> \n> >> \t/* \\todo Populate this with real values */\n> >> \tctrls.set(controls::FrameDuration,\n> >> -\t\t static_cast(33334));\n> >> +\t\t static_cast(34483));\n> >\n> > I think it's time to fix this properly by calculating the actual frame\n> > duration. It should hopefully not be more complicated than\n> >\n> > \tduration = (width + hblank) * (height + vblank) / pixel_rate;\n>\n> We need to get vblank, for each frame's exposure I think. We need to \n> plumb this into IPAIPU3 and JM seem to have the base helper class \n> in-progress. For now, I have slightly made an improvement by using the \n> minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but \n> anyway is an improvement over hard-coded value. I'll check with Paul, to \n> see if it doesn't regress CTS on soraka. Let me know you thoughts!\n\nV4L2_CID_VBLANK is set by userspace. It can be updated by drivers when\nthe format and/or selection rectangles are modified, but will not\notherwise be changed automatically at runtime. The IPA should compute\nthe desired vblank value and pass it to the pipeline handler to\nconfigure the sensor. I don't think that's related to the work that JM\nis doing.\n\n> >> \n> >> \tIPU3Action op;\n> >> \top.op = ActionMetadataReady;","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 A3C52C3206\n\tfor ;\n\tWed, 2 Jun 2021 18:10:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 111506891E;\n\tWed, 2 Jun 2021 20:10:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 409E4602AF\n\tfor ;\n\tWed, 2 Jun 2021 20:10:42 +0200 (CEST)","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 B7547D97;\n\tWed, 2 Jun 2021 20:10:41 +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=\"IPzDFiW7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622657441;\n\tbh=7naCIt+DW5xdrjsDaka7Z3lNQtqIYgLqxZi03toX2Zs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IPzDFiW7jw2GCHikt4X2kzIrfTdzQruUZBxkF6c70LdAi9TScnyEJEJC3S9bj59oz\n\tbvIMdebVwKHjYpuPszXVop4ReRTq14ienZo2N03q2OwxX8b1EIvrZYqZHBbg9y2YrU\n\tDOwHk3Odbmc+Tuni7E8X/f/er5LdGJ5/CvVz1ZmI=","Date":"Wed, 2 Jun 2021 21:10:30 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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":17392,"web_url":"https://patchwork.libcamera.org/comment/17392/","msgid":"<56d0e283-2d17-1afa-b651-6339f2ed219a@ideasonboard.com>","date":"2021-06-03T03:11:25","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 6/2/21 11:40 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:\n>> On 6/2/21 6:38 AM, Laurent Pinchart wrote:\n>>> On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n>>>> We report a hard-coded frame duration value for CTS. However, when the\n>>>> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n>>>> duration was found to be out-of-range. Since different sensors have\n>>>> different frame durations range-limits, we need to bump up our value\n>>>> to match the lower end of the 'imx258' frame duration range-limit\n>>>> (rounded up to the nearest micro-second).\n>>>>\n>>>> This allows the following CTS test to pass on nautilus platform:\n>>>> - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n>>>>\n>>>> Signed-off-by: Umang Jain \n>>>> ---\n>>>> src/ipa/ipu3/ipu3.cpp | 2 +-\n>>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>>>> index 700a5660..e51b0401 100644\n>>>> --- a/src/ipa/ipu3/ipu3.cpp\n>>>> +++ b/src/ipa/ipu3/ipu3.cpp\n>>>> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n>>>> \n>>>> \t/* \\todo Populate this with real values */\n>>>> \tctrls.set(controls::FrameDuration,\n>>>> -\t\t static_cast(33334));\n>>>> +\t\t static_cast(34483));\n>>> I think it's time to fix this properly by calculating the actual frame\n>>> duration. It should hopefully not be more complicated than\n>>>\n>>> \tduration = (width + hblank) * (height + vblank) / pixel_rate;\n>> We need to get vblank, for each frame's exposure I think. We need to\n>> plumb this into IPAIPU3 and JM seem to have the base helper class\n>> in-progress. For now, I have slightly made an improvement by using the\n>> minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but\n>> anyway is an improvement over hard-coded value. I'll check with Paul, to\n>> see if it doesn't regress CTS on soraka. Let me know you thoughts!\n> V4L2_CID_VBLANK is set by userspace. It can be updated by drivers when\n> the format and/or selection rectangles are modified, but will not\n> otherwise be changed automatically at runtime. The IPA should compute\n> the desired vblank value and pass it to the pipeline handler to\n> configure the sensor. I don't think that's related to the work that JM\n> is doing.\n\n\nOk, I will check in with JM for that,\n\nMeanwhile, I saw and JM also mentioned that, for getting the vblank - we \nneed 'frameIntegrationDiff' (referencing CamHelper::GetVBlanking from \nRPi IPA). Looking into it, it's seems to a value tied to a particular \nsensor, so that's something. Not sure if we are going to put in some \nspecific sensor-related parameters into IPU3 IPA.\n\nCC-ing JM, Would like your thoughts on the above discussion.\n\n\n>\n>>>> \n>>>> \tIPU3Action op;\n>>>> \top.op = ActionMetadataReady;","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 25743C3206\n\tfor ;\n\tThu, 3 Jun 2021 03:11:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B2626892C;\n\tThu, 3 Jun 2021 05:11:33 +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 822B7602A2\n\tfor ;\n\tThu, 3 Jun 2021 05:11:31 +0200 (CEST)","from [192.168.0.105] (unknown [103.251.226.189])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5AE554A5;\n\tThu, 3 Jun 2021 05:11:30 +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=\"ctfvcPqb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622689891;\n\tbh=CP6yxFns721/CyOBkYFwGxJQBpFxr8Od0cwunPLnudk=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=ctfvcPqbrnJLw/uVB8OEU3uxksDmT9PGlD4GWzmWhyHWgpDIpxWAlvfZe6mNqsrLC\n\t4kAtGM2ItkBUolk0Qo8BLabZ4alNpF4ND0VnaaUhZsm3W8ekJ3to/W9LVexPO7JCe8\n\tb+c72D/N86L0jisZe2m2Qb4YBT+txOYrN27balhc=","To":"Laurent Pinchart ","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>\n\t\n\t\n\t","From":"Umang Jain ","Message-ID":"<56d0e283-2d17-1afa-b651-6339f2ed219a@ideasonboard.com>","Date":"Thu, 3 Jun 2021 08:41:25 +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":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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":17396,"web_url":"https://patchwork.libcamera.org/comment/17396/","msgid":"","date":"2021-06-03T10:33:54","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent and Umang,\n\nOn Wed, 2 Jun 2021 at 19:10, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Umang,\n>\n> On Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:\n> > On 6/2/21 6:38 AM, Laurent Pinchart wrote:\n> > > On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n> > >> We report a hard-coded frame duration value for CTS. However, when the\n> > >> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n> > >> duration was found to be out-of-range. Since different sensors have\n> > >> different frame durations range-limits, we need to bump up our value\n> > >> to match the lower end of the 'imx258' frame duration range-limit\n> > >> (rounded up to the nearest micro-second).\n> > >>\n> > >> This allows the following CTS test to pass on nautilus platform:\n> > >> -\n> android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n> > >>\n> > >> Signed-off-by: Umang Jain \n> > >> ---\n> > >> src/ipa/ipu3/ipu3.cpp | 2 +-\n> > >> 1 file changed, 1 insertion(+), 1 deletion(-)\n> > >>\n> > >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > >> index 700a5660..e51b0401 100644\n> > >> --- a/src/ipa/ipu3/ipu3.cpp\n> > >> +++ b/src/ipa/ipu3/ipu3.cpp\n> > >> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> > >>\n> > >> /* \\todo Populate this with real values */\n> > >> ctrls.set(controls::FrameDuration,\n> > >> - static_cast(33334));\n> > >> + static_cast(34483));\n> > >\n> > > I think it's time to fix this properly by calculating the actual frame\n> > > duration. It should hopefully not be more complicated than\n> > >\n> > > duration = (width + hblank) * (height + vblank) / pixel_rate;\n> >\n> > We need to get vblank, for each frame's exposure I think. We need to\n> > plumb this into IPAIPU3 and JM seem to have the base helper class\n> > in-progress. For now, I have slightly made an improvement by using the\n> > minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but\n> > anyway is an improvement over hard-coded value. I'll check with Paul, to\n> > see if it doesn't regress CTS on soraka. Let me know you thoughts!\n>\n> V4L2_CID_VBLANK is set by userspace. It can be updated by drivers when\n> the format and/or selection rectangles are modified, but will not\n> otherwise be changed automatically at runtime. The IPA should compute\n> the desired vblank value and pass it to the pipeline handler to\n> configure the sensor. I don't think that's related to the work that JM\n> is doing.\n>\n\nSorry for jumping in on this topic, but for RPi IPA, it is possible for\nVBLANK to\nchange at runtime when we are running in variable framerate mode. This\ndecision is down to the AGC and what exposure it decides to use.\n\nThis is also where we run into the problem with V4L2 control limits not\nallowing\nus to update VBLANK and EXPOSURE at the same time without using\npriority writes.\n\nNaush\n\n\n> > >>\n> > >> IPU3Action op;\n> > >> op.op = ActionMetadataReady;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 1DF63C3208\n\tfor ;\n\tThu, 3 Jun 2021 10:34:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B3FF6050E;\n\tThu, 3 Jun 2021 12:34:13 +0200 (CEST)","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6352E602A2\n\tfor ;\n\tThu, 3 Jun 2021 12:34:11 +0200 (CEST)","by mail-lj1-x232.google.com with SMTP id u22so6477451ljh.7\n\tfor ;\n\tThu, 03 Jun 2021 03:34:11 -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=\"i2cM53/R\"; 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=KiyN+4j6Ckv5SGl+LoUZoPzSpBxPhEpuBYk7Iiq5z/U=;\n\tb=i2cM53/RLA1E8rSZCF3fzFv3dkBC/+KF4iMgQRxQEUnKzZi9fOOvvyW6F1fxnosdUR\n\tjyZ8A/9kNmqJOkpnxkiJ874fywUu0swkeSxSW1gHgnuxgmcSxMPxzkceywx+hmMKW2GH\n\t+Vnw4HBmo+yXit2g3/1Cmphuj3jhssiijMV4QdODziZwR6JtPRXwFtIwn1JjYTdmTPxI\n\tWb1BjfAJ6ttnnex/hUFD67TTwdozyjQbYAPCFykdvqphr8Z0ixq7AhUYpMGJuo8raZEi\n\th3Ikmz5nu2+kpzS6lBLqDeKvxUEe/Kbghvsei84vf+jqUIul+xz+IygJ37TLGn/DH9c3\n\tZe9Q==","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=KiyN+4j6Ckv5SGl+LoUZoPzSpBxPhEpuBYk7Iiq5z/U=;\n\tb=TKT/yLLf4BQ1p7wCBr1BjSDcoV5Bnq0sNYJT7DrFaF3xFe5xsk6c/4DbWx3D/kLC2i\n\tjTTicQzgM/3kLeRwq7X8rUu3ufmCc/NA2DdLrd+o+YGuZfa+HuzYfoYiYeDdjG5vOQ0r\n\tq6QqX//oqBMulfioxeQSIbYHOetqbpWIf6qcCx6bnRW9VJNXot9Vf63a5fXFJJ1bKGU4\n\t6LHkJ/lbUk9rPf2VeZm85W82bd7JbN44WMGGwY2CHszDzHks2mM2/jBmdlaMVzy5dQ3h\n\t0c8NGGQOoQtO2I9tGHuiV++YZYssH3S2BkS6b0u+VF5efrfxnsus3WZxcZ5XJhhLQ7YI\n\tOrTQ==","X-Gm-Message-State":"AOAM531AKCaNdT5hCrMxEiMVfaYbSuUrDnAdWX2wK6ablElSGozp6GjO\n\tosq0o/zaJjTA88laHrdIjW4m10BHCB+CsDi54ZNQALDQbv0=","X-Google-Smtp-Source":"ABdhPJyhRYWKhefrSvv499avSBlhkbKNg41qqgVfz/Zp7VMf6vt8K49RTopFPodhj3ccd4OerMTaJFlEEbrladQy+Lo=","X-Received":"by 2002:a2e:780c:: with SMTP id\n\tt12mr17070249ljc.253.1622716450478; \n\tThu, 03 Jun 2021 03:34:10 -0700 (PDT)","MIME-Version":"1.0","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 3 Jun 2021 11:33:54 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"0000000000009a2e7005c3da1d14\"","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17423,"web_url":"https://patchwork.libcamera.org/comment/17423/","msgid":"","date":"2021-06-07T00:05:05","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Thu, Jun 03, 2021 at 08:41:25AM +0530, Umang Jain wrote:\n> On 6/2/21 11:40 PM, Laurent Pinchart wrote:\n> > On Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:\n> >> On 6/2/21 6:38 AM, Laurent Pinchart wrote:\n> >>> On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n> >>>> We report a hard-coded frame duration value for CTS. However, when the\n> >>>> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n> >>>> duration was found to be out-of-range. Since different sensors have\n> >>>> different frame durations range-limits, we need to bump up our value\n> >>>> to match the lower end of the 'imx258' frame duration range-limit\n> >>>> (rounded up to the nearest micro-second).\n> >>>>\n> >>>> This allows the following CTS test to pass on nautilus platform:\n> >>>> - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n> >>>>\n> >>>> Signed-off-by: Umang Jain \n> >>>> ---\n> >>>> src/ipa/ipu3/ipu3.cpp | 2 +-\n> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n> >>>>\n> >>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> >>>> index 700a5660..e51b0401 100644\n> >>>> --- a/src/ipa/ipu3/ipu3.cpp\n> >>>> +++ b/src/ipa/ipu3/ipu3.cpp\n> >>>> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> >>>> \n> >>>> \t/* \\todo Populate this with real values */\n> >>>> \tctrls.set(controls::FrameDuration,\n> >>>> -\t\t static_cast(33334));\n> >>>> +\t\t static_cast(34483));\n> >>>\n> >>> I think it's time to fix this properly by calculating the actual frame\n> >>> duration. It should hopefully not be more complicated than\n> >>>\n> >>> \tduration = (width + hblank) * (height + vblank) / pixel_rate;\n> >>\n> >> We need to get vblank, for each frame's exposure I think. We need to\n> >> plumb this into IPAIPU3 and JM seem to have the base helper class\n> >> in-progress. For now, I have slightly made an improvement by using the\n> >> minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but\n> >> anyway is an improvement over hard-coded value. I'll check with Paul, to\n> >> see if it doesn't regress CTS on soraka. Let me know you thoughts!\n> >\n> > V4L2_CID_VBLANK is set by userspace. It can be updated by drivers when\n> > the format and/or selection rectangles are modified, but will not\n> > otherwise be changed automatically at runtime. The IPA should compute\n> > the desired vblank value and pass it to the pipeline handler to\n> > configure the sensor. I don't think that's related to the work that JM\n> > is doing.\n> \n> Ok, I will check in with JM for that,\n> \n> Meanwhile, I saw and JM also mentioned that, for getting the vblank - we \n> need 'frameIntegrationDiff' (referencing CamHelper::GetVBlanking from \n> RPi IPA). Looking into it, it's seems to a value tied to a particular \n> sensor, so that's something. Not sure if we are going to put in some \n> specific sensor-related parameters into IPU3 IPA.\n\nWe will, Jean-Michel is working on that.\n\nThe frame integration diff value will be used by the IPA to compute the\ndesired exposure time and/or vertical blanking, but you won't need to\ncare about it to compute the frame duration. Only the vblank value is\nneeded for that.\n\n> CC-ing JM, Would like your thoughts on the above discussion.\n> \n> >>>> \n> >>>> \tIPU3Action op;\n> >>>> \top.op = ActionMetadataReady;","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 AD3CAC3206\n\tfor ;\n\tMon, 7 Jun 2021 00:05:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 15F2B6892E;\n\tMon, 7 Jun 2021 02:05:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 09F3C68921\n\tfor ;\n\tMon, 7 Jun 2021 02:05:19 +0200 (CEST)","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 7817680F;\n\tMon, 7 Jun 2021 02:05:18 +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=\"jsIyF7ed\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623024318;\n\tbh=U2WgR7TL70xzw0WcrRiXWx/PIolhdRm9S/WE4U8iSss=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jsIyF7edurgnHFnEQWN49fngsifLCq2FzBLvo/cDy+rhDhIfGqE6y35EskVOAXxUL\n\tfXYPKb8fMbaU8o0hlDKDLDO/0OtlIoqd5bKGrR/rCMF1Y9rhrszujx+T20YIrt1Q/A\n\tDCnZ4ZodjaaWf8wsukKDEgjPcyJeCNBK3nxVwlSY=","Date":"Mon, 7 Jun 2021 03:05:05 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>\n\t\n\t\n\t\n\t<56d0e283-2d17-1afa-b651-6339f2ed219a@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<56d0e283-2d17-1afa-b651-6339f2ed219a@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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":17424,"web_url":"https://patchwork.libcamera.org/comment/17424/","msgid":"","date":"2021-06-07T00:07:04","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Thu, Jun 03, 2021 at 11:33:54AM +0100, Naushir Patuck wrote:\n> On Wed, 2 Jun 2021 at 19:10, Laurent Pinchart wrote:\n> > On Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:\n> > > On 6/2/21 6:38 AM, Laurent Pinchart wrote:\n> > > > On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n> > > >> We report a hard-coded frame duration value for CTS. However, when the\n> > > >> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n> > > >> duration was found to be out-of-range. Since different sensors have\n> > > >> different frame durations range-limits, we need to bump up our value\n> > > >> to match the lower end of the 'imx258' frame duration range-limit\n> > > >> (rounded up to the nearest micro-second).\n> > > >>\n> > > >> This allows the following CTS test to pass on nautilus platform:\n> > > >> - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n> > > >>\n> > > >> Signed-off-by: Umang Jain \n> > > >> ---\n> > > >> src/ipa/ipu3/ipu3.cpp | 2 +-\n> > > >> 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > >>\n> > > >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > >> index 700a5660..e51b0401 100644\n> > > >> --- a/src/ipa/ipu3/ipu3.cpp\n> > > >> +++ b/src/ipa/ipu3/ipu3.cpp\n> > > >> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> > > >>\n> > > >> /* \\todo Populate this with real values */\n> > > >> ctrls.set(controls::FrameDuration,\n> > > >> - static_cast(33334));\n> > > >> + static_cast(34483));\n> > > >\n> > > > I think it's time to fix this properly by calculating the actual frame\n> > > > duration. It should hopefully not be more complicated than\n> > > >\n> > > > duration = (width + hblank) * (height + vblank) / pixel_rate;\n> > >\n> > > We need to get vblank, for each frame's exposure I think. We need to\n> > > plumb this into IPAIPU3 and JM seem to have the base helper class\n> > > in-progress. For now, I have slightly made an improvement by using the\n> > > minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but\n> > > anyway is an improvement over hard-coded value. I'll check with Paul, to\n> > > see if it doesn't regress CTS on soraka. Let me know you thoughts!\n> >\n> > V4L2_CID_VBLANK is set by userspace. It can be updated by drivers when\n> > the format and/or selection rectangles are modified, but will not\n> > otherwise be changed automatically at runtime. The IPA should compute\n> > the desired vblank value and pass it to the pipeline handler to\n> > configure the sensor. I don't think that's related to the work that JM\n> > is doing.\n> \n> Sorry for jumping in on this topic, but for RPi IPA, it is possible for VBLANK to\n> change at runtime when we are running in variable framerate mode. This\n> decision is down to the AGC and what exposure it decides to use.\n\nSure, my point was that the IPA controls the vblank value, the driver\ndoesn't adjust it automatically at runtime.\n\n> This is also where we run into the problem with V4L2 control limits not allowing\n> us to update VBLANK and EXPOSURE at the same time without using\n> priority writes.\n> \n> > > >>\n> > > >> IPU3Action op;\n> > > >> op.op = ActionMetadataReady;","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 894EDC3206\n\tfor ;\n\tMon, 7 Jun 2021 00:07:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E8B3568928;\n\tMon, 7 Jun 2021 02:07:19 +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 2559C68921\n\tfor ;\n\tMon, 7 Jun 2021 02:07:18 +0200 (CEST)","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 90C6D80F;\n\tMon, 7 Jun 2021 02:07:17 +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=\"MP8B8hym\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623024437;\n\tbh=q/laNucUywAyAxjekL7Lrq+42cWttEsR7qJ/Yus51x4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MP8B8hym3hc14sB2+F86WE63OsaOPMWOUo9higu8YNnLT2kx2eKBCrc5ODRMvnRPG\n\tY24N8m50Ubj5eydGS9x/TP/dsS1uKHsFTUn7MzOYLDRFsInc8G3iheY+nGEv0YEe0j\n\tCFtDDL2ZbVf4w8peYktyhYTvToZkQNz8kXqnlNXY=","Date":"Mon, 7 Jun 2021 03:07:04 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17438,"web_url":"https://patchwork.libcamera.org/comment/17438/","msgid":"<07519bcb-bce9-1d77-99f7-d96d820c419f@ideasonboard.com>","date":"2021-06-07T13:29:57","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 6/7/21 5:35 AM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Thu, Jun 03, 2021 at 08:41:25AM +0530, Umang Jain wrote:\n>> On 6/2/21 11:40 PM, Laurent Pinchart wrote:\n>>> On Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:\n>>>> On 6/2/21 6:38 AM, Laurent Pinchart wrote:\n>>>>> On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n>>>>>> We report a hard-coded frame duration value for CTS. However, when the\n>>>>>> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n>>>>>> duration was found to be out-of-range. Since different sensors have\n>>>>>> different frame durations range-limits, we need to bump up our value\n>>>>>> to match the lower end of the 'imx258' frame duration range-limit\n>>>>>> (rounded up to the nearest micro-second).\n>>>>>>\n>>>>>> This allows the following CTS test to pass on nautilus platform:\n>>>>>> - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n>>>>>>\n>>>>>> Signed-off-by: Umang Jain \n>>>>>> ---\n>>>>>> src/ipa/ipu3/ipu3.cpp | 2 +-\n>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>>\n>>>>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>>>>>> index 700a5660..e51b0401 100644\n>>>>>> --- a/src/ipa/ipu3/ipu3.cpp\n>>>>>> +++ b/src/ipa/ipu3/ipu3.cpp\n>>>>>> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n>>>>>> \n>>>>>> \t/* \\todo Populate this with real values */\n>>>>>> \tctrls.set(controls::FrameDuration,\n>>>>>> -\t\t static_cast(33334));\n>>>>>> +\t\t static_cast(34483));\n>>>>> I think it's time to fix this properly by calculating the actual frame\n>>>>> duration. It should hopefully not be more complicated than\n>>>>>\n>>>>> \tduration = (width + hblank) * (height + vblank) / pixel_rate;\n>>>> We need to get vblank, for each frame's exposure I think. We need to\n>>>> plumb this into IPAIPU3 and JM seem to have the base helper class\n>>>> in-progress. For now, I have slightly made an improvement by using the\n>>>> minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but\n>>>> anyway is an improvement over hard-coded value. I'll check with Paul, to\n>>>> see if it doesn't regress CTS on soraka. Let me know you thoughts!\n>>> V4L2_CID_VBLANK is set by userspace. It can be updated by drivers when\n>>> the format and/or selection rectangles are modified, but will not\n>>> otherwise be changed automatically at runtime. The IPA should compute\n>>> the desired vblank value and pass it to the pipeline handler to\n>>> configure the sensor. I don't think that's related to the work that JM\n>>> is doing.\n>> Ok, I will check in with JM for that,\n>>\n>> Meanwhile, I saw and JM also mentioned that, for getting the vblank - we\n>> need 'frameIntegrationDiff' (referencing CamHelper::GetVBlanking from\n>> RPi IPA). Looking into it, it's seems to a value tied to a particular\n>> sensor, so that's something. Not sure if we are going to put in some\n>> specific sensor-related parameters into IPU3 IPA.\n> We will, Jean-Michel is working on that.\n>\n> The frame integration diff value will be used by the IPA to compute the\n> desired exposure time and/or vertical blanking, but you won't need to\n> care about it to compute the frame duration. Only the vblank value is\n> needed for that.\n\n\nYes, the vblank value is needed and currently I am using the minimum \nvblank value\nqueried from sensor controls in the \" ipa: ipu3: Calculate frame \nduration from minimum VBLANK value\"\npatch. It also contains the \\todo stating that it can be improved on, by \nusing vblank from each frame's exposure.\n\nCan you please take a look at that series too,  \"IPAIPU3 drive-by \nimprovements\"\n\n\n>\n>> CC-ing JM, Would like your thoughts on the above discussion.\n>>\n>>>>>> \n>>>>>> \tIPU3Action op;\n>>>>>> \top.op = ActionMetadataReady;","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 24A0CC320B\n\tfor ;\n\tMon, 7 Jun 2021 13:30:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 77A36602A7;\n\tMon, 7 Jun 2021 15:30:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E89B9602A1\n\tfor ;\n\tMon, 7 Jun 2021 15:30:03 +0200 (CEST)","from [192.168.0.107] (unknown [103.251.226.98])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 466CE8DB;\n\tMon, 7 Jun 2021 15:30:01 +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=\"BtycFjfC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623072603;\n\tbh=jp/grhdiejm2tG2Vu4oH8VGULAvczxwkCRiuigK3IDE=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=BtycFjfCv3GikafdicpoBLkN9oIODHSUBTYnvDud5onTRlMVlJsesaSn8B4mYYF2Q\n\tTPTi7QqKFjSbtnFjVpiagr2nwj6Ae+QJ9k2rrRT5PB+975GBOmxAP4MP++PdEkEjPE\n\t3Ioca/6iu1suU/hO0Gfuq0m/ImmEtx/qHHLDpiBc=","To":"Laurent Pinchart ","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>\n\t\n\t\n\t\n\t<56d0e283-2d17-1afa-b651-6339f2ed219a@ideasonboard.com>\n\t","From":"Umang Jain ","Message-ID":"<07519bcb-bce9-1d77-99f7-d96d820c419f@ideasonboard.com>","Date":"Mon, 7 Jun 2021 18:59:57 +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":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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":17452,"web_url":"https://patchwork.libcamera.org/comment/17452/","msgid":"","date":"2021-06-07T21:45:28","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Mon, Jun 07, 2021 at 06:59:57PM +0530, Umang Jain wrote:\n> On 6/7/21 5:35 AM, Laurent Pinchart wrote:\n> > On Thu, Jun 03, 2021 at 08:41:25AM +0530, Umang Jain wrote:\n> >> On 6/2/21 11:40 PM, Laurent Pinchart wrote:\n> >>> On Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:\n> >>>> On 6/2/21 6:38 AM, Laurent Pinchart wrote:\n> >>>>> On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:\n> >>>>>> We report a hard-coded frame duration value for CTS. However, when the\n> >>>>>> CTS is tested on nautilus, which has a 'imx258' sensor, the frame\n> >>>>>> duration was found to be out-of-range. Since different sensors have\n> >>>>>> different frame durations range-limits, we need to bump up our value\n> >>>>>> to match the lower end of the 'imx258' frame duration range-limit\n> >>>>>> (rounded up to the nearest micro-second).\n> >>>>>>\n> >>>>>> This allows the following CTS test to pass on nautilus platform:\n> >>>>>> - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl\n> >>>>>>\n> >>>>>> Signed-off-by: Umang Jain \n> >>>>>> ---\n> >>>>>> src/ipa/ipu3/ipu3.cpp | 2 +-\n> >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n> >>>>>>\n> >>>>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> >>>>>> index 700a5660..e51b0401 100644\n> >>>>>> --- a/src/ipa/ipu3/ipu3.cpp\n> >>>>>> +++ b/src/ipa/ipu3/ipu3.cpp\n> >>>>>> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> >>>>>> \n> >>>>>> \t/* \\todo Populate this with real values */\n> >>>>>> \tctrls.set(controls::FrameDuration,\n> >>>>>> -\t\t static_cast(33334));\n> >>>>>> +\t\t static_cast(34483));\n> >>>>>\n> >>>>> I think it's time to fix this properly by calculating the actual frame\n> >>>>> duration. It should hopefully not be more complicated than\n> >>>>>\n> >>>>> \tduration = (width + hblank) * (height + vblank) / pixel_rate;\n> >>>>\n> >>>> We need to get vblank, for each frame's exposure I think. We need to\n> >>>> plumb this into IPAIPU3 and JM seem to have the base helper class\n> >>>> in-progress. For now, I have slightly made an improvement by using the\n> >>>> minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but\n> >>>> anyway is an improvement over hard-coded value. I'll check with Paul, to\n> >>>> see if it doesn't regress CTS on soraka. Let me know you thoughts!\n> >>>\n> >>> V4L2_CID_VBLANK is set by userspace. It can be updated by drivers when\n> >>> the format and/or selection rectangles are modified, but will not\n> >>> otherwise be changed automatically at runtime. The IPA should compute\n> >>> the desired vblank value and pass it to the pipeline handler to\n> >>> configure the sensor. I don't think that's related to the work that JM\n> >>> is doing.\n> >>\n> >> Ok, I will check in with JM for that,\n> >>\n> >> Meanwhile, I saw and JM also mentioned that, for getting the vblank - we\n> >> need 'frameIntegrationDiff' (referencing CamHelper::GetVBlanking from\n> >> RPi IPA). Looking into it, it's seems to a value tied to a particular\n> >> sensor, so that's something. Not sure if we are going to put in some\n> >> specific sensor-related parameters into IPU3 IPA.\n> >\n> > We will, Jean-Michel is working on that.\n> >\n> > The frame integration diff value will be used by the IPA to compute the\n> > desired exposure time and/or vertical blanking, but you won't need to\n> > care about it to compute the frame duration. Only the vblank value is\n> > needed for that.\n> \n> Yes, the vblank value is needed and currently I am using the minimum \n> vblank value\n> queried from sensor controls in the \" ipa: ipu3: Calculate frame \n> duration from minimum VBLANK value\"\n> patch. It also contains the \\todo stating that it can be improved on, by \n> using vblank from each frame's exposure.\n\nAny chance we could address this \\todo now already ? I fear it may be a\ncause for regressions.\n\n> Can you please take a look at that series too,  \"IPAIPU3 drive-by \n> improvements\"\n\nNext on my list :-)\n\n> >> CC-ing JM, Would like your thoughts on the above discussion.\n> >>\n> >>>>>> \n> >>>>>> \tIPU3Action op;\n> >>>>>> \top.op = ActionMetadataReady;","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 97A95C3206\n\tfor ;\n\tMon, 7 Jun 2021 21:45:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 147E268922;\n\tMon, 7 Jun 2021 23:45:44 +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 90B3C6891F\n\tfor ;\n\tMon, 7 Jun 2021 23:45:42 +0200 (CEST)","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 04F668DB;\n\tMon, 7 Jun 2021 23:45:41 +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=\"VvQqtFZ0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623102342;\n\tbh=6P245DgRMuT/OAgX5dhUTfK6e9QFkgHaNMScS38Mg78=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VvQqtFZ0iTSBV24wpvUkc6gwFug+q4DI3ifItSufi3XiOqluYKNWstdQv1hWCAn/L\n\tHj8ovxBYvPomEXs9LYV0VqhoThULbKcJxzxE0LZ65BidZS+34DWXfe0y+DFQXeJ95l\n\tsQWJXR73vePXpeVbi6t6PUMhJLhAq4woDcKAbyu0=","Date":"Tue, 8 Jun 2021 00:45:28 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210601112456.118755-1-umang.jain@ideasonboard.com>\n\t<20210601112456.118755-3-umang.jain@ideasonboard.com>\n\t\n\t\n\t\n\t<56d0e283-2d17-1afa-b651-6339f2ed219a@ideasonboard.com>\n\t\n\t<07519bcb-bce9-1d77-99f7-d96d820c419f@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<07519bcb-bce9-1d77-99f7-d96d820c419f@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration\n\tvalue","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\" "}}]