Patch Detail
Show a patch.
GET /api/1.1/patches/25383/?format=api
{ "id": 25383, "url": "https://patchwork.libcamera.org/api/1.1/patches/25383/?format=api", "web_url": "https://patchwork.libcamera.org/patch/25383/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20251208004808.1274417-6-rui.wang@ideasonboard.com>", "date": "2025-12-08T00:48:06", "name": "[v4,5/7] ipa: rkisp1: algorithms: dpf: Use YamlObject::Getter for parsing", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "ed2c05b85746dd43193de4bc59f562b277a4c2dd", "submitter": { "id": 241, "url": "https://patchwork.libcamera.org/api/1.1/people/241/?format=api", "name": "Rui Wang", "email": "rui.wang@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/25383/mbox/", "series": [ { "id": 5641, "url": "https://patchwork.libcamera.org/api/1.1/series/5641/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5641", "date": "2025-12-08T00:48:01", "name": "rebase_dpf_refactory_patch_v4", "version": 4, "mbox": "https://patchwork.libcamera.org/series/5641/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/25383/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/25383/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 C818EC3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 8 Dec 2025 00:48:50 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B9BD613FA;\n\tMon, 8 Dec 2025 01:48:50 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 877C9613F2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 8 Dec 2025 01:48:48 +0100 (CET)", "from localhost.localdomain (unknown [209.216.103.65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 25DD51E29;\n\tMon, 8 Dec 2025 01:46:30 +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=\"t6JSnqqr\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765154790;\n\tbh=TIx7IiB0QruV3c19NCJutrJAVQRu3rhDUppRcubS3eE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=t6JSnqqrMQzdGydF4rrpgeh/rEeSlyc/GhQKKt5AjqQR13aBoPQ4dKgP94rQrrPez\n\tl7HGSrsjIrlU4KCMl0nEg9dShAkyeRj9ntlYXsjz/G5hed/x9aaHfikTwrUoC+Q9FU\n\tCjxEGGMlTnGUY9kfsrHNKfmTFKduG79TSRUTyoMM=", "From": "Rui Wang <rui.wang@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Rui Wang <rui.wang@ideasonboard.com>", "Subject": "[PATCH v4 5/7] ipa: rkisp1: algorithms: dpf: Use YamlObject::Getter\n\tfor parsing", "Date": "Sun, 7 Dec 2025 19:48:06 -0500", "Message-ID": "<20251208004808.1274417-6-rui.wang@ideasonboard.com>", "X-Mailer": "git-send-email 2.43.0", "In-Reply-To": "<20251208004808.1274417-1-rui.wang@ideasonboard.com>", "References": "<20251208004808.1274417-1-rui.wang@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Refactor DPF configuration parsing to use YamlObject::Getter\nspecializations, similar to the approach used in pwl.cpp. This\nsimplifies parseSingleConfig() by delegating the parsing logic\nto dedicated getter specializations.\n\nImplement two YamlObject::Getter specializations:\n- rkisp1_cif_isp_dpf_config: Parses filter and nll parameters\n- rkisp1_cif_isp_dpf_strength_config: Parses strength parameters\n\nThe specializations are defined at the top of the file in the\nlibcamera namespace.\n\nRemove LOG statements from the getter specializations to avoid\nnamespace visibility issues, as the LOG macro requires access\nto the log category which is defined in a nested namespace.\n\nSigned-off-by: Rui Wang <rui.wang@ideasonboard.com>\n---\n src/ipa/rkisp1/algorithms/dpf.cpp | 240 ++++++++++++++++--------------\n 1 file changed, 125 insertions(+), 115 deletions(-)", "diff": "diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\nindex dea04cc3..68a94afe 100644\n--- a/src/ipa/rkisp1/algorithms/dpf.cpp\n+++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n@@ -23,107 +23,31 @@\n \n namespace libcamera {\n \n-namespace ipa::rkisp1::algorithms {\n-\n-/**\n- * \\class Dpf\n- * \\brief RkISP1 Denoise Pre-Filter control\n- *\n- * The denoise pre-filter algorithm is a bilateral filter which combines a\n- * range filter and a domain filter. The denoise pre-filter is applied before\n- * demosaicing.\n- */\n-\n-LOG_DEFINE_CATEGORY(RkISP1Dpf)\n-\n-Dpf::Dpf()\n-\t: config_({}), strengthConfig_({}),\n-\t noiseReductionModes_({}),\n-\t runningMode_(controls::draft::NoiseReductionModeOff)\n-{\n-}\n-\n-/**\n- * \\copydoc libcamera::ipa::Algorithm::init\n- */\n-int Dpf::init([[maybe_unused]] IPAContext &context,\n-\t const YamlObject &tuningData)\n-{\n-\t/* Parse tuning block. */\n-\tif (!parseConfig(tuningData)) {\n-\t\treturn -EINVAL;\n-\t}\n-\n-\treturn 0;\n-}\n-\n-bool Dpf::parseConfig(const YamlObject &tuningData)\n+template<>\n+std::optional<rkisp1_cif_isp_dpf_strength_config>\n+YamlObject::Getter<rkisp1_cif_isp_dpf_strength_config>::get(const YamlObject &obj) const\n {\n-\t/* Parse base config. */\n-\tif (!parseSingleConfig(tuningData, config_, strengthConfig_)) {\n-\t\treturn false;\n-\t}\n+\trkisp1_cif_isp_dpf_strength_config config = {};\n \n-\t/* Parse modes. */\n-\tif (!parseModes(tuningData)) {\n-\t\treturn false;\n-\t}\n+\tconfig.r = obj[\"r\"].get<uint8_t>().value_or(64);\n+\tconfig.g = obj[\"g\"].get<uint8_t>().value_or(64);\n+\tconfig.b = obj[\"b\"].get<uint8_t>().value_or(64);\n \n-\treturn true;\n+\treturn config;\n }\n \n-bool Dpf::parseModes(const YamlObject &tuningData)\n-{\n-\t/* Parse noise reduction modes. */\n-\tif (!tuningData.contains(\"modes\")) {\n-\t\treturn true;\n-\t}\n-\n-\tnoiseReductionModes_.clear();\n-\tfor (const auto &entry : tuningData[\"modes\"].asList()) {\n-\t\tstd::optional<std::string> typeOpt =\n-\t\t\tentry[\"type\"].get<std::string>();\n-\t\tif (!typeOpt) {\n-\t\t\tLOG(RkISP1Dpf, Error) << \"Modes entry missing type\";\n-\t\t\treturn false;\n-\t\t}\n-\n-\t\tint32_t modeValue = controls::draft::NoiseReductionModeOff;\n-\t\tif (*typeOpt == \"minimal\") {\n-\t\t\tmodeValue = controls::draft::NoiseReductionModeMinimal;\n-\t\t} else if (*typeOpt == \"fast\") {\n-\t\t\tmodeValue = controls::draft::NoiseReductionModeFast;\n-\t\t} else if (*typeOpt == \"highquality\") {\n-\t\t\tmodeValue = controls::draft::NoiseReductionModeHighQuality;\n-\t\t} else if (*typeOpt == \"zsl\") {\n-\t\t\tmodeValue = controls::draft::NoiseReductionModeZSL;\n-\t\t} else {\n-\t\t\tLOG(RkISP1Dpf, Error) << \"Unknown mode type: \" << *typeOpt;\n-\t\t\treturn false;\n-\t\t}\n-\n-\t\tModeConfig mode{};\n-\t\tmode.modeValue = modeValue;\n-\t\tif (!parseSingleConfig(entry, mode.dpf, mode.strength)) {\n-\t\t\treturn false;\n-\t\t}\n-\t\tnoiseReductionModes_.push_back(mode);\n-\t}\n-\n-\treturn true;\n-}\n-\n-bool Dpf::parseSingleConfig(const YamlObject &tuningData,\n-\t\t\t rkisp1_cif_isp_dpf_config &config,\n-\t\t\t rkisp1_cif_isp_dpf_strength_config &strengthConfig)\n+template<>\n+std::optional<rkisp1_cif_isp_dpf_config>\n+YamlObject::Getter<rkisp1_cif_isp_dpf_config>::get(const YamlObject &obj) const\n {\n+\trkisp1_cif_isp_dpf_config config = {};\n \tstd::vector<uint8_t> values;\n \n \t/*\n \t * The domain kernel is configured with a 9x9 kernel for the green\n \t * pixels, and a 13x9 or 9x9 kernel for red and blue pixels.\n \t */\n-\tconst YamlObject &dFObject = tuningData[\"filter\"];\n+\tconst YamlObject &dFObject = obj[\"filter\"];\n \n \t/*\n \t * For the green component, we have the 9x9 kernel specified\n@@ -144,11 +68,7 @@ bool Dpf::parseSingleConfig(const YamlObject &tuningData,\n \t */\n \tvalues = dFObject[\"g\"].getList<uint8_t>().value_or(std::vector<uint8_t>{});\n \tif (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS) {\n-\t\tLOG(RkISP1Dpf, Error)\n-\t\t\t<< \"Invalid 'filter:g': expected \"\n-\t\t\t<< RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS\n-\t\t\t<< \" elements, got \" << values.size();\n-\t\treturn false;\n+\t\treturn std::nullopt;\n \t}\n \n \tstd::copy_n(values.begin(), values.size(),\n@@ -181,12 +101,7 @@ bool Dpf::parseSingleConfig(const YamlObject &tuningData,\n \tvalues = dFObject[\"rb\"].getList<uint8_t>().value_or(std::vector<uint8_t>{});\n \tif (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS &&\n \t values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1) {\n-\t\tLOG(RkISP1Dpf, Error)\n-\t\t\t<< \"Invalid 'filter:rb': expected \"\n-\t\t\t<< RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1\n-\t\t\t<< \" or \" << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS\n-\t\t\t<< \" elements, got \" << values.size();\n-\t\treturn false;\n+\t\treturn std::nullopt;\n \t}\n \n \tconfig.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS\n@@ -204,39 +119,134 @@ bool Dpf::parseSingleConfig(const YamlObject &tuningData,\n \t * which stores a piecewise linear function that characterizes the\n \t * sensor noise profile as a noise level function curve (NLF).\n \t */\n-\tconst YamlObject &rFObject = tuningData[\"nll\"];\n+\tconst YamlObject &rFObject = obj[\"nll\"];\n \n \tstd::vector<uint16_t> nllValues;\n \tnllValues = rFObject[\"coeff\"].getList<uint16_t>().value_or(std::vector<uint16_t>{});\n \tif (nllValues.size() != RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS) {\n-\t\tLOG(RkISP1Dpf, Error)\n-\t\t\t<< \"Invalid 'nll:coeff': expected \"\n-\t\t\t<< RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS\n-\t\t\t<< \" elements, got \" << nllValues.size();\n-\t\treturn false;\n+\t\treturn std::nullopt;\n \t}\n \n \tstd::copy_n(nllValues.begin(), nllValues.size(),\n \t\t std::begin(config.nll.coeff));\n \n-\tstd::string scaleMode = rFObject[\"scale-mode\"].get<std::string>(\"\");\n+\tstd::string scaleMode = rFObject[\"scale-mode\"].get<std::string>().value_or(\"\");\n \tif (scaleMode == \"linear\") {\n \t\tconfig.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LINEAR;\n \t} else if (scaleMode == \"logarithmic\") {\n \t\tconfig.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC;\n \t} else {\n-\t\tLOG(RkISP1Dpf, Error)\n-\t\t\t<< \"Invalid 'nll:scale-mode': expected \"\n-\t\t\t<< \"'linear' or 'logarithmic' value, got \"\n-\t\t\t<< scaleMode;\n+\t\treturn std::nullopt;\n+\t}\n+\n+\treturn config;\n+}\n+\n+namespace ipa::rkisp1::algorithms {\n+\n+/**\n+ * \\class Dpf\n+ * \\brief RkISP1 Denoise Pre-Filter control\n+ *\n+ * The denoise pre-filter algorithm is a bilateral filter which combines a\n+ * range filter and a domain filter. The denoise pre-filter is applied before\n+ * demosaicing.\n+ */\n+\n+LOG_DEFINE_CATEGORY(RkISP1Dpf)\n+\n+Dpf::Dpf()\n+\t: config_({}), strengthConfig_({}),\n+\t noiseReductionModes_({}),\n+\t runningMode_(controls::draft::NoiseReductionModeOff)\n+{\n+}\n+\n+/**\n+ * \\copydoc libcamera::ipa::Algorithm::init\n+ */\n+int Dpf::init([[maybe_unused]] IPAContext &context,\n+\t const YamlObject &tuningData)\n+{\n+\t/* Parse tuning block. */\n+\tif (!parseConfig(tuningData)) {\n+\t\treturn -EINVAL;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+bool Dpf::parseConfig(const YamlObject &tuningData)\n+{\n+\t/* Parse base config. */\n+\tif (!parseSingleConfig(tuningData, config_, strengthConfig_)) {\n+\t\treturn false;\n+\t}\n+\n+\t/* Parse modes. */\n+\tif (!parseModes(tuningData)) {\n \t\treturn false;\n \t}\n \n-\tconst YamlObject &fSObject = tuningData[\"strength\"];\n+\treturn true;\n+}\n+\n+bool Dpf::parseModes(const YamlObject &tuningData)\n+{\n+\t/* Parse noise reduction modes. */\n+\tif (!tuningData.contains(\"modes\")) {\n+\t\treturn true;\n+\t}\n+\n+\tnoiseReductionModes_.clear();\n+\tfor (const auto &entry : tuningData[\"modes\"].asList()) {\n+\t\tstd::optional<std::string> typeOpt =\n+\t\t\tentry[\"type\"].get<std::string>();\n+\t\tif (!typeOpt) {\n+\t\t\tLOG(RkISP1Dpf, Error) << \"Modes entry missing type\";\n+\t\t\treturn false;\n+\t\t}\n+\n+\t\tint32_t modeValue = controls::draft::NoiseReductionModeOff;\n+\t\tif (*typeOpt == \"minimal\") {\n+\t\t\tmodeValue = controls::draft::NoiseReductionModeMinimal;\n+\t\t} else if (*typeOpt == \"fast\") {\n+\t\t\tmodeValue = controls::draft::NoiseReductionModeFast;\n+\t\t} else if (*typeOpt == \"highquality\") {\n+\t\t\tmodeValue = controls::draft::NoiseReductionModeHighQuality;\n+\t\t} else if (*typeOpt == \"zsl\") {\n+\t\t\tmodeValue = controls::draft::NoiseReductionModeZSL;\n+\t\t} else {\n+\t\t\tLOG(RkISP1Dpf, Error) << \"Unknown mode type: \" << *typeOpt;\n+\t\t\treturn false;\n+\t\t}\n+\n+\t\tModeConfig mode{};\n+\t\tmode.modeValue = modeValue;\n+\t\tif (!parseSingleConfig(entry, mode.dpf, mode.strength)) {\n+\t\t\treturn false;\n+\t\t}\n+\t\tnoiseReductionModes_.push_back(mode);\n+\t}\n+\n+\treturn true;\n+}\n+\n+bool Dpf::parseSingleConfig(const YamlObject &tuningData,\n+\t\t\t rkisp1_cif_isp_dpf_config &config,\n+\t\t\t rkisp1_cif_isp_dpf_strength_config &strengthConfig)\n+{\n+\tauto dpfConfig = tuningData.get<rkisp1_cif_isp_dpf_config>();\n+\tif (!dpfConfig)\n+\t\treturn false;\n+\n+\tconfig = *dpfConfig;\n+\n+\tauto strength = tuningData[\"strength\"].get<rkisp1_cif_isp_dpf_strength_config>();\n+\tif (!strength)\n+\t\treturn false;\n \n-\tstrengthConfig.r = fSObject[\"r\"].get<uint8_t>().value_or(64);\n-\tstrengthConfig.g = fSObject[\"g\"].get<uint8_t>().value_or(64);\n-\tstrengthConfig.b = fSObject[\"b\"].get<uint8_t>().value_or(64);\n+\tstrengthConfig = *strength;\n \n \treturn true;\n }\n", "prefixes": [ "v4", "5/7" ] }