[{"id":34101,"web_url":"https://patchwork.libcamera.org/comment/34101/","msgid":"<174617332642.1586992.13042286476868819624@ping.linuxembedded.co.uk>","date":"2025-05-02T08:08:46","subject":"Re: [PATCH v3 10/16] ipa: rkisp1: algorithms: awb: Fix wrong colour\n\ttemperature reporting","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-04-03 16:49:15)\n> In commit b60bd37b1a49 (\"ipa: rkisp1: Move calculation of RGB means into\n> own function\") the output of the current measured colour temperature as\n> metadata was incorrectly added. Remove it.\n> \n\nOoops, it even says \"Commit doesn't contain any functional changes.\" But\nit did!\n\nIs the colour temperature set elsewhere now?\n\n\n\n> Fixes: b60bd37b1a49 (\"ipa: rkisp1: Move calculation of RGB means into own function\")\n> Signed-off-by: Stefan Klug \n> \n\nReviewed-by: Kieran Bingham \n\n> ---\n> \n> Changes in v3:\n> - Added this patch\n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 3 ---\n> 1 file changed, 3 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index a9759e53f593..79c4c658406d 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -311,9 +311,6 @@ void Awb::process(IPAContext &context,\n> \n> activeState.awb.temperatureK = awbResult.colourTemperature;\n> \n> - /* Metadata shall contain the up to date measurement */\n> - metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n> -\n> /*\n> * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> * unsigned integer values. Set the minimum just above zero to avoid\n> -- \n> 2.43.0\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 835ADBE08B\n\tfor ;\n\tFri, 2 May 2025 08:08:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 80A4868ADB;\n\tFri, 2 May 2025 10:08:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F5A168ACB\n\tfor ;\n\tFri, 2 May 2025 10:08:49 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 70575353;\n\tFri, 2 May 2025 10:08: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=\"sHqriqoL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746173321;\n\tbh=DqtuwbL+GzgjDS15dRTuM9jcoxRM1XtxJYeahX4sinI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=sHqriqoLGJEI6aXua1hqSQs70jfqQWB23pAcWko3W5xhzQOYmUYQptBINU5uF8Cog\n\tBHjpMjq5LF6Za5CDVj+Bq+ni7UL6DFKF7LOGYCr+ccW1q13DleBtgX2Ho9hlNLuYdu\n\tMpTD50gjX5yR/RBGMPmEpu9b++ZLTL1utNJ4MJIc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250403154925.382973-11-stefan.klug@ideasonboard.com>","References":"<20250403154925.382973-1-stefan.klug@ideasonboard.com>\n\t<20250403154925.382973-11-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v3 10/16] ipa: rkisp1: algorithms: awb: Fix wrong colour\n\ttemperature reporting","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 02 May 2025 09:08:46 +0100","Message-ID":"<174617332642.1586992.13042286476868819624@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":34124,"web_url":"https://patchwork.libcamera.org/comment/34124/","msgid":"<20250502210154.GB24278@pendragon.ideasonboard.com>","date":"2025-05-02T21:01:54","subject":"Re: [PATCH v3 10/16] ipa: rkisp1: algorithms: awb: Fix wrong colour\n\ttemperature reporting","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, May 02, 2025 at 09:08:46AM +0100, Kieran Bingham wrote:\n> Quoting Stefan Klug (2025-04-03 16:49:15)\n> > In commit b60bd37b1a49 (\"ipa: rkisp1: Move calculation of RGB means into\n> > own function\") the output of the current measured colour temperature as\n> > metadata was incorrectly added. Remove it.\n> > \n> \n> Ooops, it even says \"Commit doesn't contain any functional changes.\" But\n> it did!\n> \n> Is the colour temperature set elsewhere now?\n\nIt's set earlier in the process() function, to\nframeContext.awb.temperatureK instead of activeState.awb.temperatureK.\n\nThere was a discussion on whether or not we should add a metadata\ncontrol to report the colour temperature estimated for the frame, in\naddition to the one that was used to process the frame. Using\nactiveState.awb.temperatureK switches to the latter. Regardless of the\nanswer to that question, the aforementioned commit introduced a\nfunctional change, so I think this fix is right.\n\n> > Fixes: b60bd37b1a49 (\"ipa: rkisp1: Move calculation of RGB means into own function\")\n> > Signed-off-by: Stefan Klug \n> > \n> \n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\n> > ---\n> > \n> > Changes in v3:\n> > - Added this patch\n> > ---\n> > src/ipa/rkisp1/algorithms/awb.cpp | 3 ---\n> > 1 file changed, 3 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index a9759e53f593..79c4c658406d 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -311,9 +311,6 @@ void Awb::process(IPAContext &context,\n> > \n> > activeState.awb.temperatureK = awbResult.colourTemperature;\n> > \n> > - /* Metadata shall contain the up to date measurement */\n> > - metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n> > -\n> > /*\n> > * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> > * unsigned integer values. Set the minimum just above zero to avoid","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 AD6C1BE08B\n\tfor ;\n\tFri, 2 May 2025 21:02:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D3A5368ADE;\n\tFri, 2 May 2025 23:02:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 16CA368AD3\n\tfor ;\n\tFri, 2 May 2025 23:02:04 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A844422F;\n\tFri, 2 May 2025 23:01:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cWEtFz9M\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746219715;\n\tbh=X9awC51HhUGET07iVNSzBtBgDpM5KR/v8Tn39GhDUGE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cWEtFz9MdrcM9aD/7826oB00LIiJq0qNWOqqzOuXplUUQnXIyVxApYOwNKp1M4dxX\n\tvNv8VjCKnqt3C/4Rr5ROGprCmOgeRc5MgQcPFnN1r5oFK/GjXrn2lNQpw1auaBGv88\n\tES4tgn7uk7IFuMlr5L+33wxcoLSvxXOWMkuWo8A0=","Date":"Sat, 3 May 2025 00:01:54 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 10/16] ipa: rkisp1: algorithms: awb: Fix wrong colour\n\ttemperature reporting","Message-ID":"<20250502210154.GB24278@pendragon.ideasonboard.com>","References":"<20250403154925.382973-1-stefan.klug@ideasonboard.com>\n\t<20250403154925.382973-11-stefan.klug@ideasonboard.com>\n\t<174617332642.1586992.13042286476868819624@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<174617332642.1586992.13042286476868819624@ping.linuxembedded.co.uk>","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":34147,"web_url":"https://patchwork.libcamera.org/comment/34147/","msgid":"","date":"2025-05-07T16:54:02","subject":"Re: [PATCH v3 10/16] ipa: rkisp1: algorithms: awb: Fix wrong colour\n\ttemperature reporting","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Sat, May 03, 2025 at 12:01:54AM +0300, Laurent Pinchart wrote:\n> On Fri, May 02, 2025 at 09:08:46AM +0100, Kieran Bingham wrote:\n> > Quoting Stefan Klug (2025-04-03 16:49:15)\n> > > In commit b60bd37b1a49 (\"ipa: rkisp1: Move calculation of RGB means into\n> > > own function\") the output of the current measured colour temperature as\n> > > metadata was incorrectly added. Remove it.\n> > > \n> > \n> > Ooops, it even says \"Commit doesn't contain any functional changes.\" But\n> > it did!\n> > \n> > Is the colour temperature set elsewhere now?\n> \n> It's set earlier in the process() function, to\n> frameContext.awb.temperatureK instead of activeState.awb.temperatureK.\n> \n> There was a discussion on whether or not we should add a metadata\n> control to report the colour temperature estimated for the frame, in\n> addition to the one that was used to process the frame. Using\n> activeState.awb.temperatureK switches to the latter. Regardless of the\n> answer to that question, the aforementioned commit introduced a\n> functional change, so I think this fix is right.\n> \n> > > Fixes: b60bd37b1a49 (\"ipa: rkisp1: Move calculation of RGB means into own function\")\n> > > Signed-off-by: Stefan Klug \n> > > \n> > \n> > Reviewed-by: Kieran Bingham \n> \n> Reviewed-by: Laurent Pinchart \n\n\nReviewed-by: Paul Elder \n\n> \n> > > ---\n> > > \n> > > Changes in v3:\n> > > - Added this patch\n> > > ---\n> > > src/ipa/rkisp1/algorithms/awb.cpp | 3 ---\n> > > 1 file changed, 3 deletions(-)\n> > > \n> > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > index a9759e53f593..79c4c658406d 100644\n> > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > @@ -311,9 +311,6 @@ void Awb::process(IPAContext &context,\n> > > \n> > > activeState.awb.temperatureK = awbResult.colourTemperature;\n> > > \n> > > - /* Metadata shall contain the up to date measurement */\n> > > - metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n> > > -\n> > > /*\n> > > * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> > > * unsigned integer values. Set the minimum just above zero to avoid\n> \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 0BEC2C3226\n\tfor ;\n\tWed, 7 May 2025 16:54:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 77EB368B30;\n\tWed, 7 May 2025 18:54:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 300EE68B2F\n\tfor ;\n\tWed, 7 May 2025 18:54:07 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2001:861:3a80:3300:4f2f:8c2c:b3ef:17d4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7F48F6D5;\n\tWed, 7 May 2025 18:53:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"GBcWZEfC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746636835;\n\tbh=7k++dhk5JZ0Z5bcdK9KPXneObfH56GW3ADws5w/VggM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GBcWZEfCkK1XlYpnY9QHZjQ28Ee4t+I/WdcJiJj9lz0ScdPeG/W+mrqgl77ZALJTa\n\tMhqTnugyfMWMKEC0kn/jNgk6A7AhJwXH7LVOMaKjneo/FVOrsAU/AqWOvUK12FOqm5\n\t1cXLqGWOWoMQWx03ix3Yblmt3AK6vlGuaAJLJxNw=","Date":"Wed, 7 May 2025 18:54:02 +0200","From":"Paul Elder ","To":"Laurent Pinchart ","Cc":"Kieran Bingham ,\n\tStefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 10/16] ipa: rkisp1: algorithms: awb: Fix wrong colour\n\ttemperature reporting","Message-ID":"","References":"<20250403154925.382973-1-stefan.klug@ideasonboard.com>\n\t<20250403154925.382973-11-stefan.klug@ideasonboard.com>\n\t<174617332642.1586992.13042286476868819624@ping.linuxembedded.co.uk>\n\t<20250502210154.GB24278@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250502210154.GB24278@pendragon.ideasonboard.com>","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\" "}}]