[{"id":20981,"web_url":"https://patchwork.libcamera.org/comment/20981/","msgid":"<41a86f3a-a36a-59d8-620b-a1627e54894c@ideasonboard.com>","date":"2021-11-17T10:26:59","subject":"Re: [libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the\n\taverages when computing relative luminance","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Laurent,\n\nThanks for the patch !\n\nOn 16/11/2021 17:26, Laurent Pinchart wrote:\n> The relative luminance is calculated using an iterative process to\n> account for saturation in the sensor, as multiplying pixels by a gain\n> doesn't increase the relative luminance by the same factor if some\n> regions are saturated. Relative luminance estimation doesn't apply a\n> saturation, which produces a value that doesn't match what the sensor\n> will output, and defeats the point of the iterative process. Fix it.\n> \n> Fixes: f8f07f9468c6 (\"ipa: ipu3: agc: Improve gain calculation\")\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 25 ++++++++++++++++++-------\n> 1 file changed, 18 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 71398fdd96a6..46aa1b14a100 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -252,10 +252,19 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,\n> * \\param[in] gain The analogue gain to apply to the frame\n> * \\return The relative luminance\n> *\n> - * Luma is the weighted sum of gamma-compressed R′G′B′ components of a color\n> - * video. The luma values are normalized as 0.0 to 1.0, with 1.0 being a\n> - * theoretical perfect reflector of 100% reference white. We use the Rec. 601\n> - * luma here.\n> + * This function estimates the average relative luminance of the frame that\n> + * would be output by the sensor if an additional analogue \\a gain was applied.\n\ns/additional analogue \\a gain/additional \\a gain/\n\n> + *\n> + * The estimation is based on the AWB statistics for the current frame. Red,\n> + * green and blue averages for all cells are first multiplied by the gain, and\n> + * then saturated to approximate the sensor behaviour at high brightness\n> + * values. The approximation is quitte rough, as it doesn't take into account\n> + * non-linearities when approaching saturation.\n> + *\n> + * The relative luminance (Y) is computed from the linear RGB components using\n> + * the Rec. 601 formula. The values is normalized to the [0.0, 1.0] range,\n> + * where 1.0 corresponds to a theoretical perfect reflector of 100% reference\n> + * white.\n> *\n> * More detailed information can be found in:\n> * https://en.wikipedia.org/wiki/Relative_luminance\n> @@ -267,6 +276,7 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> {\n> \tdouble redSum = 0, greenSum = 0, blueSum = 0;\n> \n> +\t/* Sum the per-channel averages, saturated to 255. */\n> \tfor (unsigned int cellY = 0; cellY < grid.height; cellY++) {\n> \t\tfor (unsigned int cellX = 0; cellX < grid.width; cellX++) {\n> \t\t\tuint32_t cellPosition = cellY * stride_ + cellX;\n> @@ -275,10 +285,11 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> \t\t\t\treinterpret_cast(\n> \t\t\t\t\t&stats->awb_raw_buffer.meta_data[cellPosition]\n> \t\t\t\t);\n> +\t\t\tconst uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> \n> -\t\t\tredSum += cell->R_avg * gain;\n> -\t\t\tgreenSum += (cell->Gr_avg + cell->Gb_avg) / 2 * gain;\n> -\t\t\tblueSum += cell->B_avg * gain;\n> +\t\t\tredSum += std::min(cell->R_avg * gain, 255.0);\n> +\t\t\tgreenSum += std::min(G_avg * gain, 255.0);\n> +\t\t\tblueSum += std::min(cell->B_avg * gain, 255.0);\n> \t\t}\n> \t}\n> \n> \n\nTested-by: Jean-Michel Hautbois \nReviewed-by: Jean-Michel Hautbois ","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 B8381BDB1C\n\tfor ;\n\tWed, 17 Nov 2021 10:27:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B8F26032C;\n\tWed, 17 Nov 2021 11:27:04 +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 D9536600B5\n\tfor ;\n\tWed, 17 Nov 2021 11:27:02 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:f372:e247:b88:b5dd] (unknown\n\t[IPv6:2a01:e0a:169:7140:f372:e247:b88:b5dd])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A3564E7;\n\tWed, 17 Nov 2021 11:27:02 +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=\"Qrdmy41B\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637144822;\n\tbh=5qjA1VscBRl3oxAXiP1HV13TXCKgzw5TtV8wByM1xDE=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=Qrdmy41BykI/jzH0nc6rbYZiqoZPle61YBfoJwpeYegWLzZOk8zKcs0B4NGfziRbD\n\tCvGchft8YYmEJg1lVkCqEph+U9sXK1T4k9m53Hfa685bHuzbbPG1sQXRiKRMlkziKQ\n\tycvcWILYB1C7NBp28Y0ceFSIuguyWwdLMPl5BzGg=","Message-ID":"<41a86f3a-a36a-59d8-620b-a1627e54894c@ideasonboard.com>","Date":"Wed, 17 Nov 2021 11:26:59 +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":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211116162615.27777-1-laurent.pinchart@ideasonboard.com>\n\t<20211116162615.27777-6-laurent.pinchart@ideasonboard.com>","From":"Jean-Michel Hautbois ","In-Reply-To":"<20211116162615.27777-6-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the\n\taverages when computing relative luminance","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":20996,"web_url":"https://patchwork.libcamera.org/comment/20996/","msgid":"<163722920200.420308.11381756709537585251@Monstersaurus>","date":"2021-11-18T09:53:22","subject":"Re: [libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the\n\taverages when computing relative luminance","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-17 10:26:59)\n> Hi Laurent,\n> \n> Thanks for the patch !\n> \n> On 16/11/2021 17:26, Laurent Pinchart wrote:\n> > The relative luminance is calculated using an iterative process to\n> > account for saturation in the sensor, as multiplying pixels by a gain\n> > doesn't increase the relative luminance by the same factor if some\n> > regions are saturated. Relative luminance estimation doesn't apply a\n> > saturation, which produces a value that doesn't match what the sensor\n> > will output, and defeats the point of the iterative process. Fix it.\n> > \n> > Fixes: f8f07f9468c6 (\"ipa: ipu3: agc: Improve gain calculation\")\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/ipu3/algorithms/agc.cpp | 25 ++++++++++++++++++-------\n> > 1 file changed, 18 insertions(+), 7 deletions(-)\n> > \n> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > index 71398fdd96a6..46aa1b14a100 100644\n> > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > @@ -252,10 +252,19 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,\n> > * \\param[in] gain The analogue gain to apply to the frame\n> > * \\return The relative luminance\n> > *\n> > - * Luma is the weighted sum of gamma-compressed R′G′B′ components of a color\n> > - * video. The luma values are normalized as 0.0 to 1.0, with 1.0 being a\n> > - * theoretical perfect reflector of 100% reference white. We use the Rec. 601\n> > - * luma here.\n> > + * This function estimates the average relative luminance of the frame that\n> > + * would be output by the sensor if an additional analogue \\a gain was applied.\n> \n> s/additional analogue \\a gain/additional \\a gain/\n> \n> > + *\n> > + * The estimation is based on the AWB statistics for the current frame. Red,\n> > + * green and blue averages for all cells are first multiplied by the gain, and\n> > + * then saturated to approximate the sensor behaviour at high brightness\n> > + * values. The approximation is quitte rough, as it doesn't take into account\n\n/quitte/quite/\n\n> > + * non-linearities when approaching saturation.\n> > + *\n> > + * The relative luminance (Y) is computed from the linear RGB components using\n> > + * the Rec. 601 formula. The values is normalized to the [0.0, 1.0] range,\n\n/values is/values are/\n\n> > + * where 1.0 corresponds to a theoretical perfect reflector of 100% reference\n> > + * white.\n> > *\n> > * More detailed information can be found in:\n> > * https://en.wikipedia.org/wiki/Relative_luminance\n> > @@ -267,6 +276,7 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> > {\n> > double redSum = 0, greenSum = 0, blueSum = 0;\n> > \n> > + /* Sum the per-channel averages, saturated to 255. */\n> > for (unsigned int cellY = 0; cellY < grid.height; cellY++) {\n> > for (unsigned int cellX = 0; cellX < grid.width; cellX++) {\n> > uint32_t cellPosition = cellY * stride_ + cellX;\n> > @@ -275,10 +285,11 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> > reinterpret_cast(\n> > &stats->awb_raw_buffer.meta_data[cellPosition]\n> > );\n> > + const uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> > \n> > - redSum += cell->R_avg * gain;\n> > - greenSum += (cell->Gr_avg + cell->Gb_avg) / 2 * gain;\n> > - blueSum += cell->B_avg * gain;\n> > + redSum += std::min(cell->R_avg * gain, 255.0);\n> > + greenSum += std::min(G_avg * gain, 255.0);\n> > + blueSum += std::min(cell->B_avg * gain, 255.0);\n> > }\n> > }\n> > \n> > \n> \n\nStill works well in my harsh backlight office conditions too.\n\nTested-by: Kieran Bingham \nReviewed-by: Kieran Bingham \n\n> Tested-by: Jean-Michel Hautbois \n> Reviewed-by: Jean-Michel Hautbois ","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 4A724BDB1C\n\tfor ;\n\tThu, 18 Nov 2021 09:53:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EFEA46037A;\n\tThu, 18 Nov 2021 10:53:25 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B03B360230\n\tfor ;\n\tThu, 18 Nov 2021 10:53:24 +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 6D12A3E5;\n\tThu, 18 Nov 2021 10:53:24 +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=\"tzzDBaC9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637229204;\n\tbh=PUydf9GN36GOIcv5O2tOC4Yn+PBCPouEFtG4hgeGr0A=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=tzzDBaC9CDW7F7XSSSp1XtdIx39d2aTVAfxVNRsjleLsg6xNzEul3ULgWtlyw0QrC\n\tiDscEuN98/RU0hNbcKDQShnlHCA2RLpkyPnmeU0ctGePYuooIX46NKJH+YmK3Km51r\n\tH6apvlUFple63OQ5jF0ndlTLXbLy4F0OEzGIG7fI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<41a86f3a-a36a-59d8-620b-a1627e54894c@ideasonboard.com>","References":"<20211116162615.27777-1-laurent.pinchart@ideasonboard.com>\n\t<20211116162615.27777-6-laurent.pinchart@ideasonboard.com>\n\t<41a86f3a-a36a-59d8-620b-a1627e54894c@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tLaurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 18 Nov 2021 09:53:22 +0000","Message-ID":"<163722920200.420308.11381756709537585251@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the\n\taverages when computing relative luminance","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":21049,"web_url":"https://patchwork.libcamera.org/comment/21049/","msgid":"<163733254432.1448748.9583670640977336331@Monstersaurus>","date":"2021-11-19T14:35:44","subject":"Re: [libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the\n\taverages when computing relative luminance","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2021-11-18 09:53:22)\n> Quoting Jean-Michel Hautbois (2021-11-17 10:26:59)\n> > Hi Laurent,\n> > \n> > Thanks for the patch !\n> > \n> > On 16/11/2021 17:26, Laurent Pinchart wrote:\n> > > The relative luminance is calculated using an iterative process to\n> > > account for saturation in the sensor, as multiplying pixels by a gain\n> > > doesn't increase the relative luminance by the same factor if some\n> > > regions are saturated. Relative luminance estimation doesn't apply a\n> > > saturation, which produces a value that doesn't match what the sensor\n> > > will output, and defeats the point of the iterative process. Fix it.\n> > > \n> > > Fixes: f8f07f9468c6 (\"ipa: ipu3: agc: Improve gain calculation\")\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/ipa/ipu3/algorithms/agc.cpp | 25 ++++++++++++++++++-------\n> > > 1 file changed, 18 insertions(+), 7 deletions(-)\n> > > \n> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > > index 71398fdd96a6..46aa1b14a100 100644\n> > > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > > @@ -252,10 +252,19 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,\n> > > * \\param[in] gain The analogue gain to apply to the frame\n> > > * \\return The relative luminance\n> > > *\n> > > - * Luma is the weighted sum of gamma-compressed R′G′B′ components of a color\n> > > - * video. The luma values are normalized as 0.0 to 1.0, with 1.0 being a\n> > > - * theoretical perfect reflector of 100% reference white. We use the Rec. 601\n> > > - * luma here.\n> > > + * This function estimates the average relative luminance of the frame that\n> > > + * would be output by the sensor if an additional analogue \\a gain was applied.\n> > \n> > s/additional analogue \\a gain/additional \\a gain/\n> > \n> > > + *\n> > > + * The estimation is based on the AWB statistics for the current frame. Red,\n> > > + * green and blue averages for all cells are first multiplied by the gain, and\n> > > + * then saturated to approximate the sensor behaviour at high brightness\n> > > + * values. The approximation is quitte rough, as it doesn't take into account\n> \n> /quitte/quite/\n> \n> > > + * non-linearities when approaching saturation.\n> > > + *\n> > > + * The relative luminance (Y) is computed from the linear RGB components using\n> > > + * the Rec. 601 formula. The values is normalized to the [0.0, 1.0] range,\n> \n> /values is/values are/\n> \n> > > + * where 1.0 corresponds to a theoretical perfect reflector of 100% reference\n> > > + * white.\n> > > *\n> > > * More detailed information can be found in:\n> > > * https://en.wikipedia.org/wiki/Relative_luminance\n> > > @@ -267,6 +276,7 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> > > {\n> > > double redSum = 0, greenSum = 0, blueSum = 0;\n> > > \n> > > + /* Sum the per-channel averages, saturated to 255. */\n> > > for (unsigned int cellY = 0; cellY < grid.height; cellY++) {\n> > > for (unsigned int cellX = 0; cellX < grid.width; cellX++) {\n> > > uint32_t cellPosition = cellY * stride_ + cellX;\n> > > @@ -275,10 +285,11 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> > > reinterpret_cast(\n> > > &stats->awb_raw_buffer.meta_data[cellPosition]\n> > > );\n> > > + const uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> > > \n> > > - redSum += cell->R_avg * gain;\n> > > - greenSum += (cell->Gr_avg + cell->Gb_avg) / 2 * gain;\n> > > - blueSum += cell->B_avg * gain;\n> > > + redSum += std::min(cell->R_avg * gain, 255.0);\n> > > + greenSum += std::min(G_avg * gain, 255.0);\n> > > + blueSum += std::min(cell->B_avg * gain, 255.0);\n\nSeeing this constant in the new RKISP series again, wouldn't it make\nsense to keep the kMaximumLuminance constant that you remove at the\nbeginning of the series and use it here so that it's self-documented\nthat the min operation is clamping to the maximum luminance value?\n\nOr is 255 really not a luminance value here?\n\n\n> > > }\n> > > }\n> > > \n> > > \n> > \n> \n> Still works well in my harsh backlight office conditions too.\n> \n> Tested-by: Kieran Bingham \n> Reviewed-by: Kieran Bingham \n> \n> > Tested-by: Jean-Michel Hautbois \n> > Reviewed-by: Jean-Michel Hautbois ","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 8A9ABBF415\n\tfor ;\n\tFri, 19 Nov 2021 14:35:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC46B60378;\n\tFri, 19 Nov 2021 15:35:48 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C3EA8600B5\n\tfor ;\n\tFri, 19 Nov 2021 15:35:47 +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 51EE01959;\n\tFri, 19 Nov 2021 15:35:47 +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=\"FFrGXfpx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637332547;\n\tbh=WVX95InAcaJ1h9fAc6URYhqZdlAYTCuI3hsvWvAwPmM=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=FFrGXfpx9p8Mt1aSLiwc8EAxA5jUpvHrkIwFT6Dmz0Zn9OKL7iZ3JWtdrYyt9yEsg\n\tu0r2FeSm9FyKxHlvpX+zoO+8tT3VjfumogEji2SiHKowPvX2CFpHvhWBp1Lp6PRyC0\n\teycz4MxP9Ypw/7UwWVZA4Cs6WRAEymZ/14DBt/a0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<163722920200.420308.11381756709537585251@Monstersaurus>","References":"<20211116162615.27777-1-laurent.pinchart@ideasonboard.com>\n\t<20211116162615.27777-6-laurent.pinchart@ideasonboard.com>\n\t<41a86f3a-a36a-59d8-620b-a1627e54894c@ideasonboard.com>\n\t<163722920200.420308.11381756709537585251@Monstersaurus>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tLaurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 19 Nov 2021 14:35:44 +0000","Message-ID":"<163733254432.1448748.9583670640977336331@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the\n\taverages when computing relative luminance","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":21058,"web_url":"https://patchwork.libcamera.org/comment/21058/","msgid":"","date":"2021-11-19T16:09:05","subject":"Re: [libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the\n\taverages when computing relative luminance","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Nov 19, 2021 at 02:35:44PM +0000, Kieran Bingham wrote:\n> Quoting Kieran Bingham (2021-11-18 09:53:22)\n> > Quoting Jean-Michel Hautbois (2021-11-17 10:26:59)\n> > > Hi Laurent,\n> > > \n> > > Thanks for the patch !\n> > > \n> > > On 16/11/2021 17:26, Laurent Pinchart wrote:\n> > > > The relative luminance is calculated using an iterative process to\n> > > > account for saturation in the sensor, as multiplying pixels by a gain\n> > > > doesn't increase the relative luminance by the same factor if some\n> > > > regions are saturated. Relative luminance estimation doesn't apply a\n> > > > saturation, which produces a value that doesn't match what the sensor\n> > > > will output, and defeats the point of the iterative process. Fix it.\n> > > > \n> > > > Fixes: f8f07f9468c6 (\"ipa: ipu3: agc: Improve gain calculation\")\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/ipa/ipu3/algorithms/agc.cpp | 25 ++++++++++++++++++-------\n> > > > 1 file changed, 18 insertions(+), 7 deletions(-)\n> > > > \n> > > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > > > index 71398fdd96a6..46aa1b14a100 100644\n> > > > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > > > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > > > @@ -252,10 +252,19 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,\n> > > > * \\param[in] gain The analogue gain to apply to the frame\n> > > > * \\return The relative luminance\n> > > > *\n> > > > - * Luma is the weighted sum of gamma-compressed R′G′B′ components of a color\n> > > > - * video. The luma values are normalized as 0.0 to 1.0, with 1.0 being a\n> > > > - * theoretical perfect reflector of 100% reference white. We use the Rec. 601\n> > > > - * luma here.\n> > > > + * This function estimates the average relative luminance of the frame that\n> > > > + * would be output by the sensor if an additional analogue \\a gain was applied.\n> > > \n> > > s/additional analogue \\a gain/additional \\a gain/\n> > > \n> > > > + *\n> > > > + * The estimation is based on the AWB statistics for the current frame. Red,\n> > > > + * green and blue averages for all cells are first multiplied by the gain, and\n> > > > + * then saturated to approximate the sensor behaviour at high brightness\n> > > > + * values. The approximation is quitte rough, as it doesn't take into account\n> > \n> > /quitte/quite/\n> > \n> > > > + * non-linearities when approaching saturation.\n> > > > + *\n> > > > + * The relative luminance (Y) is computed from the linear RGB components using\n> > > > + * the Rec. 601 formula. The values is normalized to the [0.0, 1.0] range,\n> > \n> > /values is/values are/\n> > \n> > > > + * where 1.0 corresponds to a theoretical perfect reflector of 100% reference\n> > > > + * white.\n> > > > *\n> > > > * More detailed information can be found in:\n> > > > * https://en.wikipedia.org/wiki/Relative_luminance\n> > > > @@ -267,6 +276,7 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> > > > {\n> > > > double redSum = 0, greenSum = 0, blueSum = 0;\n> > > > \n> > > > + /* Sum the per-channel averages, saturated to 255. */\n> > > > for (unsigned int cellY = 0; cellY < grid.height; cellY++) {\n> > > > for (unsigned int cellX = 0; cellX < grid.width; cellX++) {\n> > > > uint32_t cellPosition = cellY * stride_ + cellX;\n> > > > @@ -275,10 +285,11 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> > > > reinterpret_cast(\n> > > > &stats->awb_raw_buffer.meta_data[cellPosition]\n> > > > );\n> > > > + const uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> > > > \n> > > > - redSum += cell->R_avg * gain;\n> > > > - greenSum += (cell->Gr_avg + cell->Gb_avg) / 2 * gain;\n> > > > - blueSum += cell->B_avg * gain;\n> > > > + redSum += std::min(cell->R_avg * gain, 255.0);\n> > > > + greenSum += std::min(G_avg * gain, 255.0);\n> > > > + blueSum += std::min(cell->B_avg * gain, 255.0);\n> \n> Seeing this constant in the new RKISP series again, wouldn't it make\n> sense to keep the kMaximumLuminance constant that you remove at the\n> beginning of the series and use it here so that it's self-documented\n> that the min operation is clamping to the maximum luminance value?\n> \n> Or is 255 really not a luminance value here?\n\n255 here is UINT8_MAX, as the red, green and blue averages are stored as\n8-bit values. The maximum values for the sums are then UINT8_MAX *\nnumber of cells, and the maximum value for the luminance is the same\ngiven that the sum of the three coefficients is 1.0.\n\n> > > > }\n> > > > }\n> > > > \n> > > > \n> > \n> > Still works well in my harsh backlight office conditions too.\n> > \n> > Tested-by: Kieran Bingham \n> > Reviewed-by: Kieran Bingham \n> > \n> > > Tested-by: Jean-Michel Hautbois \n> > > Reviewed-by: Jean-Michel Hautbois ","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 00717BF415\n\tfor ;\n\tFri, 19 Nov 2021 16:09:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B04F360371;\n\tFri, 19 Nov 2021 17:09:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CC7E600B5\n\tfor ;\n\tFri, 19 Nov 2021 17:09:28 +0100 (CET)","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 359A91C19;\n\tFri, 19 Nov 2021 17:09:28 +0100 (CET)"],"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=\"dtZPqo7K\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637338168;\n\tbh=21i9dhQ5o00K7rLLMRp869CY0YAt8brAl3y2LyehLoU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dtZPqo7KMlDPQ75zWUt0yhMCGpC4KTtPe0cqZYh8wnbHdoYrm1EZctKWiFEzCmsyV\n\towuRp4SGqWgXf2GNLU0I56TgfF7b+2iKW4tUEVqO79P/Wthwg5mE1lVCSH+kwXCVv3\n\t39aSHpzJAidnnaBwX3tx3KMnQ7hp9nwOQEpUA8RE=","Date":"Fri, 19 Nov 2021 18:09:05 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211116162615.27777-1-laurent.pinchart@ideasonboard.com>\n\t<20211116162615.27777-6-laurent.pinchart@ideasonboard.com>\n\t<41a86f3a-a36a-59d8-620b-a1627e54894c@ideasonboard.com>\n\t<163722920200.420308.11381756709537585251@Monstersaurus>\n\t<163733254432.1448748.9583670640977336331@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<163733254432.1448748.9583670640977336331@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 5/5] ipa: ipu3: agc: Saturate the\n\taverages when computing relative luminance","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\" "}}]