[{"id":38988,"web_url":"https://patchwork.libcamera.org/comment/38988/","msgid":"<178049668593.3720686.9673366505624763253@ping.linuxembedded.co.uk>","date":"2026-06-03T14:24:45","subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jai,\n\n\nQuoting Jai Luthra (2026-06-03 15:08:18)\n> Use the difference between againMin + 1 and againMin as the againMinStep\n> value. This is better than a heuristic of 1% of the whole range, which\n> doesn't match the actual allowed values by the V4L2 control, especially\n> for sensors with an exponential gain model (that map the control values\n> to equal-sized dB increments).\n> \n> Without this change I saw a lot of oscillations with IMX678. While the\n> oscillations did go away after I narrowed the gain control to only map\n> to 0 - 30dB of analogue gain, and ignore the 30.3 - 72dB digital gain\n> range, I still think we should use the minimum step allowed by V4L2\n> rather than a heuristic of 1%.\n\nThis sounds good, but there's some existing work on the list that we\nshould also consider here:\n\nCan you take a look at\nhttps://patchwork.libcamera.org/project/libcamera/list/?series=5915 and\nin particular https://patchwork.libcamera.org/patch/26662/ please ?\n\nI also imagine lots of this changing as part of our AGC rework for\nlibipa - so if there's a quick win here already I'm fine merging it -\nbut I think there could be bigger changes imminent too.\n\n--\nKieran\n\n\n> \n> Even for sensors with a linear gain model like IMX219, that allow\n> setting more than 100 values, this should make the AGC algorithm\n> smoother.\n> \n> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>\n> ---\n>  src/ipa/simple/soft_simple.cpp | 5 ++---\n>  1 file changed, 2 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index 629e1a32d..e463e38af 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -228,10 +228,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>                 context_.configuration.agc.againMin = camHelper_->gain(againMin);\n>                 context_.configuration.agc.againMax = camHelper_->gain(againMax);\n>                 context_.configuration.agc.again10 = std::max(context_.configuration.agc.againMin, 1.0);\n> +               double againNext = camHelper_->gain(std::min(againMin + 1, againMax));\n>                 context_.configuration.agc.againMinStep =\n> -                       (context_.configuration.agc.againMax -\n> -                        context_.configuration.agc.againMin) /\n> -                       100.0;\n> +                       againNext - context_.configuration.agc.againMin;\n>                 if (camHelper_->blackLevel().has_value()) {\n>                         /*\n>                          * The black level from camHelper_ is a 16 bit value, software ISP\n> -- \n> 2.54.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E5263C328C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Jun 2026 14:24:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F1EF9631AB;\n\tWed,  3 Jun 2026 16:24:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E952A630BA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Jun 2026 16:24:48 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D740A929;\n\tWed,  3 Jun 2026 16:24:24 +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=\"uD2Lz3Ig\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1780496664;\n\tbh=t1Smx1ufV4Q9nSp5AVBI8nfWj4U62YC0gjH7ZTrVsg4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uD2Lz3IglyIc/h6MHL6hARlmMGcCLv2j7uuYlLOXdT+ZYQmVG9BMtNghVjaH7azue\n\tgN4p8UoDCx1oEvY3PlHWaYnLuUdgvjdbG7UAnecnQQlP0v9rs3zZbTXGKvhBbMHnOQ\n\tmkBzutfOlnCHfKAVZCqy59mEVkbJxSj3U2ZhEvOA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260603140818.2558156-1-jai.luthra@ideasonboard.com>","References":"<20260603140818.2558156-1-jai.luthra@ideasonboard.com>","Subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jai Luthra <jai.luthra@ideasonboard.com>","To":"Jai Luthra <jai.luthra@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 03 Jun 2026 15:24:45 +0100","Message-ID":"<178049668593.3720686.9673366505624763253@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38990,"web_url":"https://patchwork.libcamera.org/comment/38990/","msgid":"<178054813438.1525445.15723266115922326335@freya>","date":"2026-06-04T04:42:14","subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","submitter":{"id":223,"url":"https://patchwork.libcamera.org/api/people/223/","name":"Jai Luthra","email":"jai.luthra@ideasonboard.com"},"content":"Hi Kieran,\n\nThanks for the quick reply.\n\nQuoting Kieran Bingham (2026-06-03 19:54:45)\n> Hi Jai,\n> \n> \n> Quoting Jai Luthra (2026-06-03 15:08:18)\n> > Use the difference between againMin + 1 and againMin as the againMinStep\n> > value. This is better than a heuristic of 1% of the whole range, which\n> > doesn't match the actual allowed values by the V4L2 control, especially\n> > for sensors with an exponential gain model (that map the control values\n> > to equal-sized dB increments).\n> > \n> > Without this change I saw a lot of oscillations with IMX678. While the\n> > oscillations did go away after I narrowed the gain control to only map\n> > to 0 - 30dB of analogue gain, and ignore the 30.3 - 72dB digital gain\n> > range, I still think we should use the minimum step allowed by V4L2\n> > rather than a heuristic of 1%.\n> \n> This sounds good, but there's some existing work on the list that we\n> should also consider here:\n> \n> Can you take a look at\n> https://patchwork.libcamera.org/project/libcamera/list/?series=5915 and\n> in particular https://patchwork.libcamera.org/patch/26662/ please ?\n> \n\nI have that particular patch already in my tree (based on master). While it\nfixed the algorithm to shrink the steps to <1% when close to the target,\nthe actual programmed values still get clipped to againMinStep (= 1%)\nwithout my patch.\n\n> I also imagine lots of this changing as part of our AGC rework for\n> libipa - so if there's a quick win here already I'm fine merging it -\n> but I think there could be bigger changes imminent too.\n> \n\nOh good point. If this whole code path is going to be dropped in the new\nlibipa based AGC, we can skip this patch.\n\n> --\n> Kieran\n> \n\nThanks,\n    Jai\n\n> \n> > \n> > Even for sensors with a linear gain model like IMX219, that allow\n> > setting more than 100 values, this should make the AGC algorithm\n> > smoother.\n> > \n> > Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>\n> > ---\n> >  src/ipa/simple/soft_simple.cpp | 5 ++---\n> >  1 file changed, 2 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> > index 629e1a32d..e463e38af 100644\n> > --- a/src/ipa/simple/soft_simple.cpp\n> > +++ b/src/ipa/simple/soft_simple.cpp\n> > @@ -228,10 +228,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> >                 context_.configuration.agc.againMin = camHelper_->gain(againMin);\n> >                 context_.configuration.agc.againMax = camHelper_->gain(againMax);\n> >                 context_.configuration.agc.again10 = std::max(context_.configuration.agc.againMin, 1.0);\n> > +               double againNext = camHelper_->gain(std::min(againMin + 1, againMax));\n> >                 context_.configuration.agc.againMinStep =\n> > -                       (context_.configuration.agc.againMax -\n> > -                        context_.configuration.agc.againMin) /\n> > -                       100.0;\n> > +                       againNext - context_.configuration.agc.againMin;\n> >                 if (camHelper_->blackLevel().has_value()) {\n> >                         /*\n> >                          * The black level from camHelper_ is a 16 bit value, software ISP\n> > -- \n> > 2.54.0\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 703C9C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Jun 2026 04:42:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9BC7D61754;\n\tThu,  4 Jun 2026 06:42:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C16361754\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jun 2026 06:42:20 +0200 (CEST)","from mail.ideasonboard.com (unknown\n\t[IPv6:2401:4900:1c66:476d:c684:fe78:389f:7375])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 637111494;\n\tThu,  4 Jun 2026 06:41:55 +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=\"oDKEgMzK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1780548115;\n\tbh=svbInGozuZUtOfQaiMNyhEQbBVV0Le/FoSnwR5VA5Uw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=oDKEgMzKy1co4ydLRgY4qpkLapZdyhB5RUfJWEVwnZrOoTt/JT10eSUAizB9Ymtl8\n\tOTkxj/63LLYTg5qk8vYcGp1wywvj7hBirlrOhxvh+aW2qiJZ6f55sKhhBBYEYbJH/H\n\tNI+vSXQT/uJluuSQoQqBsDoVjmkp9UWIY643H/L8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<178049668593.3720686.9673366505624763253@ping.linuxembedded.co.uk>","References":"<20260603140818.2558156-1-jai.luthra@ideasonboard.com>\n\t<178049668593.3720686.9673366505624763253@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","From":"Jai Luthra <jai.luthra@ideasonboard.com>","Cc":"","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 04 Jun 2026 10:12:14 +0530","Message-ID":"<178054813438.1525445.15723266115922326335@freya>","User-Agent":"alot/0.13.dev20+g31692a239","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38991,"web_url":"https://patchwork.libcamera.org/comment/38991/","msgid":"<0f1eed77-b858-4c3d-8b76-c2f22cb6b384@ideasonboard.com>","date":"2026-06-04T05:39:19","subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2026. 06. 04. 6:42 keltezéssel, Jai Luthra írta:\n> Hi Kieran,\n> \n> Thanks for the quick reply.\n> \n> Quoting Kieran Bingham (2026-06-03 19:54:45)\n>> Hi Jai,\n>>\n>>\n>> Quoting Jai Luthra (2026-06-03 15:08:18)\n>>> Use the difference between againMin + 1 and againMin as the againMinStep\n>>> value. This is better than a heuristic of 1% of the whole range, which\n>>> doesn't match the actual allowed values by the V4L2 control, especially\n>>> for sensors with an exponential gain model (that map the control values\n>>> to equal-sized dB increments).\n>>>\n>>> Without this change I saw a lot of oscillations with IMX678. While the\n>>> oscillations did go away after I narrowed the gain control to only map\n>>> to 0 - 30dB of analogue gain, and ignore the 30.3 - 72dB digital gain\n>>> range, I still think we should use the minimum step allowed by V4L2\n>>> rather than a heuristic of 1%.\n>>\n>> This sounds good, but there's some existing work on the list that we\n>> should also consider here:\n>>\n>> Can you take a look at\n>> https://patchwork.libcamera.org/project/libcamera/list/?series=5915 and\n>> in particular https://patchwork.libcamera.org/patch/26662/ please ?\n>>\n> \n> I have that particular patch already in my tree (based on master). While it\n> fixed the algorithm to shrink the steps to <1% when close to the target,\n> the actual programmed values still get clipped to againMinStep (= 1%)\n> without my patch.\n> \n>> I also imagine lots of this changing as part of our AGC rework for\n>> libipa - so if there's a quick win here already I'm fine merging it -\n>> but I think there could be bigger changes imminent too.\n>>\n> \n> Oh good point. If this whole code path is going to be dropped in the new\n> libipa based AGC, we can skip this patch.\n\nI'm afraid it won't be, since there needs to be an algorithm for sensor\nhelper-less scenarios, and the current mean luminance based AGC implementation\nmost likely does not satisfactorily with just gain codes.\n\n\n> \n>> --\n>> Kieran\n>>\n> \n> Thanks,\n>      Jai\n> \n>>\n>>>\n>>> Even for sensors with a linear gain model like IMX219, that allow\n>>> setting more than 100 values, this should make the AGC algorithm\n>>> smoother.\n>>>\n>>> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>\n>>> ---\n>>>   src/ipa/simple/soft_simple.cpp | 5 ++---\n>>>   1 file changed, 2 insertions(+), 3 deletions(-)\n>>>\n>>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n>>> index 629e1a32d..e463e38af 100644\n>>> --- a/src/ipa/simple/soft_simple.cpp\n>>> +++ b/src/ipa/simple/soft_simple.cpp\n>>> @@ -228,10 +228,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>>>                  context_.configuration.agc.againMin = camHelper_->gain(againMin);\n>>>                  context_.configuration.agc.againMax = camHelper_->gain(againMax);\n>>>                  context_.configuration.agc.again10 = std::max(context_.configuration.agc.againMin, 1.0);\n>>> +               double againNext = camHelper_->gain(std::min(againMin + 1, againMax));\n>>>                  context_.configuration.agc.againMinStep =\n>>> -                       (context_.configuration.agc.againMax -\n>>> -                        context_.configuration.agc.againMin) /\n>>> -                       100.0;\n>>> +                       againNext - context_.configuration.agc.againMin;\n\nI imagine `gain(againMax) - gain(max(againMax - 1, againMin))` would be a\nvalid choice as well? Should the min/max of the two be used? If the mapping\nis not linear, the two might be different, which one would be a better option?\n\n\n\n>>>                  if (camHelper_->blackLevel().has_value()) {\n>>>                          /*\n>>>                           * The black level from camHelper_ is a 16 bit value, software ISP\n>>> -- \n>>> 2.54.0\n>>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 455B0C328C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Jun 2026 05:39:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D5E6631AC;\n\tThu,  4 Jun 2026 07:39:25 +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 93FDE61754\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jun 2026 07:39:23 +0200 (CEST)","from [IPV6:2a00:1110:20f:5c81:24ae:6237:7f83:54a9]\n\t(2A001110020F5C8124AE62377F8354A9.mobile.pool.telekom.hu\n\t[IPv6:2a00:1110:20f:5c81:24ae:6237:7f83:54a9])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BFB0056D;\n\tThu,  4 Jun 2026 07:38:58 +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=\"ujnC8puV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1780551539;\n\tbh=UVOKRK9uJjHzxpAVrSz2YKxoBp/3Leesk3pFMooowQs=;\n\th=Date:From:Subject:To:References:In-Reply-To:From;\n\tb=ujnC8puVHazQJiblltH5bVUq5Yhm5dDa3eQVC2YBmSpmZ1A1ok1Qlsa5CLD7D28ax\n\tS1fMlTAn05fdRydqrGgUUE7ad7pgQQON4G9qpWH97Bf00DkhUplsNPByT7ZZ/8KzFs\n\ti5VRwH/895kHl2MweR+pbkTfPlfJ5no6F5tM3ROQ=","Message-ID":"<0f1eed77-b858-4c3d-8b76-c2f22cb6b384@ideasonboard.com>","Date":"Thu, 4 Jun 2026 07:39:19 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","To":"Jai Luthra <jai.luthra@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260603140818.2558156-1-jai.luthra@ideasonboard.com>\n\t<178049668593.3720686.9673366505624763253@ping.linuxembedded.co.uk>\n\t<178054813438.1525445.15723266115922326335@freya>","Content-Language":"en-US, hu-HU","In-Reply-To":"<178054813438.1525445.15723266115922326335@freya>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38998,"web_url":"https://patchwork.libcamera.org/comment/38998/","msgid":"<178057525688.815980.17470839737530644610@ping.linuxembedded.co.uk>","date":"2026-06-04T12:14:16","subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2026-06-04 06:39:19)\n> Hi\n> \n> 2026. 06. 04. 6:42 keltezéssel, Jai Luthra írta:\n> > Hi Kieran,\n> > \n> > Thanks for the quick reply.\n> > \n> > Quoting Kieran Bingham (2026-06-03 19:54:45)\n> >> Hi Jai,\n> >>\n> >>\n> >> Quoting Jai Luthra (2026-06-03 15:08:18)\n> >>> Use the difference between againMin + 1 and againMin as the againMinStep\n> >>> value. This is better than a heuristic of 1% of the whole range, which\n> >>> doesn't match the actual allowed values by the V4L2 control, especially\n> >>> for sensors with an exponential gain model (that map the control values\n> >>> to equal-sized dB increments).\n> >>>\n> >>> Without this change I saw a lot of oscillations with IMX678. While the\n> >>> oscillations did go away after I narrowed the gain control to only map\n> >>> to 0 - 30dB of analogue gain, and ignore the 30.3 - 72dB digital gain\n> >>> range, I still think we should use the minimum step allowed by V4L2\n> >>> rather than a heuristic of 1%.\n> >>\n> >> This sounds good, but there's some existing work on the list that we\n> >> should also consider here:\n> >>\n> >> Can you take a look at\n> >> https://patchwork.libcamera.org/project/libcamera/list/?series=5915 and\n> >> in particular https://patchwork.libcamera.org/patch/26662/ please ?\n> >>\n> > \n> > I have that particular patch already in my tree (based on master). While it\n> > fixed the algorithm to shrink the steps to <1% when close to the target,\n> > the actual programmed values still get clipped to againMinStep (= 1%)\n> > without my patch.\n> > \n> >> I also imagine lots of this changing as part of our AGC rework for\n> >> libipa - so if there's a quick win here already I'm fine merging it -\n> >> but I think there could be bigger changes imminent too.\n> >>\n> > \n> > Oh good point. If this whole code path is going to be dropped in the new\n> > libipa based AGC, we can skip this patch.\n> \n> I'm afraid it won't be, since there needs to be an algorithm for sensor\n> helper-less scenarios, and the current mean luminance based AGC implementation\n> most likely does not satisfactorily with just gain codes.\n> \n\nIn this instance though - we have a camera sensor helper, which means we\nshould have a correct gain model! So I think we should try to fix things\nso that when we have a gain model we /can/ use AgcMeanLuminance.\n\nI know that's ongoing work and investigation though.\n\n--\nKieran\n\n\n> \n> > \n> >> --\n> >> Kieran\n> >>\n> > \n> > Thanks,\n> >      Jai\n> > \n> >>\n> >>>\n> >>> Even for sensors with a linear gain model like IMX219, that allow\n> >>> setting more than 100 values, this should make the AGC algorithm\n> >>> smoother.\n> >>>\n> >>> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>\n> >>> ---\n> >>>   src/ipa/simple/soft_simple.cpp | 5 ++---\n> >>>   1 file changed, 2 insertions(+), 3 deletions(-)\n> >>>\n> >>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> >>> index 629e1a32d..e463e38af 100644\n> >>> --- a/src/ipa/simple/soft_simple.cpp\n> >>> +++ b/src/ipa/simple/soft_simple.cpp\n> >>> @@ -228,10 +228,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> >>>                  context_.configuration.agc.againMin = camHelper_->gain(againMin);\n> >>>                  context_.configuration.agc.againMax = camHelper_->gain(againMax);\n> >>>                  context_.configuration.agc.again10 = std::max(context_.configuration.agc.againMin, 1.0);\n> >>> +               double againNext = camHelper_->gain(std::min(againMin + 1, againMax));\n> >>>                  context_.configuration.agc.againMinStep =\n> >>> -                       (context_.configuration.agc.againMax -\n> >>> -                        context_.configuration.agc.againMin) /\n> >>> -                       100.0;\n> >>> +                       againNext - context_.configuration.agc.againMin;\n> \n> I imagine `gain(againMax) - gain(max(againMax - 1, againMin))` would be a\n> valid choice as well? Should the min/max of the two be used? If the mapping\n> is not linear, the two might be different, which one would be a better option?\n> \n> \n> \n> >>>                  if (camHelper_->blackLevel().has_value()) {\n> >>>                          /*\n> >>>                           * The black level from camHelper_ is a 16 bit value, software ISP\n> >>> -- \n> >>> 2.54.0\n> >>>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1BF96C328C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Jun 2026 12:14:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2A68563320;\n\tThu,  4 Jun 2026 14:14:21 +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 862D962DC4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jun 2026 14:14:19 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 93FBCBE1;\n\tThu,  4 Jun 2026 14:13:54 +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=\"dc7mKF6G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1780575234;\n\tbh=kuubmNFvARl7Ygn9sD4XtPUqzWBy5a1mkWIacNmHQxY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=dc7mKF6GyxRcwllvosqI5IGN7CNvTjprGY6NF6y0OXF2bgNIMqtPUJWAdh+5v8Psh\n\tTk0OWtw/TvLWAzM2YBKJTkNERCJFXLHx4vpZQ0bi5TAtziTImufFV6sPsQmdoAb4nw\n\t5gksGEtZdHIoxRailRWswZJdRx5TI/+h5ZGR8rG8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<0f1eed77-b858-4c3d-8b76-c2f22cb6b384@ideasonboard.com>","References":"<20260603140818.2558156-1-jai.luthra@ideasonboard.com>\n\t<178049668593.3720686.9673366505624763253@ping.linuxembedded.co.uk>\n\t<178054813438.1525445.15723266115922326335@freya>\n\t<0f1eed77-b858-4c3d-8b76-c2f22cb6b384@ideasonboard.com>","Subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tJai Luthra <jai.luthra@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 04 Jun 2026 13:14:16 +0100","Message-ID":"<178057525688.815980.17470839737530644610@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38999,"web_url":"https://patchwork.libcamera.org/comment/38999/","msgid":"<178058483471.305093.1860936327506618134@freya>","date":"2026-06-04T14:53:54","subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","submitter":{"id":223,"url":"https://patchwork.libcamera.org/api/people/223/","name":"Jai Luthra","email":"jai.luthra@ideasonboard.com"},"content":"Hi Barnabás,\n\nQuoting Barnabás Pőcze (2026-06-04 11:09:19)\n> Hi\n> \n> 2026. 06. 04. 6:42 keltezéssel, Jai Luthra írta:\n> > Hi Kieran,\n> > \n> > Thanks for the quick reply.\n> > \n> > Quoting Kieran Bingham (2026-06-03 19:54:45)\n> >> Hi Jai,\n> >>\n> >>\n> >> Quoting Jai Luthra (2026-06-03 15:08:18)\n> >>> Use the difference between againMin + 1 and againMin as the againMinStep\n> >>> value. This is better than a heuristic of 1% of the whole range, which\n> >>> doesn't match the actual allowed values by the V4L2 control, especially\n> >>> for sensors with an exponential gain model (that map the control values\n> >>> to equal-sized dB increments).\n> >>>\n> >>> Without this change I saw a lot of oscillations with IMX678. While the\n> >>> oscillations did go away after I narrowed the gain control to only map\n> >>> to 0 - 30dB of analogue gain, and ignore the 30.3 - 72dB digital gain\n> >>> range, I still think we should use the minimum step allowed by V4L2\n> >>> rather than a heuristic of 1%.\n> >>\n> >> This sounds good, but there's some existing work on the list that we\n> >> should also consider here:\n> >>\n> >> Can you take a look at\n> >> https://patchwork.libcamera.org/project/libcamera/list/?series=5915 and\n> >> in particular https://patchwork.libcamera.org/patch/26662/ please ?\n> >>\n> > \n> > I have that particular patch already in my tree (based on master). While it\n> > fixed the algorithm to shrink the steps to <1% when close to the target,\n> > the actual programmed values still get clipped to againMinStep (= 1%)\n> > without my patch.\n> > \n> >> I also imagine lots of this changing as part of our AGC rework for\n> >> libipa - so if there's a quick win here already I'm fine merging it -\n> >> but I think there could be bigger changes imminent too.\n> >>\n> > \n> > Oh good point. If this whole code path is going to be dropped in the new\n> > libipa based AGC, we can skip this patch.\n> \n> I'm afraid it won't be, since there needs to be an algorithm for sensor\n> helper-less scenarios, and the current mean luminance based AGC implementation\n> most likely does not satisfactorily with just gain codes.\n> \n> \n> > \n> >> --\n> >> Kieran\n> >>\n> > \n> > Thanks,\n> >      Jai\n> > \n> >>\n> >>>\n> >>> Even for sensors with a linear gain model like IMX219, that allow\n> >>> setting more than 100 values, this should make the AGC algorithm\n> >>> smoother.\n> >>>\n> >>> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>\n> >>> ---\n> >>>   src/ipa/simple/soft_simple.cpp | 5 ++---\n> >>>   1 file changed, 2 insertions(+), 3 deletions(-)\n> >>>\n> >>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> >>> index 629e1a32d..e463e38af 100644\n> >>> --- a/src/ipa/simple/soft_simple.cpp\n> >>> +++ b/src/ipa/simple/soft_simple.cpp\n> >>> @@ -228,10 +228,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> >>>                  context_.configuration.agc.againMin = camHelper_->gain(againMin);\n> >>>                  context_.configuration.agc.againMax = camHelper_->gain(againMax);\n> >>>                  context_.configuration.agc.again10 = std::max(context_.configuration.agc.againMin, 1.0);\n> >>> +               double againNext = camHelper_->gain(std::min(againMin + 1, againMax));\n> >>>                  context_.configuration.agc.againMinStep =\n> >>> -                       (context_.configuration.agc.againMax -\n> >>> -                        context_.configuration.agc.againMin) /\n> >>> -                       100.0;\n> >>> +                       againNext - context_.configuration.agc.againMin;\n> \n> I imagine `gain(againMax) - gain(max(againMax - 1, againMin))` would be a\n> valid choice as well? Should the min/max of the two be used? If the mapping\n> is not linear, the two might be different, which one would be a better option?\n> \n\nFor exponential mappings each step is a fixed multiplicative factor, so the\nvalues near max will have really big differences between them.\n\nUsing gain = 10^(dB/20) for IMX678 (0.3dB step, 1.035x multiplication) this\nwould mean againMinStep is:\n\ngain(againMin + 1) - gain(againMin) = 1.035 - 1  = 0.035x\ngain(againMax) - gain(againMax - 1) = 31.62 - 30.54 = 1.08x\n\nWith 1% step size we had before this patch, againMinStep was:\n\n(31.62 - 1) / 100 = 0.3x\n\nIt gets worse if we use 72dB as max (like I had originally in the driver\nwith combined digital gain):\n\ngain(againMax) - gain(againMax - 1) = 3981 - 3846 = 135x\n(gain(againMax) - gain(againMin)) / 100 = (3981 - 1)/100 = 398x\n\nGain jumps between 1 and 100x or 400x are noticable I would say ;-)\n\nTBH, subtracting two gain values doesn't make sense for the exponential\ncase at all, instead we should use 1.035x here as the minimum\nmultiplicative step. Or just use a better algorithm for sensors that have a\nknown gain model (like IMX678).\n\nFor unknown cases though, I think it is better to be safe and use the\nsmallest jump size, given sensors or bad drivers might expose unreasonably\nhigh values to userspace.\n\nEven if this means the algorithm is wasting iterations close to the target,\nbigger jumps are still allowed, so it shouldn't be slow or noticeable to\nthe end-user.\n\nThanks,\n    Jai\n\n> \n> \n> >>>                  if (camHelper_->blackLevel().has_value()) {\n> >>>                          /*\n> >>>                           * The black level from camHelper_ is a 16 bit value, software ISP\n> >>> -- \n> >>> 2.54.0\n> >>>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 87243C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Jun 2026 14:54:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8638D63320;\n\tThu,  4 Jun 2026 16:54:01 +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 76F4E62DC4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jun 2026 16:54:00 +0200 (CEST)","from mail.ideasonboard.com (unknown\n\t[IPv6:2401:4900:1c66:476d:c684:fe78:389f:7375])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 390C613D7;\n\tThu,  4 Jun 2026 16:53:35 +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=\"eTknhOPf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1780584815;\n\tbh=cKmvHQN1fp/PpA+tJ/7KYbC4S+CfXGkQo1LvW/Lb0mk=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=eTknhOPf3TPGjjT+Z2b+8sRP3n403t7jAMwByVvX7ygFWnWJ/wIv6uBtBKmWPkalK\n\tM9ZZStMFV2yxeAtEnbsVd3JnI5hlN1QW/sHsqwMYQP9qZMFQgRulIO+IHjEHiZ8xMF\n\tKxnpxI+HZsnHvMnFA9zvU03UjxUmCgiPQHEYTm3M=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<0f1eed77-b858-4c3d-8b76-c2f22cb6b384@ideasonboard.com>","References":"<20260603140818.2558156-1-jai.luthra@ideasonboard.com>\n\t<178049668593.3720686.9673366505624763253@ping.linuxembedded.co.uk>\n\t<178054813438.1525445.15723266115922326335@freya>\n\t<0f1eed77-b858-4c3d-8b76-c2f22cb6b384@ideasonboard.com>","Subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","From":"Jai Luthra <jai.luthra@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 04 Jun 2026 20:23:54 +0530","Message-ID":"<178058483471.305093.1860936327506618134@freya>","User-Agent":"alot/0.13.dev35+g4a69c46ca","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":39000,"web_url":"https://patchwork.libcamera.org/comment/39000/","msgid":"<178058602133.305093.18056208999048459988@freya>","date":"2026-06-04T15:13:41","subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","submitter":{"id":223,"url":"https://patchwork.libcamera.org/api/people/223/","name":"Jai Luthra","email":"jai.luthra@ideasonboard.com"},"content":"Quoting Kieran Bingham (2026-06-04 17:44:16)\n> Quoting Barnabás Pőcze (2026-06-04 06:39:19)\n> > Hi\n> > \n> > 2026. 06. 04. 6:42 keltezéssel, Jai Luthra írta:\n> > > Hi Kieran,\n> > > \n> > > Thanks for the quick reply.\n> > > \n> > > Quoting Kieran Bingham (2026-06-03 19:54:45)\n> > >> Hi Jai,\n> > >>\n> > >>\n> > >> Quoting Jai Luthra (2026-06-03 15:08:18)\n> > >>> Use the difference between againMin + 1 and againMin as the againMinStep\n> > >>> value. This is better than a heuristic of 1% of the whole range, which\n> > >>> doesn't match the actual allowed values by the V4L2 control, especially\n> > >>> for sensors with an exponential gain model (that map the control values\n> > >>> to equal-sized dB increments).\n> > >>>\n> > >>> Without this change I saw a lot of oscillations with IMX678. While the\n> > >>> oscillations did go away after I narrowed the gain control to only map\n> > >>> to 0 - 30dB of analogue gain, and ignore the 30.3 - 72dB digital gain\n> > >>> range, I still think we should use the minimum step allowed by V4L2\n> > >>> rather than a heuristic of 1%.\n> > >>\n> > >> This sounds good, but there's some existing work on the list that we\n> > >> should also consider here:\n> > >>\n> > >> Can you take a look at\n> > >> https://patchwork.libcamera.org/project/libcamera/list/?series=5915 and\n> > >> in particular https://patchwork.libcamera.org/patch/26662/ please ?\n> > >>\n> > > \n> > > I have that particular patch already in my tree (based on master). While it\n> > > fixed the algorithm to shrink the steps to <1% when close to the target,\n> > > the actual programmed values still get clipped to againMinStep (= 1%)\n> > > without my patch.\n> > > \n> > >> I also imagine lots of this changing as part of our AGC rework for\n> > >> libipa - so if there's a quick win here already I'm fine merging it -\n> > >> but I think there could be bigger changes imminent too.\n> > >>\n> > > \n> > > Oh good point. If this whole code path is going to be dropped in the new\n> > > libipa based AGC, we can skip this patch.\n> > \n> > I'm afraid it won't be, since there needs to be an algorithm for sensor\n> > helper-less scenarios, and the current mean luminance based AGC implementation\n> > most likely does not satisfactorily with just gain codes.\n> > \n> \n> In this instance though - we have a camera sensor helper, which means we\n> should have a correct gain model! So I think we should try to fix things\n> so that when we have a gain model we /can/ use AgcMeanLuminance.\n> \n> I know that's ongoing work and investigation though.\n\nAh you're right, for cases where we don't have a camHelper_ we anyway do:\n\t\tcontext_.configuration.agc.againMax = againMax;\n\t\tcontext_.configuration.agc.again10 = againDef;\n\t\tcontext_.configuration.agc.againMin = againMin;\n\t\tcontext_.configuration.agc.againMinStep = 1.0;\n\nAnd for the cases where we have model, I think using AgcMeanLuminance is\nthe correct way forward.\n\nYou can ignore this patch I think.\n\nThanks,\n    Jai\n\n> \n> --\n> Kieran\n> \n> \n> > \n> > > \n> > >> --\n> > >> Kieran\n> > >>\n> > > \n> > > Thanks,\n> > >      Jai\n> > > \n> > >>\n> > >>>\n> > >>> Even for sensors with a linear gain model like IMX219, that allow\n> > >>> setting more than 100 values, this should make the AGC algorithm\n> > >>> smoother.\n> > >>>\n> > >>> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>\n> > >>> ---\n> > >>>   src/ipa/simple/soft_simple.cpp | 5 ++---\n> > >>>   1 file changed, 2 insertions(+), 3 deletions(-)\n> > >>>\n> > >>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> > >>> index 629e1a32d..e463e38af 100644\n> > >>> --- a/src/ipa/simple/soft_simple.cpp\n> > >>> +++ b/src/ipa/simple/soft_simple.cpp\n> > >>> @@ -228,10 +228,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n> > >>>                  context_.configuration.agc.againMin = camHelper_->gain(againMin);\n> > >>>                  context_.configuration.agc.againMax = camHelper_->gain(againMax);\n> > >>>                  context_.configuration.agc.again10 = std::max(context_.configuration.agc.againMin, 1.0);\n> > >>> +               double againNext = camHelper_->gain(std::min(againMin + 1, againMax));\n> > >>>                  context_.configuration.agc.againMinStep =\n> > >>> -                       (context_.configuration.agc.againMax -\n> > >>> -                        context_.configuration.agc.againMin) /\n> > >>> -                       100.0;\n> > >>> +                       againNext - context_.configuration.agc.againMin;\n> > \n> > I imagine `gain(againMax) - gain(max(againMax - 1, againMin))` would be a\n> > valid choice as well? Should the min/max of the two be used? If the mapping\n> > is not linear, the two might be different, which one would be a better option?\n> > \n> > \n> > \n> > >>>                  if (camHelper_->blackLevel().has_value()) {\n> > >>>                          /*\n> > >>>                           * The black level from camHelper_ is a 16 bit value, software ISP\n> > >>> -- \n> > >>> 2.54.0\n> > >>>\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7EFEAC324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Jun 2026 15:13:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD4A0633EF;\n\tThu,  4 Jun 2026 17:13:48 +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 108CD62DC4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jun 2026 17:13:47 +0200 (CEST)","from mail.ideasonboard.com (unknown\n\t[IPv6:2401:4900:1c66:476d:c684:fe78:389f:7375])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A5CADBE1;\n\tThu,  4 Jun 2026 17:13:21 +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=\"MFnGEe4G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1780586002;\n\tbh=hFKsyILgJGlceWWEzXI2nKDKpHvN8g9789gc2Mr8dl4=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=MFnGEe4GBfJMj58AaMzp43LU9Md1DaHSqKaYVD1Kxow3WezmdLWyG80CNm1sz0e6b\n\t08UCObcqGiv4jT+vWkxdZelrUP977vOiJ0hsDs1g0qzh6Cn0BKXt1wmA6ZZzH2mq3N\n\tHmWjwwzioOFCnPgKt6PN+2zC2Ur+/kuG9DbZoSoY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<178057525688.815980.17470839737530644610@ping.linuxembedded.co.uk>","References":"<20260603140818.2558156-1-jai.luthra@ideasonboard.com>\n\t<178049668593.3720686.9673366505624763253@ping.linuxembedded.co.uk>\n\t<178054813438.1525445.15723266115922326335@freya>\n\t<0f1eed77-b858-4c3d-8b76-c2f22cb6b384@ideasonboard.com>\n\t<178057525688.815980.17470839737530644610@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH] ipa: simple: Fix againMinStep calculation","From":"Jai Luthra <jai.luthra@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 04 Jun 2026 20:43:41 +0530","Message-ID":"<178058602133.305093.18056208999048459988@freya>","User-Agent":"alot/0.13.dev35+g4a69c46ca","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]