[{"id":20193,"web_url":"https://patchwork.libcamera.org/comment/20193/","msgid":"<163420699338.3829429.4509588483126382716@Monstersaurus>","date":"2021-10-14T10:23:13","subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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:13)\n> We can have a saturation ratio per cell, giving the percentage of pixels\n> over a threshold within a cell where 100% is set to 0xff.\n> \n> The parameter structure 'ipu3_uapi_awb_config_s' contains four fields to\n> set the threshold, one per channel.\n> The blue field is also used to configure the ImgU and make it calculate\n> the saturation ratio or not.\n> \n> Set a green value saturated when it is more than 230 (90% of the maximum\n> value 255, coded as 8191).\n\nWhy do we only determine a lower saturation on one component?\n\nShouldn't they be balanced (perhaps in accordance with the white balance\nsomehow?).\n\n\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/ipu3/algorithms/awb.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index e2b18336..5574bd44 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -326,9 +326,9 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n> \n> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> {\n> - params->acc_param.awb.config.rgbs_thr_gr = 8191;\n> + params->acc_param.awb.config.rgbs_thr_gr = (90 * 8191 / 100);\n> params->acc_param.awb.config.rgbs_thr_r = 8191;\n> - params->acc_param.awb.config.rgbs_thr_gb = 8191;\n> + params->acc_param.awb.config.rgbs_thr_gb = (90 * 8191 / 100);\n> params->acc_param.awb.config.rgbs_thr_b = IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT\n> | IPU3_UAPI_AWB_RGBS_THR_B_EN\n> | 8191;\n\nThat's interesting that thr_b has those enable flags. Do they apply\nonly to B? or does that enable all of them?\n\nI wonder if a helper function would make the values clearer:\n\n(Awb private:)\nuint16_t Awb::Threshold(float value)\n{\n\t/* AWB Thresholds are represented in FixedPoint 3.13 */\n\tconstexpr uint16_t kFixedPoint3_13 = 8191;\n\n\treturn value * kFixedPoint3_13;\n}\n\n\nvoid Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n{\n\t/*\n\t * Green saturation thresholds are reduced because...\n\t */\n\tparams->acc_param.awb.config.rgbs_thr_r = Threshold(1.0);\n\tparams->acc_param.awb.config.rgbs_thr_gr = Threshold(0.9);\n\tparams->acc_param.awb.config.rgbs_thr_gb = Threshold(0.9);\n\tparams->acc_param.awb.config.rgbs_thr_b = Threshold(1.0);\n\n\t/* Enable saturation inclusion on thr_b because ... */\n\tparams->acc_param.awb.config.rgbs_thr_b |= IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n\t\t\t\t\t \t IPU3_UAPI_AWB_RGBS_THR_B_EN;\n\t...\n}\n\n\n\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 478AEC323E\n\tfor ;\n\tThu, 14 Oct 2021 10:23:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D1F0568F4D;\n\tThu, 14 Oct 2021 12:23:18 +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 A491668541\n\tfor ;\n\tThu, 14 Oct 2021 12:23:16 +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 40A2D2F3;\n\tThu, 14 Oct 2021 12:23:16 +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=\"uxwvC49w\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634206996;\n\tbh=oM9ihEOepKW+RR+qfuPP4TApqJMYywsPyysiF6/rorM=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=uxwvC49wBgH7qkmqREuBKi2tW0AJJWvHqoefUvsjl9z1YSf17JO4JztgJW5JUlve7\n\tyJYThz1WEj3mTGk3doMG6NsiarjT9Vs31sXrB3DWq41PLMSObCdJo4c8JYquKgBwMz\n\tQTX2cO2AK2QAmnN8fS112JSrOi8BII8VU7q19LbI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 14 Oct 2021 11:23:13 +0100","Message-ID":"<163420699338.3829429.4509588483126382716@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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":20211,"web_url":"https://patchwork.libcamera.org/comment/20211/","msgid":"","date":"2021-10-14T21:56:51","subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Thu, Oct 14, 2021 at 11:23:13AM +0100, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-10-13 16:41:13)\n> > We can have a saturation ratio per cell, giving the percentage of pixels\n> > over a threshold within a cell where 100% is set to 0xff.\n> > \n> > The parameter structure 'ipu3_uapi_awb_config_s' contains four fields to\n> > set the threshold, one per channel.\n> > The blue field is also used to configure the ImgU and make it calculate\n> > the saturation ratio or not.\n> > \n> > Set a green value saturated when it is more than 230 (90% of the maximum\n> > value 255, coded as 8191).\n> \n> Why do we only determine a lower saturation on one component?\n\nI assume because we only use the stats for that component :-)\n\n> Shouldn't they be balanced (perhaps in accordance with the white balance\n> somehow?).\n\nIf the assumption is correct, then we could, and it would make no\ndifference.\n\n> > Signed-off-by: Jean-Michel Hautbois \n> > ---\n> > src/ipa/ipu3/algorithms/awb.cpp | 4 ++--\n> > 1 file changed, 2 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> > index e2b18336..5574bd44 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > @@ -326,9 +326,9 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n> > \n> > void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> > {\n> > - params->acc_param.awb.config.rgbs_thr_gr = 8191;\n> > + params->acc_param.awb.config.rgbs_thr_gr = (90 * 8191 / 100);\n\nNo need for parentheses.\n\n> > params->acc_param.awb.config.rgbs_thr_r = 8191;\n> > - params->acc_param.awb.config.rgbs_thr_gb = 8191;\n> > + params->acc_param.awb.config.rgbs_thr_gb = (90 * 8191 / 100);\n> > params->acc_param.awb.config.rgbs_thr_b = IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT\n> > | IPU3_UAPI_AWB_RGBS_THR_B_EN\n> > | 8191;\n> \n> That's interesting that thr_b has those enable flags. Do they apply\n> only to B? or does that enable all of them?\n\nAs far as I undertand, the single flag applies to all channels.\n\n> I wonder if a helper function would make the values clearer:\n> \n> (Awb private:)\n> uint16_t Awb::Threshold(float value)\n\nMake it\n\nconstexpr uint16_t Awb::Threshold(float value)\n\n> {\n> \t/* AWB Thresholds are represented in FixedPoint 3.13 */\n> \tconstexpr uint16_t kFixedPoint3_13 = 8191;\n> \n> \treturn value * kFixedPoint3_13;\n> }\n\nIn a 3.13 fixed-point format, 1.0 would be 8192, not 8191. The above\nwould thus be misleading. 8191 is the maximum value of a 12bpp pixel,\nit's not a fixed-point number in this context.\n\n> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> {\n> \t/*\n> \t * Green saturation thresholds are reduced because...\n> \t */\n> \tparams->acc_param.awb.config.rgbs_thr_r = Threshold(1.0);\n> \tparams->acc_param.awb.config.rgbs_thr_gr = Threshold(0.9);\n> \tparams->acc_param.awb.config.rgbs_thr_gb = Threshold(0.9);\n> \tparams->acc_param.awb.config.rgbs_thr_b = Threshold(1.0);\n> \n> \t/* Enable saturation inclusion on thr_b because ... */\n> \tparams->acc_param.awb.config.rgbs_thr_b |= IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n> \t\t\t\t\t \t IPU3_UAPI_AWB_RGBS_THR_B_EN;\n\nI like splitting this from the previous line.\n\n> \t...\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 8E510C324C\n\tfor ;\n\tThu, 14 Oct 2021 21:57:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1072468F4D;\n\tThu, 14 Oct 2021 23:57:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2BB5A68541\n\tfor ;\n\tThu, 14 Oct 2021 23:57:07 +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 975C22F3;\n\tThu, 14 Oct 2021 23:57:06 +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=\"wUUd2WxS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634248626;\n\tbh=lpuQE7YXycTwH4HXl8qJZtznszRPtcAve/2bm0wT69c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wUUd2WxST4Do7QPbAirUDMhuCzfhzSfW92kYYqEDCcpoaW+bPca7CBptWqYYCOiVC\n\tDMhZDHA8HTYepMYBcRZBcKWt+1i0PPOmy+W2D0cYf0XD+g71bopcMiZl4g1xe9X+o/\n\tf7Ubyc1lcrVpqcST+cjlcPHnmap9t7lHqyk7WwDk=","Date":"Fri, 15 Oct 2021 00:56:51 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163420699338.3829429.4509588483126382716@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163420699338.3829429.4509588483126382716@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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\" "}},{"id":20229,"web_url":"https://patchwork.libcamera.org/comment/20229/","msgid":"<40a7aa2b-5673-9547-acf7-9ba5bfa65e7a@ideasonboard.com>","date":"2021-10-15T05:38:54","subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi !\n\nOn 14/10/2021 23:56, Laurent Pinchart wrote:\n> Hello,\n> \n> On Thu, Oct 14, 2021 at 11:23:13AM +0100, Kieran Bingham wrote:\n>> Quoting Jean-Michel Hautbois (2021-10-13 16:41:13)\n>>> We can have a saturation ratio per cell, giving the percentage of pixels\n>>> over a threshold within a cell where 100% is set to 0xff.\n>>>\n>>> The parameter structure 'ipu3_uapi_awb_config_s' contains four fields to\n>>> set the threshold, one per channel.\n>>> The blue field is also used to configure the ImgU and make it calculate\n>>> the saturation ratio or not.\n>>>\n>>> Set a green value saturated when it is more than 230 (90% of the maximum\n>>> value 255, coded as 8191).\n>>\n>> Why do we only determine a lower saturation on one component?\n> \n> I assume because we only use the stats for that component :-)\n> \n\nExactly, and also because most of the perceived luminance is well\nrepresented by the green channel (red and blud have smaller\ncontributions, 60% for green, 30% for red and 10% for blue to simplify).\n\n>> Shouldn't they be balanced (perhaps in accordance with the white balance\n>> somehow?).\n> \n> If the assumption is correct, then we could, and it would make no\n> difference.\n> \n\nYes, it is not a bad thing but does not improve anything :-).\n\n>>> Signed-off-by: Jean-Michel Hautbois \n>>> ---\n>>> src/ipa/ipu3/algorithms/awb.cpp | 4 ++--\n>>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n>>> index e2b18336..5574bd44 100644\n>>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n>>> @@ -326,9 +326,9 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>> \n>>> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>>> {\n>>> - params->acc_param.awb.config.rgbs_thr_gr = 8191;\n>>> + params->acc_param.awb.config.rgbs_thr_gr = (90 * 8191 / 100);\n> \n> No need for parentheses.\n> \n>>> params->acc_param.awb.config.rgbs_thr_r = 8191;\n>>> - params->acc_param.awb.config.rgbs_thr_gb = 8191;\n>>> + params->acc_param.awb.config.rgbs_thr_gb = (90 * 8191 / 100);\n>>> params->acc_param.awb.config.rgbs_thr_b = IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT\n>>> | IPU3_UAPI_AWB_RGBS_THR_B_EN\n>>> | 8191;\n>>\n>> That's interesting that thr_b has those enable flags. Do they apply\n>> only to B? or does that enable all of them?\n> \n> As far as I undertand, the single flag applies to all channels.\n> \n>> I wonder if a helper function would make the values clearer:\n>>\n>> (Awb private:)\n>> uint16_t Awb::Threshold(float value)\n> \n> Make it\n> \n> constexpr uint16_t Awb::Threshold(float value)\n> \n>> {\n>> \t/* AWB Thresholds are represented in FixedPoint 3.13 */\n>> \tconstexpr uint16_t kFixedPoint3_13 = 8191;\n>>\n>> \treturn value * kFixedPoint3_13;\n>> }\n> \n> In a 3.13 fixed-point format, 1.0 would be 8192, not 8191. The above\n> would thus be misleading. 8191 is the maximum value of a 12bpp pixel,\n> it's not a fixed-point number in this context.\n> \n>> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>> {\n>> \t/*\n>> \t * Green saturation thresholds are reduced because...\n>> \t */\n>> \tparams->acc_param.awb.config.rgbs_thr_r = Threshold(1.0);\n>> \tparams->acc_param.awb.config.rgbs_thr_gr = Threshold(0.9);\n>> \tparams->acc_param.awb.config.rgbs_thr_gb = Threshold(0.9);\n>> \tparams->acc_param.awb.config.rgbs_thr_b = Threshold(1.0);\n>>\n>> \t/* Enable saturation inclusion on thr_b because ... */\n>> \tparams->acc_param.awb.config.rgbs_thr_b |= IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n>> \t\t\t\t\t \t IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> \n> I like splitting this from the previous line.\n> \n>> \t...\n>> }\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 079C3C324C\n\tfor ;\n\tFri, 15 Oct 2021 05:39:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7EA2860239;\n\tFri, 15 Oct 2021 07:38:59 +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 AEA7960239\n\tfor ;\n\tFri, 15 Oct 2021 07:38:57 +0200 (CEST)","from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:ae2a:a11b:484b:c825])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3B7652E3;\n\tFri, 15 Oct 2021 07:38:57 +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=\"UQSAq2Qo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634276337;\n\tbh=g/5JjQeuC5eMTpueLEjmN70X0/3F7AdcqCx4lBZkB5M=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=UQSAq2QoRMuSNnDG3xgVECYWZL4CMyq+GcaXOFp0VN5wRKdx20ZxmCcp2D0k97tto\n\trpjeRuelqoAj7Cn5h91Auiiti9MGTJibM76ANecgkyg/gWD/HbItx2EsH6iJNpn2UA\n\tCMVf12cJxVoWUa6IOeLo/wgZQuCNzcCkU+4OMtyo=","To":"Laurent Pinchart ,\n\tKieran Bingham ","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163420699338.3829429.4509588483126382716@Monstersaurus>\n\t","From":"Jean-Michel Hautbois ","Message-ID":"<40a7aa2b-5673-9547-acf7-9ba5bfa65e7a@ideasonboard.com>","Date":"Fri, 15 Oct 2021 07:38:54 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.13.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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\" "}},{"id":20328,"web_url":"https://patchwork.libcamera.org/comment/20328/","msgid":"<3a8877f6-06b8-125f-5d96-c4414cf79d7f@ideasonboard.com>","date":"2021-10-20T08:41:05","subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 14/10/2021 23:56, Laurent Pinchart wrote:\n> Hello,\n> \n> On Thu, Oct 14, 2021 at 11:23:13AM +0100, Kieran Bingham wrote:\n>> Quoting Jean-Michel Hautbois (2021-10-13 16:41:13)\n>>> We can have a saturation ratio per cell, giving the percentage of pixels\n>>> over a threshold within a cell where 100% is set to 0xff.\n>>>\n>>> The parameter structure 'ipu3_uapi_awb_config_s' contains four fields to\n>>> set the threshold, one per channel.\n>>> The blue field is also used to configure the ImgU and make it calculate\n>>> the saturation ratio or not.\n>>>\n>>> Set a green value saturated when it is more than 230 (90% of the maximum\n>>> value 255, coded as 8191).\n>>\n>> Why do we only determine a lower saturation on one component?\n> \n> I assume because we only use the stats for that component :-)\n> \n>> Shouldn't they be balanced (perhaps in accordance with the white balance\n>> somehow?).\n> \n> If the assumption is correct, then we could, and it would make no\n> difference.\n> \n>>> Signed-off-by: Jean-Michel Hautbois \n>>> ---\n>>> src/ipa/ipu3/algorithms/awb.cpp | 4 ++--\n>>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n>>> index e2b18336..5574bd44 100644\n>>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n>>> @@ -326,9 +326,9 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>> \n>>> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>>> {\n>>> - params->acc_param.awb.config.rgbs_thr_gr = 8191;\n>>> + params->acc_param.awb.config.rgbs_thr_gr = (90 * 8191 / 100);\n> \n> No need for parentheses.\n> \n>>> params->acc_param.awb.config.rgbs_thr_r = 8191;\n>>> - params->acc_param.awb.config.rgbs_thr_gb = 8191;\n>>> + params->acc_param.awb.config.rgbs_thr_gb = (90 * 8191 / 100);\n>>> params->acc_param.awb.config.rgbs_thr_b = IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT\n>>> | IPU3_UAPI_AWB_RGBS_THR_B_EN\n>>> | 8191;\n>>\n>> That's interesting that thr_b has those enable flags. Do they apply\n>> only to B? or does that enable all of them?\n> \n> As far as I undertand, the single flag applies to all channels.\n> \n>> I wonder if a helper function would make the values clearer:\n>>\n>> (Awb private:)\n>> uint16_t Awb::Threshold(float value)\n> \n> Make it\n> \n> constexpr uint16_t Awb::Threshold(float value)\n> \n>> {\n>> \t/* AWB Thresholds are represented in FixedPoint 3.13 */\n>> \tconstexpr uint16_t kFixedPoint3_13 = 8191;\n>>\n>> \treturn value * kFixedPoint3_13;\n>> }\n> \n> In a 3.13 fixed-point format, 1.0 would be 8192, not 8191. The above\n> would thus be misleading. 8191 is the maximum value of a 12bpp pixel,\n> it's not a fixed-point number in this context.\n\nI am not sure I understand this comment :-/. I started by changing\n- constexpr uint16_t kFixedPoint3_13 = 8191;\n+ constexpr uint16_t kFixedPoint3_13 = 8192;\n\nBut now I read it again, and I think I might have misunderstand ?\n\n> \n>> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>> {\n>> \t/*\n>> \t * Green saturation thresholds are reduced because...\n>> \t */\n>> \tparams->acc_param.awb.config.rgbs_thr_r = Threshold(1.0);\n>> \tparams->acc_param.awb.config.rgbs_thr_gr = Threshold(0.9);\n>> \tparams->acc_param.awb.config.rgbs_thr_gb = Threshold(0.9);\n>> \tparams->acc_param.awb.config.rgbs_thr_b = Threshold(1.0);\n>>\n>> \t/* Enable saturation inclusion on thr_b because ... */\n>> \tparams->acc_param.awb.config.rgbs_thr_b |= IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n>> \t\t\t\t\t \t IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> \n> I like splitting this from the previous line.\n> \n>> \t...\n>> }\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 4A61CBF415\n\tfor ;\n\tWed, 20 Oct 2021 08:41:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 701B968F5B;\n\tWed, 20 Oct 2021 10:41:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A65246012A\n\tfor ;\n\tWed, 20 Oct 2021 10:41:07 +0200 (CEST)","from [IPV6:2a01:e0a:169:7140:ce4b:1c5f:7302:b899] (unknown\n\t[IPv6:2a01:e0a:169:7140:ce4b:1c5f:7302:b899])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4B7FD3F6;\n\tWed, 20 Oct 2021 10:41:07 +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=\"tNJGPWae\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634719267;\n\tbh=dzDrebwXG5eYLpR9tN8wot5HfqahxWQnet9c8rvoOuU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=tNJGPWaeEKyfOivQXk1fYhmHSgFKNmnhJTck/FN/pkt+WERJeB3GWctcp+nlclqPA\n\tyt3ATKkNWajyFQR7trmV+OiHTBvlOKyQTn86lBpk8LMmfPNMHGo/PIyUYV6ZU0kxJe\n\tZToIE5nJwqAr+6pS4s2rxySJ3vnOTxa9i833yU+Q=","Message-ID":"<3a8877f6-06b8-125f-5d96-c4414cf79d7f@ideasonboard.com>","Date":"Wed, 20 Oct 2021 10:41:05 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.1.2","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tKieran Bingham ","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163420699338.3829429.4509588483126382716@Monstersaurus>\n\t","From":"Jean-Michel Hautbois ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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\" "}},{"id":20329,"web_url":"https://patchwork.libcamera.org/comment/20329/","msgid":"<7666bf85-f9f9-6f5d-e5cf-304af5acb7b2@ideasonboard.com>","date":"2021-10-20T08:53:54","subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"On 20/10/2021 10:41, Jean-Michel Hautbois wrote:\n> Hi Laurent,\n> \n> On 14/10/2021 23:56, Laurent Pinchart wrote:\n>> Hello,\n>>\n>> On Thu, Oct 14, 2021 at 11:23:13AM +0100, Kieran Bingham wrote:\n>>> Quoting Jean-Michel Hautbois (2021-10-13 16:41:13)\n>>>> We can have a saturation ratio per cell, giving the percentage of \n>>>> pixels\n>>>> over a threshold within a cell where 100% is set to 0xff.\n>>>>\n>>>> The parameter structure 'ipu3_uapi_awb_config_s' contains four \n>>>> fields to\n>>>> set the threshold, one per channel.\n>>>> The blue field is also used to configure the ImgU and make it calculate\n>>>> the saturation ratio or not.\n>>>>\n>>>> Set a green value saturated when it is more than 230 (90% of the \n>>>> maximum\n>>>> value 255, coded as 8191).\n>>>\n>>> Why do we only determine a lower saturation on one component?\n>>\n>> I assume because we only use the stats for that component :-)\n>>\n>>> Shouldn't they be balanced (perhaps in accordance with the white balance\n>>> somehow?).\n>>\n>> If the assumption is correct, then we could, and it would make no\n>> difference.\n>>\n>>>> Signed-off-by: Jean-Michel Hautbois \n>>>> \n>>>> ---\n>>>>   src/ipa/ipu3/algorithms/awb.cpp | 4 ++--\n>>>>   1 file changed, 2 insertions(+), 2 deletions(-)\n>>>>\n>>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp \n>>>> b/src/ipa/ipu3/algorithms/awb.cpp\n>>>> index e2b18336..5574bd44 100644\n>>>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n>>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n>>>> @@ -326,9 +326,9 @@ void Awb::process(IPAContext &context, const \n>>>> ipu3_uapi_stats_3a *stats)\n>>>>   void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>>>>   {\n>>>> -       params->acc_param.awb.config.rgbs_thr_gr = 8191;\n>>>> +       params->acc_param.awb.config.rgbs_thr_gr = (90 * 8191 / 100);\n>>\n>> No need for parentheses.\n>>\n>>>>          params->acc_param.awb.config.rgbs_thr_r = 8191;\n>>>> -       params->acc_param.awb.config.rgbs_thr_gb = 8191;\n>>>> +       params->acc_param.awb.config.rgbs_thr_gb = (90 * 8191 / 100);\n>>>>          params->acc_param.awb.config.rgbs_thr_b = \n>>>> IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT\n>>>>                                                 | \n>>>> IPU3_UAPI_AWB_RGBS_THR_B_EN\n>>>>                                                 | 8191;\n>>>\n>>> That's interesting that thr_b has those enable flags. Do they apply\n>>> only to B? or does that enable all of them?\n>>\n>> As far as I undertand, the single flag applies to all channels.\n>>\n>>> I wonder if a helper function would make the values clearer:\n>>>\n>>> (Awb private:)\n>>> uint16_t Awb::Threshold(float value)\n>>\n>> Make it\n>>\n>> constexpr uint16_t Awb::Threshold(float value)\n>>\n>>> {\n>>>     /* AWB Thresholds are represented in FixedPoint 3.13 */\n>>>     constexpr uint16_t kFixedPoint3_13 = 8191;\n>>>\n>>>     return value * kFixedPoint3_13;\n>>> }\n>>\n>> In a 3.13 fixed-point format, 1.0 would be 8192, not 8191. The above\n>> would thus be misleading. 8191 is the maximum value of a 12bpp pixel,\n>> it's not a fixed-point number in this context.\n> \n> I am not sure I understand this comment :-/. I started by changing\n> - constexpr uint16_t kFixedPoint3_13 = 8191;\n> + constexpr uint16_t kFixedPoint3_13 = 8192;\n> \n> But now I read it again, and I think I might have misunderstand ?\n\nI have done that, I think this is what you meant ?\n\n+constexpr uint16_t Awb::Threshold(float value)\n+{\n+ /* AWB Thresholds are in the range [0, 8191] */\n+ constexpr uint16_t kMaxThreshold = 8191;\n+\n+ return value * kMaxThreshold;\n+}\n+\n\n> \n>>\n>>> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>>> {\n>>>     /*\n>>>      * Green saturation thresholds are reduced because...\n>>>      */\n>>>     params->acc_param.awb.config.rgbs_thr_r = Threshold(1.0);\n>>>     params->acc_param.awb.config.rgbs_thr_gr = Threshold(0.9);\n>>>     params->acc_param.awb.config.rgbs_thr_gb = Threshold(0.9);\n>>>     params->acc_param.awb.config.rgbs_thr_b = Threshold(1.0);\n>>>\n>>>     /* Enable saturation inclusion on thr_b because ... */\n>>>     params->acc_param.awb.config.rgbs_thr_b |= \n>>> IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n>>>                             IPU3_UAPI_AWB_RGBS_THR_B_EN;\n>>\n>> I like splitting this from the previous line.\n>>\n>>>     ...\n>>> }\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 194C8BDB1C\n\tfor ;\n\tWed, 20 Oct 2021 08:53:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 87FCB68F56;\n\tWed, 20 Oct 2021 10:53:58 +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 DE6676012A\n\tfor ;\n\tWed, 20 Oct 2021 10:53:56 +0200 (CEST)","from [IPV6:2a01:e0a:169:7140:ce4b:1c5f:7302:b899] (unknown\n\t[IPv6:2a01:e0a:169:7140:ce4b:1c5f:7302:b899])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 70FD72A5;\n\tWed, 20 Oct 2021 10:53:56 +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=\"uNQwlHhl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634720036;\n\tbh=iScAMcE2ta9hyExHIuLi09uIQHXg3UF7FU5zAICgvic=;\n\th=Date:Subject:From:To:Cc:References:In-Reply-To:From;\n\tb=uNQwlHhl6oNlEC+bxgRhk6wAGNSwtzUAAfA75vhv+OQ/Qo0Z4u+970mNwaoj90b/m\n\tDD42H4dqb2fMVt0MVZSGZxi41836pklUDmEE8JsOr8/AFwH75HsY6edIzBVzmYAX5u\n\tUTU/Htc/tDHVci1d7fqPzrLitp9zXQ4pHbA4ArhE=","Message-ID":"<7666bf85-f9f9-6f5d-e5cf-304af5acb7b2@ideasonboard.com>","Date":"Wed, 20 Oct 2021 10:53:54 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.1.2","Content-Language":"en-US","From":"Jean-Michel Hautbois ","To":"Laurent Pinchart ,\n\tKieran Bingham ","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163420699338.3829429.4509588483126382716@Monstersaurus>\n\t\n\t<3a8877f6-06b8-125f-5d96-c4414cf79d7f@ideasonboard.com>","In-Reply-To":"<3a8877f6-06b8-125f-5d96-c4414cf79d7f@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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\" "}},{"id":20330,"web_url":"https://patchwork.libcamera.org/comment/20330/","msgid":"<163472246151.2358410.3674984978338528087@Monstersaurus>","date":"2021-10-20T09:34:21","subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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-20 09:53:54)\n> \n> \n> On 20/10/2021 10:41, Jean-Michel Hautbois wrote:\n> > Hi Laurent,\n> > \n> > On 14/10/2021 23:56, Laurent Pinchart wrote:\n> >> Hello,\n> >>\n> >> On Thu, Oct 14, 2021 at 11:23:13AM +0100, Kieran Bingham wrote:\n> >>> Quoting Jean-Michel Hautbois (2021-10-13 16:41:13)\n> >>>> We can have a saturation ratio per cell, giving the percentage of \n> >>>> pixels\n> >>>> over a threshold within a cell where 100% is set to 0xff.\n> >>>>\n> >>>> The parameter structure 'ipu3_uapi_awb_config_s' contains four \n> >>>> fields to\n> >>>> set the threshold, one per channel.\n> >>>> The blue field is also used to configure the ImgU and make it calculate\n> >>>> the saturation ratio or not.\n> >>>>\n> >>>> Set a green value saturated when it is more than 230 (90% of the \n> >>>> maximum\n> >>>> value 255, coded as 8191).\n> >>>\n> >>> Why do we only determine a lower saturation on one component?\n> >>\n> >> I assume because we only use the stats for that component :-)\n> >>\n> >>> Shouldn't they be balanced (perhaps in accordance with the white balance\n> >>> somehow?).\n> >>\n> >> If the assumption is correct, then we could, and it would make no\n> >> difference.\n> >>\n> >>>> Signed-off-by: Jean-Michel Hautbois \n> >>>> \n> >>>> ---\n> >>>>   src/ipa/ipu3/algorithms/awb.cpp | 4 ++--\n> >>>>   1 file changed, 2 insertions(+), 2 deletions(-)\n> >>>>\n> >>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp \n> >>>> b/src/ipa/ipu3/algorithms/awb.cpp\n> >>>> index e2b18336..5574bd44 100644\n> >>>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> >>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> >>>> @@ -326,9 +326,9 @@ void Awb::process(IPAContext &context, const \n> >>>> ipu3_uapi_stats_3a *stats)\n> >>>>   void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> >>>>   {\n> >>>> -       params->acc_param.awb.config.rgbs_thr_gr = 8191;\n> >>>> +       params->acc_param.awb.config.rgbs_thr_gr = (90 * 8191 / 100);\n> >>\n> >> No need for parentheses.\n> >>\n> >>>>          params->acc_param.awb.config.rgbs_thr_r = 8191;\n> >>>> -       params->acc_param.awb.config.rgbs_thr_gb = 8191;\n> >>>> +       params->acc_param.awb.config.rgbs_thr_gb = (90 * 8191 / 100);\n> >>>>          params->acc_param.awb.config.rgbs_thr_b = \n> >>>> IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT\n> >>>>                                                 | \n> >>>> IPU3_UAPI_AWB_RGBS_THR_B_EN\n> >>>>                                                 | 8191;\n> >>>\n> >>> That's interesting that thr_b has those enable flags. Do they apply\n> >>> only to B? or does that enable all of them?\n> >>\n> >> As far as I undertand, the single flag applies to all channels.\n> >>\n> >>> I wonder if a helper function would make the values clearer:\n> >>>\n> >>> (Awb private:)\n> >>> uint16_t Awb::Threshold(float value)\n> >>\n> >> Make it\n> >>\n> >> constexpr uint16_t Awb::Threshold(float value)\n> >>\n> >>> {\n> >>>     /* AWB Thresholds are represented in FixedPoint 3.13 */\n> >>>     constexpr uint16_t kFixedPoint3_13 = 8191;\n> >>>\n> >>>     return value * kFixedPoint3_13;\n> >>> }\n> >>\n> >> In a 3.13 fixed-point format, 1.0 would be 8192, not 8191. The above\n> >> would thus be misleading. 8191 is the maximum value of a 12bpp pixel,\n> >> it's not a fixed-point number in this context.\n> > \n> > I am not sure I understand this comment :-/. I started by changing\n> > - constexpr uint16_t kFixedPoint3_13 = 8191;\n> > + constexpr uint16_t kFixedPoint3_13 = 8192;\n> > \n> > But now I read it again, and I think I might have misunderstand ?\n> \n> I have done that, I think this is what you meant ?\n> \n> +constexpr uint16_t Awb::Threshold(float value)\n> +{\n> + /* AWB Thresholds are in the range [0, 8191] */\n> + constexpr uint16_t kMaxThreshold = 8191;\n> +\n\nThat's probably better, yes.\n\nDo we need to clamp the input values? Maybe not as we're in control of\nthem anyway. Perhaps a compile time ASSERT(value >= 0 && value <= 1.0) ? or such?\n\n\n> + return value * kMaxThreshold;\n> +}\n> +\n> \n> > \n> >>\n> >>> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> >>> {\n> >>>     /*\n> >>>      * Green saturation thresholds are reduced because...\n> >>>      */\n> >>>     params->acc_param.awb.config.rgbs_thr_r = Threshold(1.0);\n> >>>     params->acc_param.awb.config.rgbs_thr_gr = Threshold(0.9);\n> >>>     params->acc_param.awb.config.rgbs_thr_gb = Threshold(0.9);\n> >>>     params->acc_param.awb.config.rgbs_thr_b = Threshold(1.0);\n> >>>\n> >>>     /* Enable saturation inclusion on thr_b because ... */\n> >>>     params->acc_param.awb.config.rgbs_thr_b |= \n> >>> IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n> >>>                             IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> >>\n> >> I like splitting this from the previous line.\n> >>\n> >>>     ...\n> >>> }\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 5712CBDB1C\n\tfor ;\n\tWed, 20 Oct 2021 09:34:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D051268F59;\n\tWed, 20 Oct 2021 11:34:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 88EA86012A\n\tfor ;\n\tWed, 20 Oct 2021 11:34:24 +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 153C92A5;\n\tWed, 20 Oct 2021 11:34: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=\"dLbHDcsN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634722464;\n\tbh=ipGMNXDVaVKnWEnpLmMtUL3E/ygP6k/rkMTeJtp3bEo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=dLbHDcsNHXdxQoPlslhUNOHlHt5LiPxqDI5O2wRlhC7F0n990NDWHxrMCi30zI1RA\n\tWQ+mmYA+YfdYSU9kqjDLCY9xlsEzbX4RN+eQNGMLEbcaVCaTwhXo5sl87S+Gg5RGWK\n\tLcVk0V0Qwl85QOma4SaAXpWa2RxdGXn+KRLHzCW8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<7666bf85-f9f9-6f5d-e5cf-304af5acb7b2@ideasonboard.com>","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163420699338.3829429.4509588483126382716@Monstersaurus>\n\t\n\t<3a8877f6-06b8-125f-5d96-c4414cf79d7f@ideasonboard.com>\n\t<7666bf85-f9f9-6f5d-e5cf-304af5acb7b2@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tLaurent Pinchart ","Date":"Wed, 20 Oct 2021 10:34:21 +0100","Message-ID":"<163472246151.2358410.3674984978338528087@Monstersaurus>","User-Agent":"alot/0.9.2","Subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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\" "}},{"id":20331,"web_url":"https://patchwork.libcamera.org/comment/20331/","msgid":"","date":"2021-10-20T09:46:01","subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Oct 20, 2021 at 10:34:21AM +0100, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-10-20 09:53:54)\n> > On 20/10/2021 10:41, Jean-Michel Hautbois wrote:\n> > > On 14/10/2021 23:56, Laurent Pinchart wrote:\n> > >> On Thu, Oct 14, 2021 at 11:23:13AM +0100, Kieran Bingham wrote:\n> > >>> Quoting Jean-Michel Hautbois (2021-10-13 16:41:13)\n> > >>>> We can have a saturation ratio per cell, giving the percentage of \n> > >>>> pixels\n> > >>>> over a threshold within a cell where 100% is set to 0xff.\n> > >>>>\n> > >>>> The parameter structure 'ipu3_uapi_awb_config_s' contains four \n> > >>>> fields to\n> > >>>> set the threshold, one per channel.\n> > >>>> The blue field is also used to configure the ImgU and make it calculate\n> > >>>> the saturation ratio or not.\n> > >>>>\n> > >>>> Set a green value saturated when it is more than 230 (90% of the \n> > >>>> maximum\n> > >>>> value 255, coded as 8191).\n> > >>>\n> > >>> Why do we only determine a lower saturation on one component?\n> > >>\n> > >> I assume because we only use the stats for that component :-)\n> > >>\n> > >>> Shouldn't they be balanced (perhaps in accordance with the white balance\n> > >>> somehow?).\n> > >>\n> > >> If the assumption is correct, then we could, and it would make no\n> > >> difference.\n> > >>\n> > >>>> Signed-off-by: Jean-Michel Hautbois \n> > >>>> ---\n> > >>>>   src/ipa/ipu3/algorithms/awb.cpp | 4 ++--\n> > >>>>   1 file changed, 2 insertions(+), 2 deletions(-)\n> > >>>>\n> > >>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp \n> > >>>> b/src/ipa/ipu3/algorithms/awb.cpp\n> > >>>> index e2b18336..5574bd44 100644\n> > >>>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > >>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > >>>> @@ -326,9 +326,9 @@ void Awb::process(IPAContext &context, const \n> > >>>> ipu3_uapi_stats_3a *stats)\n> > >>>>   void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> > >>>>   {\n> > >>>> -       params->acc_param.awb.config.rgbs_thr_gr = 8191;\n> > >>>> +       params->acc_param.awb.config.rgbs_thr_gr = (90 * 8191 / 100);\n> > >>\n> > >> No need for parentheses.\n> > >>\n> > >>>>          params->acc_param.awb.config.rgbs_thr_r = 8191;\n> > >>>> -       params->acc_param.awb.config.rgbs_thr_gb = 8191;\n> > >>>> +       params->acc_param.awb.config.rgbs_thr_gb = (90 * 8191 / 100);\n> > >>>>          params->acc_param.awb.config.rgbs_thr_b = \n> > >>>> IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT\n> > >>>>                                                 | \n> > >>>> IPU3_UAPI_AWB_RGBS_THR_B_EN\n> > >>>>                                                 | 8191;\n> > >>>\n> > >>> That's interesting that thr_b has those enable flags. Do they apply\n> > >>> only to B? or does that enable all of them?\n> > >>\n> > >> As far as I undertand, the single flag applies to all channels.\n> > >>\n> > >>> I wonder if a helper function would make the values clearer:\n> > >>>\n> > >>> (Awb private:)\n> > >>> uint16_t Awb::Threshold(float value)\n> > >>\n> > >> Make it\n> > >>\n> > >> constexpr uint16_t Awb::Threshold(float value)\n> > >>\n> > >>> {\n> > >>>     /* AWB Thresholds are represented in FixedPoint 3.13 */\n> > >>>     constexpr uint16_t kFixedPoint3_13 = 8191;\n> > >>>\n> > >>>     return value * kFixedPoint3_13;\n> > >>> }\n> > >>\n> > >> In a 3.13 fixed-point format, 1.0 would be 8192, not 8191. The above\n> > >> would thus be misleading. 8191 is the maximum value of a 12bpp pixel,\n> > >> it's not a fixed-point number in this context.\n> > > \n> > > I am not sure I understand this comment :-/. I started by changing\n> > > - constexpr uint16_t kFixedPoint3_13 = 8191;\n> > > + constexpr uint16_t kFixedPoint3_13 = 8192;\n> > > \n> > > But now I read it again, and I think I might have misunderstand ?\n\nThe point was that in U3.13 fixed-point format, 1.0 would be encoded as\n8192, not 8191. The values we deal with here are not fixed-point decimal\nnumbers, they're just integer pixel values in the range [0, 8191]. If we\nwant to map these to a floating-point [0.0, 1.0] range internally to\nmake it more readable (0.8 immediately shows as 80%, while 6553\ndoesn't), that's fine, but we shouldn't pretend the values are\nfixed-point decimals.\n\n> > I have done that, I think this is what you meant ?\n> > \n> > +constexpr uint16_t Awb::Threshold(float value)\n\ns/Threshold/threshold/\n\n> > +{\n> > + /* AWB Thresholds are in the range [0, 8191] */\n> > + constexpr uint16_t kMaxThreshold = 8191;\n> > +\n> \n> That's probably better, yes.\n> \n> Do we need to clamp the input values? Maybe not as we're in control of\n> them anyway. Perhaps a compile time ASSERT(value >= 0 && value <= 1.0) ? or such?\n\nThat's not compile-time :-) static_assert could be used, but that would\nrequire the argument to be a constexpr. That's not a problem here.\n\n> > + return value * kMaxThreshold;\n> > +}\n> > +\n> > \n> > >>> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> > >>> {\n> > >>>     /*\n> > >>>      * Green saturation thresholds are reduced because...\n> > >>>      */\n> > >>>     params->acc_param.awb.config.rgbs_thr_r = Threshold(1.0);\n> > >>>     params->acc_param.awb.config.rgbs_thr_gr = Threshold(0.9);\n> > >>>     params->acc_param.awb.config.rgbs_thr_gb = Threshold(0.9);\n> > >>>     params->acc_param.awb.config.rgbs_thr_b = Threshold(1.0);\n> > >>>\n> > >>>     /* Enable saturation inclusion on thr_b because ... */\n> > >>>     params->acc_param.awb.config.rgbs_thr_b |= \n> > >>> IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n> > >>>                             IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> > >>\n> > >> I like splitting this from the previous line.\n> > >>\n> > >>>     ...\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 DF2C5BF415\n\tfor ;\n\tWed, 20 Oct 2021 09:46:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1783068F59;\n\tWed, 20 Oct 2021 11:46: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 591A66012A\n\tfor ;\n\tWed, 20 Oct 2021 11:46:20 +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 C04BE2A5;\n\tWed, 20 Oct 2021 11:46:19 +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=\"kJhydext\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634723180;\n\tbh=uCl04u9O6rRxQIX2Sx7dqN5D/JZmDJ/WbxQGO9Ya6RA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kJhydextijpolE0vozxlHlF9J4mF/Q6qNSeW9sdmWt5YP2yJl5a89rrsMtN9ohJnq\n\t+J2lKrk9Hmv79C1Gmt9n9oRtjRlhCEzUvX/23FNwqpNn+ISVvS+sg4a/bfIMP3yLYp\n\t3mYyVDsl5VXnU0hDPcAf5SMOOGU1F0ZANuWq8K/U=","Date":"Wed, 20 Oct 2021 12:46:01 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163420699338.3829429.4509588483126382716@Monstersaurus>\n\t\n\t<3a8877f6-06b8-125f-5d96-c4414cf79d7f@ideasonboard.com>\n\t<7666bf85-f9f9-6f5d-e5cf-304af5acb7b2@ideasonboard.com>\n\t<163472246151.2358410.3674984978338528087@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<163472246151.2358410.3674984978338528087@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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\" "}},{"id":20332,"web_url":"https://patchwork.libcamera.org/comment/20332/","msgid":"<163472357833.2382401.161545392944996934@Monstersaurus>","date":"2021-10-20T09:52:58","subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2021-10-20 10:46:01)\n> On Wed, Oct 20, 2021 at 10:34:21AM +0100, Kieran Bingham wrote:\n> > Quoting Jean-Michel Hautbois (2021-10-20 09:53:54)\n> > > On 20/10/2021 10:41, Jean-Michel Hautbois wrote:\n> > > > On 14/10/2021 23:56, Laurent Pinchart wrote:\n> > > >> On Thu, Oct 14, 2021 at 11:23:13AM +0100, Kieran Bingham wrote:\n> > > >>> Quoting Jean-Michel Hautbois (2021-10-13 16:41:13)\n> > > >>>> We can have a saturation ratio per cell, giving the percentage of \n> > > >>>> pixels\n> > > >>>> over a threshold within a cell where 100% is set to 0xff.\n> > > >>>>\n> > > >>>> The parameter structure 'ipu3_uapi_awb_config_s' contains four \n> > > >>>> fields to\n> > > >>>> set the threshold, one per channel.\n> > > >>>> The blue field is also used to configure the ImgU and make it calculate\n> > > >>>> the saturation ratio or not.\n> > > >>>>\n> > > >>>> Set a green value saturated when it is more than 230 (90% of the \n> > > >>>> maximum\n> > > >>>> value 255, coded as 8191).\n> > > >>>\n> > > >>> Why do we only determine a lower saturation on one component?\n> > > >>\n> > > >> I assume because we only use the stats for that component :-)\n> > > >>\n> > > >>> Shouldn't they be balanced (perhaps in accordance with the white balance\n> > > >>> somehow?).\n> > > >>\n> > > >> If the assumption is correct, then we could, and it would make no\n> > > >> difference.\n> > > >>\n> > > >>>> Signed-off-by: Jean-Michel Hautbois \n> > > >>>> ---\n> > > >>>>   src/ipa/ipu3/algorithms/awb.cpp | 4 ++--\n> > > >>>>   1 file changed, 2 insertions(+), 2 deletions(-)\n> > > >>>>\n> > > >>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp \n> > > >>>> b/src/ipa/ipu3/algorithms/awb.cpp\n> > > >>>> index e2b18336..5574bd44 100644\n> > > >>>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > > >>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > > >>>> @@ -326,9 +326,9 @@ void Awb::process(IPAContext &context, const \n> > > >>>> ipu3_uapi_stats_3a *stats)\n> > > >>>>   void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> > > >>>>   {\n> > > >>>> -       params->acc_param.awb.config.rgbs_thr_gr = 8191;\n> > > >>>> +       params->acc_param.awb.config.rgbs_thr_gr = (90 * 8191 / 100);\n> > > >>\n> > > >> No need for parentheses.\n> > > >>\n> > > >>>>          params->acc_param.awb.config.rgbs_thr_r = 8191;\n> > > >>>> -       params->acc_param.awb.config.rgbs_thr_gb = 8191;\n> > > >>>> +       params->acc_param.awb.config.rgbs_thr_gb = (90 * 8191 / 100);\n> > > >>>>          params->acc_param.awb.config.rgbs_thr_b = \n> > > >>>> IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT\n> > > >>>>                                                 | \n> > > >>>> IPU3_UAPI_AWB_RGBS_THR_B_EN\n> > > >>>>                                                 | 8191;\n> > > >>>\n> > > >>> That's interesting that thr_b has those enable flags. Do they apply\n> > > >>> only to B? or does that enable all of them?\n> > > >>\n> > > >> As far as I undertand, the single flag applies to all channels.\n> > > >>\n> > > >>> I wonder if a helper function would make the values clearer:\n> > > >>>\n> > > >>> (Awb private:)\n> > > >>> uint16_t Awb::Threshold(float value)\n> > > >>\n> > > >> Make it\n> > > >>\n> > > >> constexpr uint16_t Awb::Threshold(float value)\n> > > >>\n> > > >>> {\n> > > >>>     /* AWB Thresholds are represented in FixedPoint 3.13 */\n> > > >>>     constexpr uint16_t kFixedPoint3_13 = 8191;\n> > > >>>\n> > > >>>     return value * kFixedPoint3_13;\n> > > >>> }\n> > > >>\n> > > >> In a 3.13 fixed-point format, 1.0 would be 8192, not 8191. The above\n> > > >> would thus be misleading. 8191 is the maximum value of a 12bpp pixel,\n> > > >> it's not a fixed-point number in this context.\n> > > > \n> > > > I am not sure I understand this comment :-/. I started by changing\n> > > > - constexpr uint16_t kFixedPoint3_13 = 8191;\n> > > > + constexpr uint16_t kFixedPoint3_13 = 8192;\n> > > > \n> > > > But now I read it again, and I think I might have misunderstand ?\n> \n> The point was that in U3.13 fixed-point format, 1.0 would be encoded as\n> 8192, not 8191. The values we deal with here are not fixed-point decimal\n> numbers, they're just integer pixel values in the range [0, 8191]. If we\n> want to map these to a floating-point [0.0, 1.0] range internally to\n> make it more readable (0.8 immediately shows as 80%, while 6553\n> doesn't), that's fine, but we shouldn't pretend the values are\n> fixed-point decimals.\n> \n> > > I have done that, I think this is what you meant ?\n> > > \n> > > +constexpr uint16_t Awb::Threshold(float value)\n> \n> s/Threshold/threshold/\n> \n> > > +{\n> > > + /* AWB Thresholds are in the range [0, 8191] */\n> > > + constexpr uint16_t kMaxThreshold = 8191;\n> > > +\n> > \n> > That's probably better, yes.\n> > \n> > Do we need to clamp the input values? Maybe not as we're in control of\n> > them anyway. Perhaps a compile time ASSERT(value >= 0 && value <= 1.0) ? or such?\n> \n> That's not compile-time :-) static_assert could be used, but that would\n> require the argument to be a constexpr. That's not a problem here.\n> \n\nWell then, that's what I meant ;-)\n--\nKieran\n\n\n> > > + return value * kMaxThreshold;\n> > > +}\n> > > +\n> > > \n> > > >>> void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> > > >>> {\n> > > >>>     /*\n> > > >>>      * Green saturation thresholds are reduced because...\n> > > >>>      */\n> > > >>>     params->acc_param.awb.config.rgbs_thr_r = Threshold(1.0);\n> > > >>>     params->acc_param.awb.config.rgbs_thr_gr = Threshold(0.9);\n> > > >>>     params->acc_param.awb.config.rgbs_thr_gb = Threshold(0.9);\n> > > >>>     params->acc_param.awb.config.rgbs_thr_b = Threshold(1.0);\n> > > >>>\n> > > >>>     /* Enable saturation inclusion on thr_b because ... */\n> > > >>>     params->acc_param.awb.config.rgbs_thr_b |= \n> > > >>> IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n> > > >>>                             IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> > > >>\n> > > >> I like splitting this from the previous line.\n> > > >>\n> > > >>>     ...\n> > > >>> }\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 0EC57BDB1C\n\tfor ;\n\tWed, 20 Oct 2021 09:53:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 66B9A68F5B;\n\tWed, 20 Oct 2021 11:53:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 054396012A\n\tfor ;\n\tWed, 20 Oct 2021 11:53:01 +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 865B72A5;\n\tWed, 20 Oct 2021 11:53:00 +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=\"GomwHCg8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634723580;\n\tbh=VWE5lCFF/xUgTQ7YzKW8ZQ5GO9isZQR3cYw0lwOMQZs=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=GomwHCg8XRI5ZSWzY1XTXBzvNdBcx+Ir6DEQRCmg81jbN/pttrTPxYzVELofpeBWb\n\tXzTos3uJOHKafVL15NS9/Ll/kF5zYoglvI1uomPsS9FFVF01eiunu7OU+qfiHpQmC3\n\tRokYxD0QYFduOvAhQCELARcNx9JD69vrb5wcYcho=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163420699338.3829429.4509588483126382716@Monstersaurus>\n\t\n\t<3a8877f6-06b8-125f-5d96-c4414cf79d7f@ideasonboard.com>\n\t<7666bf85-f9f9-6f5d-e5cf-304af5acb7b2@ideasonboard.com>\n\t<163472246151.2358410.3674984978338528087@Monstersaurus>\n\t","From":"Kieran Bingham ","To":"Laurent Pinchart ","Date":"Wed, 20 Oct 2021 10:52:58 +0100","Message-ID":"<163472357833.2382401.161545392944996934@Monstersaurus>","User-Agent":"alot/0.9.2","Subject":"Re: [libcamera-devel] [PATCH 01/13] ipa: ipu3: awb: Set a threshold\n\tfor the green saturation","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\" "}}]