[{"id":24209,"web_url":"https://patchwork.libcamera.org/comment/24209/","msgid":"<20220728075149.x3f5qkaqnbyesgcw@uno.localdomain>","date":"2022-07-28T07:51:49","subject":"Re: [libcamera-devel] [PATCH v5 8/9] ipa: rkisp1: Add support of\n\tFilter control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Thu, Jul 28, 2022 at 01:21:48AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> From: Florian Sylvestre \n>\n> Denoise and Sharpness filters will be applied by RkISP1 during the\n> demosaicing step. The denoise filter is responsible for removing noise from\n> the image, while the sharpness filter will enhance its acutance.\n>\n> Add filter algorithm with denoise and sharpness values based on user controls.\n>\n> Signed-off-by: Florian Sylvestre \n> Reviewed-by: Laurent Pinchart \n> ---\n> src/ipa/rkisp1/algorithms/filter.cpp | 201 +++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/filter.h | 30 ++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> src/ipa/rkisp1/data/ov5640.yaml | 1 +\n> src/ipa/rkisp1/ipa_context.cpp | 14 ++\n> src/ipa/rkisp1/ipa_context.h | 6 +\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +\n> 7 files changed, 261 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/filter.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/filter.h\n>\n> diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp\n> new file mode 100644\n> index 000000000000..8ca10fd1ee9d\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/filter.cpp\n> @@ -0,0 +1,201 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021-2022, Ideas On Board\n> + *\n> + * filter.cpp - RkISP1 Filter control\n> + */\n> +\n> +#include \"filter.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +/**\n> + * \\file filter.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class Filter\n> + * \\brief RkISP1 Filter control\n> + *\n> + * Denoise and Sharpness filters will be applied by RkISP1 during the\n> + * demosaicing step. The denoise filter is responsible for removing noise from\n> + * the image, while the sharpness filter will enhance its acutance.\n> + *\n> + * \\todo In current version the denoise and sharpness control is based on user\n> + * controls. In a future version it should be controlled automatically by the\n> + * algorithm.\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Filter)\n> +\n> +static constexpr uint32_t kFiltLumWeightDefault = 0x00022040;\n> +static constexpr uint32_t kFiltModeDefault = 0x000004f2;\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void Filter::queueRequest(IPAContext &context,\n> +\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t const ControlList &controls)\n> +{\n> +\tauto &filter = context.frameContext.filter;\n> +\n> +\tconst auto &sharpness = controls.get(controls::Sharpness);\n> +\tif (sharpness) {\n> +\t\tfilter.sharpness = std::round(std::clamp(*sharpness, 0.0f, 10.0f));\n> +\t\tfilter.updateParams = true;\n> +\n> +\t\tLOG(RkISP1Filter, Debug) << \"Set sharpness to \" << *sharpness;\n> +\t}\n> +\n> +\tconst auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> +\tif (denoise) {\n> +\t\tLOG(RkISP1Filter, Debug) << \"Set denoise to \" << *denoise;\n> +\n> +\t\tswitch (*denoise) {\n> +\t\tcase controls::draft::NoiseReductionModeOff:\n> +\t\t\tfilter.denoise = 0;\n> +\t\t\tfilter.updateParams = true;\n> +\t\t\tbreak;\n> +\t\tcase controls::draft::NoiseReductionModeMinimal:\n> +\t\t\tfilter.denoise = 1;\n> +\t\t\tfilter.updateParams = true;\n> +\t\t\tbreak;\n> +\t\tcase controls::draft::NoiseReductionModeHighQuality:\n> +\t\tcase controls::draft::NoiseReductionModeFast:\n> +\t\t\tfilter.denoise = 3;\n> +\t\t\tfilter.updateParams = true;\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tLOG(RkISP1Filter, Error)\n> +\t\t\t\t<< \"Unsupported denoise value \"\n> +\t\t\t\t<< *denoise;\n> +\t\t}\n> +\t}\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> +{\n> +\tauto &filter = context.frameContext.filter;\n> +\n> +\t/* Check if the algorithm configuration has been updated. */\n> +\tif (!filter.updateParams)\n> +\t\treturn;\n> +\n> +\tfilter.updateParams = false;\n> +\n> +\tstatic constexpr uint16_t filt_fac_sh0[] = {\n> +\t\t0x04, 0x07, 0x0a, 0x0c, 0x10, 0x14, 0x1a, 0x1e, 0x24, 0x2a, 0x30\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_fac_sh1[] = {\n> +\t\t0x04, 0x08, 0x0c, 0x10, 0x16, 0x1b, 0x20, 0x26, 0x2c, 0x30, 0x3f\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_fac_mid[] = {\n> +\t\t0x04, 0x06, 0x08, 0x0a, 0x0c, 0x10, 0x13, 0x17, 0x1d, 0x22, 0x28\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_fac_bl0[] = {\n> +\t\t0x02, 0x02, 0x04, 0x06, 0x08, 0x0a, 0x0c, 0x10, 0x15, 0x1a, 0x24\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_fac_bl1[] = {\n> +\t\t0x00, 0x00, 0x00, 0x02, 0x04, 0x04, 0x06, 0x08, 0x0d, 0x14, 0x20\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_thresh_sh0[] = {\n> +\t\t0, 18, 26, 36, 41, 75, 90, 120, 170, 250, 1023\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_thresh_sh1[] = {\n> +\t\t0, 33, 44, 51, 67, 100, 120, 150, 200, 300, 1023\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_thresh_bl0[] = {\n> +\t\t0, 8, 13, 23, 26, 50, 60, 80, 140, 180, 1023\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_thresh_bl1[] = {\n> +\t\t0, 2, 5, 10, 15, 20, 26, 51, 100, 150, 1023\n> +\t};\n> +\n> +\tstatic constexpr uint16_t stage1_select[] = {\n> +\t\t6, 6, 4, 4, 3, 3, 2, 2, 2, 1, 0\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_chr_v_mode[] = {\n> +\t\t1, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3\n> +\t};\n> +\n> +\tstatic constexpr uint16_t filt_chr_h_mode[] = {\n> +\t\t0, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3\n> +\t};\n> +\n> +\tuint8_t denoise = filter.denoise;\n> +\tuint8_t sharpness = filter.sharpness;\n> +\tauto &flt_config = params->others.flt_config;\n> +\n> +\tflt_config.fac_sh0 = filt_fac_sh0[sharpness];\n> +\tflt_config.fac_sh1 = filt_fac_sh1[sharpness];\n> +\tflt_config.fac_mid = filt_fac_mid[sharpness];\n> +\tflt_config.fac_bl0 = filt_fac_bl0[sharpness];\n> +\tflt_config.fac_bl1 = filt_fac_bl1[sharpness];\n> +\n> +\tflt_config.lum_weight = kFiltLumWeightDefault;\n> +\tflt_config.mode = kFiltModeDefault;\n> +\tflt_config.thresh_sh0 = filt_thresh_sh0[denoise];\n> +\tflt_config.thresh_sh1 = filt_thresh_sh1[denoise];\n> +\tflt_config.thresh_bl0 = filt_thresh_bl0[denoise];\n> +\tflt_config.thresh_bl1 = filt_thresh_bl1[denoise];\n> +\tflt_config.grn_stage1 = stage1_select[denoise];\n> +\tflt_config.chr_v_mode = filt_chr_v_mode[denoise];\n> +\tflt_config.chr_h_mode = filt_chr_h_mode[denoise];\n> +\n> +\t/*\n> +\t * Combined high denoising and high sharpening requires some\n> +\t * adjustments to the configuration of the filters. A first stage\n> +\t * filter with a lower strength must be selected, and the blur factors\n> +\t * must be decreased.\n> +\t */\n> +\tif (denoise == 9) {\n> +\t\tif (sharpness > 3)\n> +\t\t\tflt_config.grn_stage1 = 2;\n> +\t} else if (denoise == 10) {\n> +\t\tif (sharpness > 5)\n> +\t\t\tflt_config.grn_stage1 = 2;\n> +\t\telse if (sharpness > 3)\n> +\t\t\tflt_config.grn_stage1 = 1;\n> +\t}\n> +\n> +\tif (denoise > 7) {\n> +\t\tif (sharpness > 7) {\n> +\t\t\tflt_config.fac_bl0 /= 2;\n> +\t\t\tflt_config.fac_bl1 /= 4;\n> +\t\t} else if (sharpness > 4) {\n> +\t\t\tflt_config.fac_bl0 = flt_config.fac_bl0 * 3 / 4;\n> +\t\t\tflt_config.fac_bl1 /= 2;\n> +\t\t}\n> +\t}\n> +\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_FLT;\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_FLT;\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_FLT;\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Filter, \"Filter\")\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/filter.h b/src/ipa/rkisp1/algorithms/filter.h\n> new file mode 100644\n> index 000000000000..9eb170eb7da1\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/filter.h\n> @@ -0,0 +1,30 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021-2022, Ideas On Board\n> + *\n> + * filter.h - RkISP1 Filter control\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class Filter : public Algorithm\n> +{\n> +public:\n> +\tFilter() = default;\n> +\t~Filter() = default;\n> +\n> +\tvoid queueRequest(IPAContext &context, const uint32_t frame,\n> +\t\t\t const ControlList &controls) override;\n> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\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 870074939d6a..dcd24fe07d23 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([\n> 'awb.cpp',\n> 'blc.cpp',\n> 'dpcc.cpp',\n> + 'filter.cpp',\n> 'gsl.cpp',\n> 'lsc.cpp',\n> ])\n> diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml\n> index 2315ec437f77..99529481f593 100644\n> --- a/src/ipa/rkisp1/data/ov5640.yaml\n> +++ b/src/ipa/rkisp1/data/ov5640.yaml\n> @@ -155,4 +155,5 @@ algorithms:\n> rnd-offsets:\n> green: 2\n> red-blue: 2\n> + - Filter:\n> ...\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 30bb87a967bd..4b1171865f1f 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -136,6 +136,20 @@ namespace libcamera::ipa::rkisp1 {\n> * \\brief Estimated color temperature\n> */\n>\n> +/**\n> + * \\var IPAFrameContext::filter\n> + * \\brief Context for the Filter algorithm\n> + *\n> + * \\struct IPAFrameContext::filter.denoise\n> + * \\brief Denoising level\n> + *\n> + * \\var IPAFrameContext::filter.sharpness\n> + * \\brief Sharpness level\n> + *\n> + * \\var IPAFrameContext::filter.updateParams\n> + * \\brief Indicates if ISP parameters need to be updated\n> + */\n> +\n> /**\n> * \\var IPAFrameContext::sensor\n> * \\brief Effective sensor values\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 3bfb262c8eb3..3b2f6af1276f 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -57,6 +57,12 @@ struct IPAFrameContext {\n> \t\tdouble temperatureK;\n> \t} awb;\n>\n> +\tstruct {\n> +\t\tuint8_t denoise;\n> +\t\tuint8_t sharpness;\n> +\t\tbool updateParams;\n> +\t} filter;\n> +\n> \tstruct {\n> \t\tuint32_t exposure;\n> \t\tdouble gain;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 99eecd444f95..4e000d3122fb 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -968,6 +968,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \t\t\t\t\t\t hasSelfPath_ ? &selfPath_ : nullptr);\n>\n> \tControlInfoMap::Map ctrls;\n> +\tctrls.emplace(std::piecewise_construct,\n> +\t\t std::forward_as_tuple(&controls::Sharpness),\n> +\t\t std::forward_as_tuple(0.0f, 10.0f, 1.0f));\n> +\n> +\tctrls.emplace(std::piecewise_construct,\n> +\t\t std::forward_as_tuple(&controls::draft::NoiseReductionMode),\n> +\t\t std::forward_as_tuple(controls::draft::NoiseReductionModeValues));\n> +\n\nShouldn't these be registered by the IPA ?\n\nI see below AeEnable is also registered here and it should be moved\nthere as well, so I guess it will have to be done on top ?\n\n> \tctrls.emplace(std::piecewise_construct,\n> \t\t std::forward_as_tuple(&controls::AeEnable),\n> \t\t std::forward_as_tuple(false, true));\n> --\n> Regards,\n>\n> Laurent Pinchart\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 52EA1BE173\n\tfor ;\n\tThu, 28 Jul 2022 07:51:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A37FE63311;\n\tThu, 28 Jul 2022 09:51:54 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 357E1603EB\n\tfor ;\n\tThu, 28 Jul 2022 09:51:53 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id E76551BF208;\n\tThu, 28 Jul 2022 07:51:51 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658994714;\n\tbh=HC7syf7ucP65+7L/l4u8Y0x05oT9hsCTZx34MwgSCUc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Bb14CXC0o/RKluuJRGn58a9ew9ktBas4DltzWY3V0bKz5wB7ZAkrKoCTLIDLNm0Mm\n\tKM5y+9QxuBLRrF130MApqjhQSfzDFsAWC7FNTPMdhwaUyzKWpF8NjJeQiXn13nvxMQ\n\tNG1YLLy68h19J4x1Ak3o36XBTHysHecAVmpkpEN7hAcHbASQe2YDEgIxj9ixHtiOIv\n\tdLr0+nOvTI9FNm+LB7frr2kRhhaWDhfOlyMcwXofvMUbCLUMzkBM8EfTyPGrRAlNbA\n\tQS5JPKMskWYwdGneP44oG7eFHzipts5QPCCZOX6roEtyYSPn54GhdlSN5OqtmMxXlV\n\tBL1baoLMU06yQ==","Date":"Thu, 28 Jul 2022 09:51:49 +0200","To":"Laurent Pinchart ","Message-ID":"<20220728075149.x3f5qkaqnbyesgcw@uno.localdomain>","References":"<20220727222149.30627-1-laurent.pinchart@ideasonboard.com>\n\t<20220727222149.30627-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220727222149.30627-9-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 8/9] ipa: rkisp1: Add support of\n\tFilter control","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24215,"web_url":"https://patchwork.libcamera.org/comment/24215/","msgid":"","date":"2022-07-28T10:29:08","subject":"Re: [libcamera-devel] [PATCH v5 8/9] ipa: rkisp1: Add support of\n\tFilter control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jul 28, 2022 at 09:51:49AM +0200, Jacopo Mondi wrote:\n> On Thu, Jul 28, 2022 at 01:21:48AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > From: Florian Sylvestre \n> >\n> > Denoise and Sharpness filters will be applied by RkISP1 during the\n> > demosaicing step. The denoise filter is responsible for removing noise from\n> > the image, while the sharpness filter will enhance its acutance.\n> >\n> > Add filter algorithm with denoise and sharpness values based on user controls.\n> >\n> > Signed-off-by: Florian Sylvestre \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > src/ipa/rkisp1/algorithms/filter.cpp | 201 +++++++++++++++++++++++\n> > src/ipa/rkisp1/algorithms/filter.h | 30 ++++\n> > src/ipa/rkisp1/algorithms/meson.build | 1 +\n> > src/ipa/rkisp1/data/ov5640.yaml | 1 +\n> > src/ipa/rkisp1/ipa_context.cpp | 14 ++\n> > src/ipa/rkisp1/ipa_context.h | 6 +\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +\n> > 7 files changed, 261 insertions(+)\n> > create mode 100644 src/ipa/rkisp1/algorithms/filter.cpp\n> > create mode 100644 src/ipa/rkisp1/algorithms/filter.h\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp\n> > new file mode 100644\n> > index 000000000000..8ca10fd1ee9d\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/filter.cpp\n> > @@ -0,0 +1,201 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021-2022, Ideas On Board\n> > + *\n> > + * filter.cpp - RkISP1 Filter control\n> > + */\n> > +\n> > +#include \"filter.h\"\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +/**\n> > + * \\file filter.h\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +/**\n> > + * \\class Filter\n> > + * \\brief RkISP1 Filter control\n> > + *\n> > + * Denoise and Sharpness filters will be applied by RkISP1 during the\n> > + * demosaicing step. The denoise filter is responsible for removing noise from\n> > + * the image, while the sharpness filter will enhance its acutance.\n> > + *\n> > + * \\todo In current version the denoise and sharpness control is based on user\n> > + * controls. In a future version it should be controlled automatically by the\n> > + * algorithm.\n> > + */\n> > +\n> > +LOG_DEFINE_CATEGORY(RkISP1Filter)\n> > +\n> > +static constexpr uint32_t kFiltLumWeightDefault = 0x00022040;\n> > +static constexpr uint32_t kFiltModeDefault = 0x000004f2;\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > + */\n> > +void Filter::queueRequest(IPAContext &context,\n> > +\t\t\t [[maybe_unused]] const uint32_t frame,\n> > +\t\t\t const ControlList &controls)\n> > +{\n> > +\tauto &filter = context.frameContext.filter;\n> > +\n> > +\tconst auto &sharpness = controls.get(controls::Sharpness);\n> > +\tif (sharpness) {\n> > +\t\tfilter.sharpness = std::round(std::clamp(*sharpness, 0.0f, 10.0f));\n> > +\t\tfilter.updateParams = true;\n> > +\n> > +\t\tLOG(RkISP1Filter, Debug) << \"Set sharpness to \" << *sharpness;\n> > +\t}\n> > +\n> > +\tconst auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n> > +\tif (denoise) {\n> > +\t\tLOG(RkISP1Filter, Debug) << \"Set denoise to \" << *denoise;\n> > +\n> > +\t\tswitch (*denoise) {\n> > +\t\tcase controls::draft::NoiseReductionModeOff:\n> > +\t\t\tfilter.denoise = 0;\n> > +\t\t\tfilter.updateParams = true;\n> > +\t\t\tbreak;\n> > +\t\tcase controls::draft::NoiseReductionModeMinimal:\n> > +\t\t\tfilter.denoise = 1;\n> > +\t\t\tfilter.updateParams = true;\n> > +\t\t\tbreak;\n> > +\t\tcase controls::draft::NoiseReductionModeHighQuality:\n> > +\t\tcase controls::draft::NoiseReductionModeFast:\n> > +\t\t\tfilter.denoise = 3;\n> > +\t\t\tfilter.updateParams = true;\n> > +\t\t\tbreak;\n> > +\t\tdefault:\n> > +\t\t\tLOG(RkISP1Filter, Error)\n> > +\t\t\t\t<< \"Unsupported denoise value \"\n> > +\t\t\t\t<< *denoise;\n> > +\t\t}\n> > +\t}\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::prepare\n> > + */\n> > +void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> > +{\n> > +\tauto &filter = context.frameContext.filter;\n> > +\n> > +\t/* Check if the algorithm configuration has been updated. */\n> > +\tif (!filter.updateParams)\n> > +\t\treturn;\n> > +\n> > +\tfilter.updateParams = false;\n> > +\n> > +\tstatic constexpr uint16_t filt_fac_sh0[] = {\n> > +\t\t0x04, 0x07, 0x0a, 0x0c, 0x10, 0x14, 0x1a, 0x1e, 0x24, 0x2a, 0x30\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_fac_sh1[] = {\n> > +\t\t0x04, 0x08, 0x0c, 0x10, 0x16, 0x1b, 0x20, 0x26, 0x2c, 0x30, 0x3f\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_fac_mid[] = {\n> > +\t\t0x04, 0x06, 0x08, 0x0a, 0x0c, 0x10, 0x13, 0x17, 0x1d, 0x22, 0x28\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_fac_bl0[] = {\n> > +\t\t0x02, 0x02, 0x04, 0x06, 0x08, 0x0a, 0x0c, 0x10, 0x15, 0x1a, 0x24\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_fac_bl1[] = {\n> > +\t\t0x00, 0x00, 0x00, 0x02, 0x04, 0x04, 0x06, 0x08, 0x0d, 0x14, 0x20\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_thresh_sh0[] = {\n> > +\t\t0, 18, 26, 36, 41, 75, 90, 120, 170, 250, 1023\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_thresh_sh1[] = {\n> > +\t\t0, 33, 44, 51, 67, 100, 120, 150, 200, 300, 1023\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_thresh_bl0[] = {\n> > +\t\t0, 8, 13, 23, 26, 50, 60, 80, 140, 180, 1023\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_thresh_bl1[] = {\n> > +\t\t0, 2, 5, 10, 15, 20, 26, 51, 100, 150, 1023\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t stage1_select[] = {\n> > +\t\t6, 6, 4, 4, 3, 3, 2, 2, 2, 1, 0\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_chr_v_mode[] = {\n> > +\t\t1, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3\n> > +\t};\n> > +\n> > +\tstatic constexpr uint16_t filt_chr_h_mode[] = {\n> > +\t\t0, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3\n> > +\t};\n> > +\n> > +\tuint8_t denoise = filter.denoise;\n> > +\tuint8_t sharpness = filter.sharpness;\n> > +\tauto &flt_config = params->others.flt_config;\n> > +\n> > +\tflt_config.fac_sh0 = filt_fac_sh0[sharpness];\n> > +\tflt_config.fac_sh1 = filt_fac_sh1[sharpness];\n> > +\tflt_config.fac_mid = filt_fac_mid[sharpness];\n> > +\tflt_config.fac_bl0 = filt_fac_bl0[sharpness];\n> > +\tflt_config.fac_bl1 = filt_fac_bl1[sharpness];\n> > +\n> > +\tflt_config.lum_weight = kFiltLumWeightDefault;\n> > +\tflt_config.mode = kFiltModeDefault;\n> > +\tflt_config.thresh_sh0 = filt_thresh_sh0[denoise];\n> > +\tflt_config.thresh_sh1 = filt_thresh_sh1[denoise];\n> > +\tflt_config.thresh_bl0 = filt_thresh_bl0[denoise];\n> > +\tflt_config.thresh_bl1 = filt_thresh_bl1[denoise];\n> > +\tflt_config.grn_stage1 = stage1_select[denoise];\n> > +\tflt_config.chr_v_mode = filt_chr_v_mode[denoise];\n> > +\tflt_config.chr_h_mode = filt_chr_h_mode[denoise];\n> > +\n> > +\t/*\n> > +\t * Combined high denoising and high sharpening requires some\n> > +\t * adjustments to the configuration of the filters. A first stage\n> > +\t * filter with a lower strength must be selected, and the blur factors\n> > +\t * must be decreased.\n> > +\t */\n> > +\tif (denoise == 9) {\n> > +\t\tif (sharpness > 3)\n> > +\t\t\tflt_config.grn_stage1 = 2;\n> > +\t} else if (denoise == 10) {\n> > +\t\tif (sharpness > 5)\n> > +\t\t\tflt_config.grn_stage1 = 2;\n> > +\t\telse if (sharpness > 3)\n> > +\t\t\tflt_config.grn_stage1 = 1;\n> > +\t}\n> > +\n> > +\tif (denoise > 7) {\n> > +\t\tif (sharpness > 7) {\n> > +\t\t\tflt_config.fac_bl0 /= 2;\n> > +\t\t\tflt_config.fac_bl1 /= 4;\n> > +\t\t} else if (sharpness > 4) {\n> > +\t\t\tflt_config.fac_bl0 = flt_config.fac_bl0 * 3 / 4;\n> > +\t\t\tflt_config.fac_bl1 /= 2;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_FLT;\n> > +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_FLT;\n> > +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_FLT;\n> > +}\n> > +\n> > +REGISTER_IPA_ALGORITHM(Filter, \"Filter\")\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/filter.h b/src/ipa/rkisp1/algorithms/filter.h\n> > new file mode 100644\n> > index 000000000000..9eb170eb7da1\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/filter.h\n> > @@ -0,0 +1,30 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021-2022, Ideas On Board\n> > + *\n> > + * filter.h - RkISP1 Filter control\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +class Filter : public Algorithm\n> > +{\n> > +public:\n> > +\tFilter() = default;\n> > +\t~Filter() = default;\n> > +\n> > +\tvoid queueRequest(IPAContext &context, const uint32_t frame,\n> > +\t\t\t const ControlList &controls) override;\n> > +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\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 870074939d6a..dcd24fe07d23 100644\n> > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([\n> > 'awb.cpp',\n> > 'blc.cpp',\n> > 'dpcc.cpp',\n> > + 'filter.cpp',\n> > 'gsl.cpp',\n> > 'lsc.cpp',\n> > ])\n> > diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml\n> > index 2315ec437f77..99529481f593 100644\n> > --- a/src/ipa/rkisp1/data/ov5640.yaml\n> > +++ b/src/ipa/rkisp1/data/ov5640.yaml\n> > @@ -155,4 +155,5 @@ algorithms:\n> > rnd-offsets:\n> > green: 2\n> > red-blue: 2\n> > + - Filter:\n> > ...\n> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > index 30bb87a967bd..4b1171865f1f 100644\n> > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > @@ -136,6 +136,20 @@ namespace libcamera::ipa::rkisp1 {\n> > * \\brief Estimated color temperature\n> > */\n> >\n> > +/**\n> > + * \\var IPAFrameContext::filter\n> > + * \\brief Context for the Filter algorithm\n> > + *\n> > + * \\struct IPAFrameContext::filter.denoise\n> > + * \\brief Denoising level\n> > + *\n> > + * \\var IPAFrameContext::filter.sharpness\n> > + * \\brief Sharpness level\n> > + *\n> > + * \\var IPAFrameContext::filter.updateParams\n> > + * \\brief Indicates if ISP parameters need to be updated\n> > + */\n> > +\n> > /**\n> > * \\var IPAFrameContext::sensor\n> > * \\brief Effective sensor values\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 3bfb262c8eb3..3b2f6af1276f 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -57,6 +57,12 @@ struct IPAFrameContext {\n> > \t\tdouble temperatureK;\n> > \t} awb;\n> >\n> > +\tstruct {\n> > +\t\tuint8_t denoise;\n> > +\t\tuint8_t sharpness;\n> > +\t\tbool updateParams;\n> > +\t} filter;\n> > +\n> > \tstruct {\n> > \t\tuint32_t exposure;\n> > \t\tdouble gain;\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 99eecd444f95..4e000d3122fb 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -968,6 +968,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > \t\t\t\t\t\t hasSelfPath_ ? &selfPath_ : nullptr);\n> >\n> > \tControlInfoMap::Map ctrls;\n> > +\tctrls.emplace(std::piecewise_construct,\n> > +\t\t std::forward_as_tuple(&controls::Sharpness),\n> > +\t\t std::forward_as_tuple(0.0f, 10.0f, 1.0f));\n> > +\n> > +\tctrls.emplace(std::piecewise_construct,\n> > +\t\t std::forward_as_tuple(&controls::draft::NoiseReductionMode),\n> > +\t\t std::forward_as_tuple(controls::draft::NoiseReductionModeValues));\n> > +\n> \n> Shouldn't these be registered by the IPA ?\n> \n> I see below AeEnable is also registered here and it should be moved\n> there as well, so I guess it will have to be done on top ?\n\nYes, they both should move to the IPA. I've told Florian I would handle\nthis on top indeed, to avoid delaying the implementation of the\nalgorithms. I'll get to it shortly.\n\n> > \tctrls.emplace(std::piecewise_construct,\n> > \t\t std::forward_as_tuple(&controls::AeEnable),\n> > \t\t std::forward_as_tuple(false, true));","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 02334C3275\n\tfor ;\n\tThu, 28 Jul 2022 10:29:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AEB3C63312;\n\tThu, 28 Jul 2022 12:29:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7D5DA6330D\n\tfor ;\n\tThu, 28 Jul 2022 12:29:11 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 163FA56D;\n\tThu, 28 Jul 2022 12:29:11 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659004152;\n\tbh=Pgk0I2DFoiPm/DvO6LMbo/HN/aRTmEJM6Chh4xUnTiU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=NvVXQcqKcaRzMXHku3Y5YWAcC+bbYc/Zecr60gncFzy2lusJanB5OVvzSioH3H5cB\n\toksqsrPr084bonkWeFLUbB8/ikgVVcXhf9uFd6w7CweIMPoh7Fb/UgKqYjO0XknnIp\n\tC7O2E2xS+I3CEWn5cQWXWKD4M7IK1B6QlTvW0O6hqryVT/tIi9e5YU0zUbu2eXgVOV\n\tQRCZiR/zqs73eQmAUaPw0dTtMycUCoxp+RkMITs+vc/79ceNfGMb6dkOgNobNh5T2l\n\tNztEfuhDq3mY6Q8/GkDUIbS/tVWD7fUhLEiqL7PFdu//fuEVM2n2RFek6aw5IO8obs\n\tl6tVcBjxKgpZw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659004151;\n\tbh=Pgk0I2DFoiPm/DvO6LMbo/HN/aRTmEJM6Chh4xUnTiU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=I8+l6X2X8nOktRIl6ORHi9XEZ0bCoHW6HDBoa3HBwV4bNGFlFV133ZrrdUghHQhzr\n\tcNac3oqqLCi8VQ8a3MM5+NnqfarIVnymgM73pStkym6RvDYSLMKe2Yk/we8GYCW5Pf\n\trXGvepisUYqxVTBYcqEZ4oiRxB8/rnbMqcNRBgSM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"I8+l6X2X\"; dkim-atps=neutral","Date":"Thu, 28 Jul 2022 13:29:08 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220727222149.30627-1-laurent.pinchart@ideasonboard.com>\n\t<20220727222149.30627-9-laurent.pinchart@ideasonboard.com>\n\t<20220728075149.x3f5qkaqnbyesgcw@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220728075149.x3f5qkaqnbyesgcw@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v5 8/9] ipa: rkisp1: Add support of\n\tFilter control","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]