[{"id":25043,"web_url":"https://patchwork.libcamera.org/comment/25043/","msgid":"<166371728479.18961.12352226090138469247@Monstersaurus>","date":"2022-09-20T23:41:24","subject":"Re: [libcamera-devel] [PATCH v4 30/32] ipa: rkisp1: awb: Clamp\n\tgains to prevent divisions by zero","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:58)\n> The gain values are currently clamped to the range [0.0, 3.996] used by\n> the hardware. A zero value makes little sense, as it would completely\n> remove the contribution of the corresponding color channel from the AWB\n> accumulators, but worse, would lead to divisions by zero when\n> calculating the raw means in subsequent iterations. Prevent this by\n> setting the minimum gain value to 1/256.\n> \n> While at it, clamp the gain values before filtering them, to improve the\n> stability of the control loop.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 22 +++++++++++++++-------\n> 1 file changed, 15 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index de54c4d24650..5f2535688c93 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -265,21 +265,29 @@ void Awb::process(IPAContext &context,\n> \n> frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> \n> - /* Estimate the red and blue gains to apply in a grey world. */\n> + /*\n> + * Estimate the red and blue gains to apply in a grey world. The green\n> + * gain is hardcoded to 0.\n\n0 or 1?\n\nWith that \n\nReviewed-by: Kieran Bingham \n\n> + */\n> double redGain = greenMean / (redMean + 1);\n> double blueGain = greenMean / (blueMean + 1);\n> \n> + /*\n> + * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> + * unsigned integer values. Set the minimum just above zero to avoid\n> + * divisions by zero when computing the raw means in subsequent\n> + * iterations.\n> + */\n> + redGain = std::clamp(redGain, 1.0 / 256, 1023.0 / 256);\n> + blueGain = std::clamp(blueGain, 1.0 / 256, 1023.0 / 256);\n> +\n> /* Filter the values to avoid oscillations. */\n> double speed = 0.2;\n> redGain = speed * redGain + (1 - speed) * activeState.awb.gains.automatic.red;\n> blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.automatic.blue;\n> \n> - /*\n> - * Gain values are unsigned integer value, range 0 to 4 with 8 bit\n> - * fractional part. Hardcode the green gain to 1.0.\n> - */\n> - activeState.awb.gains.automatic.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> - activeState.awb.gains.automatic.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> + activeState.awb.gains.automatic.red = redGain;\n> + activeState.awb.gains.automatic.blue = blueGain;\n> activeState.awb.gains.automatic.green = 1.0;\n> \n> LOG(RkISP1Awb, Debug) << std::showpoint\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 01A60C0DA4\n\tfor ;\n\tTue, 20 Sep 2022 23:41:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 53684621D8;\n\tWed, 21 Sep 2022 01:41:30 +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 AB59361F7D\n\tfor ;\n\tWed, 21 Sep 2022 01:41:28 +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 2A05B415;\n\tWed, 21 Sep 2022 01:41:28 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663717290;\n\tbh=YHcl3dOYBW53L3rKKeEngZ+EGbwFuzeGPchYaiogH24=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=mvGXy2EZ2iAjd1HYDtJGr6m9sKwzgnZM4F3oBf5Yon+bCfsyZzhlfjqLGu02MoeQS\n\t2Exh1wXZylgk7Jk0jcUJq81n/cewHFY6FESFr5wJnqF65PNNWTUg0Y52OZLBNWxRK5\n\tx1Jgo9iV/yFVjd3krY4sOrnIeyxmzkjZYjq6fA9/xgKTaG1bXzOPTKkgO1upeJDX7M\n\tWb+6tsY8KUEtarMikyH/2MOmTWboEJJOk1MiL4972TP0bHN9xeN6kc7NZR0H+OxzO8\n\tw6qGUtHg0engEUNlA3Y4yuYWewpreImqIfJSQHvmfZ0bvx4Ro7NyYxvGOlivU2Qbm1\n\t6fmcW60JuZiHg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663717288;\n\tbh=YHcl3dOYBW53L3rKKeEngZ+EGbwFuzeGPchYaiogH24=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=SS0NmN37WZULNmy/u0ivXVmvnBJfyQHFGAAysxs1UYAGPcDV4b01Mgjfe5HnQ/T6r\n\tJWJeZs71MYwKy41yI6RIJ0p05YgvRAIRl47LtgckDb4a6aaUvFxdnCsa0kQ68JN48Y\n\tltERwLCftW22PKCv2KtwYshpM7X+yFjVpbumigyM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"SS0NmN37\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220908014200.28728-31-laurent.pinchart@ideasonboard.com>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-31-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 21 Sep 2022 00:41:24 +0100","Message-ID":"<166371728479.18961.12352226090138469247@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 30/32] ipa: rkisp1: awb: Clamp\n\tgains to prevent divisions by zero","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25080,"web_url":"https://patchwork.libcamera.org/comment/25080/","msgid":"<20220922104423.y23mq64pqztkz7ud@uno.localdomain>","date":"2022-09-22T10:44:23","subject":"Re: [libcamera-devel] [PATCH v4 30/32] ipa: rkisp1: awb: Clamp\n\tgains to prevent divisions by zero","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi\n\nOn Wed, Sep 21, 2022 at 12:41:24AM +0100, Kieran Bingham via libcamera-devel wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:58)\n> > The gain values are currently clamped to the range [0.0, 3.996] used by\n> > the hardware. A zero value makes little sense, as it would completely\n> > remove the contribution of the corresponding color channel from the AWB\n> > accumulators, but worse, would lead to divisions by zero when\n> > calculating the raw means in subsequent iterations. Prevent this by\n> > setting the minimum gain value to 1/256.\n> >\n> > While at it, clamp the gain values before filtering them, to improve the\n> > stability of the control loop.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/rkisp1/algorithms/awb.cpp | 22 +++++++++++++++-------\n> > 1 file changed, 15 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index de54c4d24650..5f2535688c93 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -265,21 +265,29 @@ void Awb::process(IPAContext &context,\n> >\n> > frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> >\n> > - /* Estimate the red and blue gains to apply in a grey world. */\n> > + /*\n> > + * Estimate the red and blue gains to apply in a grey world. The green\n> > + * gain is hardcoded to 0.\n>\n> 0 or 1?\n\nI guess a gain of 0 means multiplying the color channel by 1 :) ?\n\n>\n> With that\n>\n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> > + */\n> > double redGain = greenMean / (redMean + 1);\n> > double blueGain = greenMean / (blueMean + 1);\n> >\n> > + /*\n> > + * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> > + * unsigned integer values. Set the minimum just above zero to avoid\n> > + * divisions by zero when computing the raw means in subsequent\n> > + * iterations.\n> > + */\n> > + redGain = std::clamp(redGain, 1.0 / 256, 1023.0 / 256);\n> > + blueGain = std::clamp(blueGain, 1.0 / 256, 1023.0 / 256);\n> > +\n> > /* Filter the values to avoid oscillations. */\n> > double speed = 0.2;\n> > redGain = speed * redGain + (1 - speed) * activeState.awb.gains.automatic.red;\n> > blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.automatic.blue;\n> >\n> > - /*\n> > - * Gain values are unsigned integer value, range 0 to 4 with 8 bit\n> > - * fractional part. Hardcode the green gain to 1.0.\n> > - */\n> > - activeState.awb.gains.automatic.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> > - activeState.awb.gains.automatic.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> > + activeState.awb.gains.automatic.red = redGain;\n> > + activeState.awb.gains.automatic.blue = blueGain;\n> > activeState.awb.gains.automatic.green = 1.0;\n> >\n> > LOG(RkISP1Awb, Debug) << std::showpoint\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 64E1DC0DA4\n\tfor ;\n\tThu, 22 Sep 2022 10:44:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2742862207;\n\tThu, 22 Sep 2022 12:44:27 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D35366219A\n\tfor ;\n\tThu, 22 Sep 2022 12:44:25 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 17621240005;\n\tThu, 22 Sep 2022 10:44:24 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663843467;\n\tbh=bmIg0UPd4w+lDyXdruo8h+sMDVdVVsLFRLtDfuzSsJk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=JiM5qi4RJMdcil2ooLmqfa22TvVqKZqwAVkdkAAAeMDg26f5b8f+378QBBF/YxzAa\n\tFBw1/v/wBXGoFJOvxcn78xDCvjcTYA4mel6RtsGI8PgdI5P+2kLdmcT6B6suNaY/x/\n\tLSxeURsNkut8YId7rXULS8kYjN+BqfaMvAJTUQfwwU2r0pMD1jk7GADwnmgwGMbZYJ\n\tpyGyq80Gi+oqtwyjhOHUXLiz6HCpG4j3HCb6ZpMgBXMCw1TSpOhsiGNnTbdDswGTXd\n\tvcWubsanlGD9p9KeDh0w9T6IBqQV0xcd8kDymQ4FVpmFepxT5OB83GAFpVHlI0w9IM\n\tP0OWltA6UkI/w==","Date":"Thu, 22 Sep 2022 12:44:23 +0200","To":"Kieran Bingham ","Message-ID":"<20220922104423.y23mq64pqztkz7ud@uno.localdomain>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-31-laurent.pinchart@ideasonboard.com>\n\t<166371728479.18961.12352226090138469247@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166371728479.18961.12352226090138469247@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v4 30/32] ipa: rkisp1: awb: Clamp\n\tgains to prevent divisions by zero","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]