[v3,6/9] ipa: rkisp1: Move loader functions into helper class
diff mbox series

Message ID 20240920133941.90629-7-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • Implement polynomial lsc support
Related show

Commit Message

Stefan Klug Sept. 20, 2024, 1:39 p.m. UTC
In preparation for supporting polynomial LSC data, move the current
loader into its own helper class.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---

Changes in v3:
- Renamed LscClassicLoader to LscTableLoader
- Collected tags
- Removed type detection from this patch to the one adding the
  polynomial loader
---
 src/ipa/rkisp1/algorithms/lsc.cpp | 120 +++++++++++++++++-------------
 1 file changed, 69 insertions(+), 51 deletions(-)

Comments

Laurent Pinchart Sept. 23, 2024, 8:21 p.m. UTC | #1
On Fri, Sep 20, 2024 at 03:39:21PM +0200, Stefan Klug wrote:
> In preparation for supporting polynomial LSC data, move the current
> loader into its own helper class.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> 
> Changes in v3:
> - Renamed LscClassicLoader to LscTableLoader
> - Collected tags
> - Removed type detection from this patch to the one adding the
>   polynomial loader
> ---
>  src/ipa/rkisp1/algorithms/lsc.cpp | 120 +++++++++++++++++-------------
>  1 file changed, 69 insertions(+), 51 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index fe84062bbc70..44c97f0e1a10 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -70,6 +70,70 @@ namespace ipa::rkisp1::algorithms {
>  
>  LOG_DEFINE_CATEGORY(RkISP1Lsc)
>  
> +class LscTableLoader
> +{
> +public:
> +	int parseLscData(const YamlObject &yamlSets,
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +	{

As mentioned in the review of other patches in the series, please don't
make those functions inline. In hinders readability.

> +		const auto &sets = yamlSets.asList();
> +
> +		for (const auto &yamlSet : sets) {
> +			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> +
> +			if (lscData.count(ct)) {
> +				LOG(RkISP1Lsc, Error)
> +					<< "Multiple sets found for color temperature "
> +					<< ct;
> +				return -EINVAL;
> +			}
> +
> +			LensShadingCorrection::Components &set = lscData[ct];
> +
> +			set.ct = ct;
> +			set.r = parseTable(yamlSet, "r");
> +			set.gr = parseTable(yamlSet, "gr");
> +			set.gb = parseTable(yamlSet, "gb");
> +			set.b = parseTable(yamlSet, "b");
> +
> +			if (set.r.empty() || set.gr.empty() ||
> +			    set.gb.empty() || set.b.empty()) {
> +				LOG(RkISP1Lsc, Error)
> +					<< "Set for color temperature " << ct
> +					<< " is missing tables";
> +				return -EINVAL;
> +			}
> +		}
> +
> +		if (lscData.empty()) {
> +			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +			return -EINVAL;
> +		}
> +
> +		return 0;
> +	}
> +
> +private:
> +	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> +					 const char *prop)
> +	{
> +		static constexpr unsigned int kLscNumSamples =
> +			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +		std::vector<uint16_t> table =
> +			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> +		if (table.size() != kLscNumSamples) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Invalid '" << prop << "' values: expected "
> +				<< kLscNumSamples
> +				<< " elements, got " << table.size();
> +			return {};
> +		}
> +
> +		return table;
> +	}
> +};
> +
>  static std::vector<double> parseSizes(const YamlObject &tuningData,
>  				      const char *prop)
>  {
> @@ -100,25 +164,6 @@ static std::vector<double> parseSizes(const YamlObject &tuningData,
>  	return sizes;
>  }
>  
> -static std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> -					const char *prop)
> -{
> -	static constexpr unsigned int kLscNumSamples =
> -		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> -
> -	std::vector<uint16_t> table =
> -		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> -	if (table.size() != kLscNumSamples) {
> -		LOG(RkISP1Lsc, Error)
> -			<< "Invalid '" << prop << "' values: expected "
> -			<< kLscNumSamples
> -			<< " elements, got " << table.size();
> -		return {};
> -	}
> -
> -	return table;
> -}
> -
>  LensShadingCorrection::LensShadingCorrection()
>  	: lastAppliedCt_(0), lastAppliedQuantizedCt_(0)
>  {
> @@ -145,39 +190,12 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>  		return -EINVAL;
>  	}
>  
> -	const auto &sets = yamlSets.asList();
>  	std::map<unsigned int, Components> lscData;
> -	for (const auto &yamlSet : sets) {
> -		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -		if (lscData.count(ct)) {
> -			LOG(RkISP1Lsc, Error)
> -				<< "Multiple sets found for color temperature "
> -				<< ct;
> -			return -EINVAL;
> -		}
> -
> -		Components &set = lscData[ct];
> -
> -		set.ct = ct;
> -		set.r = parseTable(yamlSet, "r");
> -		set.gr = parseTable(yamlSet, "gr");
> -		set.gb = parseTable(yamlSet, "gb");
> -		set.b = parseTable(yamlSet, "b");
> -
> -		if (set.r.empty() || set.gr.empty() ||
> -		    set.gb.empty() || set.b.empty()) {
> -			LOG(RkISP1Lsc, Error)
> -				<< "Set for color temperature " << ct
> -				<< " is missing tables";
> -			return -EINVAL;
> -		}
> -	}
> -
> -	if (lscData.empty()) {
> -		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> -		return -EINVAL;
> -	}
> +	int res = 0;
> +	auto loader = LscTableLoader();
> +	res = loader.parseLscData(yamlSets, lscData);
> +	if (res)
> +		return res;
>  
>  	sets_.setData(std::move(lscData));
>  
> -- 
> 2.43.0
>

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
index fe84062bbc70..44c97f0e1a10 100644
--- a/src/ipa/rkisp1/algorithms/lsc.cpp
+++ b/src/ipa/rkisp1/algorithms/lsc.cpp
@@ -70,6 +70,70 @@  namespace ipa::rkisp1::algorithms {
 
 LOG_DEFINE_CATEGORY(RkISP1Lsc)
 
+class LscTableLoader
+{
+public:
+	int parseLscData(const YamlObject &yamlSets,
+			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
+	{
+		const auto &sets = yamlSets.asList();
+
+		for (const auto &yamlSet : sets) {
+			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
+
+			if (lscData.count(ct)) {
+				LOG(RkISP1Lsc, Error)
+					<< "Multiple sets found for color temperature "
+					<< ct;
+				return -EINVAL;
+			}
+
+			LensShadingCorrection::Components &set = lscData[ct];
+
+			set.ct = ct;
+			set.r = parseTable(yamlSet, "r");
+			set.gr = parseTable(yamlSet, "gr");
+			set.gb = parseTable(yamlSet, "gb");
+			set.b = parseTable(yamlSet, "b");
+
+			if (set.r.empty() || set.gr.empty() ||
+			    set.gb.empty() || set.b.empty()) {
+				LOG(RkISP1Lsc, Error)
+					<< "Set for color temperature " << ct
+					<< " is missing tables";
+				return -EINVAL;
+			}
+		}
+
+		if (lscData.empty()) {
+			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+
+private:
+	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
+					 const char *prop)
+	{
+		static constexpr unsigned int kLscNumSamples =
+			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
+
+		std::vector<uint16_t> table =
+			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
+		if (table.size() != kLscNumSamples) {
+			LOG(RkISP1Lsc, Error)
+				<< "Invalid '" << prop << "' values: expected "
+				<< kLscNumSamples
+				<< " elements, got " << table.size();
+			return {};
+		}
+
+		return table;
+	}
+};
+
 static std::vector<double> parseSizes(const YamlObject &tuningData,
 				      const char *prop)
 {
@@ -100,25 +164,6 @@  static std::vector<double> parseSizes(const YamlObject &tuningData,
 	return sizes;
 }
 
-static std::vector<uint16_t> parseTable(const YamlObject &tuningData,
-					const char *prop)
-{
-	static constexpr unsigned int kLscNumSamples =
-		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
-
-	std::vector<uint16_t> table =
-		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
-	if (table.size() != kLscNumSamples) {
-		LOG(RkISP1Lsc, Error)
-			<< "Invalid '" << prop << "' values: expected "
-			<< kLscNumSamples
-			<< " elements, got " << table.size();
-		return {};
-	}
-
-	return table;
-}
-
 LensShadingCorrection::LensShadingCorrection()
 	: lastAppliedCt_(0), lastAppliedQuantizedCt_(0)
 {
@@ -145,39 +190,12 @@  int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
 		return -EINVAL;
 	}
 
-	const auto &sets = yamlSets.asList();
 	std::map<unsigned int, Components> lscData;
-	for (const auto &yamlSet : sets) {
-		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
-
-		if (lscData.count(ct)) {
-			LOG(RkISP1Lsc, Error)
-				<< "Multiple sets found for color temperature "
-				<< ct;
-			return -EINVAL;
-		}
-
-		Components &set = lscData[ct];
-
-		set.ct = ct;
-		set.r = parseTable(yamlSet, "r");
-		set.gr = parseTable(yamlSet, "gr");
-		set.gb = parseTable(yamlSet, "gb");
-		set.b = parseTable(yamlSet, "b");
-
-		if (set.r.empty() || set.gr.empty() ||
-		    set.gb.empty() || set.b.empty()) {
-			LOG(RkISP1Lsc, Error)
-				<< "Set for color temperature " << ct
-				<< " is missing tables";
-			return -EINVAL;
-		}
-	}
-
-	if (lscData.empty()) {
-		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
-		return -EINVAL;
-	}
+	int res = 0;
+	auto loader = LscTableLoader();
+	res = loader.parseLscData(yamlSets, lscData);
+	if (res)
+		return res;
 
 	sets_.setData(std::move(lscData));