[{"id":32984,"web_url":"https://patchwork.libcamera.org/comment/32984/","msgid":"<173644245058.2992722.2830487320880500052@ping.linuxembedded.co.uk>","date":"2025-01-09T17:07:30","subject":"Re: [PATCH v3 6/9] libcamera: software_isp: Add CCM algorithm","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2024-12-10 15:34:36)\n> This patch adds color correction matrix (CCM) algorithm to software ISP.\n> It is based on the corresponding algorithm in rkisp1.\n> \n> The primary difference against hardware pipelines is that applying the\n> CCM is optional. Applying CCM causes a significant slowdown, time\n> needed to process a frame raises by 40-90% on tested platforms. If CCM\n\nOuch... :-(\n\n> is really needed, it can be applied, if not, it's better to stick\n> without it. This can be configured by presence or omission of Ccm\n> algorithm in the tuning file.\n> \n> CCM is changed only if the determined temperature changes by at least\n> 100 K (an arbitrarily selected value), to avoid recomputing the matrices\n\nI think I'd be comfortable with anything between 50 and 500 here, so I\nthink this is a good starting point.\n\nThe real answer will come from looking at how much the measured colour\nchanges by in a given capture session!\n\n\n> and lookup tables all the time. Some ugly unsigned->signed int\n> typecasting is needed to compare the temperatures.\n> \n> The outputs of the algorithm are not used yet, they will be enabled in\n> followup patches.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/ipa/simple/algorithms/ccm.cpp | 88 +++++++++++++++++++++++++++\n> src/ipa/simple/algorithms/ccm.h | 45 ++++++++++++++\n> src/ipa/simple/algorithms/meson.build | 1 +\n> src/ipa/simple/ipa_context.h | 13 ++++\n> 4 files changed, 147 insertions(+)\n> create mode 100644 src/ipa/simple/algorithms/ccm.cpp\n> create mode 100644 src/ipa/simple/algorithms/ccm.h\n> \n> diff --git a/src/ipa/simple/algorithms/ccm.cpp b/src/ipa/simple/algorithms/ccm.cpp\n> new file mode 100644\n> index 00000000..40fe12c8\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/ccm.cpp\n> @@ -0,0 +1,88 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Color correction matrix\n> + */\n> +\n> +#include \"ccm.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +LOG_DEFINE_CATEGORY(IPASoftCcm)\n> +\n> +unsigned int Ccm::kTemperatureThreshold = 100;\n> +\n> +int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> +{\n> + int ret = ccm_.readYaml(tuningData[\"ccms\"], \"ct\", \"ccm\");\n> + if (ret < 0) {\n> + LOG(IPASoftCcm, Warning)\n> + << \"Failed to parse 'ccm' \"\n> + << \"parameter from tuning file; falling back to unit matrix\";\n> + ccmEnabled_ = false;\n> + } else {\n> + ccmEnabled_ = true;\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +void Ccm::prepare(IPAContext &context, const uint32_t frame,\n> + IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params)\n> +{\n> + context.activeState.ccm.enabled = ccmEnabled_;\n> +\n> + if (!ccmEnabled_)\n> + return;\n> +\n> + unsigned int ct = context.activeState.awb.temperatureK;\n> +\n> + /* Change CCM only on bigger temperature changes. */\n> + if (frame > 0 &&\n> + abs(static_cast(ct) - static_cast(ct_)) <\n> + static_cast(kTemperatureThreshold)) {\n\nDoes utils::abs_diff() solve this ?\n\n\nAside from that:\n\n\nReviewed-by: Kieran Bingham \n\n> + frameContext.ccm.ccm = context.activeState.ccm.ccm;\n> + context.activeState.ccm.changed = false;\n> + return;\n> + }\n> +\n> + ct_ = ct;\n> + Matrix ccm = ccm_.getInterpolated(ct);\n> +\n> + context.activeState.ccm.ccm = ccm;\n> + frameContext.ccm.ccm = ccm;\n> + context.activeState.ccm.changed = true;\n> +}\n> +\n> +void Ccm::process([[maybe_unused]] IPAContext &context,\n> + [[maybe_unused]] const uint32_t frame,\n> + IPAFrameContext &frameContext,\n> + [[maybe_unused]] const SwIspStats *stats,\n> + ControlList &metadata)\n> +{\n> + if (!ccmEnabled_)\n> + return;\n> +\n> + float m[9];\n> + for (unsigned int i = 0; i < 3; i++) {\n> + for (unsigned int j = 0; j < 3; j++)\n> + m[i * 3 + j] = frameContext.ccm.ccm[i][j];\n> + }\n> + metadata.set(controls::ColourCorrectionMatrix, m);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Ccm, \"Ccm\")\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/ccm.h b/src/ipa/simple/algorithms/ccm.h\n> new file mode 100644\n> index 00000000..23481a08\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/ccm.h\n> @@ -0,0 +1,45 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Color correction matrix\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"libcamera/internal/matrix.h\"\n> +\n> +#include \n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +class Ccm : public Algorithm\n> +{\n> +public:\n> + Ccm() = default;\n> + ~Ccm() = default;\n> +\n> + int init(IPAContext &context, const YamlObject &tuningData) override;\n> + void prepare(IPAContext &context,\n> + const uint32_t frame,\n> + IPAFrameContext &frameContext,\n> + DebayerParams *params) override;\n> + void process(IPAContext &context, const uint32_t frame,\n> + IPAFrameContext &frameContext,\n> + const SwIspStats *stats,\n> + ControlList &metadata) override;\n> +\n> +private:\n> + static unsigned int kTemperatureThreshold;\n> + unsigned int ct_;\n> + bool ccmEnabled_;\n> + Interpolator> ccm_;\n> +};\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n> index 37a2eb53..2d0adb05 100644\n> --- a/src/ipa/simple/algorithms/meson.build\n> +++ b/src/ipa/simple/algorithms/meson.build\n> @@ -4,5 +4,6 @@ soft_simple_ipa_algorithms = files([\n> 'awb.cpp',\n> 'agc.cpp',\n> 'blc.cpp',\n> + 'ccm.cpp',\n> 'lut.cpp',\n> ])\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index 607af45a..0def3eef 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -13,6 +13,8 @@\n> \n> #include \n> \n> +#include \"libcamera/internal/matrix.h\"\n> +\n> #include \n> \n> namespace libcamera {\n> @@ -50,6 +52,13 @@ struct IPAActiveState {\n> uint8_t blackLevel;\n> double contrast;\n> } gamma;\n> +\n> + struct {\n> + Matrix ccm;\n> + bool enabled;\n> + bool changed;\n> + } ccm;\n> +\n> struct {\n> /* 0..2 range, 1.0 = normal */\n> std::optional contrast;\n> @@ -57,6 +66,10 @@ struct IPAActiveState {\n> };\n> \n> struct IPAFrameContext : public FrameContext {\n> + struct {\n> + Matrix ccm;\n> + } ccm;\n> +\n> struct {\n> int32_t exposure;\n> double gain;\n> -- \n> 2.44.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 15F19C32EA\n\tfor ;\n\tThu, 9 Jan 2025 17:07:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B40A684EA;\n\tThu, 9 Jan 2025 18:07:34 +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 CCC1B61880\n\tfor ;\n\tThu, 9 Jan 2025 18:07:32 +0100 (CET)","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 22D649FC;\n\tThu, 9 Jan 2025 18:06:39 +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=\"cYzi/xVh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1736442399;\n\tbh=mzPLpZ0GwHpUSYrfVbwwLsR86nasb6v2JD01AqUj4Ds=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=cYzi/xVhBkkiPRrxYCCSGL8la9/yAvc/hOmNRyexuKaezIXL1ITABxSWHlj2A6z0r\n\tw5o2l9PhnGyB6XQs3h8hvfBlJ+sfmXi1/7doee+OvAQ8vxYf2zdIQHshc4QMgVdpMV\n\tytxuCuTbXF5rjhuQoMjbcx4VTy//u6+SqOzX+ZO8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241210153440.1007470-7-mzamazal@redhat.com>","References":"<20241210153440.1007470-1-mzamazal@redhat.com>\n\t<20241210153440.1007470-7-mzamazal@redhat.com>","Subject":"Re: [PATCH v3 6/9] libcamera: software_isp: Add CCM algorithm","From":"Kieran Bingham ","Cc":"Milan Zamazal ,\n\tRobert Mader ,\n\tHans de Goede , \n\tLaurent Pinchart ","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Date":"Thu, 09 Jan 2025 17:07:30 +0000","Message-ID":"<173644245058.2992722.2830487320880500052@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":32985,"web_url":"https://patchwork.libcamera.org/comment/32985/","msgid":"<20250109170948.GC6159@pendragon.ideasonboard.com>","date":"2025-01-09T17:09:48","subject":"Re: [PATCH v3 6/9] libcamera: software_isp: Add CCM algorithm","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Jan 09, 2025 at 05:07:30PM +0000, Kieran Bingham wrote:\n> Quoting Milan Zamazal (2024-12-10 15:34:36)\n> > This patch adds color correction matrix (CCM) algorithm to software ISP.\n> > It is based on the corresponding algorithm in rkisp1.\n> > \n> > The primary difference against hardware pipelines is that applying the\n> > CCM is optional. Applying CCM causes a significant slowdown, time\n> > needed to process a frame raises by 40-90% on tested platforms. If CCM\n> \n> Ouch... :-(\n\nThat's where I expect GPU acceleration to really help.\n\n> > is really needed, it can be applied, if not, it's better to stick\n> > without it. This can be configured by presence or omission of Ccm\n> > algorithm in the tuning file.\n> > \n> > CCM is changed only if the determined temperature changes by at least\n> > 100 K (an arbitrarily selected value), to avoid recomputing the matrices\n> \n> I think I'd be comfortable with anything between 50 and 500 here, so I\n> think this is a good starting point.\n> \n> The real answer will come from looking at how much the measured colour\n> changes by in a given capture session!\n> \n> > and lookup tables all the time. Some ugly unsigned->signed int\n> > typecasting is needed to compare the temperatures.\n> > \n> > The outputs of the algorithm are not used yet, they will be enabled in\n> > followup patches.\n> > \n> > Signed-off-by: Milan Zamazal \n> > ---\n> > src/ipa/simple/algorithms/ccm.cpp | 88 +++++++++++++++++++++++++++\n> > src/ipa/simple/algorithms/ccm.h | 45 ++++++++++++++\n> > src/ipa/simple/algorithms/meson.build | 1 +\n> > src/ipa/simple/ipa_context.h | 13 ++++\n> > 4 files changed, 147 insertions(+)\n> > create mode 100644 src/ipa/simple/algorithms/ccm.cpp\n> > create mode 100644 src/ipa/simple/algorithms/ccm.h\n> > \n> > diff --git a/src/ipa/simple/algorithms/ccm.cpp b/src/ipa/simple/algorithms/ccm.cpp\n> > new file mode 100644\n> > index 00000000..40fe12c8\n> > --- /dev/null\n> > +++ b/src/ipa/simple/algorithms/ccm.cpp\n> > @@ -0,0 +1,88 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board\n> > + * Copyright (C) 2024, Red Hat Inc.\n> > + *\n> > + * Color correction matrix\n> > + */\n> > +\n> > +#include \"ccm.h\"\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::soft::algorithms {\n> > +\n> > +LOG_DEFINE_CATEGORY(IPASoftCcm)\n> > +\n> > +unsigned int Ccm::kTemperatureThreshold = 100;\n> > +\n> > +int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> > +{\n> > + int ret = ccm_.readYaml(tuningData[\"ccms\"], \"ct\", \"ccm\");\n> > + if (ret < 0) {\n> > + LOG(IPASoftCcm, Warning)\n> > + << \"Failed to parse 'ccm' \"\n> > + << \"parameter from tuning file; falling back to unit matrix\";\n> > + ccmEnabled_ = false;\n> > + } else {\n> > + ccmEnabled_ = true;\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +void Ccm::prepare(IPAContext &context, const uint32_t frame,\n> > + IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params)\n> > +{\n> > + context.activeState.ccm.enabled = ccmEnabled_;\n> > +\n> > + if (!ccmEnabled_)\n> > + return;\n> > +\n> > + unsigned int ct = context.activeState.awb.temperatureK;\n> > +\n> > + /* Change CCM only on bigger temperature changes. */\n> > + if (frame > 0 &&\n> > + abs(static_cast(ct) - static_cast(ct_)) <\n> > + static_cast(kTemperatureThreshold)) {\n> \n> Does utils::abs_diff() solve this ?\n> \n> \n> Aside from that:\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > + frameContext.ccm.ccm = context.activeState.ccm.ccm;\n> > + context.activeState.ccm.changed = false;\n> > + return;\n> > + }\n> > +\n> > + ct_ = ct;\n> > + Matrix ccm = ccm_.getInterpolated(ct);\n> > +\n> > + context.activeState.ccm.ccm = ccm;\n> > + frameContext.ccm.ccm = ccm;\n> > + context.activeState.ccm.changed = true;\n> > +}\n> > +\n> > +void Ccm::process([[maybe_unused]] IPAContext &context,\n> > + [[maybe_unused]] const uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + [[maybe_unused]] const SwIspStats *stats,\n> > + ControlList &metadata)\n> > +{\n> > + if (!ccmEnabled_)\n> > + return;\n> > +\n> > + float m[9];\n> > + for (unsigned int i = 0; i < 3; i++) {\n> > + for (unsigned int j = 0; j < 3; j++)\n> > + m[i * 3 + j] = frameContext.ccm.ccm[i][j];\n> > + }\n> > + metadata.set(controls::ColourCorrectionMatrix, m);\n> > +}\n> > +\n> > +REGISTER_IPA_ALGORITHM(Ccm, \"Ccm\")\n> > +\n> > +} /* namespace ipa::soft::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/simple/algorithms/ccm.h b/src/ipa/simple/algorithms/ccm.h\n> > new file mode 100644\n> > index 00000000..23481a08\n> > --- /dev/null\n> > +++ b/src/ipa/simple/algorithms/ccm.h\n> > @@ -0,0 +1,45 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Red Hat Inc.\n> > + *\n> > + * Color correction matrix\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \"libcamera/internal/matrix.h\"\n> > +\n> > +#include \n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::soft::algorithms {\n> > +\n> > +class Ccm : public Algorithm\n> > +{\n> > +public:\n> > + Ccm() = default;\n> > + ~Ccm() = default;\n> > +\n> > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > + void prepare(IPAContext &context,\n> > + const uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + DebayerParams *params) override;\n> > + void process(IPAContext &context, const uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + const SwIspStats *stats,\n> > + ControlList &metadata) override;\n> > +\n> > +private:\n> > + static unsigned int kTemperatureThreshold;\n> > + unsigned int ct_;\n> > + bool ccmEnabled_;\n> > + Interpolator> ccm_;\n> > +};\n> > +\n> > +} /* namespace ipa::soft::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n> > index 37a2eb53..2d0adb05 100644\n> > --- a/src/ipa/simple/algorithms/meson.build\n> > +++ b/src/ipa/simple/algorithms/meson.build\n> > @@ -4,5 +4,6 @@ soft_simple_ipa_algorithms = files([\n> > 'awb.cpp',\n> > 'agc.cpp',\n> > 'blc.cpp',\n> > + 'ccm.cpp',\n> > 'lut.cpp',\n> > ])\n> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> > index 607af45a..0def3eef 100644\n> > --- a/src/ipa/simple/ipa_context.h\n> > +++ b/src/ipa/simple/ipa_context.h\n> > @@ -13,6 +13,8 @@\n> > \n> > #include \n> > \n> > +#include \"libcamera/internal/matrix.h\"\n> > +\n> > #include \n> > \n> > namespace libcamera {\n> > @@ -50,6 +52,13 @@ struct IPAActiveState {\n> > uint8_t blackLevel;\n> > double contrast;\n> > } gamma;\n> > +\n> > + struct {\n> > + Matrix ccm;\n> > + bool enabled;\n> > + bool changed;\n> > + } ccm;\n> > +\n> > struct {\n> > /* 0..2 range, 1.0 = normal */\n> > std::optional contrast;\n> > @@ -57,6 +66,10 @@ struct IPAActiveState {\n> > };\n> > \n> > struct IPAFrameContext : public FrameContext {\n> > + struct {\n> > + Matrix ccm;\n> > + } ccm;\n> > +\n> > struct {\n> > int32_t exposure;\n> > double gain;","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 E3281C32EA\n\tfor ;\n\tThu, 9 Jan 2025 17:09:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1BFAB68500;\n\tThu, 9 Jan 2025 18:09:53 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F2CFA61884\n\tfor ;\n\tThu, 9 Jan 2025 18:09:51 +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 083E39FC;\n\tThu, 9 Jan 2025 18:08:57 +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=\"Kf9WleQ5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1736442538;\n\tbh=JdvHyEU6prx6NjLy5b1akV0ZHDC6YsGNrCJ7wyhMxJQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kf9WleQ5hZ95ULwZcLtZI4ARR45ZuPVEKDX8HaNeNPp5KkQW9J9GUABNQ+yGFd9QC\n\tPXEt43Udf4ao5sN4lErgt+NO/gsaXlaswkfpeIQI3CMclKjg1w95tCwLZf8syfxt1j\n\tR6BVz2Cp8nDg2FPxY2JNTnDiEjM/smy+djS4oYqk=","Date":"Thu, 9 Jan 2025 19:09:48 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Milan Zamazal , libcamera-devel@lists.libcamera.org,\n\tRobert Mader ,\n\tHans de Goede ","Subject":"Re: [PATCH v3 6/9] libcamera: software_isp: Add CCM algorithm","Message-ID":"<20250109170948.GC6159@pendragon.ideasonboard.com>","References":"<20241210153440.1007470-1-mzamazal@redhat.com>\n\t<20241210153440.1007470-7-mzamazal@redhat.com>\n\t<173644245058.2992722.2830487320880500052@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<173644245058.2992722.2830487320880500052@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":33023,"web_url":"https://patchwork.libcamera.org/comment/33023/","msgid":"<858qrio69d.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","date":"2025-01-10T18:47:42","subject":"Re: [PATCH v3 6/9] libcamera: software_isp: Add CCM algorithm","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Kieran,\n\nthank you for review.\n\nKieran Bingham writes:\n\n> Quoting Milan Zamazal (2024-12-10 15:34:36)\n>> This patch adds color correction matrix (CCM) algorithm to software ISP.\n>> It is based on the corresponding algorithm in rkisp1.\n>\n>> \n>> The primary difference against hardware pipelines is that applying the\n>> CCM is optional. Applying CCM causes a significant slowdown, time\n>> needed to process a frame raises by 40-90% on tested platforms. If CCM\n>\n> Ouch... :-(\n\nIt's not that bad, considering how heavy-weight the operation it is.\nIt's usable on reasonable CPUs, especially for testing. And as Laurent\npointed out, is likely to be much better with GPU.\n\n>> is really needed, it can be applied, if not, it's better to stick\n>> without it. This can be configured by presence or omission of Ccm\n>> algorithm in the tuning file.\n>> \n>> CCM is changed only if the determined temperature changes by at least\n>> 100 K (an arbitrarily selected value), to avoid recomputing the matrices\n>\n> I think I'd be comfortable with anything between 50 and 500 here, so I\n> think this is a good starting point.\n>\n> The real answer will come from looking at how much the measured colour\n> changes by in a given capture session!\n>\n>\n>> and lookup tables all the time. Some ugly unsigned->signed int\n>> typecasting is needed to compare the temperatures.\n>> \n>> The outputs of the algorithm are not used yet, they will be enabled in\n>> followup patches.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/ipa/simple/algorithms/ccm.cpp | 88 +++++++++++++++++++++++++++\n>> src/ipa/simple/algorithms/ccm.h | 45 ++++++++++++++\n>> src/ipa/simple/algorithms/meson.build | 1 +\n>> src/ipa/simple/ipa_context.h | 13 ++++\n>> 4 files changed, 147 insertions(+)\n>> create mode 100644 src/ipa/simple/algorithms/ccm.cpp\n>> create mode 100644 src/ipa/simple/algorithms/ccm.h\n>> \n>> diff --git a/src/ipa/simple/algorithms/ccm.cpp b/src/ipa/simple/algorithms/ccm.cpp\n>> new file mode 100644\n>> index 00000000..40fe12c8\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/ccm.cpp\n>> @@ -0,0 +1,88 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Ideas On Board\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Color correction matrix\n>> + */\n>> +\n>> +#include \"ccm.h\"\n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +LOG_DEFINE_CATEGORY(IPASoftCcm)\n>> +\n>> +unsigned int Ccm::kTemperatureThreshold = 100;\n>> +\n>> +int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n>> +{\n>> + int ret = ccm_.readYaml(tuningData[\"ccms\"], \"ct\", \"ccm\");\n>> + if (ret < 0) {\n>> + LOG(IPASoftCcm, Warning)\n>> + << \"Failed to parse 'ccm' \"\n>> + << \"parameter from tuning file; falling back to unit matrix\";\n>> + ccmEnabled_ = false;\n>> + } else {\n>> + ccmEnabled_ = true;\n>> + }\n>> +\n>> + return 0;\n>> +}\n>> +\n>> +void Ccm::prepare(IPAContext &context, const uint32_t frame,\n>> + IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params)\n>> +{\n>> + context.activeState.ccm.enabled = ccmEnabled_;\n>> +\n>> + if (!ccmEnabled_)\n>> + return;\n>> +\n>> + unsigned int ct = context.activeState.awb.temperatureK;\n>> +\n>> + /* Change CCM only on bigger temperature changes. */\n>> + if (frame > 0 &&\n>> + abs(static_cast(ct) - static_cast(ct_)) <\n>> + static_cast(kTemperatureThreshold)) {\n>\n> Does utils::abs_diff() solve this ?\n\nIt does, thank you.\n\n> Aside from that:\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n>> + frameContext.ccm.ccm = context.activeState.ccm.ccm;\n>> + context.activeState.ccm.changed = false;\n>> + return;\n>> + }\n>> +\n>> + ct_ = ct;\n>> + Matrix ccm = ccm_.getInterpolated(ct);\n>> +\n>> + context.activeState.ccm.ccm = ccm;\n>> + frameContext.ccm.ccm = ccm;\n>> + context.activeState.ccm.changed = true;\n>> +}\n>> +\n>> +void Ccm::process([[maybe_unused]] IPAContext &context,\n>> + [[maybe_unused]] const uint32_t frame,\n>> + IPAFrameContext &frameContext,\n>> + [[maybe_unused]] const SwIspStats *stats,\n>> + ControlList &metadata)\n>> +{\n>> + if (!ccmEnabled_)\n>> + return;\n>> +\n>> + float m[9];\n>> + for (unsigned int i = 0; i < 3; i++) {\n>> + for (unsigned int j = 0; j < 3; j++)\n>> + m[i * 3 + j] = frameContext.ccm.ccm[i][j];\n>> + }\n>> + metadata.set(controls::ColourCorrectionMatrix, m);\n>> +}\n>> +\n>> +REGISTER_IPA_ALGORITHM(Ccm, \"Ccm\")\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/ccm.h b/src/ipa/simple/algorithms/ccm.h\n>> new file mode 100644\n>> index 00000000..23481a08\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/ccm.h\n>> @@ -0,0 +1,45 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Color correction matrix\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \"libcamera/internal/matrix.h\"\n>> +\n>> +#include \n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +class Ccm : public Algorithm\n>> +{\n>> +public:\n>> + Ccm() = default;\n>> + ~Ccm() = default;\n>> +\n>> + int init(IPAContext &context, const YamlObject &tuningData) override;\n>> + void prepare(IPAContext &context,\n>> + const uint32_t frame,\n>> + IPAFrameContext &frameContext,\n>> + DebayerParams *params) override;\n>> + void process(IPAContext &context, const uint32_t frame,\n>> + IPAFrameContext &frameContext,\n>> + const SwIspStats *stats,\n>> + ControlList &metadata) override;\n>> +\n>> +private:\n>> + static unsigned int kTemperatureThreshold;\n>> + unsigned int ct_;\n>> + bool ccmEnabled_;\n>> + Interpolator> ccm_;\n>> +};\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n>> index 37a2eb53..2d0adb05 100644\n>> --- a/src/ipa/simple/algorithms/meson.build\n>> +++ b/src/ipa/simple/algorithms/meson.build\n>> @@ -4,5 +4,6 @@ soft_simple_ipa_algorithms = files([\n>> 'awb.cpp',\n>> 'agc.cpp',\n>> 'blc.cpp',\n>> + 'ccm.cpp',\n>> 'lut.cpp',\n>> ])\n>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>> index 607af45a..0def3eef 100644\n>> --- a/src/ipa/simple/ipa_context.h\n>> +++ b/src/ipa/simple/ipa_context.h\n>> @@ -13,6 +13,8 @@\n>> \n>> #include \n>> \n>> +#include \"libcamera/internal/matrix.h\"\n>> +\n>> #include \n>> \n>> namespace libcamera {\n>> @@ -50,6 +52,13 @@ struct IPAActiveState {\n>> uint8_t blackLevel;\n>> double contrast;\n>> } gamma;\n>> +\n>> + struct {\n>> + Matrix ccm;\n>> + bool enabled;\n>> + bool changed;\n>> + } ccm;\n>> +\n>> struct {\n>> /* 0..2 range, 1.0 = normal */\n>> std::optional contrast;\n>> @@ -57,6 +66,10 @@ struct IPAActiveState {\n>> };\n>> \n>> struct IPAFrameContext : public FrameContext {\n>> + struct {\n>> + Matrix ccm;\n>> + } ccm;\n>> +\n>> struct {\n>> int32_t exposure;\n>> double gain;\n>> -- \n>> 2.44.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 4DD8AC32F9\n\tfor ;\n\tFri, 10 Jan 2025 18:47:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 63D13684E7;\n\tFri, 10 Jan 2025 19:47:51 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4CBE608AA\n\tfor ;\n\tFri, 10 Jan 2025 19:47:48 +0100 (CET)","from mail-wr1-f70.google.com (mail-wr1-f70.google.com\n\t[209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-441-LI-0xIbHPvCdP2A5JPyLGQ-1; Fri, 10 Jan 2025 13:47:46 -0500","by mail-wr1-f70.google.com with SMTP id\n\tffacd0b85a97d-3862e986d17so968119f8f.3\n\tfor ;\n\tFri, 10 Jan 2025 10:47:46 -0800 (PST)","from mzamazal-thinkpadp1gen3.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-38a8e37d17dsm5112772f8f.21.2025.01.10.10.47.42\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 10 Jan 2025 10:47:43 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"HcFqPUC1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1736534867;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=21fz5vJoowQBVkFBh+L1JSuG6ZIX+JXWncSbQFmWs6Y=;\n\tb=HcFqPUC1X0r0Oku2GKRbYdsS3qg+LZSEcQG4ItBuLgoWOGbdAqWyXwRUd5+qO94aQQL5L5\n\thoEqvYzAds0Isc4qH0UjCpw8CIIEjFdAHl92Dcgc1QW1DEH4ZABXGYUYPLVEW20IEFuZgL\n\t6g9JbKbat5nymAg3+qN/KMpwNoA4f+c=","X-MC-Unique":"LI-0xIbHPvCdP2A5JPyLGQ-1","X-Mimecast-MFC-AGG-ID":"LI-0xIbHPvCdP2A5JPyLGQ","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1736534865; x=1737139665;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=21fz5vJoowQBVkFBh+L1JSuG6ZIX+JXWncSbQFmWs6Y=;\n\tb=NqQkVrguC/TzK/mTtkJ4j9jM4DnWmvxEf+k78mS3tQqKxZEug22BrdWQdQhZgdgx4O\n\tCeXvNwlp9f8sdtXfryhPj0r6flmzvQZHY5R+v01+MeyMa0QjiRxogkYzs3wpEM/QyvYS\n\to/TmxBoYDQX7kFdKDTr88tVbiMhcaCqyQZGpIpCul4leYnLbBnTAMFkbFTVsew1fbX50\n\txV3JLfJU/IpA6i2kO7tFgiSNfsAmb97xixRnaUY3Gda8TDPwNHFYXOXZkpBZYoTT44ob\n\tffF+AmM8XKhWJkr2oYA9grQqMWTB7BBbWOCAQOQjk/jl3pQb1L+SDOF708eqP3Eej1Hz\n\t8pBg==","X-Gm-Message-State":"AOJu0Yzl3lHGqsn5Twbv8GAS8vC+LCs7d18A5/zJurwrajKxDVgiSO4c\n\tkOyvN5fy+y6E29rPxXAOMdz5kkZwGw+nqj+eLS+l/wzPLFbicaHQGXD2NtLTscz1N1rz2StR+Ma\n\tBkzwt1NcC0CtGe+2OQlxC9da8ccRHPtlf2SMkDjPyLE+n9ATWwab7Ea/eLMYPhckTvCjmhyk=","X-Gm-Gg":"ASbGncsLnmxOC0YDlnZ8yZARHW11vCCr0tAUgiqEaS3PaDAS7ZEJZqRaQey/QLk4uu6\n\tnweSflRtryf0M9ZXbjHKzZCLnoDwQf8ItOJSfYrWT+z/8BHaLp8leEoDOOEc7xxoShRPm5mya/2\n\tE0wR2JVkfmWtJdRJ03ALzfc73EZaOCYn5ZIRLTfC0wbOJa9kJEQm1m5AT/LPTLKKxCLLJVhPZqV\n\t6JyonUzkMlQj01G7UG9+8EyNlwMUnPxdu/nqlozU4lQ4agoaHYeLI01k/FqRtdwIFPJdwXtsMmI\n\tSj5yhU6k54UkrJTdFfXMfE5hlgC656vgZw==","X-Received":["by 2002:a05:6000:1f85:b0:38a:418e:bee with SMTP id\n\tffacd0b85a97d-38a87336d5cmr9494347f8f.42.1736534865094; \n\tFri, 10 Jan 2025 10:47:45 -0800 (PST)","by 2002:a05:6000:1f85:b0:38a:418e:bee with SMTP id\n\tffacd0b85a97d-38a87336d5cmr9494332f8f.42.1736534864623; \n\tFri, 10 Jan 2025 10:47:44 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHPBGAxy0c2YA8CeHIvIXk4wVfin3fbW2y9insVom/OF/ZP60vHShOScdgxon5/H3fplqKDWQ==","From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Robert Mader\n\t, Hans de Goede ,\n\tLaurent Pinchart ","Subject":"Re: [PATCH v3 6/9] libcamera: software_isp: Add CCM algorithm","In-Reply-To":"<173644245058.2992722.2830487320880500052@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Thu, 09 Jan 2025 17:07:30 +0000\")","References":"<20241210153440.1007470-1-mzamazal@redhat.com>\n\t<20241210153440.1007470-7-mzamazal@redhat.com>\n\t<173644245058.2992722.2830487320880500052@ping.linuxembedded.co.uk>","Date":"Fri, 10 Jan 2025 19:47:42 +0100","Message-ID":"<858qrio69d.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"zlGqtc2JD2Qj6vd6Ro23COnBaNOfpenP7QBFFCd90uI_1736534865","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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\" "}}]