[{"id":29551,"web_url":"https://patchwork.libcamera.org/comment/29551/","msgid":"<20240517100141.3ksm33w6hg2ptxyy@jasper>","date":"2024-05-17T10:01:41","subject":"Re: [PATCH v4 3/3] ipa: rkisp1: algorithms: Add crosstalk algorithm","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nthanks for the patch.\n\nI thinks thats ready to go now.\nReviewed-by: Stefan Klug \n\nCheers,\nStefan\n\nOn Fri, May 17, 2024 at 05:01:29PM +0900, Paul Elder wrote:\n> Add an algorithm module to the rkisp1 IPA for crosstalk correction.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v4:\n> - remove stray semicolons\n> - use the new matrix interpolator readYaml\n> - use the new matrix operator[] getter\n> \n> Changes in v3:\n> - read ccm offsets from tuning data, and write these offsets to the\n> parameters buffer\n> - make parseYaml return void, as it should fill in default data if\n> unable to read, thus never failing\n> \n> Changes in v2:\n> - rename ctk to ccm\n> - reset the matrix interpolator to identity matrix if failed to read\n> from tuning file\n> ---\n> src/ipa/rkisp1/algorithms/ccm.cpp | 112 ++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/ccm.h | 44 ++++++++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> 3 files changed, 157 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/ccm.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/ccm.h\n> \n> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> new file mode 100644\n> index 000000000..522ea0041\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> @@ -0,0 +1,112 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * ccm.cpp - RkISP1 Cross Talk Correction control algorithm\n> + */\n> +\n> +#include \"ccm.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"libipa/matrix_interpolator.h\"\n> +\n> +/**\n> + * \\file ctk.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class Ccm\n> + * \\brief A cross talk correction algorithm\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Ccm)\n> +\n> +void Ccm::parseYaml(const YamlObject &tuningData)\n> +{\n> +\tint ret = ccm_.readYaml(tuningData[\"ccms\"], \"ct\", \"ccm\");\n> +\tif (ret < 0) {\n> +\t\tLOG(RkISP1Ccm, Warning)\n> +\t\t\t<< \"Failed to parse 'ccms' \"\n> +\t\t\t<< \"parameter from tuning file; falling back to unit matrix\";\n> +\t\tccm_.reset();\n> +\t}\n> +\n> +\tret = offsets_.readYaml(tuningData[\"ccms\"], \"ct\", \"offsets\");\n> +\tif (ret < 0) {\n> +\t\tLOG(RkISP1Ccm, Warning)\n> +\t\t\t<< \"Failed to parse 'offsets' \"\n> +\t\t\t<< \"parameter from tuning file; falling back to zero offsets\";\n> +\t\toffsets_.reset();\n> +\t}\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> +{\n> +\tparseYaml(tuningData);\n> +\treturn 0;\n> +}\n> +\n> +void Ccm::setParameters(rkisp1_params_cfg *params,\n> +\t\t\tconst Matrix &matrix,\n> +\t\t\tconst Matrix &offsets)\n> +{\n> +\tstruct rkisp1_cif_isp_ctk_config &config = params->others.ctk_config;\n> +\n> +\t/*\n> +\t * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to\n> +\t * +7.992 (0x3FF)\n> +\t */\n> +\tfor (unsigned int i = 0; i < 3; i++)\n> +\t\tfor (unsigned int j = 0; j < 3; j++)\n> +\t\t\tconfig.coeff[i][j] =\n> +\t\t\t\tutils::floatingToFixedPoint<4, 7, uint16_t, double>(matrix[i][j]);\n> +\n> +\tfor (unsigned int i = 0; i < 3; i++)\n> +\t\tconfig.ct_offset[i] = offsets[i][0] & 0xFFF;\n> +\n> +\tLOG(RkISP1Ccm, Debug) << \"Setting matrix \" << matrix;\n> +\tLOG(RkISP1Ccm, Debug) << \"Setting offsets \" << offsets;\n> +\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_CTK;\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_CTK;\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_CTK;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Ccm::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params)\n> +{\n> +\tuint32_t ct = context.activeState.awb.temperatureK;\n> +\tMatrix ccm = ccm_.get(ct);\n> +\tMatrix offsets = offsets_.get(ct);\n> +\n> +\tsetParameters(params, ccm, offsets);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Ccm, \"Ccm\")\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h\n> new file mode 100644\n> index 000000000..80a4e7beb\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/ccm.h\n> @@ -0,0 +1,44 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * ccm.h - RkISP1 Cross Talk Correction control algorithm\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"libipa/matrix.h\"\n> +#include \"libipa/matrix_interpolator.h\"\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class Ccm : public Algorithm\n> +{\n> +public:\n> +\tCcm() {}\n> +\t~Ccm() = default;\n> +\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> +\tvoid prepare(IPAContext &context, const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params) override;\n> +\n> +private:\n> +\tvoid parseYaml(const YamlObject &tuningData);\n> +\tvoid setParameters(rkisp1_params_cfg *params,\n> +\t\t\t const Matrix &matrix,\n> +\t\t\t const Matrix &offsets);\n> +\n> +\tMatrixInterpolator ccm_;\n> +\tMatrixInterpolator offsets_;\n> +};\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> index 93a483292..16de71332 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -4,6 +4,7 @@ rkisp1_ipa_algorithms = files([\n> 'agc.cpp',\n> 'awb.cpp',\n> 'blc.cpp',\n> + 'ccm.cpp',\n> 'cproc.cpp',\n> 'dpcc.cpp',\n> 'dpf.cpp',\n> -- \n> 2.39.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 2356BBD78E\n\tfor ;\n\tFri, 17 May 2024 10:01:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A047163480;\n\tFri, 17 May 2024 12:01:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1AFBE63466\n\tfor ;\n\tFri, 17 May 2024 12:01:44 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:d572:8aa2:3e8e:5b99])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E41AF82E;\n\tFri, 17 May 2024 12:01:34 +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=\"nXGzwWjD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715940095;\n\tbh=xZICJnZ//S4ZSUTtfo1fczgJvFbimqONSl6dXaRNoE8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nXGzwWjDtBdAjcHLpWBwvmqYYJNHRB5a/B2DCqQWEoaBwuB9Dt6lIvc84Rg4FvXf4\n\tkYSjwI9ks7u1l6k3QDxGGl4ihjzhuiIziuCIN6O/TcuQ5U4QgzKvt+2HYI6ClVvhHy\n\tQID6mipYxlJIUHWO002HLE2/MC9mGPJXbHoXjgJc=","Date":"Fri, 17 May 2024 12:01:41 +0200","From":"Stefan Klug ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 3/3] ipa: rkisp1: algorithms: Add crosstalk algorithm","Message-ID":"<20240517100141.3ksm33w6hg2ptxyy@jasper>","References":"<20240517080129.3876981-1-paul.elder@ideasonboard.com>\n\t<20240517080129.3876981-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240517080129.3876981-4-paul.elder@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\" "}},{"id":29583,"web_url":"https://patchwork.libcamera.org/comment/29583/","msgid":"<95f60683-70ec-4496-987a-34e37993b884@ideasonboard.com>","date":"2024-05-20T14:48:45","subject":"Re: [PATCH v4 3/3] ipa: rkisp1: algorithms: Add crosstalk algorithm","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Paul, thanks for the patch\n\nOn 17/05/2024 09:01, Paul Elder wrote:\n> Add an algorithm module to the rkisp1 IPA for crosstalk correction.\n>\n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v4:\n> - remove stray semicolons\n> - use the new matrix interpolator readYaml\n> - use the new matrix operator[] getter\n>\n> Changes in v3:\n> - read ccm offsets from tuning data, and write these offsets to the\n> parameters buffer\n> - make parseYaml return void, as it should fill in default data if\n> unable to read, thus never failing\n>\n> Changes in v2:\n> - rename ctk to ccm\n> - reset the matrix interpolator to identity matrix if failed to read\n> from tuning file\n> ---\n> src/ipa/rkisp1/algorithms/ccm.cpp | 112 ++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/ccm.h | 44 ++++++++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> 3 files changed, 157 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/ccm.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/ccm.h\n>\n> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> new file mode 100644\n> index 000000000..522ea0041\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> @@ -0,0 +1,112 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * ccm.cpp - RkISP1 Cross Talk Correction control algorithm\n> + */\nNo more filenames in top-of-file comment\n> +\n> +#include \"ccm.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"libipa/matrix_interpolator.h\"\n> +\n> +/**\n> + * \\file ctk.h\n> + */\nWrong filename here.\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class Ccm\n> + * \\brief A cross talk correction algorithm\nI think call it a colour correction matrix algorithm here\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Ccm)\n> +\n> +void Ccm::parseYaml(const YamlObject &tuningData)\n> +{\n> +\tint ret = ccm_.readYaml(tuningData[\"ccms\"], \"ct\", \"ccm\");\n> +\tif (ret < 0) {\n> +\t\tLOG(RkISP1Ccm, Warning)\n> +\t\t\t<< \"Failed to parse 'ccms' \"\ns/ccms/ccm?\n> +\t\t\t<< \"parameter from tuning file; falling back to unit matrix\";\n> +\t\tccm_.reset();\n> +\t}\n> +\n> +\tret = offsets_.readYaml(tuningData[\"ccms\"], \"ct\", \"offsets\");\n> +\tif (ret < 0) {\n> +\t\tLOG(RkISP1Ccm, Warning)\n> +\t\t\t<< \"Failed to parse 'offsets' \"\n> +\t\t\t<< \"parameter from tuning file; falling back to zero offsets\";\n> +\t\toffsets_.reset();\n> +\t}\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> +{\n> +\tparseYaml(tuningData);\n> +\treturn 0;\n> +}\n\n\nWhy not just have the calls from parseYaml here and drop that function?\n\n> +\n> +void Ccm::setParameters(rkisp1_params_cfg *params,\n> +\t\t\tconst Matrix &matrix,\n> +\t\t\tconst Matrix &offsets)\n> +{\n> +\tstruct rkisp1_cif_isp_ctk_config &config = params->others.ctk_config;\n> +\n> +\t/*\n> +\t * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to\n> +\t * +7.992 (0x3FF)\n> +\t */\n> +\tfor (unsigned int i = 0; i < 3; i++)\n> +\t\tfor (unsigned int j = 0; j < 3; j++)\n> +\t\t\tconfig.coeff[i][j] =\n> +\t\t\t\tutils::floatingToFixedPoint<4, 7, uint16_t, double>(matrix[i][j]);\n> +\n> +\tfor (unsigned int i = 0; i < 3; i++)\n> +\t\tconfig.ct_offset[i] = offsets[i][0] & 0xFFF;\n> +\n> +\tLOG(RkISP1Ccm, Debug) << \"Setting matrix \" << matrix;\n> +\tLOG(RkISP1Ccm, Debug) << \"Setting offsets \" << offsets;\n> +\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_CTK;\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_CTK;\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_CTK;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Ccm::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params)\n> +{\n> +\tuint32_t ct = context.activeState.awb.temperatureK;\n> +\tMatrix ccm = ccm_.get(ct);\n> +\tMatrix offsets = offsets_.get(ct);\n> +\n> +\tsetParameters(params, ccm, offsets);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Ccm, \"Ccm\")\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h\n> new file mode 100644\n> index 000000000..80a4e7beb\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/ccm.h\n> @@ -0,0 +1,44 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * ccm.h - RkISP1 Cross Talk Correction control algorithm\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"libipa/matrix.h\"\n> +#include \"libipa/matrix_interpolator.h\"\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class Ccm : public Algorithm\n> +{\n> +public:\n> +\tCcm() {}\n> +\t~Ccm() = default;\n> +\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> +\tvoid prepare(IPAContext &context, const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params) override;\n> +\n> +private:\n> +\tvoid parseYaml(const YamlObject &tuningData);\n> +\tvoid setParameters(rkisp1_params_cfg *params,\n> +\t\t\t const Matrix &matrix,\n> +\t\t\t const Matrix &offsets);\n> +\n> +\tMatrixInterpolator ccm_;\n> +\tMatrixInterpolator offsets_;\n> +};\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> index 93a483292..16de71332 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -4,6 +4,7 @@ rkisp1_ipa_algorithms = files([\n> 'agc.cpp',\n> 'awb.cpp',\n> 'blc.cpp',\n> + 'ccm.cpp',\n> 'cproc.cpp',\n> 'dpcc.cpp',\n> 'dpf.cpp',","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 4918BBDE6B\n\tfor ;\n\tMon, 20 May 2024 14:48:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 608D763471;\n\tMon, 20 May 2024 16:48:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E77E61A57\n\tfor ;\n\tMon, 20 May 2024 16:48:49 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A499AD01\n\tfor ;\n\tMon, 20 May 2024 16:48:37 +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=\"bDuwG6GI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716216518;\n\tbh=z+JWD3AHk1VoCrq788P7wHB5SEys6VJ8ZwnpP1vnA5A=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=bDuwG6GIMI7q+m3ftrlmbeXJxOadJhlYODa/yDUB4WviBSXRTr0juQoRP8LLJO8d8\n\tHI7HWWOExtrzF1Bzu7suo2NKH7UiKg4GUaS+Iq5k3nr1nCgU7PiLUZDLsAXa+1x7L4\n\tomzhWQI5PgyzzNwfdRcxcxbjtNH+wZTtW/+hdQEo=","Message-ID":"<95f60683-70ec-4496-987a-34e37993b884@ideasonboard.com>","Date":"Mon, 20 May 2024 15:48:45 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 3/3] ipa: rkisp1: algorithms: Add crosstalk algorithm","To":"libcamera-devel@lists.libcamera.org","References":"<20240517080129.3876981-1-paul.elder@ideasonboard.com>\n\t<20240517080129.3876981-4-paul.elder@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20240517080129.3876981-4-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":29584,"web_url":"https://patchwork.libcamera.org/comment/29584/","msgid":"<171621844849.2248009.5920983432652367740@ping.linuxembedded.co.uk>","date":"2024-05-20T15:20:48","subject":"Re: [PATCH v4 3/3] ipa: rkisp1: algorithms: Add crosstalk algorithm","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Dan Scally (2024-05-20 15:48:45)\n> Hi Paul, thanks for the patch\n> \n> On 17/05/2024 09:01, Paul Elder wrote:\n> > Add an algorithm module to the rkisp1 IPA for crosstalk correction.\n> >\n> > Signed-off-by: Paul Elder \n> >\n> > ---\n> > Changes in v4:\n> > - remove stray semicolons\n> > - use the new matrix interpolator readYaml\n> > - use the new matrix operator[] getter\n> >\n> > Changes in v3:\n> > - read ccm offsets from tuning data, and write these offsets to the\n> > parameters buffer\n> > - make parseYaml return void, as it should fill in default data if\n> > unable to read, thus never failing\n> >\n> > Changes in v2:\n> > - rename ctk to ccm\n> > - reset the matrix interpolator to identity matrix if failed to read\n> > from tuning file\n> > ---\n> > src/ipa/rkisp1/algorithms/ccm.cpp | 112 ++++++++++++++++++++++++++\n> > src/ipa/rkisp1/algorithms/ccm.h | 44 ++++++++++\n> > src/ipa/rkisp1/algorithms/meson.build | 1 +\n> > 3 files changed, 157 insertions(+)\n> > create mode 100644 src/ipa/rkisp1/algorithms/ccm.cpp\n> > create mode 100644 src/ipa/rkisp1/algorithms/ccm.h\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> > new file mode 100644\n> > index 000000000..522ea0041\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> > @@ -0,0 +1,112 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board\n> > + *\n> > + * ccm.cpp - RkISP1 Cross Talk Correction control algorithm\n> > + */\n> No more filenames in top-of-file comment\n> > +\n> > +#include \"ccm.h\"\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"libipa/matrix_interpolator.h\"\n> > +\n> > +/**\n> > + * \\file ctk.h\n> > + */\n> Wrong filename here.\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +/**\n> > + * \\class Ccm\n> > + * \\brief A cross talk correction algorithm\n> I think call it a colour correction matrix algorithm here\n> > + */\n> > +\n> > +LOG_DEFINE_CATEGORY(RkISP1Ccm)\n> > +\n> > +void Ccm::parseYaml(const YamlObject &tuningData)\n> > +{\n> > + int ret = ccm_.readYaml(tuningData[\"ccms\"], \"ct\", \"ccm\");\n> > + if (ret < 0) {\n> > + LOG(RkISP1Ccm, Warning)\n> > + << \"Failed to parse 'ccms' \"\n> s/ccms/ccm?\n> > + << \"parameter from tuning file; falling back to unit matrix\";\n> > + ccm_.reset();\n> > + }\n> > +\n> > + ret = offsets_.readYaml(tuningData[\"ccms\"], \"ct\", \"offsets\");\n> > + if (ret < 0) {\n> > + LOG(RkISP1Ccm, Warning)\n> > + << \"Failed to parse 'offsets' \"\n> > + << \"parameter from tuning file; falling back to zero offsets\";\n> > + offsets_.reset();\n> > + }\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::init\n> > + */\n> > +int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> > +{\n> > + parseYaml(tuningData);\n> > + return 0;\n> > +}\n> \n> \n> Why not just have the calls from parseYaml here and drop that function?\n> \n> > +\n> > +void Ccm::setParameters(rkisp1_params_cfg *params,\n> > + const Matrix &matrix,\n> > + const Matrix &offsets)\n> > +{\n> > + struct rkisp1_cif_isp_ctk_config &config = params->others.ctk_config;\n> > +\n> > + /*\n> > + * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to\n> > + * +7.992 (0x3FF)\n> > + */\n> > + for (unsigned int i = 0; i < 3; i++)\n> > + for (unsigned int j = 0; j < 3; j++)\n> > + config.coeff[i][j] =\n> > + utils::floatingToFixedPoint<4, 7, uint16_t, double>(matrix[i][j]);\n> > +\n> > + for (unsigned int i = 0; i < 3; i++)\n> > + config.ct_offset[i] = offsets[i][0] & 0xFFF;\n> > +\n> > + LOG(RkISP1Ccm, Debug) << \"Setting matrix \" << matrix;\n> > + LOG(RkISP1Ccm, Debug) << \"Setting offsets \" << offsets;\n> > +\n> > + params->module_en_update |= RKISP1_CIF_ISP_MODULE_CTK;\n> > + params->module_ens |= RKISP1_CIF_ISP_MODULE_CTK;\n> > + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_CTK;\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::prepare\n> > + */\n> > +void Ccm::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > + rkisp1_params_cfg *params)\n> > +{\n> > + uint32_t ct = context.activeState.awb.temperatureK;\n\nCan the CCM module cache what the current configured colour temperature\nis and only perform the next operations if it has changed? Possibly even\nwith a small threshold even.\n\nOtherwise I suspect we'll be doing far more work in this module than is\nrequired? Or do we need to process every frame individually through here\nregardless?\n\n\n> > + Matrix ccm = ccm_.get(ct);\n> > + Matrix offsets = offsets_.get(ct);\n> > +\n> > + setParameters(params, ccm, offsets);\n> > +}\n> > +\n> > +REGISTER_IPA_ALGORITHM(Ccm, \"Ccm\")\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h\n> > new file mode 100644\n> > index 000000000..80a4e7beb\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/ccm.h\n> > @@ -0,0 +1,44 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board\n> > + *\n> > + * ccm.h - RkISP1 Cross Talk Correction control algorithm\n\nThis line can be dropped too.\n\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +\n> > +#include \"libipa/matrix.h\"\n> > +#include \"libipa/matrix_interpolator.h\"\n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +class Ccm : public Algorithm\n> > +{\n> > +public:\n> > + Ccm() {}\n> > + ~Ccm() = default;\n> > +\n> > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > + void prepare(IPAContext &context, const uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + rkisp1_params_cfg *params) override;\n> > +\n> > +private:\n> > + void parseYaml(const YamlObject &tuningData);\n> > + void setParameters(rkisp1_params_cfg *params,\n> > + const Matrix &matrix,\n> > + const Matrix &offsets);\n> > +\n> > + MatrixInterpolator ccm_;\n> > + MatrixInterpolator offsets_;\n> > +};\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > index 93a483292..16de71332 100644\n> > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > @@ -4,6 +4,7 @@ rkisp1_ipa_algorithms = files([\n> > 'agc.cpp',\n> > 'awb.cpp',\n> > 'blc.cpp',\n> > + 'ccm.cpp',\n> > 'cproc.cpp',\n> > 'dpcc.cpp',\n> > 'dpf.cpp',","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 5BE29BDE6B\n\tfor ;\n\tMon, 20 May 2024 15:20:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 558D663483;\n\tMon, 20 May 2024 17:20:53 +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 7DCD961A57\n\tfor ;\n\tMon, 20 May 2024 17:20:51 +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 F38C1581;\n\tMon, 20 May 2024 17:20:39 +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=\"UrIuMmXM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716218440;\n\tbh=QyujMqeB4ypoR63NeOTPe/OkHDNA6AAlxvYTOBdy904=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=UrIuMmXM9raCZeU+YAu1hUSgd5Cd/XAly8AUBw7bDZyh9Dlke1k4jm1bgs+0Oz5fM\n\t4a76fZNXfkOtQsPmga9qTbEgTEuOwsesv7HpFSwFQYfsqPboQhUVbwYLNz58qtEVQ/\n\tZgg63Aptgp0LpXcYxG1hHS0d/qnQA5tasOmr7nNI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<95f60683-70ec-4496-987a-34e37993b884@ideasonboard.com>","References":"<20240517080129.3876981-1-paul.elder@ideasonboard.com>\n\t<20240517080129.3876981-4-paul.elder@ideasonboard.com>\n\t<95f60683-70ec-4496-987a-34e37993b884@ideasonboard.com>","Subject":"Re: [PATCH v4 3/3] ipa: rkisp1: algorithms: Add crosstalk algorithm","From":"Kieran Bingham ","To":"Dan Scally ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 20 May 2024 16:20:48 +0100","Message-ID":"<171621844849.2248009.5920983432652367740@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\" "}}]