[{"id":20823,"web_url":"https://patchwork.libcamera.org/comment/20823/","msgid":"<163658101535.2121661.3753490321961392926@Monstersaurus>","date":"2021-11-10T21:50:15","subject":"Re: [libcamera-devel] [PATCH v2 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi JM,\n\nQuoting Jean-Michel Hautbois (2021-11-10 19:58:50)\n> The pipeline handler populates the new sensorControls ControlList, to\n> have the effective exposure and gain values for the current frame. This\n> is done when a statistics buffer is received.\n> \n> Make those values the frameContext::agc values for the frame when the\n> EventStatReady event is received.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/ipu3/ipu3.cpp | 4 ++++\n> 1 file changed, 4 insertions(+)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index bcc3863b..1111f200 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -549,6 +549,10 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n> const ipu3_uapi_stats_3a *stats =\n> reinterpret_cast(mem.data());\n> \n> + /* \\todo move those into processControls ? */\n> + context_.frameContext.agc.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get();\n> + context_.frameContext.agc.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n\nMy only worry with this reading it now, is that we are storing the\nexposure and gain of the frame that that is associated with the\nstatistics in what I had thought was a structure that maps the 'output'\nof the algorithms.\n\nYou've explored this a bit more now - so you probably have a much better\nidea than me. This is ultimately an 'input' to the algorithms. Do we\nneed to distinguish between the 'input' and 'output' ? (I.e. do we need\nto have both values kept so that the algorithm can determine how much it\nis changing? Or is that not a problem because it's all handled\ninternally anyway....\n\n> +\n> parseStatistics(event.frame, event.frameTimestamp, stats);\n> break;\n> }\n> -- \n> 2.32.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 5BC7BBF415\n\tfor ;\n\tWed, 10 Nov 2021 21:50:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B410F6034E;\n\tWed, 10 Nov 2021 22:50:19 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C3626033C\n\tfor ;\n\tWed, 10 Nov 2021 22:50:18 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 315298B6;\n\tWed, 10 Nov 2021 22:50:18 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YIfOIvqy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636581018;\n\tbh=at48mE+JLVqzqkCR0l+wC0tILki4sHJuIeEbYmwzrv4=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=YIfOIvqypIhZqMVUNObP0qg14dAEsELvh6CDuJWTASqH76xMpOBwR/loAvS5mdVTX\n\tEppIdy7grGuBFHLmOeybT6silYrXMm2RA94BSn1YM47v9h32//aIFRRdpRg5QUz9LN\n\tVpW+F164qw/f8IbC4Or6Cwwf2unnVkVl/vBqlPS4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211110195901.85597-4-jeanmichel.hautbois@ideasonboard.com>","References":"<20211110195901.85597-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211110195901.85597-4-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 10 Nov 2021 21:50:15 +0000","Message-ID":"<163658101535.2121661.3753490321961392926@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","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":20827,"web_url":"https://patchwork.libcamera.org/comment/20827/","msgid":"<15ff6e9e-1ad6-f59a-e37f-9b9cf29883ec@ideasonboard.com>","date":"2021-11-10T22:10:54","subject":"Re: [libcamera-devel] [PATCH v2 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 10/11/2021 22:50, Kieran Bingham wrote:\n> Hi JM,\n> \n> Quoting Jean-Michel Hautbois (2021-11-10 19:58:50)\n>> The pipeline handler populates the new sensorControls ControlList, to\n>> have the effective exposure and gain values for the current frame. This\n>> is done when a statistics buffer is received.\n>>\n>> Make those values the frameContext::agc values for the frame when the\n>> EventStatReady event is received.\n>>\n>> Signed-off-by: Jean-Michel Hautbois \n>> ---\n>> src/ipa/ipu3/ipu3.cpp | 4 ++++\n>> 1 file changed, 4 insertions(+)\n>>\n>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>> index bcc3863b..1111f200 100644\n>> --- a/src/ipa/ipu3/ipu3.cpp\n>> +++ b/src/ipa/ipu3/ipu3.cpp\n>> @@ -549,6 +549,10 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n>> const ipu3_uapi_stats_3a *stats =\n>> reinterpret_cast(mem.data());\n>> \n>> + /* \\todo move those into processControls ? */\n>> + context_.frameContext.agc.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get();\n>> + context_.frameContext.agc.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> \n> My only worry with this reading it now, is that we are storing the\n> exposure and gain of the frame that that is associated with the\n> statistics in what I had thought was a structure that maps the 'output'\n> of the algorithms.\n\nI am not sure it is only the output, it is a frame context, input and \noutput.\n\n> \n> You've explored this a bit more now - so you probably have a much better\n> idea than me. This is ultimately an 'input' to the algorithms. Do we\n> need to distinguish between the 'input' and 'output' ? (I.e. do we need\n> to have both values kept so that the algorithm can determine how much it\n> is changing? Or is that not a problem because it's all handled\n> internally anyway....\n\nWe update the value for the input of the algorithms, and they will \nupdate those values. It is debatable though: should we have two \nframeContexts, one before and one after the algorithm for instance ? I \ncan't see a real value to add this though.\n\n> \n>> +\n>> parseStatistics(event.frame, event.frameTimestamp, stats);\n>> break;\n>> }\n>> -- \n>> 2.32.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 1024EBDB1C\n\tfor ;\n\tWed, 10 Nov 2021 22:11:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E18D6034E;\n\tWed, 10 Nov 2021 23:10:59 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B830C6033C\n\tfor ;\n\tWed, 10 Nov 2021 23:10:57 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:7713:3465:89e6:c5cd] (unknown\n\t[IPv6:2a01:e0a:169:7140:7713:3465:89e6:c5cd])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 586378B6;\n\tWed, 10 Nov 2021 23:10:57 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tcxojpdy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636582257;\n\tbh=ntAYiOkPiw4q1Ergpg0xkzJ7OtGKxlGGawWW3lWs+gg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=tcxojpdy+D/79JiXYNRK9G6jmZviEWRewKMRxUiQlySt78eObWJj5k3Qq7pEame9L\n\tbBNxxgEd+qF8zpZuDLmtvINJ7+c+hSNBuuxsuQj4bfsZWyWsU/eT21byMbla+HwUK4\n\tFwni5sApDIdeEdWd2jI3U4uNma0iENiTveVGiNZg=","Message-ID":"<15ff6e9e-1ad6-f59a-e37f-9b9cf29883ec@ideasonboard.com>","Date":"Wed, 10 Nov 2021 23:10:54 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.2.1","Content-Language":"en-US","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211110195901.85597-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211110195901.85597-4-jeanmichel.hautbois@ideasonboard.com>\n\t<163658101535.2121661.3753490321961392926@Monstersaurus>","From":"Jean-Michel Hautbois ","In-Reply-To":"<163658101535.2121661.3753490321961392926@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","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":20836,"web_url":"https://patchwork.libcamera.org/comment/20836/","msgid":"<163658888504.2121661.2660154728195722587@Monstersaurus>","date":"2021-11-11T00:01:25","subject":"Re: [libcamera-devel] [PATCH v2 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-10 22:10:54)\n> Hi Kieran,\n> \n> On 10/11/2021 22:50, Kieran Bingham wrote:\n> > Hi JM,\n> > \n> > Quoting Jean-Michel Hautbois (2021-11-10 19:58:50)\n> >> The pipeline handler populates the new sensorControls ControlList, to\n> >> have the effective exposure and gain values for the current frame. This\n> >> is done when a statistics buffer is received.\n> >>\n> >> Make those values the frameContext::agc values for the frame when the\n> >> EventStatReady event is received.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois \n> >> ---\n> >> src/ipa/ipu3/ipu3.cpp | 4 ++++\n> >> 1 file changed, 4 insertions(+)\n> >>\n> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> >> index bcc3863b..1111f200 100644\n> >> --- a/src/ipa/ipu3/ipu3.cpp\n> >> +++ b/src/ipa/ipu3/ipu3.cpp\n> >> @@ -549,6 +549,10 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n> >> const ipu3_uapi_stats_3a *stats =\n> >> reinterpret_cast(mem.data());\n> >> \n> >> + /* \\todo move those into processControls ? */\n> >> + context_.frameContext.agc.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get();\n> >> + context_.frameContext.agc.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> > \n> > My only worry with this reading it now, is that we are storing the\n> > exposure and gain of the frame that that is associated with the\n> > statistics in what I had thought was a structure that maps the 'output'\n> > of the algorithms.\n> \n> I am not sure it is only the output, it is a frame context, input and \n> output.\n> \n> > \n> > You've explored this a bit more now - so you probably have a much better\n> > idea than me. This is ultimately an 'input' to the algorithms. Do we\n> > need to distinguish between the 'input' and 'output' ? (I.e. do we need\n> > to have both values kept so that the algorithm can determine how much it\n> > is changing? Or is that not a problem because it's all handled\n> > internally anyway....\n> \n> We update the value for the input of the algorithms, and they will \n> update those values. It is debatable though: should we have two \n> frameContexts, one before and one after the algorithm for instance ? I \n> can't see a real value to add this though.\n\nI don't think it would be 'two' frameContexts... it could just be:\n\tstruct sensor {\n\t\tint32_t exposure;\n\t\tint32_t gain;\n\t}\n\nadded to the current frameContext structure perhaps and stored so it can\nbe identified by the algorithms when they run as an input/metadata from\nthe sensor?\n\n> \n> > \n> >> +\n> >> parseStatistics(event.frame, event.frameTimestamp, stats);\n> >> break;\n> >> }\n> >> -- \n> >> 2.32.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 7D66EBDB1C\n\tfor ;\n\tThu, 11 Nov 2021 00:01:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B68B16035A;\n\tThu, 11 Nov 2021 01:01:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 81AE1600BF\n\tfor ;\n\tThu, 11 Nov 2021 01:01:27 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2B30355C;\n\tThu, 11 Nov 2021 01:01:27 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cFc313i9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636588887;\n\tbh=erCBo52UzuCp/7QemjW9En/ibvELOhC5SGc2dX2kNlY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=cFc313i9LzMlftgPg3SeFAmGdPhbzGzTOO6tgraokCaFKH1W0SO4oBcMc1J+XjSAb\n\tpdYWyJTHcidwW8nJeGXzxF0KOFXwpTQ+o90TN/wynK3SiDblpeBQShaxirvGGmeHvc\n\tabQFFVRbWiwPDRwPuFRJd+ymIibAfhhJ56HgUatw=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<15ff6e9e-1ad6-f59a-e37f-9b9cf29883ec@ideasonboard.com>","References":"<20211110195901.85597-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211110195901.85597-4-jeanmichel.hautbois@ideasonboard.com>\n\t<163658101535.2121661.3753490321961392926@Monstersaurus>\n\t<15ff6e9e-1ad6-f59a-e37f-9b9cf29883ec@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 11 Nov 2021 00:01:25 +0000","Message-ID":"<163658888504.2121661.2660154728195722587@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","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\" "}}]