| Message ID | 20251214181646.573675-2-rui.wang@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Rui On Sun, Dec 14, 2025 at 01:16:41PM -0500, Rui Wang wrote: > Split DPF configuration parsing and initialization into clearer, > self-contained helpers and modernized initialization patterns. > > Introduce parseSingleConfig() as DPF tuning file parser helper > make future extensions and maintenance easier. > > Change strengthconfig.r/g/b parser from uint16 to uint8 > to match rkisp1_cif_isp_dpf_strength_config defination. As commented in the previous version s/defination/definition > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> When you receive a tag on a patch, like the one I gave on the previous version, you should collect it and paste it here. It's a tedious and manual process, but that's what we have at the moment... Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > --- > > changelog: > - propagates specific integer error codes (e.g. -EINVAL) from > parseConfig and parseSingleConfig instead of > returning boolean false values, improving error reporting. > - format the code : Unbraced if (single-statement if) > > src/ipa/rkisp1/algorithms/dpf.cpp | 52 ++++++++++++++++++++++--------- > src/ipa/rkisp1/algorithms/dpf.h | 5 +++ > 2 files changed, 42 insertions(+), 15 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp > index 39f3e461..dd3effa1 100644 > --- a/src/ipa/rkisp1/algorithms/dpf.cpp > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp > @@ -46,6 +46,28 @@ Dpf::Dpf() > */ > int Dpf::init([[maybe_unused]] IPAContext &context, > const YamlObject &tuningData) > +{ > + /* Parse tuning block. */ > + int ret = parseConfig(tuningData); > + if (ret) > + return ret; > + > + return 0; > +} > + > +int Dpf::parseConfig(const YamlObject &tuningData) > +{ > + /* Parse base config. */ > + int ret = parseSingleConfig(tuningData, config_, strengthConfig_); > + if (ret) > + return ret; > + > + return 0; > +} > + > +int Dpf::parseSingleConfig(const YamlObject &tuningData, > + rkisp1_cif_isp_dpf_config &config, > + rkisp1_cif_isp_dpf_strength_config &strengthConfig) > { > std::vector<uint8_t> values; > > @@ -82,10 +104,10 @@ int Dpf::init([[maybe_unused]] IPAContext &context, > } > > std::copy_n(values.begin(), values.size(), > - std::begin(config_.g_flt.spatial_coeff)); > + std::begin(config.g_flt.spatial_coeff)); > > - config_.g_flt.gr_enable = true; > - config_.g_flt.gb_enable = true; > + config.g_flt.gr_enable = true; > + config.g_flt.gb_enable = true; > > /* > * For the red and blue components, we have the 13x9 kernel specified > @@ -119,15 +141,15 @@ int Dpf::init([[maybe_unused]] IPAContext &context, > return -EINVAL; > } > > - config_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS > - ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 > - : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9; > + config.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS > + ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 > + : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9; > > std::copy_n(values.begin(), values.size(), > - std::begin(config_.rb_flt.spatial_coeff)); > + std::begin(config.rb_flt.spatial_coeff)); > > - config_.rb_flt.r_enable = true; > - config_.rb_flt.b_enable = true; > + config.rb_flt.r_enable = true; > + config.rb_flt.b_enable = true; > > /* > * The range kernel is configured with a noise level lookup table (NLL) > @@ -147,13 +169,13 @@ int Dpf::init([[maybe_unused]] IPAContext &context, > } > > std::copy_n(nllValues.begin(), nllValues.size(), > - std::begin(config_.nll.coeff)); > + std::begin(config.nll.coeff)); > > std::string scaleMode = rFObject["scale-mode"].get<std::string>(""); > if (scaleMode == "linear") { > - config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LINEAR; > + config.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LINEAR; > } else if (scaleMode == "logarithmic") { > - config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC; > + config.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC; > } else { > LOG(RkISP1Dpf, Error) > << "Invalid 'RangeFilter:scale-mode': expected " > @@ -164,9 +186,9 @@ int Dpf::init([[maybe_unused]] IPAContext &context, > > const YamlObject &fSObject = tuningData["FilterStrength"]; > > - strengthConfig_.r = fSObject["r"].get<uint16_t>(64); > - strengthConfig_.g = fSObject["g"].get<uint16_t>(64); > - strengthConfig_.b = fSObject["b"].get<uint16_t>(64); > + strengthConfig.r = fSObject["r"].get<uint8_t>().value_or(64); > + strengthConfig.g = fSObject["g"].get<uint8_t>().value_or(64); > + strengthConfig.b = fSObject["b"].get<uint8_t>().value_or(64); > > return 0; > } > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h > index 2dd8cd36..39186c55 100644 > --- a/src/ipa/rkisp1/algorithms/dpf.h > +++ b/src/ipa/rkisp1/algorithms/dpf.h > @@ -30,6 +30,11 @@ public: > RkISP1Params *params) override; > > private: > + int parseConfig(const YamlObject &tuningData); > + int parseSingleConfig(const YamlObject &tuningData, > + rkisp1_cif_isp_dpf_config &config, > + rkisp1_cif_isp_dpf_strength_config &strengthConfig); > + > struct rkisp1_cif_isp_dpf_config config_; > struct rkisp1_cif_isp_dpf_strength_config strengthConfig_; > }; > -- > 2.43.0 >
diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp index 39f3e461..dd3effa1 100644 --- a/src/ipa/rkisp1/algorithms/dpf.cpp +++ b/src/ipa/rkisp1/algorithms/dpf.cpp @@ -46,6 +46,28 @@ Dpf::Dpf() */ int Dpf::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData) +{ + /* Parse tuning block. */ + int ret = parseConfig(tuningData); + if (ret) + return ret; + + return 0; +} + +int Dpf::parseConfig(const YamlObject &tuningData) +{ + /* Parse base config. */ + int ret = parseSingleConfig(tuningData, config_, strengthConfig_); + if (ret) + return ret; + + return 0; +} + +int Dpf::parseSingleConfig(const YamlObject &tuningData, + rkisp1_cif_isp_dpf_config &config, + rkisp1_cif_isp_dpf_strength_config &strengthConfig) { std::vector<uint8_t> values; @@ -82,10 +104,10 @@ int Dpf::init([[maybe_unused]] IPAContext &context, } std::copy_n(values.begin(), values.size(), - std::begin(config_.g_flt.spatial_coeff)); + std::begin(config.g_flt.spatial_coeff)); - config_.g_flt.gr_enable = true; - config_.g_flt.gb_enable = true; + config.g_flt.gr_enable = true; + config.g_flt.gb_enable = true; /* * For the red and blue components, we have the 13x9 kernel specified @@ -119,15 +141,15 @@ int Dpf::init([[maybe_unused]] IPAContext &context, return -EINVAL; } - config_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 - : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9; + config.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS + ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 + : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9; std::copy_n(values.begin(), values.size(), - std::begin(config_.rb_flt.spatial_coeff)); + std::begin(config.rb_flt.spatial_coeff)); - config_.rb_flt.r_enable = true; - config_.rb_flt.b_enable = true; + config.rb_flt.r_enable = true; + config.rb_flt.b_enable = true; /* * The range kernel is configured with a noise level lookup table (NLL) @@ -147,13 +169,13 @@ int Dpf::init([[maybe_unused]] IPAContext &context, } std::copy_n(nllValues.begin(), nllValues.size(), - std::begin(config_.nll.coeff)); + std::begin(config.nll.coeff)); std::string scaleMode = rFObject["scale-mode"].get<std::string>(""); if (scaleMode == "linear") { - config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LINEAR; + config.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LINEAR; } else if (scaleMode == "logarithmic") { - config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC; + config.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC; } else { LOG(RkISP1Dpf, Error) << "Invalid 'RangeFilter:scale-mode': expected " @@ -164,9 +186,9 @@ int Dpf::init([[maybe_unused]] IPAContext &context, const YamlObject &fSObject = tuningData["FilterStrength"]; - strengthConfig_.r = fSObject["r"].get<uint16_t>(64); - strengthConfig_.g = fSObject["g"].get<uint16_t>(64); - strengthConfig_.b = fSObject["b"].get<uint16_t>(64); + strengthConfig.r = fSObject["r"].get<uint8_t>().value_or(64); + strengthConfig.g = fSObject["g"].get<uint8_t>().value_or(64); + strengthConfig.b = fSObject["b"].get<uint8_t>().value_or(64); return 0; } diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h index 2dd8cd36..39186c55 100644 --- a/src/ipa/rkisp1/algorithms/dpf.h +++ b/src/ipa/rkisp1/algorithms/dpf.h @@ -30,6 +30,11 @@ public: RkISP1Params *params) override; private: + int parseConfig(const YamlObject &tuningData); + int parseSingleConfig(const YamlObject &tuningData, + rkisp1_cif_isp_dpf_config &config, + rkisp1_cif_isp_dpf_strength_config &strengthConfig); + struct rkisp1_cif_isp_dpf_config config_; struct rkisp1_cif_isp_dpf_strength_config strengthConfig_; };
Split DPF configuration parsing and initialization into clearer, self-contained helpers and modernized initialization patterns. Introduce parseSingleConfig() as DPF tuning file parser helper make future extensions and maintenance easier. Change strengthconfig.r/g/b parser from uint16 to uint8 to match rkisp1_cif_isp_dpf_strength_config defination. Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> --- changelog: - propagates specific integer error codes (e.g. -EINVAL) from parseConfig and parseSingleConfig instead of returning boolean false values, improving error reporting. - format the code : Unbraced if (single-statement if) src/ipa/rkisp1/algorithms/dpf.cpp | 52 ++++++++++++++++++++++--------- src/ipa/rkisp1/algorithms/dpf.h | 5 +++ 2 files changed, 42 insertions(+), 15 deletions(-)