[{"id":33417,"web_url":"https://patchwork.libcamera.org/comment/33417/","msgid":"<20250223222709.GB16159@pendragon.ideasonboard.com>","date":"2025-02-23T22:27:09","subject":"Re: [PATCH v2 07/17] ipa: rkisp1: Use grey world algorithm from\n\tlibipa","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stefan,\n\nOn Thu, Jan 23, 2025 at 12:40:57PM +0100, Stefan Klug wrote:\n> Now that libipa contains a grey world algorithm, use that.\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Paul Elder \n> Reviewed-by: Daniel Scally \n> \n> ---\n> \n> Changes in v2:\n> - Collected tags\n> - Replaced r variable with awbResult\n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 73 ++++++++++++++++++++-----------\n> src/ipa/rkisp1/algorithms/awb.h | 4 +-\n> 2 files changed, 50 insertions(+), 27 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index f6449565834b..b21d0d4c03bb 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -16,6 +16,7 @@\n> \n> #include \n> \n> +#include \"libipa/awb_grey.h\"\n> #include \"libipa/colours.h\"\n> \n> /**\n> @@ -40,6 +41,28 @@ constexpr int32_t kDefaultColourTemperature = 5000;\n> /* Minimum mean value below which AWB can't operate. */\n> constexpr double kMeanMinThreshold = 2.0;\n> \n> +class RkISP1AwbStats : public AwbStats\n> +{\n> +public:\n> +\tRkISP1AwbStats(const RGB &rgbMeans)\n> +\t\t: rgbMeans_(rgbMeans) {}\n> +\n> +\tdouble computeColourError([[maybe_unused]] const RGB &gains) const override\n> +\t{\n> +\t\tLOG(RkISP1Awb, Error)\n> +\t\t\t<< \"RkISP1AwbStats::computeColourError is not implemented\";\n> +\t\treturn 0.0;\n> +\t}\n> +\n> +\tRGB getRGBMeans() const override\n> +\t{\n> +\t\treturn rgbMeans_;\n> +\t};\n> +\n> +private:\n> +\tRGB rgbMeans_;\n> +};\n> +\n> Awb::Awb()\n> \t: rgbMode_(false)\n> {\n> @@ -55,15 +78,12 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> \t\t\t\t\t\t\t kMaxColourTemperature,\n> \t\t\t\t\t\t\t kDefaultColourTemperature);\n> \n> -\tInterpolator> gainCurve;\n> -\tint ret = gainCurve.readYaml(tuningData[\"colourGains\"], \"ct\", \"gains\");\n> -\tif (ret < 0)\n> -\t\tLOG(RkISP1Awb, Warning)\n> -\t\t\t<< \"Failed to parse 'colourGains' \"\n> -\t\t\t<< \"parameter from tuning file; \"\n> -\t\t\t<< \"manual colour temperature will not work properly\";\n> -\telse\n> -\t\tcolourGainCurve_ = gainCurve;\n> +\tawbAlgo_ = std::make_unique();\n> +\tint ret = awbAlgo_->init(tuningData);\n> +\tif (ret) {\n> +\t\tLOG(RkISP1Awb, Error) << \"Failed to init awb algorithm\";\n> +\t\treturn ret;\n> +\t}\n> \n> \treturn 0;\n> }\n> @@ -128,10 +148,10 @@ void Awb::queueRequest(IPAContext &context,\n> \t\t * This will be fixed with the bayes AWB algorithm.\n> \t\t */\n> \t\tupdate = true;\n> -\t} else if (colourTemperature && colourGainCurve_) {\n> -\t\tconst auto &gains = colourGainCurve_->getInterpolated(*colourTemperature);\n> -\t\tawb.gains.manual.r() = gains[0];\n> -\t\tawb.gains.manual.b() = gains[1];\n> +\t} else if (colourTemperature) {\n> +\t\tconst auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> +\t\tawb.gains.manual.r() = gains.r();\n> +\t\tawb.gains.manual.b() = gains.b();\n\nThis is a change in behaviour. Previously, if the tuning file didn't\ncontain a colour curve, we would just ignore the manual colour\ntemperature. With this patch, the gains will all be set to 1.0. Is that\nreally desired ?\n\n> \t\tawb.temperatureK = *colourTemperature;\n> \t\tupdate = true;\n> \t}\n> @@ -251,33 +271,34 @@ void Awb::process(IPAContext &context,\n> \t rgbMeans.b() < kMeanMinThreshold)\n> \t\treturn;\n> \n> -\tactiveState.awb.temperatureK = estimateCCT(rgbMeans);\n> +\t/*\n> +\t * \\Todo: Hardcode lux to a fixed value, until an estimation is\n> +\t * implemented.\n> +\t */\n> +\tint lux = 1000;\n> +\n> +\tRkISP1AwbStats awbStats{ rgbMeans };\n> +\tAwbResult awbResult = awbAlgo_->calculateAwb(awbStats, lux);\n> +\n> +\tactiveState.awb.temperatureK = awbResult.colourTemperature;\n> \n> \t/* Metadata shall contain the up to date measurement */\n> \tmetadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n> \n> -\t/*\n> -\t * Estimate the red and blue gains to apply in a grey world. The green\n> -\t * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the\n> -\t * divisor to a minimum value of 1.0.\n> -\t */\n> -\tRGB gains({ rgbMeans.g() / std::max(rgbMeans.r(), 1.0),\n> -\t\t\t 1.0,\n> -\t\t\t rgbMeans.g() / std::max(rgbMeans.b(), 1.0) });\n> -\n> \t/*\n> \t * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> \t * unsigned integer values. Set the minimum just above zero to avoid\n> \t * divisions by zero when computing the raw means in subsequent\n> \t * iterations.\n> \t */\n> -\tgains = gains.max(1.0 / 256).min(1023.0 / 256);\n> +\tawbResult.gains = awbResult.gains.max(1.0 / 256).min(1023.0 / 256);\n> \n> \t/* Filter the values to avoid oscillations. */\n> \tdouble speed = 0.2;\n> -\tgains = gains * speed + activeState.awb.gains.automatic * (1 - speed);\n> +\tawbResult.gains = awbResult.gains * speed +\n> +\t\t\t activeState.awb.gains.automatic * (1 - speed);\n> \n> -\tactiveState.awb.gains.automatic = gains;\n> +\tactiveState.awb.gains.automatic = awbResult.gains;\n> \n> \tLOG(RkISP1Awb, Debug)\n> \t\t<< std::showpoint\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index 2a64cb60d5f4..3382b2178567 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -9,6 +9,7 @@\n> \n> #include \n> \n> +#include \"libipa/awb.h\"\n> #include \"libipa/interpolator.h\"\n> #include \"libipa/vector.h\"\n> \n> @@ -41,7 +42,8 @@ private:\n> \tRGB calculateRgbMeans(const IPAFrameContext &frameContext,\n> \t\t\t\t const rkisp1_cif_isp_awb_stat *awb) const;\n> \n> -\tstd::optional>> colourGainCurve_;\n> +\tstd::unique_ptr awbAlgo_;\n> +\n> \tbool rgbMode_;\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 2717ABDB13\n\tfor ;\n\tSun, 23 Feb 2025 22:27:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40522686B6;\n\tSun, 23 Feb 2025 23:27:29 +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 ED966686A7\n\tfor ;\n\tSun, 23 Feb 2025 23:27:26 +0100 (CET)","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 C1163496;\n\tSun, 23 Feb 2025 23:26:00 +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=\"OGFzPfuN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740349561;\n\tbh=AxaKSu2PyF8FoaR0TKE5qLARG5tYCt78AskuUP9RdYA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OGFzPfuNJhDfkOwG2GRJfqMd0b8jKrffFIhYLd9+7UEicpBYScYJMROXjeMwdPWN9\n\tH3Mgf6cT9W5gbwf+JY9MqxuE3HE4nDQhvONYZHQwUSOFwpCWV+yC3yvnxgyy+cfNgj\n\tbAxJotsUZJeg137gqa4tjKHiaVagb43D61aW+JDA=","Date":"Mon, 24 Feb 2025 00:27:09 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder ,\n\tDaniel Scally ","Subject":"Re: [PATCH v2 07/17] ipa: rkisp1: Use grey world algorithm from\n\tlibipa","Message-ID":"<20250223222709.GB16159@pendragon.ideasonboard.com>","References":"<20250123114204.79321-1-stefan.klug@ideasonboard.com>\n\t<20250123114204.79321-8-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250123114204.79321-8-stefan.klug@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\" "}}]