[{"id":20196,"web_url":"https://patchwork.libcamera.org/comment/20196/","msgid":"<163420755797.3829429.7733738697496073861@Monstersaurus>","date":"2021-10-14T10:32:37","subject":"Re: [libcamera-devel] [PATCH 03/13] ipa: ipu3: awb: Use saturation\n\tunder 90%","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:15)\n> The AWB grey world algorithm tries to find a grey value and it can't do\n> it on over-exposed images. To exclude those, the saturation ratio is\n> used for each cell, and the cell is included only if this ratio is 0.\n> \n> Now that we have changed the threshold, more cells may be considered as\n> partially saturated and excluded, making the algorithm to not run.\n\n\", preventing the algorithm from running efficiently.\" ?\n\nor s/efficiently/correctly/ ?\n\n> \n> Change that behaviour, and consider 90% as a good enough ratio.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/ipu3/algorithms/awb.cpp | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index 5574bd44..30693923 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -161,6 +161,7 @@ int Awb::configure(IPAContext &context,\n> * \\todo This proportion could be configured.\n> */\n> cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * 80 / 100;\n> + LOG(IPU3Awb, Debug) << \"Threshold for AWB is set to \" << cellsPerZoneThreshold_;\n\nThis is now a bit confusing. Your commit message mentions 90%, but the\ncellsPerZoneThreshold is 80%?\n\nI guess these are two different thresholds...\n\n\n> \n> return 0;\n> }\n> @@ -232,7 +233,7 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)\n> reinterpret_cast(\n> &stats->awb_raw_buffer.meta_data[cellPosition]\n> );\n\nThis probably now deserves a comment explaining why 90% is used.\n\n\t\t\t /*\n\t\t\t * Use cells which have less than 90%\n\t\t\t * saturation as an initial means to include\n\t\t\t * otherwise bright cells which are not fully\n\t\t\t * saturated.\n\t\t\t *\n\t\t\t * \\todo The 90% saturation rate may require\n\t\t\t * further empirical measurements and\n\t\t\t * optimisation during camera tuning phases.\n\t\t\t */\n\n> - if (currentCell->sat_ratio == 0) {\n> + if (currentCell->sat_ratio <= 255 * 90 / 100) {\n> /* The cell is not saturated, use the current cell */\n> awbStats_[awbZonePosition].counted++;\n> uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg;\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 8D1BAC323E\n\tfor ;\n\tThu, 14 Oct 2021 10:32:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0BBCC68F4F;\n\tThu, 14 Oct 2021 12:32:43 +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 78BF768541\n\tfor ;\n\tThu, 14 Oct 2021 12:32:41 +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 0CD3E2F3;\n\tThu, 14 Oct 2021 12:32:41 +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=\"ZfCtyp7r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634207561;\n\tbh=H99K0r7TAlAwLoI0ug+byRlL/QSXEOrdDo0RrD19Rd0=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=ZfCtyp7rAkd/7QiGpMzGNYaOvYS3YpPlMwY/QojU+dwMvhwvEEmzKHmdLlESkDGeG\n\txsliJyBh1d6SU2/aR0d9pGE/g66dEGVebgSFxgBnxl5RV1hpHT5C8G9YOiw06p9HEs\n\tdoYUuK13JEeyRVMm4R1cv9d9CZBmyWjdPwsyTgkQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211013154125.133419-4-jeanmichel.hautbois@ideasonboard.com>","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-4-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 14 Oct 2021 11:32:37 +0100","Message-ID":"<163420755797.3829429.7733738697496073861@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 03/13] ipa: ipu3: awb: Use saturation\n\tunder 90%","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":20213,"web_url":"https://patchwork.libcamera.org/comment/20213/","msgid":"","date":"2021-10-14T22:12:08","subject":"Re: [libcamera-devel] [PATCH 03/13] ipa: ipu3: awb: Use saturation\n\tunder 90%","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Oct 14, 2021 at 11:32:37AM +0100, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-10-13 16:41:15)\n> > The AWB grey world algorithm tries to find a grey value and it can't do\n> > it on over-exposed images. To exclude those, the saturation ratio is\n> > used for each cell, and the cell is included only if this ratio is 0.\n> > \n> > Now that we have changed the threshold, more cells may be considered as\n> > partially saturated and excluded, making the algorithm to not run.\n> \n> \", preventing the algorithm from running efficiently.\" ?\n> \n> or s/efficiently/correctly/ ?\n> \n> > Change that behaviour, and consider 90% as a good enough ratio.\n> > \n> > Signed-off-by: Jean-Michel Hautbois \n> > ---\n> > src/ipa/ipu3/algorithms/awb.cpp | 3 ++-\n> > 1 file changed, 2 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> > index 5574bd44..30693923 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > @@ -161,6 +161,7 @@ int Awb::configure(IPAContext &context,\n> > * \\todo This proportion could be configured.\n> > */\n> > cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * 80 / 100;\n> > + LOG(IPU3Awb, Debug) << \"Threshold for AWB is set to \" << cellsPerZoneThreshold_;\n> \n> This is now a bit confusing. Your commit message mentions 90%, but the\n> cellsPerZoneThreshold is 80%?\n> \n> I guess these are two different thresholds...\n\nTo be considered, a zone needs 80% of the cells below a 90% saturation\nratio.\n\nI'd add constexpr constants to these values.\n\n> > return 0;\n> > }\n> > @@ -232,7 +233,7 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)\n> > reinterpret_cast(\n> > &stats->awb_raw_buffer.meta_data[cellPosition]\n> > );\n> \n> This probably now deserves a comment explaining why 90% is used.\n> \n> \t\t\t /*\n> \t\t\t * Use cells which have less than 90%\n> \t\t\t * saturation as an initial means to include\n> \t\t\t * otherwise bright cells which are not fully\n> \t\t\t * saturated.\n> \t\t\t *\n> \t\t\t * \\todo The 90% saturation rate may require\n> \t\t\t * further empirical measurements and\n> \t\t\t * optimisation during camera tuning phases.\n> \t\t\t */\n> \n> > - if (currentCell->sat_ratio == 0) {\n> > + if (currentCell->sat_ratio <= 255 * 90 / 100) {\n> > /* The cell is not saturated, use the current cell */\n> > awbStats_[awbZonePosition].counted++;\n> > uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg;","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 BA5C6C324C\n\tfor ;\n\tThu, 14 Oct 2021 22:12:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D5A3E68F4F;\n\tFri, 15 Oct 2021 00:12:26 +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 A73AA60501\n\tfor ;\n\tFri, 15 Oct 2021 00:12:24 +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 110FCB91;\n\tFri, 15 Oct 2021 00:12:24 +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=\"meOVVv+1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634249544;\n\tbh=l2VELFwLZQPtGEhSGDFQHBwSQ8SSHz4RTnls394V2r4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=meOVVv+1c3mq3tlEwQ9cGs6LCEYrebqHCI1kFLI3s1vmu5u9KyQTKSorH4Owm1VAR\n\tUUsmQO8jQ87G0rXSPFUPCCmy8+ojqUUHvLUaHgwwq5ZKEZPUxI+J1T7pZgSZaGPYKj\n\tHd2f5KV0oJpgUhLJe0eDKJTLgpJSu+Zk25nQamB4=","Date":"Fri, 15 Oct 2021 01:12:08 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-4-jeanmichel.hautbois@ideasonboard.com>\n\t<163420755797.3829429.7733738697496073861@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163420755797.3829429.7733738697496073861@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 03/13] ipa: ipu3: awb: Use saturation\n\tunder 90%","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":20231,"web_url":"https://patchwork.libcamera.org/comment/20231/","msgid":"<6424fd58-c64e-fa69-8970-2b8cb3ad1acc@ideasonboard.com>","date":"2021-10-15T05:42:16","subject":"Re: [libcamera-devel] [PATCH 03/13] ipa: ipu3: awb: Use saturation\n\tunder 90%","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi,\n\nOn 15/10/2021 00:12, Laurent Pinchart wrote:\n> On Thu, Oct 14, 2021 at 11:32:37AM +0100, Kieran Bingham wrote:\n>> Quoting Jean-Michel Hautbois (2021-10-13 16:41:15)\n>>> The AWB grey world algorithm tries to find a grey value and it can't do\n>>> it on over-exposed images. To exclude those, the saturation ratio is\n>>> used for each cell, and the cell is included only if this ratio is 0.\n>>>\n>>> Now that we have changed the threshold, more cells may be considered as\n>>> partially saturated and excluded, making the algorithm to not run.\n>>\n>> \", preventing the algorithm from running efficiently.\" ?\n>>\n>> or s/efficiently/correctly/ ?\n>>\n>>> Change that behaviour, and consider 90% as a good enough ratio.\n>>>\n>>> Signed-off-by: Jean-Michel Hautbois \n>>> ---\n>>> src/ipa/ipu3/algorithms/awb.cpp | 3 ++-\n>>> 1 file changed, 2 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n>>> index 5574bd44..30693923 100644\n>>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n>>> @@ -161,6 +161,7 @@ int Awb::configure(IPAContext &context,\n>>> * \\todo This proportion could be configured.\n>>> */\n>>> cellsPerZoneThreshold_ = cellsPerZoneX_ * cellsPerZoneY_ * 80 / 100;\n>>> + LOG(IPU3Awb, Debug) << \"Threshold for AWB is set to \" << cellsPerZoneThreshold_;\n>>\n>> This is now a bit confusing. Your commit message mentions 90%, but the\n>> cellsPerZoneThreshold is 80%?\n>>\n>> I guess these are two different thresholds...\n> \n> To be considered, a zone needs 80% of the cells below a 90% saturation\n> ratio.\n\nPrecisely.\n\n> \n> I'd add constexpr constants to these values.\n\nAck.\n\n> \n>>> return 0;\n>>> }\n>>> @@ -232,7 +233,7 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)\n>>> reinterpret_cast(\n>>> &stats->awb_raw_buffer.meta_data[cellPosition]\n>>> );\n>>\n>> This probably now deserves a comment explaining why 90% is used.\n>>\n>> \t\t\t /*\n>> \t\t\t * Use cells which have less than 90%\n>> \t\t\t * saturation as an initial means to include\n>> \t\t\t * otherwise bright cells which are not fully\n>> \t\t\t * saturated.\n>> \t\t\t *\n>> \t\t\t * \\todo The 90% saturation rate may require\n>> \t\t\t * further empirical measurements and\n>> \t\t\t * optimisation during camera tuning phases.\n>> \t\t\t */\n>>\n>>> - if (currentCell->sat_ratio == 0) {\n>>> + if (currentCell->sat_ratio <= 255 * 90 / 100) {\n>>> /* The cell is not saturated, use the current cell */\n>>> awbStats_[awbZonePosition].counted++;\n>>> uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg;\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 AF93CC323E\n\tfor ;\n\tFri, 15 Oct 2021 05:42:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C10F68F50;\n\tFri, 15 Oct 2021 07:42:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B53D60239\n\tfor ;\n\tFri, 15 Oct 2021 07:42:19 +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 1FDEB2E3;\n\tFri, 15 Oct 2021 07:42:19 +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=\"uCbPH964\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634276539;\n\tbh=sDZlR/1Xqc15DSAluT8C74diondnH6t7fikcNfreSig=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=uCbPH964Xg2Eb6zX9FTtvfkFcH/aNN1bNTQ9Y2XrB8jsQoswV5UvNZOZntlfEny0C\n\tzYfYOnSHK+vxJtbG4WQOgzbW85zl2jGB4ITfZt3ky53pZd8lZQKLAKol4bbKJ4QeHi\n\tlTOMxoLc+kkk9YnbRSwEb4uEjQI6WT6XS8V50LWc=","To":"Laurent Pinchart ,\n\tKieran Bingham ","References":"<20211013154125.133419-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211013154125.133419-4-jeanmichel.hautbois@ideasonboard.com>\n\t<163420755797.3829429.7733738697496073861@Monstersaurus>\n\t","From":"Jean-Michel Hautbois ","Message-ID":"<6424fd58-c64e-fa69-8970-2b8cb3ad1acc@ideasonboard.com>","Date":"Fri, 15 Oct 2021 07:42:16 +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 03/13] ipa: ipu3: awb: Use saturation\n\tunder 90%","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\" "}}]