[{"id":24175,"web_url":"https://patchwork.libcamera.org/comment/24175/","msgid":"","date":"2022-07-27T04:01:19","subject":"Re: [libcamera-devel] [PATCH v3 1/2] 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 Florian,\n\nThank you for the patch.\n\nOn Tue, Jul 26, 2022 at 04:32:10PM +0200, Florian Sylvestre via libcamera-devel wrote:\n> Denoise and Sharpness filters will be applied by RKISP1 during the\n\ns/RKISP1/RkISP1/\n\nSame below.\n\n> demosaicing step. The denoise filter is responsible to remove noise from the\n> 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> ---\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 00000000..3a9f9be5\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 to remove noise from the\n\ns/to remove/for removing/\n\n> + * 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,\n> +\t\t rkisp1_params_cfg *params)\n\nThis holds on a single line.\n\n> +{\n\n\tauto &filter = context.frameContext.filter;\n\nto shorten lines below, like in queueRequest() ? Up to you.\n\n> +\t/* Check if the algorithm configuration has been updated. */\n> +\tif (!context.frameContext.filter.updateParams)\n> +\t\treturn;\n> +\n> +\tcontext.frameContext.filter.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 = context.frameContext.filter.denoise;\n> +\tuint8_t sharpness = context.frameContext.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 =\n> +\t\t\t\tflt_config.fac_bl0 * 3 / 4;\n\nThis holds on a single line.\n\nReviewed-by: Laurent Pinchart \n\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 00000000..9eb170eb\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 87007493..dcd24fe0 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 2315ec43..99529481 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 30bb87a9..4b117186 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 3bfb262c..3b2f6af1 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 99eecd44..4e000d31 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> \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 E670EBE173\n\tfor ;\n\tWed, 27 Jul 2022 04:01:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CF1063312;\n\tWed, 27 Jul 2022 06:01:23 +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 1BC3D603E8\n\tfor ;\n\tWed, 27 Jul 2022 06:01:21 +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 6E19956D;\n\tWed, 27 Jul 2022 06:01:20 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658894483;\n\tbh=Eq7yx0InLQj1SuyJuD0eqttGcgAfHEtEJeDN6ZevmjY=;\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=smgUV/oBPhUx4i4g0I/XO5M7BO3+WYV1VVLzzs7QoiYsU1t19HgX3cuRaHdtIPpq1\n\to8YJ4ECJ9PMT4Q6c6iF8LF5WAm3Mk+XTt8ZjZvU7k38bVlgHVBWGlPv/6t97EUEBLQ\n\tJz8eZSHb4MTwD1tmW8TmJE0ueZm8sImuXZHY2rNwlyD+tiaqH+m13O/MgCLcl/vzwj\n\t2mXlHWY9zE2q5oL4F6l2SOgMl8mM2pK1dvkJOmu4Run643+80qw2BnmfNL/D0kUGOY\n\tGD7IYglnE8wXD6N0q3c1jxA8JTqQbV2fop45dhyGLGnpnYPiFWjpNLfinb5SjKm/rl\n\td4k8MB2cBVM4g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658894480;\n\tbh=Eq7yx0InLQj1SuyJuD0eqttGcgAfHEtEJeDN6ZevmjY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rXC8GIGN+WbCExsW99R86cU9u9biZwdcuE6OGZHT+SUSNY4TNVFj2Ua/3CCAkQXGq\n\taFhOY6Zq2fk1A34DUFGxOWHWtIcx3ahNLtlcTo/Tpi7FCQ7meosiaLooR5vTUocnqR\n\taV/Ds0wLrq4M9kn3Q6cMZ72Tw8HFzl+vJfBVVcZA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rXC8GIGN\"; dkim-atps=neutral","Date":"Wed, 27 Jul 2022 07:01:19 +0300","To":"Florian Sylvestre ","Message-ID":"","References":"<20220726143211.517231-1-fsylvestre@baylibre.com>\n\t<20220726143211.517231-2-fsylvestre@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220726143211.517231-2-fsylvestre@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] 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\" "}}]