[{"id":22190,"web_url":"https://patchwork.libcamera.org/comment/22190/","msgid":"","date":"2022-02-24T07:13:08","subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: rkisp1: Introduce AWB","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Wed, Feb 23, 2022 at 11:48:24AM +0100, Jean-Michel Hautbois wrote:\n> The RkISP1 ISP calculates a mean value for Y, Cr and Cb at each frame.\n> There is a RGB mode which could theoretically give us the values for R,\n> G and B directly, but it seems to be failing right now.\n> \n> Convert those values into R, G and B and estimate the gain to apply in a\n> grey world.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Tested-by: Peter Griffin \n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 155 ++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/awb.h | 33 ++++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> src/ipa/rkisp1/ipa_context.cpp | 28 +++++\n> src/ipa/rkisp1/ipa_context.h | 16 +++\n> src/ipa/rkisp1/rkisp1.cpp | 2 +\n> 6 files changed, 235 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/awb.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/awb.h\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> new file mode 100644\n> index 00000000..8fa8c2f7\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -0,0 +1,155 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n\n2021-2022\n\nTime flies :-)\n\n> + *\n> + * awb.cpp - AWB control algorithm\n> + */\n> +\n> +#include \"awb.h\"\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +/**\n> + * \\file awb.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class Awb\n> + * \\brief A Grey world white balance correction algorithm\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Awb)\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::configure\n> + */\n> +int Awb::configure(IPAContext &context,\n> +\t\t const IPACameraSensorInfo &configInfo)\n> +{\n> +\tcontext.frameContext.awb.gains.red = 1.0;\n> +\tcontext.frameContext.awb.gains.blue = 1.0;\n> +\tcontext.frameContext.awb.gains.green = 1.0;\n> +\n> +\t/* Define the measurement window for AGC. */\n\nSame comment as for patch 3/4.\n\n> +\tcontext.configuration.awb.measureWindow.h_offs = configInfo.outputSize.width / 8;\n> +\tcontext.configuration.awb.measureWindow.v_offs = configInfo.outputSize.height / 8;\n> +\tcontext.configuration.awb.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;\n> +\tcontext.configuration.awb.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n> +\n> +\treturn 0;\n> +}\n> +\n> +uint32_t Awb::estimateCCT(double red, double green, double blue)\n> +{\n> +\t/* Convert the RGB values to CIE tristimulus values (XYZ) */\n> +\tdouble X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);\n> +\tdouble Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);\n> +\tdouble Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);\n> +\n> +\t/* Calculate the normalized chromaticity values */\n> +\tdouble x = X / (X + Y + Z);\n> +\tdouble y = Y / (X + Y + Z);\n> +\n> +\t/* Calculate CCT */\n> +\tdouble n = (x - 0.3320) / (0.1858 - y);\n> +\treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::process\n> + */\n> +void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer *stats)\n\nPlease move this after prepare() to match the order of the .h file.\n\n> +{\n> +\tconst rkisp1_cif_isp_stat *params = &stats->params;\n> +\tconst rkisp1_cif_isp_awb_stat *awb = ¶ms->awb;\n> +\n> +\t/* Get the YCbCr mean values */\n> +\tdouble yMean = awb->awb_mean[0].mean_y_or_g;\n> +\tdouble crMean = awb->awb_mean[0].mean_cr_or_r;\n> +\tdouble cbMean = awb->awb_mean[0].mean_cb_or_b;\n> +\n> +\t/* Convert from YCbCr to RGB. */\n> +\tdouble redMean = yMean + 1.402 * (crMean - 128);\n> +\tdouble blueMean = yMean + 1.772 * (cbMean - 128);\n> +\tdouble greenMean = yMean - 0.34414 * (cbMean - 128) - 0.71414 * (crMean - 128);\n\nWhere do those values come from ? Unless I'm mistaken, the hardware uses\nthe following formulas to convert from RGB to YCbCr.\n\nY = 16 + 0.2500 R + 0.5000 G + 0.1094 B\nCb = 128 - 0.1406 R - 0.2969 G + 0.4375 B\nCr = 128 + 0.4375 R - 0.3750 G - 0.0625 B\n\n> +\n> +\t/* Estimate the red and blue gains to apply in a grey world. */\n> +\tdouble redGain = greenMean / (redMean + 1);\n> +\tdouble blueGain = greenMean / (blueMean + 1);\n> +\n> +\t/* Filter the values to avoid oscillations. */\n> +\tIPAFrameContext &frameContext = context.frameContext;\n> +\n> +\tframeContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> +\tframeContext.awb.gains.red = 0.2 * redGain +\n> +\t\t\t\t 0.8 * frameContext.awb.gains.red;\n> +\tframeContext.awb.gains.blue = 0.2 * blueGain +\n> +\t\t\t\t 0.8 * frameContext.awb.gains.blue;\n\nDon't you need to saturate the gains, either here, or in prepare() ?\n\n> +\t/* Hardcode the green gain to 1.0. */\n> +\tframeContext.awb.gains.green = 1.0;\n> +\n> +\tLOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.frameContext.awb.gains.red\n> +\t\t\t << \" and for blue: \" << context.frameContext.awb.gains.blue;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Awb::prepare(IPAContext &context,\n> +\t\t rkisp1_params_cfg *params)\n\nThis holds on a single line.\n\n> +{\n> +\tparams->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green;\n> +\tparams->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue;\n> +\tparams->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red;\n> +\tparams->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green;\n> +\n> +\tif (context.frameContext.frameId == 0) {\n> +\t\t/* Configure the gains to apply. */\n> +\t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n> +\t\t/* Update the ISP to apply the gains configured. */\n> +\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n> +\t}\n> +\t/* Update the gains. */\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n\nI'd move this line just after setting the gains above.\n\n> +\n> +\t/* Configure the measure window for AWB. */\n> +\tparams->meas.awb_meas_config.awb_wnd = context.configuration.awb.measureWindow;\n> +\t/*\n> +\t * Measure Y, Cr and Cb means.\n> +\t * \\todo RGB is not working, the kernel seems to not configure it ?\n\nDoes this mean that if RGB worked, you would prefer using that ?\n\n> +\t */\n> +\tparams->meas.awb_meas_config.awb_mode = RKISP1_CIF_ISP_AWB_MODE_YCBCR;\n> +\t/* Reference Cr and Cb. */\n> +\tparams->meas.awb_meas_config.awb_ref_cb = 128;\n> +\tparams->meas.awb_meas_config.awb_ref_cr = 128;\n> +\t/* Y values to include are between min_y and max_y only. */\n> +\tparams->meas.awb_meas_config.min_y = 16;\n> +\tparams->meas.awb_meas_config.max_y = 250;\n> +\t/* Maximum Cr+Cb value to take into account for awb. */\n> +\tparams->meas.awb_meas_config.max_csum = 250;\n> +\t/* Minimum Cr and Cb values to take into account. */\n> +\tparams->meas.awb_meas_config.min_c = 16;\n> +\t/* Number of frames to use to estimate the mean (0 means 1 frame). */\n> +\tparams->meas.awb_meas_config.frames = 0;\n\nAll this can move inside the if () below.\n\n> +\n> +\tif (context.frameContext.frameId == 0) {\n> +\t\t/* Update AWB measurement unit configuration. */\n> +\t\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB;\n> +\t\t/* Make sure the ISP is measuring the means for the next frame. */\n> +\t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AWB;\n> +\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB;\n> +\t}\n> +}\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> new file mode 100644\n> index 00000000..0a9fb82c\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -0,0 +1,33 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n\n2021-2022\n\n> + *\n> + * awb.h - AWB control algorithm\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class Awb : public Algorithm\n> +{\n> +public:\n> +\tAwb() = default;\n> +\t~Awb() = default;\n> +\n> +\tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> +\tvoid process(IPAContext &context, const rkisp1_stat_buffer *stats) override;\n> +\n> +private:\n> +\tuint32_t estimateCCT(double red, double green, double blue);\n> +};\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> index 27c97731..7ec53d89 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -2,5 +2,6 @@\n> \n> rkisp1_ipa_algorithms = files([\n> 'agc.cpp',\n> + 'awb.cpp',\n> 'blc.cpp',\n> ])\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 39acef47..c25f44ec 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -81,6 +81,14 @@ namespace libcamera::ipa::rkisp1 {\n> * \\brief Hardware revision of the ISP\n> */\n> \n> +/**\n> + * \\var IPASessionConfiguration::awb\n> + * \\brief AWB parameters configuration of the IPA\n> + *\n> + * \\var IPASessionConfiguration::awb.measureWindow\n> + * \\brief AWB measure window\n> + */\n> +\n> /**\n> * \\var IPASessionConfiguration::sensor\n> * \\brief Sensor-specific configuration of the IPA\n> @@ -105,6 +113,26 @@ namespace libcamera::ipa::rkisp1 {\n> * The gain should be adapted to the sensor specific gain code before applying.\n> */\n> \n> +/**\n> + * \\var IPAFrameContext::awb\n> + * \\brief Context for the Automatic White Balance algorithm\n> + *\n> + * \\struct IPAFrameContext::awb.gains\n> + * \\brief White balance gains\n> + *\n> + * \\var IPAFrameContext::awb.gains.red\n> + * \\brief White balance gain for R channel\n> + *\n> + * \\var IPAFrameContext::awb.gains.green\n> + * \\brief White balance gain for G channel\n> + *\n> + * \\var IPAFrameContext::awb.gains.blue\n> + * \\brief White balance gain for B channel\n> + *\n> + * \\var IPAFrameContext::awb.temperatureK\n> + * \\brief Estimated color temperature\n> + */\n> +\n> /**\n> * \\var IPAFrameContext::sensor\n> * \\brief Effective sensor values\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 9ac3b40c..51eae8b6 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -12,6 +12,8 @@\n> \n> #include \n> \n> +#include \n> +\n> namespace libcamera {\n> \n> namespace ipa::rkisp1 {\n> @@ -25,6 +27,10 @@ struct IPASessionConfiguration {\n> \t\tstruct rkisp1_cif_isp_window measureWindow;\n> \t} agc;\n> \n> +\tstruct {\n> +\t\tstruct rkisp1_cif_isp_window measureWindow;\n> +\t} awb;\n> +\n> \tstruct {\n> \t\tutils::Duration lineDuration;\n> \t} sensor;\n> @@ -40,6 +46,16 @@ struct IPAFrameContext {\n> \t\tdouble gain;\n> \t} agc;\n> \n> +\tstruct {\n> +\t\tstruct {\n> +\t\t\tdouble red;\n> +\t\t\tdouble green;\n> +\t\t\tdouble blue;\n> +\t\t} gains;\n> +\n> +\t\tdouble temperatureK;\n> +\t} awb;\n> +\n> \tstruct {\n> \t\tuint32_t exposure;\n> \t\tdouble gain;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index f82b7cb3..381fc0e0 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -27,6 +27,7 @@\n> \n> #include \"algorithms/agc.h\"\n> #include \"algorithms/algorithm.h\"\n> +#include \"algorithms/awb.h\"\n> #include \"algorithms/blc.h\"\n> #include \"libipa/camera_sensor_helper.h\"\n> \n> @@ -126,6 +127,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n> \n> \t/* Construct our Algorithms */\n> \talgorithms_.push_back(std::make_unique());\n> +\talgorithms_.push_back(std::make_unique());\n> \talgorithms_.push_back(std::make_unique());\n> \n> \treturn 0;","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 CB4BABE08A\n\tfor ;\n\tThu, 24 Feb 2022 07:13:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1427A61154;\n\tThu, 24 Feb 2022 08:13:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 77C4B601F2\n\tfor ;\n\tThu, 24 Feb 2022 08:13:19 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A151DDD;\n\tThu, 24 Feb 2022 08:13:18 +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=\"ToM8o31G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1645686798;\n\tbh=06xeJTRxp434vcHXmmho9wk7d67JvNLSgLQl6O2tweg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ToM8o31GiAG5u8rUZSqq3usEQkX3ipe8QwyogAbTbQGBO1FwR+d7wcCktnv+xqWa6\n\txa+Rj3uyOy0PwbUBLgRS5VkfcMWXdD8DREWz3W49dHyETMeI0e0B+a53aYHxq50Qji\n\ttwR7pDpWmWSApX+gmhZR76P/c1gsfOcYKqOPVB90=","Date":"Thu, 24 Feb 2022 09:13:08 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20220223104824.25723-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220223104824.25723-5-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220223104824.25723-5-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: rkisp1: Introduce AWB","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":22191,"web_url":"https://patchwork.libcamera.org/comment/22191/","msgid":"<37112622-791c-90a9-9a70-4a95e153864d@ideasonboard.com>","date":"2022-02-24T08:11:14","subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: rkisp1: Introduce AWB","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 24/02/2022 08:13, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> Thank you for the patch.\n> \n> On Wed, Feb 23, 2022 at 11:48:24AM +0100, Jean-Michel Hautbois wrote:\n>> The RkISP1 ISP calculates a mean value for Y, Cr and Cb at each frame.\n>> There is a RGB mode which could theoretically give us the values for R,\n>> G and B directly, but it seems to be failing right now.\n>>\n>> Convert those values into R, G and B and estimate the gain to apply in a\n>> grey world.\n>>\n>> Signed-off-by: Jean-Michel Hautbois \n>> Tested-by: Peter Griffin \n>> ---\n>> src/ipa/rkisp1/algorithms/awb.cpp | 155 ++++++++++++++++++++++++++\n>> src/ipa/rkisp1/algorithms/awb.h | 33 ++++++\n>> src/ipa/rkisp1/algorithms/meson.build | 1 +\n>> src/ipa/rkisp1/ipa_context.cpp | 28 +++++\n>> src/ipa/rkisp1/ipa_context.h | 16 +++\n>> src/ipa/rkisp1/rkisp1.cpp | 2 +\n>> 6 files changed, 235 insertions(+)\n>> create mode 100644 src/ipa/rkisp1/algorithms/awb.cpp\n>> create mode 100644 src/ipa/rkisp1/algorithms/awb.h\n>>\n>> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n>> new file mode 100644\n>> index 00000000..8fa8c2f7\n>> --- /dev/null\n>> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n>> @@ -0,0 +1,155 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Ideas On Board\n> \n> 2021-2022\n> \n> Time flies :-)\n\nThanks, I will apply it on all the other files too then ;-).\n\n> \n>> + *\n>> + * awb.cpp - AWB control algorithm\n>> + */\n>> +\n>> +#include \"awb.h\"\n>> +\n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +/**\n>> + * \\file awb.h\n>> + */\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::rkisp1::algorithms {\n>> +\n>> +/**\n>> + * \\class Awb\n>> + * \\brief A Grey world white balance correction algorithm\n>> + */\n>> +\n>> +LOG_DEFINE_CATEGORY(RkISP1Awb)\n>> +\n>> +/**\n>> + * \\copydoc libcamera::ipa::Algorithm::configure\n>> + */\n>> +int Awb::configure(IPAContext &context,\n>> +\t\t const IPACameraSensorInfo &configInfo)\n>> +{\n>> +\tcontext.frameContext.awb.gains.red = 1.0;\n>> +\tcontext.frameContext.awb.gains.blue = 1.0;\n>> +\tcontext.frameContext.awb.gains.green = 1.0;\n>> +\n>> +\t/* Define the measurement window for AGC. */\n> \n> Same comment as for patch 3/4.\n> \n>> +\tcontext.configuration.awb.measureWindow.h_offs = configInfo.outputSize.width / 8;\n>> +\tcontext.configuration.awb.measureWindow.v_offs = configInfo.outputSize.height / 8;\n>> +\tcontext.configuration.awb.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;\n>> +\tcontext.configuration.awb.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +uint32_t Awb::estimateCCT(double red, double green, double blue)\n>> +{\n>> +\t/* Convert the RGB values to CIE tristimulus values (XYZ) */\n>> +\tdouble X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);\n>> +\tdouble Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);\n>> +\tdouble Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);\n>> +\n>> +\t/* Calculate the normalized chromaticity values */\n>> +\tdouble x = X / (X + Y + Z);\n>> +\tdouble y = Y / (X + Y + Z);\n>> +\n>> +\t/* Calculate CCT */\n>> +\tdouble n = (x - 0.3320) / (0.1858 - y);\n>> +\treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n>> +}\n>> +\n>> +/**\n>> + * \\copydoc libcamera::ipa::Algorithm::process\n>> + */\n>> +void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer *stats)\n> \n> Please move this after prepare() to match the order of the .h file.\n> \n>> +{\n>> +\tconst rkisp1_cif_isp_stat *params = &stats->params;\n>> +\tconst rkisp1_cif_isp_awb_stat *awb = ¶ms->awb;\n>> +\n>> +\t/* Get the YCbCr mean values */\n>> +\tdouble yMean = awb->awb_mean[0].mean_y_or_g;\n>> +\tdouble crMean = awb->awb_mean[0].mean_cr_or_r;\n>> +\tdouble cbMean = awb->awb_mean[0].mean_cb_or_b;\n>> +\n>> +\t/* Convert from YCbCr to RGB. */\n>> +\tdouble redMean = yMean + 1.402 * (crMean - 128);\n>> +\tdouble blueMean = yMean + 1.772 * (cbMean - 128);\n>> +\tdouble greenMean = yMean - 0.34414 * (cbMean - 128) - 0.71414 * (crMean - 128);\n> \n> Where do those values come from ? Unless I'm mistaken, the hardware uses\n> the following formulas to convert from RGB to YCbCr.\n> \n> Y = 16 + 0.2500 R + 0.5000 G + 0.1094 B\n> Cb = 128 - 0.1406 R - 0.2969 G + 0.4375 B\n> Cr = 128 + 0.4375 R - 0.3750 G - 0.0625 B\n> \n\nI took the JPEG File Interchange Format conversion formula, and not the \ninverse matrix of the one the HW is using indeed.\nI will need to calculate it then (keeping in mind that I convert from \nYCrCb because the RGB mode is not working -yet- )...\n\n>> +\n>> +\t/* Estimate the red and blue gains to apply in a grey world. */\n>> +\tdouble redGain = greenMean / (redMean + 1);\n>> +\tdouble blueGain = greenMean / (blueMean + 1);\n>> +\n>> +\t/* Filter the values to avoid oscillations. */\n>> +\tIPAFrameContext &frameContext = context.frameContext;\n>> +\n>> +\tframeContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n>> +\tframeContext.awb.gains.red = 0.2 * redGain +\n>> +\t\t\t\t 0.8 * frameContext.awb.gains.red;\n>> +\tframeContext.awb.gains.blue = 0.2 * blueGain +\n>> +\t\t\t\t 0.8 * frameContext.awb.gains.blue;\n> \n> Don't you need to saturate the gains, either here, or in prepare() ?\n> \n>> +\t/* Hardcode the green gain to 1.0. */\n>> +\tframeContext.awb.gains.green = 1.0;\n>> +\n>> +\tLOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.frameContext.awb.gains.red\n>> +\t\t\t << \" and for blue: \" << context.frameContext.awb.gains.blue;\n>> +}\n>> +\n>> +/**\n>> + * \\copydoc libcamera::ipa::Algorithm::prepare\n>> + */\n>> +void Awb::prepare(IPAContext &context,\n>> +\t\t rkisp1_params_cfg *params)\n> \n> This holds on a single line.\n> \n>> +{\n>> +\tparams->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green;\n>> +\tparams->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue;\n>> +\tparams->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red;\n>> +\tparams->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green;\n>> +\n>> +\tif (context.frameContext.frameId == 0) {\n>> +\t\t/* Configure the gains to apply. */\n>> +\t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n>> +\t\t/* Update the ISP to apply the gains configured. */\n>> +\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n>> +\t}\n>> +\t/* Update the gains. */\n>> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n> \n> I'd move this line just after setting the gains above.\n> \n>> +\n>> +\t/* Configure the measure window for AWB. */\n>> +\tparams->meas.awb_meas_config.awb_wnd = context.configuration.awb.measureWindow;\n>> +\t/*\n>> +\t * Measure Y, Cr and Cb means.\n>> +\t * \\todo RGB is not working, the kernel seems to not configure it ?\n> \n> Does this mean that if RGB worked, you would prefer using that ?\n\nYes, as it would remove the need for the YCrCb to RGB conversion.\n\n> \n>> +\t */\n>> +\tparams->meas.awb_meas_config.awb_mode = RKISP1_CIF_ISP_AWB_MODE_YCBCR;\n>> +\t/* Reference Cr and Cb. */\n>> +\tparams->meas.awb_meas_config.awb_ref_cb = 128;\n>> +\tparams->meas.awb_meas_config.awb_ref_cr = 128;\n>> +\t/* Y values to include are between min_y and max_y only. */\n>> +\tparams->meas.awb_meas_config.min_y = 16;\n>> +\tparams->meas.awb_meas_config.max_y = 250;\n>> +\t/* Maximum Cr+Cb value to take into account for awb. */\n>> +\tparams->meas.awb_meas_config.max_csum = 250;\n>> +\t/* Minimum Cr and Cb values to take into account. */\n>> +\tparams->meas.awb_meas_config.min_c = 16;\n>> +\t/* Number of frames to use to estimate the mean (0 means 1 frame). */\n>> +\tparams->meas.awb_meas_config.frames = 0;\n> \n> All this can move inside the if () below.\n> \n>> +\n>> +\tif (context.frameContext.frameId == 0) {\n>> +\t\t/* Update AWB measurement unit configuration. */\n>> +\t\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB;\n>> +\t\t/* Make sure the ISP is measuring the means for the next frame. */\n>> +\t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AWB;\n>> +\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB;\n>> +\t}\n>> +}\n>> +\n>> +} /* namespace ipa::rkisp1::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n>> new file mode 100644\n>> index 00000000..0a9fb82c\n>> --- /dev/null\n>> +++ b/src/ipa/rkisp1/algorithms/awb.h\n>> @@ -0,0 +1,33 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Ideas On Board\n> \n> 2021-2022\n> \n>> + *\n>> + * awb.h - AWB control algorithm\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::rkisp1::algorithms {\n>> +\n>> +class Awb : public Algorithm\n>> +{\n>> +public:\n>> +\tAwb() = default;\n>> +\t~Awb() = default;\n>> +\n>> +\tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n>> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n>> +\tvoid process(IPAContext &context, const rkisp1_stat_buffer *stats) override;\n>> +\n>> +private:\n>> +\tuint32_t estimateCCT(double red, double green, double blue);\n>> +};\n>> +\n>> +} /* namespace ipa::rkisp1::algorithms */\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n>> index 27c97731..7ec53d89 100644\n>> --- a/src/ipa/rkisp1/algorithms/meson.build\n>> +++ b/src/ipa/rkisp1/algorithms/meson.build\n>> @@ -2,5 +2,6 @@\n>> \n>> rkisp1_ipa_algorithms = files([\n>> 'agc.cpp',\n>> + 'awb.cpp',\n>> 'blc.cpp',\n>> ])\n>> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n>> index 39acef47..c25f44ec 100644\n>> --- a/src/ipa/rkisp1/ipa_context.cpp\n>> +++ b/src/ipa/rkisp1/ipa_context.cpp\n>> @@ -81,6 +81,14 @@ namespace libcamera::ipa::rkisp1 {\n>> * \\brief Hardware revision of the ISP\n>> */\n>> \n>> +/**\n>> + * \\var IPASessionConfiguration::awb\n>> + * \\brief AWB parameters configuration of the IPA\n>> + *\n>> + * \\var IPASessionConfiguration::awb.measureWindow\n>> + * \\brief AWB measure window\n>> + */\n>> +\n>> /**\n>> * \\var IPASessionConfiguration::sensor\n>> * \\brief Sensor-specific configuration of the IPA\n>> @@ -105,6 +113,26 @@ namespace libcamera::ipa::rkisp1 {\n>> * The gain should be adapted to the sensor specific gain code before applying.\n>> */\n>> \n>> +/**\n>> + * \\var IPAFrameContext::awb\n>> + * \\brief Context for the Automatic White Balance algorithm\n>> + *\n>> + * \\struct IPAFrameContext::awb.gains\n>> + * \\brief White balance gains\n>> + *\n>> + * \\var IPAFrameContext::awb.gains.red\n>> + * \\brief White balance gain for R channel\n>> + *\n>> + * \\var IPAFrameContext::awb.gains.green\n>> + * \\brief White balance gain for G channel\n>> + *\n>> + * \\var IPAFrameContext::awb.gains.blue\n>> + * \\brief White balance gain for B channel\n>> + *\n>> + * \\var IPAFrameContext::awb.temperatureK\n>> + * \\brief Estimated color temperature\n>> + */\n>> +\n>> /**\n>> * \\var IPAFrameContext::sensor\n>> * \\brief Effective sensor values\n>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n>> index 9ac3b40c..51eae8b6 100644\n>> --- a/src/ipa/rkisp1/ipa_context.h\n>> +++ b/src/ipa/rkisp1/ipa_context.h\n>> @@ -12,6 +12,8 @@\n>> \n>> #include \n>> \n>> +#include \n>> +\n>> namespace libcamera {\n>> \n>> namespace ipa::rkisp1 {\n>> @@ -25,6 +27,10 @@ struct IPASessionConfiguration {\n>> \t\tstruct rkisp1_cif_isp_window measureWindow;\n>> \t} agc;\n>> \n>> +\tstruct {\n>> +\t\tstruct rkisp1_cif_isp_window measureWindow;\n>> +\t} awb;\n>> +\n>> \tstruct {\n>> \t\tutils::Duration lineDuration;\n>> \t} sensor;\n>> @@ -40,6 +46,16 @@ struct IPAFrameContext {\n>> \t\tdouble gain;\n>> \t} agc;\n>> \n>> +\tstruct {\n>> +\t\tstruct {\n>> +\t\t\tdouble red;\n>> +\t\t\tdouble green;\n>> +\t\t\tdouble blue;\n>> +\t\t} gains;\n>> +\n>> +\t\tdouble temperatureK;\n>> +\t} awb;\n>> +\n>> \tstruct {\n>> \t\tuint32_t exposure;\n>> \t\tdouble gain;\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index f82b7cb3..381fc0e0 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -27,6 +27,7 @@\n>> \n>> #include \"algorithms/agc.h\"\n>> #include \"algorithms/algorithm.h\"\n>> +#include \"algorithms/awb.h\"\n>> #include \"algorithms/blc.h\"\n>> #include \"libipa/camera_sensor_helper.h\"\n>> \n>> @@ -126,6 +127,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n>> \n>> \t/* Construct our Algorithms */\n>> \talgorithms_.push_back(std::make_unique());\n>> +\talgorithms_.push_back(std::make_unique());\n>> \talgorithms_.push_back(std::make_unique());\n>> \n>> \treturn 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 E5C3FBF415\n\tfor ;\n\tThu, 24 Feb 2022 08:11:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1A5F961145;\n\tThu, 24 Feb 2022 09:11:19 +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 9D408601F7\n\tfor ;\n\tThu, 24 Feb 2022 09:11:17 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:ce74:6df2:4b76:b230] (unknown\n\t[IPv6:2a01:e0a:169:7140:ce74:6df2:4b76:b230])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2B3D5DD;\n\tThu, 24 Feb 2022 09:11:17 +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=\"oncKrh3A\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1645690277;\n\tbh=ZlnBcKIqERlrwZef9SxifBNaWqK9d0ZGc6jfFUYk5Ec=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=oncKrh3AR5Ynd/WxJ2WpUKslaKIEydZUomdYCLnY/ZVbsw/P16HuBvql93tBRtNq2\n\tiritgG6hfHDTtYaf4NWWxFU0x9SmgE3kRZ1ShU5IqzODCpwnpWBG5Mjc5HAxsgXvEf\n\t8iESjACh9uWL/+NF8lPz1ljFZe6/jvfByyTYuaKs=","Message-ID":"<37112622-791c-90a9-9a70-4a95e153864d@ideasonboard.com>","Date":"Thu, 24 Feb 2022 09:11:14 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.5.0","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20220223104824.25723-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220223104824.25723-5-jeanmichel.hautbois@ideasonboard.com>\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 v2 4/4] ipa: rkisp1: Introduce AWB","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":22192,"web_url":"https://patchwork.libcamera.org/comment/22192/","msgid":"","date":"2022-02-24T09:06:55","subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: rkisp1: Introduce AWB","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Thu, Feb 24, 2022 at 09:11:14AM +0100, Jean-Michel Hautbois wrote:\n> On 24/02/2022 08:13, Laurent Pinchart wrote:\n> > On Wed, Feb 23, 2022 at 11:48:24AM +0100, Jean-Michel Hautbois wrote:\n> >> The RkISP1 ISP calculates a mean value for Y, Cr and Cb at each frame.\n> >> There is a RGB mode which could theoretically give us the values for R,\n> >> G and B directly, but it seems to be failing right now.\n> >>\n> >> Convert those values into R, G and B and estimate the gain to apply in a\n> >> grey world.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois \n> >> Tested-by: Peter Griffin \n> >> ---\n> >> src/ipa/rkisp1/algorithms/awb.cpp | 155 ++++++++++++++++++++++++++\n> >> src/ipa/rkisp1/algorithms/awb.h | 33 ++++++\n> >> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> >> src/ipa/rkisp1/ipa_context.cpp | 28 +++++\n> >> src/ipa/rkisp1/ipa_context.h | 16 +++\n> >> src/ipa/rkisp1/rkisp1.cpp | 2 +\n> >> 6 files changed, 235 insertions(+)\n> >> create mode 100644 src/ipa/rkisp1/algorithms/awb.cpp\n> >> create mode 100644 src/ipa/rkisp1/algorithms/awb.h\n> >>\n> >> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> >> new file mode 100644\n> >> index 00000000..8fa8c2f7\n> >> --- /dev/null\n> >> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> >> @@ -0,0 +1,155 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Ideas On Board\n> > \n> > 2021-2022\n> > \n> > Time flies :-)\n> \n> Thanks, I will apply it on all the other files too then ;-).\n> \n> >> + *\n> >> + * awb.cpp - AWB control algorithm\n> >> + */\n> >> +\n> >> +#include \"awb.h\"\n> >> +\n> >> +#include \n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +/**\n> >> + * \\file awb.h\n> >> + */\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +namespace ipa::rkisp1::algorithms {\n> >> +\n> >> +/**\n> >> + * \\class Awb\n> >> + * \\brief A Grey world white balance correction algorithm\n> >> + */\n> >> +\n> >> +LOG_DEFINE_CATEGORY(RkISP1Awb)\n> >> +\n> >> +/**\n> >> + * \\copydoc libcamera::ipa::Algorithm::configure\n> >> + */\n> >> +int Awb::configure(IPAContext &context,\n> >> +\t\t const IPACameraSensorInfo &configInfo)\n> >> +{\n> >> +\tcontext.frameContext.awb.gains.red = 1.0;\n> >> +\tcontext.frameContext.awb.gains.blue = 1.0;\n> >> +\tcontext.frameContext.awb.gains.green = 1.0;\n> >> +\n> >> +\t/* Define the measurement window for AGC. */\n> > \n> > Same comment as for patch 3/4.\n> > \n> >> +\tcontext.configuration.awb.measureWindow.h_offs = configInfo.outputSize.width / 8;\n> >> +\tcontext.configuration.awb.measureWindow.v_offs = configInfo.outputSize.height / 8;\n> >> +\tcontext.configuration.awb.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;\n> >> +\tcontext.configuration.awb.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +uint32_t Awb::estimateCCT(double red, double green, double blue)\n> >> +{\n> >> +\t/* Convert the RGB values to CIE tristimulus values (XYZ) */\n> >> +\tdouble X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);\n> >> +\tdouble Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);\n> >> +\tdouble Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);\n> >> +\n> >> +\t/* Calculate the normalized chromaticity values */\n> >> +\tdouble x = X / (X + Y + Z);\n> >> +\tdouble y = Y / (X + Y + Z);\n> >> +\n> >> +\t/* Calculate CCT */\n> >> +\tdouble n = (x - 0.3320) / (0.1858 - y);\n> >> +\treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\copydoc libcamera::ipa::Algorithm::process\n> >> + */\n> >> +void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer *stats)\n> > \n> > Please move this after prepare() to match the order of the .h file.\n> > \n> >> +{\n> >> +\tconst rkisp1_cif_isp_stat *params = &stats->params;\n> >> +\tconst rkisp1_cif_isp_awb_stat *awb = ¶ms->awb;\n> >> +\n> >> +\t/* Get the YCbCr mean values */\n> >> +\tdouble yMean = awb->awb_mean[0].mean_y_or_g;\n> >> +\tdouble crMean = awb->awb_mean[0].mean_cr_or_r;\n> >> +\tdouble cbMean = awb->awb_mean[0].mean_cb_or_b;\n> >> +\n> >> +\t/* Convert from YCbCr to RGB. */\n> >> +\tdouble redMean = yMean + 1.402 * (crMean - 128);\n> >> +\tdouble blueMean = yMean + 1.772 * (cbMean - 128);\n> >> +\tdouble greenMean = yMean - 0.34414 * (cbMean - 128) - 0.71414 * (crMean - 128);\n> > \n> > Where do those values come from ? Unless I'm mistaken, the hardware uses\n> > the following formulas to convert from RGB to YCbCr.\n> > \n> > Y = 16 + 0.2500 R + 0.5000 G + 0.1094 B\n> > Cb = 128 - 0.1406 R - 0.2969 G + 0.4375 B\n> > Cr = 128 + 0.4375 R - 0.3750 G - 0.0625 B\n> \n> I took the JPEG File Interchange Format conversion formula, and not the \n> inverse matrix of the one the HW is using indeed.\n> I will need to calculate it then (keeping in mind that I convert from \n> YCrCb because the RGB mode is not working -yet- )...\n\nnumpy to the rescue.\n\nimport numpy as np\nprint(np.linalg.inv(np.array([[0.2500, 0.5000, 0.1094],\n [-0.1406, -0.2969, 0.4375],\n [0.4375, -0.3750, -0.0625]])))\n\n[[ 1.16360251 -0.06228394 1.60078229]\n [ 1.16360251 -0.404527 -0.79491914]\n [ 1.16360251 1.99117443 -0.02500915]]\n\n> >> +\n> >> +\t/* Estimate the red and blue gains to apply in a grey world. */\n> >> +\tdouble redGain = greenMean / (redMean + 1);\n> >> +\tdouble blueGain = greenMean / (blueMean + 1);\n> >> +\n> >> +\t/* Filter the values to avoid oscillations. */\n> >> +\tIPAFrameContext &frameContext = context.frameContext;\n> >> +\n> >> +\tframeContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> >> +\tframeContext.awb.gains.red = 0.2 * redGain +\n> >> +\t\t\t\t 0.8 * frameContext.awb.gains.red;\n> >> +\tframeContext.awb.gains.blue = 0.2 * blueGain +\n> >> +\t\t\t\t 0.8 * frameContext.awb.gains.blue;\n> > \n> > Don't you need to saturate the gains, either here, or in prepare() ?\n> > \n> >> +\t/* Hardcode the green gain to 1.0. */\n> >> +\tframeContext.awb.gains.green = 1.0;\n> >> +\n> >> +\tLOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.frameContext.awb.gains.red\n> >> +\t\t\t << \" and for blue: \" << context.frameContext.awb.gains.blue;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> >> + */\n> >> +void Awb::prepare(IPAContext &context,\n> >> +\t\t rkisp1_params_cfg *params)\n> > \n> > This holds on a single line.\n> > \n> >> +{\n> >> +\tparams->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green;\n> >> +\tparams->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue;\n> >> +\tparams->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red;\n> >> +\tparams->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green;\n> >> +\n> >> +\tif (context.frameContext.frameId == 0) {\n> >> +\t\t/* Configure the gains to apply. */\n> >> +\t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n> >> +\t\t/* Update the ISP to apply the gains configured. */\n> >> +\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n> >> +\t}\n> >> +\t/* Update the gains. */\n> >> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n> > \n> > I'd move this line just after setting the gains above.\n> > \n> >> +\n> >> +\t/* Configure the measure window for AWB. */\n> >> +\tparams->meas.awb_meas_config.awb_wnd = context.configuration.awb.measureWindow;\n> >> +\t/*\n> >> +\t * Measure Y, Cr and Cb means.\n> >> +\t * \\todo RGB is not working, the kernel seems to not configure it ?\n> > \n> > Does this mean that if RGB worked, you would prefer using that ?\n> \n> Yes, as it would remove the need for the YCrCb to RGB conversion.\n> \n> >> +\t */\n> >> +\tparams->meas.awb_meas_config.awb_mode = RKISP1_CIF_ISP_AWB_MODE_YCBCR;\n> >> +\t/* Reference Cr and Cb. */\n> >> +\tparams->meas.awb_meas_config.awb_ref_cb = 128;\n> >> +\tparams->meas.awb_meas_config.awb_ref_cr = 128;\n> >> +\t/* Y values to include are between min_y and max_y only. */\n> >> +\tparams->meas.awb_meas_config.min_y = 16;\n> >> +\tparams->meas.awb_meas_config.max_y = 250;\n> >> +\t/* Maximum Cr+Cb value to take into account for awb. */\n> >> +\tparams->meas.awb_meas_config.max_csum = 250;\n> >> +\t/* Minimum Cr and Cb values to take into account. */\n> >> +\tparams->meas.awb_meas_config.min_c = 16;\n> >> +\t/* Number of frames to use to estimate the mean (0 means 1 frame). */\n> >> +\tparams->meas.awb_meas_config.frames = 0;\n> > \n> > All this can move inside the if () below.\n> > \n> >> +\n> >> +\tif (context.frameContext.frameId == 0) {\n> >> +\t\t/* Update AWB measurement unit configuration. */\n> >> +\t\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB;\n> >> +\t\t/* Make sure the ISP is measuring the means for the next frame. */\n> >> +\t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AWB;\n> >> +\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB;\n> >> +\t}\n> >> +}\n> >> +\n> >> +} /* namespace ipa::rkisp1::algorithms */\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> >> new file mode 100644\n> >> index 00000000..0a9fb82c\n> >> --- /dev/null\n> >> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> >> @@ -0,0 +1,33 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Ideas On Board\n> > \n> > 2021-2022\n> > \n> >> + *\n> >> + * awb.h - AWB control algorithm\n> >> + */\n> >> +\n> >> +#pragma once\n> >> +\n> >> +#include \n> >> +\n> >> +#include \"algorithm.h\"\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +namespace ipa::rkisp1::algorithms {\n> >> +\n> >> +class Awb : public Algorithm\n> >> +{\n> >> +public:\n> >> +\tAwb() = default;\n> >> +\t~Awb() = default;\n> >> +\n> >> +\tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> >> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> >> +\tvoid process(IPAContext &context, const rkisp1_stat_buffer *stats) override;\n> >> +\n> >> +private:\n> >> +\tuint32_t estimateCCT(double red, double green, double blue);\n> >> +};\n> >> +\n> >> +} /* namespace ipa::rkisp1::algorithms */\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> >> index 27c97731..7ec53d89 100644\n> >> --- a/src/ipa/rkisp1/algorithms/meson.build\n> >> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> >> @@ -2,5 +2,6 @@\n> >> \n> >> rkisp1_ipa_algorithms = files([\n> >> 'agc.cpp',\n> >> + 'awb.cpp',\n> >> 'blc.cpp',\n> >> ])\n> >> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> >> index 39acef47..c25f44ec 100644\n> >> --- a/src/ipa/rkisp1/ipa_context.cpp\n> >> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> >> @@ -81,6 +81,14 @@ namespace libcamera::ipa::rkisp1 {\n> >> * \\brief Hardware revision of the ISP\n> >> */\n> >> \n> >> +/**\n> >> + * \\var IPASessionConfiguration::awb\n> >> + * \\brief AWB parameters configuration of the IPA\n> >> + *\n> >> + * \\var IPASessionConfiguration::awb.measureWindow\n> >> + * \\brief AWB measure window\n> >> + */\n> >> +\n> >> /**\n> >> * \\var IPASessionConfiguration::sensor\n> >> * \\brief Sensor-specific configuration of the IPA\n> >> @@ -105,6 +113,26 @@ namespace libcamera::ipa::rkisp1 {\n> >> * The gain should be adapted to the sensor specific gain code before applying.\n> >> */\n> >> \n> >> +/**\n> >> + * \\var IPAFrameContext::awb\n> >> + * \\brief Context for the Automatic White Balance algorithm\n> >> + *\n> >> + * \\struct IPAFrameContext::awb.gains\n> >> + * \\brief White balance gains\n> >> + *\n> >> + * \\var IPAFrameContext::awb.gains.red\n> >> + * \\brief White balance gain for R channel\n> >> + *\n> >> + * \\var IPAFrameContext::awb.gains.green\n> >> + * \\brief White balance gain for G channel\n> >> + *\n> >> + * \\var IPAFrameContext::awb.gains.blue\n> >> + * \\brief White balance gain for B channel\n> >> + *\n> >> + * \\var IPAFrameContext::awb.temperatureK\n> >> + * \\brief Estimated color temperature\n> >> + */\n> >> +\n> >> /**\n> >> * \\var IPAFrameContext::sensor\n> >> * \\brief Effective sensor values\n> >> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> >> index 9ac3b40c..51eae8b6 100644\n> >> --- a/src/ipa/rkisp1/ipa_context.h\n> >> +++ b/src/ipa/rkisp1/ipa_context.h\n> >> @@ -12,6 +12,8 @@\n> >> \n> >> #include \n> >> \n> >> +#include \n> >> +\n> >> namespace libcamera {\n> >> \n> >> namespace ipa::rkisp1 {\n> >> @@ -25,6 +27,10 @@ struct IPASessionConfiguration {\n> >> \t\tstruct rkisp1_cif_isp_window measureWindow;\n> >> \t} agc;\n> >> \n> >> +\tstruct {\n> >> +\t\tstruct rkisp1_cif_isp_window measureWindow;\n> >> +\t} awb;\n> >> +\n> >> \tstruct {\n> >> \t\tutils::Duration lineDuration;\n> >> \t} sensor;\n> >> @@ -40,6 +46,16 @@ struct IPAFrameContext {\n> >> \t\tdouble gain;\n> >> \t} agc;\n> >> \n> >> +\tstruct {\n> >> +\t\tstruct {\n> >> +\t\t\tdouble red;\n> >> +\t\t\tdouble green;\n> >> +\t\t\tdouble blue;\n> >> +\t\t} gains;\n> >> +\n> >> +\t\tdouble temperatureK;\n> >> +\t} awb;\n> >> +\n> >> \tstruct {\n> >> \t\tuint32_t exposure;\n> >> \t\tdouble gain;\n> >> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> >> index f82b7cb3..381fc0e0 100644\n> >> --- a/src/ipa/rkisp1/rkisp1.cpp\n> >> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> >> @@ -27,6 +27,7 @@\n> >> \n> >> #include \"algorithms/agc.h\"\n> >> #include \"algorithms/algorithm.h\"\n> >> +#include \"algorithms/awb.h\"\n> >> #include \"algorithms/blc.h\"\n> >> #include \"libipa/camera_sensor_helper.h\"\n> >> \n> >> @@ -126,6 +127,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n> >> \n> >> \t/* Construct our Algorithms */\n> >> \talgorithms_.push_back(std::make_unique());\n> >> +\talgorithms_.push_back(std::make_unique());\n> >> \talgorithms_.push_back(std::make_unique());\n> >> \n> >> \treturn 0;","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 A189FBE08A\n\tfor ;\n\tThu, 24 Feb 2022 09:07:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA07461159;\n\tThu, 24 Feb 2022 10:07:07 +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 2B921601F7\n\tfor ;\n\tThu, 24 Feb 2022 10:07:06 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8415CDD;\n\tThu, 24 Feb 2022 10:07:05 +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=\"E60qB0gb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1645693625;\n\tbh=Ho4gI6faZlpoQy2vnp4kgQd6dkqdkaOolvpb0vuCCJ4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=E60qB0gbk8FACT8S2NDamEefyffaXIoXKuZABcWXePhpq4ZD/f9s8fnB+sp0qHxvC\n\tKqDul/TDGqnQkuoN7zOhPr37mrNl0JZv3XwVXZWdrOM9T3Wl3aQPweS2ZisjL/58d5\n\tKSuUN+WItOHUNPPFjfoBTPZp4ElvRYahqN6kS8Lg=","Date":"Thu, 24 Feb 2022 11:06:55 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20220223104824.25723-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220223104824.25723-5-jeanmichel.hautbois@ideasonboard.com>\n\t\n\t<37112622-791c-90a9-9a70-4a95e153864d@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<37112622-791c-90a9-9a70-4a95e153864d@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: rkisp1: Introduce AWB","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\" "}}]