[{"id":29788,"web_url":"https://patchwork.libcamera.org/comment/29788/","msgid":"","date":"2024-06-06T06:25:05","subject":"Re: [PATCH v4 3/3] ipa: rkisp1: Add GammaOutCorrection algorithm","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi,\n\nI missed the changelog on this one. So here it goes...\n\nOn Wed, Jun 05, 2024 at 11:53:51AM +0200, Stefan Klug wrote:\n> Add a gamma algorithm for the rkisp1. It defaults to a gamma of 2.2 which\n> closely resembles sRGB. No seperate sRGB mode was implemented because the pwl\n> that models the gamma curve is so coarse that there is basically no difference\n> between srgb and gamma=2.2. The default can be overridden within the tuning\n> file or set at runtime using the gamma control.\n> \n> The gamma algorithm is not enabled by default. This will be done in future\n> tuning file updates.\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Kieran Bingham \n> ---\n\nv3 -> v4:\n- Changed structure in IPAActiveState and IPAFrameContext to be in line with \n the other algorithms.\n- Changed the logic to check if hw needs updating to a bool inside frameContext \n to be in line with the other algorithms\n\nv2 -> v3:\n- Squashed with patch 1/4 from previous series\n- Renamed from Gamma to GammaOutCorrection\n- Read default gamma value from tuning file\n- Some stylistic fixes from review\n\n> src/ipa/rkisp1/algorithms/goc.cpp | 153 ++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/goc.h | 42 +++++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> src/ipa/rkisp1/ipa_context.h | 9 ++\n> 4 files changed, 205 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/goc.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/goc.h\n> \n> diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp\n> new file mode 100644\n> index 000000000000..ab634c1c9709\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/goc.cpp\n> @@ -0,0 +1,153 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * RkISP1 Gamma out control\n> + */\n> +#include \"goc.h\"\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"linux/rkisp1-config.h\"\n> +\n> +/**\n> + * \\file goc.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class GammaOutCorrection\n> + * \\brief RkISP1 Gamma out correction\n> + *\n> + * This algorithm implements the gamma out curve for the RkISP1.\n> + * It defaults to a gamma value of 2.2\n> + * As gamma is internally represented as a piecewise linear function with only\n> + * 17 knots, the difference between gamma=2.2 and sRGB gamma is minimal.\n> + * Therefore sRGB gamma was not implemented as special case.\n> + *\n> + * Useful links:\n> + * https://www.cambridgeincolour.com/tutorials/gamma-correction.htm\n> + * https://en.wikipedia.org/wiki/SRGB\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Gamma)\n> +\n> +const float kDefaultGamma = 2.2f;\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int GammaOutCorrection::init([[maybe_unused]] IPAContext &context,\n> +\t\t\t [[maybe_unused]] const YamlObject &tuningData)\n> +{\n> +\tif (context.hw->numGammaOutSamples !=\n> +\t RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) {\n> +\t\tLOG(RkISP1Gamma, Error)\n> +\t\t\t<< \"Gamma is not implemented for RkISP1 V12\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcontext.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, kDefaultGamma);\n> +\tdefaultGamma_ = tuningData[\"gamma\"].get(kDefaultGamma);\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Configure the Gamma given a configInfo\n> + * \\param[in] context The shared IPA context\n> + * \\param[in] configInfo The IPA configuration data\n> + *\n> + * \\return 0\n> + */\n> +int GammaOutCorrection::configure(IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> +{\n> +\tcontext.activeState.goc.gamma = defaultGamma_;\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void GammaOutCorrection::queueRequest([[maybe_unused]] IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t IPAFrameContext &frameContext,\n> +\t\t\t\t const ControlList &controls)\n> +{\n> +\tif (frame == 0)\n> +\t\tframeContext.goc.update = true;\n> +\n> +\tconst auto &gamma = controls.get(controls::Gamma);\n> +\tif (gamma) {\n> +\t\tcontext.activeState.goc.gamma = *gamma;\n> +\t\tframeContext.goc.update = true;\n> +\t\tLOG(RkISP1Gamma, Debug) << \"Set gamma to \" << *gamma;\n> +\t}\n> +\n> +\tframeContext.goc.gamma = context.activeState.goc.gamma;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void GammaOutCorrection::prepare([[maybe_unused]] IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t\t\t rkisp1_params_cfg *params)\n> +{\n> +\tASSERT(context.hw->numGammaOutSamples ==\n> +\t RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10);\n> +\n> +\t/*\n> +\t * The logarithmic segments as specified in the reference.\n> +\t * Plus an additional 0 to make the loop easier\n> +\t */\n> +\tstd::array segments = {\n> +\t\t64, 64, 64, 64, 128, 128, 128, 128, 256,\n> +\t\t256, 256, 512, 512, 512, 512, 512, 0\n> +\t};\n> +\tauto gamma_y = params->others.goc_config.gamma_y;\n> +\n> +\tif (!frameContext.goc.update)\n> +\t\treturn;\n> +\n> +\tunsigned x = 0;\n> +\tfor (const auto [i, size] : utils::enumerate(segments)) {\n> +\t\tgamma_y[i] = std::pow(x / 4096.0, 1.0 / frameContext.goc.gamma) * 1023.0;\n> +\t\tx += size;\n> +\t}\n> +\n> +\tparams->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC;\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC;\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC;\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_GOC;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::process\n> + */\n> +void GammaOutCorrection::process([[maybe_unused]] IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t IPAFrameContext &frameContext,\n> +\t\t\t\t [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> +\t\t\t\t ControlList &metadata)\n> +{\n> +\tmetadata.set(controls::Gamma, frameContext.goc.gamma);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(GammaOutCorrection, \"GammaOutCorrection\")\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h\n> new file mode 100644\n> index 000000000000..3be8e500be67\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/goc.h\n> @@ -0,0 +1,42 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * RkISP1 Gamma out control\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class GammaOutCorrection : public Algorithm\n> +{\n> +public:\n> +\tGammaOutCorrection() = default;\n> +\t~GammaOutCorrection() = default;\n> +\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> +\tint configure(IPAContext &context,\n> +\t\t const IPACameraSensorInfo &configInfo) override;\n> +\tvoid queueRequest(IPAContext &context,\n> +\t\t\t const uint32_t frame,\n> +\t\t\t IPAFrameContext &frameContext,\n> +\t\t\t const ControlList &controls) override;\n> +\tvoid prepare(IPAContext &context, const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params) override;\n> +\tvoid process(IPAContext &context, const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t const rkisp1_stat_buffer *stats,\n> +\t\t ControlList &metadata) override;\n> +\n> +private:\n> +\tdouble defaultGamma_;\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 93a483292753..6ee71a9b5da3 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -8,6 +8,7 @@ rkisp1_ipa_algorithms = files([\n> 'dpcc.cpp',\n> 'dpf.cpp',\n> 'filter.cpp',\n> + 'goc.cpp',\n> 'gsl.cpp',\n> 'lsc.cpp',\n> ])\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index bd02a7a24fdd..f261e42fc2fd 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -104,6 +104,10 @@ struct IPAActiveState {\n> \t\tuint8_t denoise;\n> \t\tuint8_t sharpness;\n> \t} filter;\n> +\n> +\tstruct {\n> +\t\tdouble gamma;\n> +\t} goc;\n> };\n> \n> struct IPAFrameContext : public FrameContext {\n> @@ -146,6 +150,11 @@ struct IPAFrameContext : public FrameContext {\n> \t\tuint32_t exposure;\n> \t\tdouble gain;\n> \t} sensor;\n> +\n> +\tstruct {\n> +\t\tdouble gamma;\n> +\t\tbool update;\n> +\t} goc;\n> };\n> \n> struct IPAContext {\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8B295C31E9\n\tfor ;\n\tThu, 6 Jun 2024 06:25:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EEA4865446;\n\tThu, 6 Jun 2024 08:25:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C63E633DD\n\tfor ;\n\tThu, 6 Jun 2024 08:25:08 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7b40:34c7:ae34:72b0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 240A9EAB;\n\tThu, 6 Jun 2024 08:24:59 +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=\"snD5MIlZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1717655099;\n\tbh=Y6awKbHNTwllVzQ4wooSLsgSXZiEAhs3wNwy8r2Os+Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=snD5MIlZ3uk+DYpkkc9gw+4rTIJsHT1paTm028ivbJv/nOt8GFe2ot1pfvqnELIz7\n\tMs5oRD692LPkqxmBgRlrMZeDgFdYXXnk8YktmJwMqROrX2yL9HwkGIiobxbdS79GJv\n\tqVrlBdC3iFGKBHzcWBXmc9wU32XHmeOeu09r30ac=","Date":"Thu, 6 Jun 2024 08:25:05 +0200","From":"Stefan Klug ","To":"libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham ","Subject":"Re: [PATCH v4 3/3] ipa: rkisp1: Add GammaOutCorrection algorithm","Message-ID":"","References":"<20240605095417.157703-1-stefan.klug@ideasonboard.com>\n\t<20240605095417.157703-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240605095417.157703-4-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29804,"web_url":"https://patchwork.libcamera.org/comment/29804/","msgid":"","date":"2024-06-10T12:41:50","subject":"Re: [PATCH v4 3/3] ipa: rkisp1: Add GammaOutCorrection algorithm","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Wed, Jun 05, 2024 at 11:53:51AM +0200, Stefan Klug wrote:\n> Add a gamma algorithm for the rkisp1. It defaults to a gamma of 2.2 which\n> closely resembles sRGB. No seperate sRGB mode was implemented because the pwl\n> that models the gamma curve is so coarse that there is basically no difference\n> between srgb and gamma=2.2. The default can be overridden within the tuning\n> file or set at runtime using the gamma control.\n> \n> The gamma algorithm is not enabled by default. This will be done in future\n> tuning file updates.\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Kieran Bingham \n> ---\n> src/ipa/rkisp1/algorithms/goc.cpp | 153 ++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/goc.h | 42 +++++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> src/ipa/rkisp1/ipa_context.h | 9 ++\n> 4 files changed, 205 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/goc.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/goc.h\n> \n> diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp\n> new file mode 100644\n> index 000000000000..ab634c1c9709\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/goc.cpp\n> @@ -0,0 +1,153 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * RkISP1 Gamma out control\n> + */\n> +#include \"goc.h\"\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"linux/rkisp1-config.h\"\n> +\n> +/**\n> + * \\file goc.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class GammaOutCorrection\n> + * \\brief RkISP1 Gamma out correction\n> + *\n> + * This algorithm implements the gamma out curve for the RkISP1.\n> + * It defaults to a gamma value of 2.2\n> + * As gamma is internally represented as a piecewise linear function with only\n> + * 17 knots, the difference between gamma=2.2 and sRGB gamma is minimal.\n> + * Therefore sRGB gamma was not implemented as special case.\n> + *\n> + * Useful links:\n> + * https://www.cambridgeincolour.com/tutorials/gamma-correction.htm\n> + * https://en.wikipedia.org/wiki/SRGB\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Gamma)\n> +\n> +const float kDefaultGamma = 2.2f;\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int GammaOutCorrection::init([[maybe_unused]] IPAContext &context,\n> +\t\t\t [[maybe_unused]] const YamlObject &tuningData)\n> +{\n> +\tif (context.hw->numGammaOutSamples !=\n> +\t RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) {\n> +\t\tLOG(RkISP1Gamma, Error)\n> +\t\t\t<< \"Gamma is not implemented for RkISP1 V12\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcontext.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, kDefaultGamma);\n> +\tdefaultGamma_ = tuningData[\"gamma\"].get(kDefaultGamma);\n\nI think these should be switched so that the default gamma that's read\nfrom the tuning file will be set as the default gamma in the\nControlInfo. Otherwise the default that's set and reported via metadata\nwouldn't be consistent with the default that's reported in the\nControlInfo.\n\nWith that changed:\n\nReviewed-by: Paul Elder \n\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Configure the Gamma given a configInfo\n> + * \\param[in] context The shared IPA context\n> + * \\param[in] configInfo The IPA configuration data\n> + *\n> + * \\return 0\n> + */\n> +int GammaOutCorrection::configure(IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> +{\n> +\tcontext.activeState.goc.gamma = defaultGamma_;\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void GammaOutCorrection::queueRequest([[maybe_unused]] IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t IPAFrameContext &frameContext,\n> +\t\t\t\t const ControlList &controls)\n> +{\n> +\tif (frame == 0)\n> +\t\tframeContext.goc.update = true;\n> +\n> +\tconst auto &gamma = controls.get(controls::Gamma);\n> +\tif (gamma) {\n> +\t\tcontext.activeState.goc.gamma = *gamma;\n> +\t\tframeContext.goc.update = true;\n> +\t\tLOG(RkISP1Gamma, Debug) << \"Set gamma to \" << *gamma;\n> +\t}\n> +\n> +\tframeContext.goc.gamma = context.activeState.goc.gamma;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void GammaOutCorrection::prepare([[maybe_unused]] IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t\t\t rkisp1_params_cfg *params)\n> +{\n> +\tASSERT(context.hw->numGammaOutSamples ==\n> +\t RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10);\n> +\n> +\t/*\n> +\t * The logarithmic segments as specified in the reference.\n> +\t * Plus an additional 0 to make the loop easier\n> +\t */\n> +\tstd::array segments = {\n> +\t\t64, 64, 64, 64, 128, 128, 128, 128, 256,\n> +\t\t256, 256, 512, 512, 512, 512, 512, 0\n> +\t};\n> +\tauto gamma_y = params->others.goc_config.gamma_y;\n> +\n> +\tif (!frameContext.goc.update)\n> +\t\treturn;\n> +\n> +\tunsigned x = 0;\n> +\tfor (const auto [i, size] : utils::enumerate(segments)) {\n> +\t\tgamma_y[i] = std::pow(x / 4096.0, 1.0 / frameContext.goc.gamma) * 1023.0;\n> +\t\tx += size;\n> +\t}\n> +\n> +\tparams->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC;\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC;\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC;\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_GOC;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::process\n> + */\n> +void GammaOutCorrection::process([[maybe_unused]] IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t IPAFrameContext &frameContext,\n> +\t\t\t\t [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> +\t\t\t\t ControlList &metadata)\n> +{\n> +\tmetadata.set(controls::Gamma, frameContext.goc.gamma);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(GammaOutCorrection, \"GammaOutCorrection\")\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h\n> new file mode 100644\n> index 000000000000..3be8e500be67\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/goc.h\n> @@ -0,0 +1,42 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * RkISP1 Gamma out control\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class GammaOutCorrection : public Algorithm\n> +{\n> +public:\n> +\tGammaOutCorrection() = default;\n> +\t~GammaOutCorrection() = default;\n> +\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> +\tint configure(IPAContext &context,\n> +\t\t const IPACameraSensorInfo &configInfo) override;\n> +\tvoid queueRequest(IPAContext &context,\n> +\t\t\t const uint32_t frame,\n> +\t\t\t IPAFrameContext &frameContext,\n> +\t\t\t const ControlList &controls) override;\n> +\tvoid prepare(IPAContext &context, const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params) override;\n> +\tvoid process(IPAContext &context, const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t const rkisp1_stat_buffer *stats,\n> +\t\t ControlList &metadata) override;\n> +\n> +private:\n> +\tdouble defaultGamma_;\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 93a483292753..6ee71a9b5da3 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -8,6 +8,7 @@ rkisp1_ipa_algorithms = files([\n> 'dpcc.cpp',\n> 'dpf.cpp',\n> 'filter.cpp',\n> + 'goc.cpp',\n> 'gsl.cpp',\n> 'lsc.cpp',\n> ])\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index bd02a7a24fdd..f261e42fc2fd 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -104,6 +104,10 @@ struct IPAActiveState {\n> \t\tuint8_t denoise;\n> \t\tuint8_t sharpness;\n> \t} filter;\n> +\n> +\tstruct {\n> +\t\tdouble gamma;\n> +\t} goc;\n> };\n> \n> struct IPAFrameContext : public FrameContext {\n> @@ -146,6 +150,11 @@ struct IPAFrameContext : public FrameContext {\n> \t\tuint32_t exposure;\n> \t\tdouble gain;\n> \t} sensor;\n> +\n> +\tstruct {\n> +\t\tdouble gamma;\n> +\t\tbool update;\n> +\t} goc;\n> };\n> \n> struct IPAContext {\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3F8CDC31E9\n\tfor ;\n\tMon, 10 Jun 2024 12:42:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 272C465462;\n\tMon, 10 Jun 2024 14:41:59 +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 2B641634D5\n\tfor ;\n\tMon, 10 Jun 2024 14:41:57 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4F48230;\n\tMon, 10 Jun 2024 14:41:43 +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=\"vKf9Uwnj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718023305;\n\tbh=dWCnmFVnAwLfXHkRtvCFimS6tOZ6y2SLG8B/uNy5dAo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vKf9UwnjXl/FwRYkkaVrkf82eLy0lqW/Mh4jUWTew1V8HgrSJg6RCD/dknstAp03S\n\tTmDg+CwtR1YEqrI2EWjIv+YhZwLxmy10hKLSprgQXIvDuUIrn86Wc20i1DT/u4K7L3\n\tfUQklycEtUbVfn1yH/GcO8Ww6mW508MLwk2Y6cdg=","Date":"Mon, 10 Jun 2024 21:41:50 +0900","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ","Subject":"Re: [PATCH v4 3/3] ipa: rkisp1: Add GammaOutCorrection algorithm","Message-ID":"","References":"<20240605095417.157703-1-stefan.klug@ideasonboard.com>\n\t<20240605095417.157703-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240605095417.157703-4-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29806,"web_url":"https://patchwork.libcamera.org/comment/29806/","msgid":"","date":"2024-06-10T12:46:43","subject":"Re: [PATCH v4 3/3] ipa: rkisp1: Add GammaOutCorrection algorithm","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Mon, Jun 10, 2024 at 09:41:50PM +0900, Paul Elder wrote:\n> On Wed, Jun 05, 2024 at 11:53:51AM +0200, Stefan Klug wrote:\n> > Add a gamma algorithm for the rkisp1. It defaults to a gamma of 2.2 which\n> > closely resembles sRGB. No seperate sRGB mode was implemented because the pwl\n> > that models the gamma curve is so coarse that there is basically no difference\n> > between srgb and gamma=2.2. The default can be overridden within the tuning\n> > file or set at runtime using the gamma control.\n> > \n> > The gamma algorithm is not enabled by default. This will be done in future\n> > tuning file updates.\n> > \n> > Signed-off-by: Stefan Klug \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > src/ipa/rkisp1/algorithms/goc.cpp | 153 ++++++++++++++++++++++++++\n> > src/ipa/rkisp1/algorithms/goc.h | 42 +++++++\n> > src/ipa/rkisp1/algorithms/meson.build | 1 +\n> > src/ipa/rkisp1/ipa_context.h | 9 ++\n> > 4 files changed, 205 insertions(+)\n> > create mode 100644 src/ipa/rkisp1/algorithms/goc.cpp\n> > create mode 100644 src/ipa/rkisp1/algorithms/goc.h\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp\n> > new file mode 100644\n> > index 000000000000..ab634c1c9709\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/goc.cpp\n> > @@ -0,0 +1,153 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board\n> > + *\n> > + * RkISP1 Gamma out control\n> > + */\n> > +#include \"goc.h\"\n> > +\n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"linux/rkisp1-config.h\"\n> > +\n> > +/**\n> > + * \\file goc.h\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +/**\n> > + * \\class GammaOutCorrection\n> > + * \\brief RkISP1 Gamma out correction\n> > + *\n> > + * This algorithm implements the gamma out curve for the RkISP1.\n> > + * It defaults to a gamma value of 2.2\n> > + * As gamma is internally represented as a piecewise linear function with only\n> > + * 17 knots, the difference between gamma=2.2 and sRGB gamma is minimal.\n> > + * Therefore sRGB gamma was not implemented as special case.\n> > + *\n> > + * Useful links:\n> > + * https://www.cambridgeincolour.com/tutorials/gamma-correction.htm\n> > + * https://en.wikipedia.org/wiki/SRGB\n> > + */\n> > +\n> > +LOG_DEFINE_CATEGORY(RkISP1Gamma)\n> > +\n> > +const float kDefaultGamma = 2.2f;\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::init\n> > + */\n> > +int GammaOutCorrection::init([[maybe_unused]] IPAContext &context,\n> > +\t\t\t [[maybe_unused]] const YamlObject &tuningData)\n> > +{\n> > +\tif (context.hw->numGammaOutSamples !=\n> > +\t RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) {\n> > +\t\tLOG(RkISP1Gamma, Error)\n> > +\t\t\t<< \"Gamma is not implemented for RkISP1 V12\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tcontext.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, kDefaultGamma);\n> > +\tdefaultGamma_ = tuningData[\"gamma\"].get(kDefaultGamma);\n> \n> I think these should be switched so that the default gamma that's read\n> from the tuning file will be set as the default gamma in the\n> ControlInfo. Otherwise the default that's set and reported via metadata\n> wouldn't be consistent with the default that's reported in the\n> ControlInfo.\n\nOh, good catch. I'll fix that. \nThank you.\n\n> \n> With that changed:\n> \n> Reviewed-by: Paul Elder \n> \n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Configure the Gamma given a configInfo\n> > + * \\param[in] context The shared IPA context\n> > + * \\param[in] configInfo The IPA configuration data\n> > + *\n> > + * \\return 0\n> > + */\n> > +int GammaOutCorrection::configure(IPAContext &context,\n> > +\t\t\t\t [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> > +{\n> > +\tcontext.activeState.goc.gamma = defaultGamma_;\n> > +\treturn 0;\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > + */\n> > +void GammaOutCorrection::queueRequest([[maybe_unused]] IPAContext &context,\n> > +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> > +\t\t\t\t IPAFrameContext &frameContext,\n> > +\t\t\t\t const ControlList &controls)\n> > +{\n> > +\tif (frame == 0)\n> > +\t\tframeContext.goc.update = true;\n> > +\n> > +\tconst auto &gamma = controls.get(controls::Gamma);\n> > +\tif (gamma) {\n> > +\t\tcontext.activeState.goc.gamma = *gamma;\n> > +\t\tframeContext.goc.update = true;\n> > +\t\tLOG(RkISP1Gamma, Debug) << \"Set gamma to \" << *gamma;\n> > +\t}\n> > +\n> > +\tframeContext.goc.gamma = context.activeState.goc.gamma;\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::prepare\n> > + */\n> > +void GammaOutCorrection::prepare([[maybe_unused]] IPAContext &context,\n> > +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> > +\t\t\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> > +\t\t\t\t rkisp1_params_cfg *params)\n> > +{\n> > +\tASSERT(context.hw->numGammaOutSamples ==\n> > +\t RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10);\n> > +\n> > +\t/*\n> > +\t * The logarithmic segments as specified in the reference.\n> > +\t * Plus an additional 0 to make the loop easier\n> > +\t */\n> > +\tstd::array segments = {\n> > +\t\t64, 64, 64, 64, 128, 128, 128, 128, 256,\n> > +\t\t256, 256, 512, 512, 512, 512, 512, 0\n> > +\t};\n> > +\tauto gamma_y = params->others.goc_config.gamma_y;\n> > +\n> > +\tif (!frameContext.goc.update)\n> > +\t\treturn;\n> > +\n> > +\tunsigned x = 0;\n> > +\tfor (const auto [i, size] : utils::enumerate(segments)) {\n> > +\t\tgamma_y[i] = std::pow(x / 4096.0, 1.0 / frameContext.goc.gamma) * 1023.0;\n> > +\t\tx += size;\n> > +\t}\n> > +\n> > +\tparams->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC;\n> > +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC;\n> > +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC;\n> > +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_GOC;\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::process\n> > + */\n> > +void GammaOutCorrection::process([[maybe_unused]] IPAContext &context,\n> > +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> > +\t\t\t\t IPAFrameContext &frameContext,\n> > +\t\t\t\t [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> > +\t\t\t\t ControlList &metadata)\n> > +{\n> > +\tmetadata.set(controls::Gamma, frameContext.goc.gamma);\n> > +}\n> > +\n> > +REGISTER_IPA_ALGORITHM(GammaOutCorrection, \"GammaOutCorrection\")\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h\n> > new file mode 100644\n> > index 000000000000..3be8e500be67\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/goc.h\n> > @@ -0,0 +1,42 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board\n> > + *\n> > + * RkISP1 Gamma out control\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +class GammaOutCorrection : public Algorithm\n> > +{\n> > +public:\n> > +\tGammaOutCorrection() = default;\n> > +\t~GammaOutCorrection() = default;\n> > +\n> > +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> > +\tint configure(IPAContext &context,\n> > +\t\t const IPACameraSensorInfo &configInfo) override;\n> > +\tvoid queueRequest(IPAContext &context,\n> > +\t\t\t const uint32_t frame,\n> > +\t\t\t IPAFrameContext &frameContext,\n> > +\t\t\t const ControlList &controls) override;\n> > +\tvoid prepare(IPAContext &context, const uint32_t frame,\n> > +\t\t IPAFrameContext &frameContext,\n> > +\t\t rkisp1_params_cfg *params) override;\n> > +\tvoid process(IPAContext &context, const uint32_t frame,\n> > +\t\t IPAFrameContext &frameContext,\n> > +\t\t const rkisp1_stat_buffer *stats,\n> > +\t\t ControlList &metadata) override;\n> > +\n> > +private:\n> > +\tdouble defaultGamma_;\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 93a483292753..6ee71a9b5da3 100644\n> > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > @@ -8,6 +8,7 @@ rkisp1_ipa_algorithms = files([\n> > 'dpcc.cpp',\n> > 'dpf.cpp',\n> > 'filter.cpp',\n> > + 'goc.cpp',\n> > 'gsl.cpp',\n> > 'lsc.cpp',\n> > ])\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index bd02a7a24fdd..f261e42fc2fd 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -104,6 +104,10 @@ struct IPAActiveState {\n> > \t\tuint8_t denoise;\n> > \t\tuint8_t sharpness;\n> > \t} filter;\n> > +\n> > +\tstruct {\n> > +\t\tdouble gamma;\n> > +\t} goc;\n> > };\n> > \n> > struct IPAFrameContext : public FrameContext {\n> > @@ -146,6 +150,11 @@ struct IPAFrameContext : public FrameContext {\n> > \t\tuint32_t exposure;\n> > \t\tdouble gain;\n> > \t} sensor;\n> > +\n> > +\tstruct {\n> > +\t\tdouble gamma;\n> > +\t\tbool update;\n> > +\t} goc;\n> > };\n> > \n> > struct IPAContext {\n> > -- \n> > 2.43.0\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C1756C32CF\n\tfor ;\n\tMon, 10 Jun 2024 12:46:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4526865468;\n\tMon, 10 Jun 2024 14:46:49 +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 2E1E96545E\n\tfor ;\n\tMon, 10 Jun 2024 14:46:47 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:5729:5358:82cd:263f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 05EED230;\n\tMon, 10 Jun 2024 14:46: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=\"tfk11oxT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718023595;\n\tbh=bTB2gMlJWxpkCkEDLxOipS8cnGRwwqQKgRgWd5BKoRU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tfk11oxTTPYuZ/mTZj31fNe3Aan207YpSuaZyy0EXfCy8lCi9VH4XBwwok32CZnW9\n\tbv1WqxyxZIDqIr+R12zxj3j8So64VCsRi6v4IBdARM2l0lfasSo8pK8QFZonBEN8xq\n\twYdU6SnUUql1eHhbcahpGARqTz++Bdl5KDfvn30w=","Date":"Mon, 10 Jun 2024 14:46:43 +0200","From":"Stefan Klug ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, \n\tKieran Bingham ","Subject":"Re: [PATCH v4 3/3] ipa: rkisp1: Add GammaOutCorrection algorithm","Message-ID":"","References":"<20240605095417.157703-1-stefan.klug@ideasonboard.com>\n\t<20240605095417.157703-4-stefan.klug@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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\" "}}]