[{"id":20205,"web_url":"https://patchwork.libcamera.org/comment/20205/","msgid":"<163421149526.3829429.9612646638694744908@Monstersaurus>","date":"2021-10-14T11:38:15","subject":"Re: [libcamera-devel] [PATCH 12/13] ipa: ipu3: agc: Increase IIR\n\tfilter speed","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-10-13 16:41:24)\n> The filter used is an infinite response filter which is controlled by\n> the speed variable. It is used to limit the gap between two exposure\n> values, and avoid nasty oscillations. The higher the speed, the less the\n> result oscillates.\n\nDoes that also mean it will take longer to obtain a good exposure?\n\n> As we are only calculating the exposure every 6 frames, as controled by\n> the 'kFrameSkipCount' constant, we can increase the speed, to avoid a\n> slow response (it can be more than 1 second to get a stable enough\n> output).\n\nAre we able to move towards calculating on every frame? or would that\ncause more oscillations? If so that implies to me that we're using the\nwrong inputs to perform the calculations.\n\nBut ... although we might need to do further investigations on the\ninputs if we hit oscillations, I don't think this is a problem if it's a\nshort term gain.\n\nBut if we do need to do more work to improve the calculatoins to allow\nus to run the calculations faster, then we might need to record a todo\nsomewhere.\n\n\nReviewed-by: Kieran Bingham \n\n\n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index b922bcdf..81eaf436 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -92,7 +92,7 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,\n> \n> void Agc::filterExposure()\n> {\n> - double speed = 0.2;\n> + double speed = 0.9;\n> if (filteredExposure_ == 0s) {\n> /* DG stands for digital gain.*/\n> filteredExposure_ = currentExposure_;\n> -- \n> 2.30.2\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 0944AC323E\n\tfor ;\n\tThu, 14 Oct 2021 11:38:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7570E68F4F;\n\tThu, 14 Oct 2021 13:38: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 CC7B268541\n\tfor ;\n\tThu, 14 Oct 2021 13:38:17 +0200 (CEST)","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 68D212F3;\n\tThu, 14 Oct 2021 13:38:17 +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=\"pzSpy9lP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634211497;\n\tbh=VeEZV+TCATxL1RG1MQtmDy0T+AgwWTCLpyCqpD8T3VQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=pzSpy9lPIiroVW93G2cpePuxGWvohpHJHcVYxLwVlKk3uPOJEl7UJXKSbOnZd9Sgd\n\thbLzzt23cASZKyQM8a9xWo6oYMCLx9m2zBr9/TEuUSoIUNzatVq72gHrPjm4Fbp+qO\n\tc52EovVQq8inkz3FBozMUAwA028TsTAmt6kDgasI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211013154125.133419-13-jeanmichel.hautbois@ideasonboard.com>","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-13-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 14 Oct 2021 12:38:15 +0100","Message-ID":"<163421149526.3829429.9612646638694744908@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 12/13] ipa: ipu3: agc: Increase IIR\n\tfilter speed","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":20222,"web_url":"https://patchwork.libcamera.org/comment/20222/","msgid":"","date":"2021-10-15T01:36:15","subject":"Re: [libcamera-devel] [PATCH 12/13] ipa: ipu3: agc: Increase IIR\n\tfilter speed","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Oct 14, 2021 at 12:38:15PM +0100, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-10-13 16:41:24)\n> > The filter used is an infinite response filter which is controlled by\n> > the speed variable. It is used to limit the gap between two exposure\n> > values, and avoid nasty oscillations. The higher the speed, the less the\n> > result oscillates.\n> \n> Does that also mean it will take longer to obtain a good exposure?\n> \n> > As we are only calculating the exposure every 6 frames, as controled by\n> > the 'kFrameSkipCount' constant, we can increase the speed, to avoid a\n> > slow response (it can be more than 1 second to get a stable enough\n> > output).\n> \n> Are we able to move towards calculating on every frame? or would that\n> cause more oscillations? If so that implies to me that we're using the\n> wrong inputs to perform the calculations.\n> \n> But ... although we might need to do further investigations on the\n> inputs if we hit oscillations, I don't think this is a problem if it's a\n> short term gain.\n> \n> But if we do need to do more work to improve the calculatoins to allow\n> us to run the calculations faster, then we might need to record a todo\n> somewhere.\n\nI think this patch tries to hide a problem instead of solving it, I'm\nnot sure I like that :-S (I'm actually pretty sure I dislike it)\n\n> Reviewed-by: Kieran Bingham \n> \n> > Signed-off-by: Jean-Michel Hautbois \n> > ---\n> > src/ipa/ipu3/algorithms/agc.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > index b922bcdf..81eaf436 100644\n> > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > @@ -92,7 +92,7 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,\n> > \n> > void Agc::filterExposure()\n> > {\n> > - double speed = 0.2;\n> > + double speed = 0.9;\n> > if (filteredExposure_ == 0s) {\n> > /* DG stands for digital gain.*/\n> > filteredExposure_ = currentExposure_;","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 32981C323E\n\tfor ;\n\tFri, 15 Oct 2021 01:36:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9F27168F4D;\n\tFri, 15 Oct 2021 03:36:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6301868F4D\n\tfor ;\n\tFri, 15 Oct 2021 03:36:31 +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 C75FC2E3;\n\tFri, 15 Oct 2021 03:36: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=\"Ld3AlwXv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634261791;\n\tbh=+2d7s1nmEsfJvknptFaJalOAu+bXjCF9o55CzaP8n/4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ld3AlwXvo6S+KvY/h4okvkCXJwNIJL9h2+EK1JRJpA6BHtOJG2J3bPJc1tH8xlx9J\n\tY6c2sG00oRL9epchy9Jp35OFh5frWXhsqcd/XVdvgmwF4V60dvwBII+hjNRFXUzv/h\n\t0nqAmmV3UTdPBXrIzqIAId0MdOsF+pivxUsaoWx8=","Date":"Fri, 15 Oct 2021 04:36:15 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-13-jeanmichel.hautbois@ideasonboard.com>\n\t<163421149526.3829429.9612646638694744908@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163421149526.3829429.9612646638694744908@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 12/13] ipa: ipu3: agc: Increase IIR\n\tfilter speed","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\" "}}]