[10/15] libipa: awb: Provide read-only accessor to modes_
diff mbox series

Message ID 20250223230403.1226-11-laurent.pinchart@ideasonboard.com
State Rejected
Headers show
Series
  • libipa: awb: Drive-by fixes
Related show

Commit Message

Laurent Pinchart Feb. 23, 2025, 11:03 p.m. UTC 
Derived classes don't need to modify the modes_ member variable. Make
it private and provide a read-only accessor.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/libipa/awb.cpp       | 6 +++---
 src/ipa/libipa/awb.h         | 6 +++++-
 src/ipa/libipa/awb_bayes.cpp | 6 +++---
 src/ipa/libipa/awb_bayes.h   | 2 +-
 4 files changed, 12 insertions(+), 8 deletions(-)

Comments

Stefan Klug Feb. 24, 2025, 9:32 a.m. UTC | #1 
Hi Laurent,

Thank you for the patch. 

On Mon, Feb 24, 2025 at 01:03:58AM +0200, Laurent Pinchart wrote:
> Derived classes don't need to modify the modes_ member variable. Make
> it private and provide a read-only accessor.

As parseModeConfigs() and modes_ are only helper functions (I was
undecided if they should be added to the Awb class at all - but they
seem to be quite useful for most implementations) and parseModeConfigs()
is only called by the derived classes, the derived classes should have
full control over the results. I would keep that protected.

Regards,
Stefan

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/ipa/libipa/awb.cpp       | 6 +++---
>  src/ipa/libipa/awb.h         | 6 +++++-
>  src/ipa/libipa/awb_bayes.cpp | 6 +++---
>  src/ipa/libipa/awb_bayes.h   | 2 +-
>  4 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp
> index 04cf2d55ed00..0f54655d3234 100644
> --- a/src/ipa/libipa/awb.cpp
> +++ b/src/ipa/libipa/awb.cpp
> @@ -250,9 +250,9 @@ int AwbAlgorithm::parseModeConfigs(const YamlObject &tuningData,
>   */
>  
>  /**
> - * \var AwbAlgorithm::modes_
> - * \brief Map of all configured modes
> - * \sa AwbAlgorithm::parseModeConfigs
> + * \fn AwbAlgorithm::modes()
> + * \brief Get the available AWB modes
> + * \return The map of available AWB modes
>   */
>  
>  } /* namespace ipa */
> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h
> index 36c05c37f5e9..017b7a0c3dc1 100644
> --- a/src/ipa/libipa/awb.h
> +++ b/src/ipa/libipa/awb.h
> @@ -54,9 +54,13 @@ protected:
>  		double ctLo;
>  	};
>  
> -	std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;
> +	const std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> &modes() const
> +	{
> +		return modes_;
> +	}
>  
>  private:
> +	std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;
>  	ControlInfoMap::Map controls_;
>  };
>  
> diff --git a/src/ipa/libipa/awb_bayes.cpp b/src/ipa/libipa/awb_bayes.cpp
> index 9287b884cb95..97bd256526a4 100644
> --- a/src/ipa/libipa/awb_bayes.cpp
> +++ b/src/ipa/libipa/awb_bayes.cpp
> @@ -170,7 +170,7 @@ int AwbBayes::init(const YamlObject &tuningData)
>  			<< "Failed to parse mode parameter from tuning file";
>  		return ret;
>  	}
> -	currentMode_ = &modes_[controls::AwbAuto];
> +	currentMode_ = &modes().at(controls::AwbAuto);
>  
>  	transversePos_ = tuningData["transversePos"].get<double>(0.01);
>  	transverseNeg_ = tuningData["transverseNeg"].get<double>(0.01);
> @@ -256,8 +256,8 @@ void AwbBayes::handleControls(const ControlList &controls)
>  {
>  	auto mode = controls.get(controls::AwbMode);
>  	if (mode) {
> -		auto it = modes_.find(static_cast<controls::AwbModeEnum>(*mode));
> -		if (it != modes_.end())
> +		auto it = modes().find(static_cast<controls::AwbModeEnum>(*mode));
> +		if (it != modes().end())
>  			currentMode_ = &it->second;
>  		else
>  			LOG(Awb, Error) << "Unsupported AWB mode " << *mode;
> diff --git a/src/ipa/libipa/awb_bayes.h b/src/ipa/libipa/awb_bayes.h
> index 23bf88061118..b74201b542fb 100644
> --- a/src/ipa/libipa/awb_bayes.h
> +++ b/src/ipa/libipa/awb_bayes.h
> @@ -59,7 +59,7 @@ private:
>  	double transversePos_;
>  	double transverseNeg_;
>  
> -	ModeConfig *currentMode_ = nullptr;
> +	const ModeConfig *currentMode_ = nullptr;
>  };
>  
>  } /* namespace ipa */
> -- 
> Regards,
> 
> Laurent Pinchart
>
Laurent Pinchart Feb. 24, 2025, 9:35 a.m. UTC | #2 
On Mon, Feb 24, 2025 at 10:32:44AM +0100, Stefan Klug wrote:
> Hi Laurent,
> 
> Thank you for the patch. 
> 
> On Mon, Feb 24, 2025 at 01:03:58AM +0200, Laurent Pinchart wrote:
> > Derived classes don't need to modify the modes_ member variable. Make
> > it private and provide a read-only accessor.
> 
> As parseModeConfigs() and modes_ are only helper functions (I was
> undecided if they should be added to the Awb class at all - but they
> seem to be quite useful for most implementations) and parseModeConfigs()
> is only called by the derived classes, the derived classes should have
> full control over the results. I would keep that protected.

OK, I'll drop this.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/ipa/libipa/awb.cpp       | 6 +++---
> >  src/ipa/libipa/awb.h         | 6 +++++-
> >  src/ipa/libipa/awb_bayes.cpp | 6 +++---
> >  src/ipa/libipa/awb_bayes.h   | 2 +-
> >  4 files changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp
> > index 04cf2d55ed00..0f54655d3234 100644
> > --- a/src/ipa/libipa/awb.cpp
> > +++ b/src/ipa/libipa/awb.cpp
> > @@ -250,9 +250,9 @@ int AwbAlgorithm::parseModeConfigs(const YamlObject &tuningData,
> >   */
> >  
> >  /**
> > - * \var AwbAlgorithm::modes_
> > - * \brief Map of all configured modes
> > - * \sa AwbAlgorithm::parseModeConfigs
> > + * \fn AwbAlgorithm::modes()
> > + * \brief Get the available AWB modes
> > + * \return The map of available AWB modes
> >   */
> >  
> >  } /* namespace ipa */
> > diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h
> > index 36c05c37f5e9..017b7a0c3dc1 100644
> > --- a/src/ipa/libipa/awb.h
> > +++ b/src/ipa/libipa/awb.h
> > @@ -54,9 +54,13 @@ protected:
> >  		double ctLo;
> >  	};
> >  
> > -	std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;
> > +	const std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> &modes() const
> > +	{
> > +		return modes_;
> > +	}
> >  
> >  private:
> > +	std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;
> >  	ControlInfoMap::Map controls_;
> >  };
> >  
> > diff --git a/src/ipa/libipa/awb_bayes.cpp b/src/ipa/libipa/awb_bayes.cpp
> > index 9287b884cb95..97bd256526a4 100644
> > --- a/src/ipa/libipa/awb_bayes.cpp
> > +++ b/src/ipa/libipa/awb_bayes.cpp
> > @@ -170,7 +170,7 @@ int AwbBayes::init(const YamlObject &tuningData)
> >  			<< "Failed to parse mode parameter from tuning file";
> >  		return ret;
> >  	}
> > -	currentMode_ = &modes_[controls::AwbAuto];
> > +	currentMode_ = &modes().at(controls::AwbAuto);
> >  
> >  	transversePos_ = tuningData["transversePos"].get<double>(0.01);
> >  	transverseNeg_ = tuningData["transverseNeg"].get<double>(0.01);
> > @@ -256,8 +256,8 @@ void AwbBayes::handleControls(const ControlList &controls)
> >  {
> >  	auto mode = controls.get(controls::AwbMode);
> >  	if (mode) {
> > -		auto it = modes_.find(static_cast<controls::AwbModeEnum>(*mode));
> > -		if (it != modes_.end())
> > +		auto it = modes().find(static_cast<controls::AwbModeEnum>(*mode));
> > +		if (it != modes().end())
> >  			currentMode_ = &it->second;
> >  		else
> >  			LOG(Awb, Error) << "Unsupported AWB mode " << *mode;
> > diff --git a/src/ipa/libipa/awb_bayes.h b/src/ipa/libipa/awb_bayes.h
> > index 23bf88061118..b74201b542fb 100644
> > --- a/src/ipa/libipa/awb_bayes.h
> > +++ b/src/ipa/libipa/awb_bayes.h
> > @@ -59,7 +59,7 @@ private:
> >  	double transversePos_;
> >  	double transverseNeg_;
> >  
> > -	ModeConfig *currentMode_ = nullptr;
> > +	const ModeConfig *currentMode_ = nullptr;
> >  };
> >  
> >  } /* namespace ipa */

Patch
diff mbox series 

diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp
index 04cf2d55ed00..0f54655d3234 100644
--- a/src/ipa/libipa/awb.cpp
+++ b/src/ipa/libipa/awb.cpp
@@ -250,9 +250,9 @@  int AwbAlgorithm::parseModeConfigs(const YamlObject &tuningData,
  */
 
 /**
- * \var AwbAlgorithm::modes_
- * \brief Map of all configured modes
- * \sa AwbAlgorithm::parseModeConfigs
+ * \fn AwbAlgorithm::modes()
+ * \brief Get the available AWB modes
+ * \return The map of available AWB modes
  */
 
 } /* namespace ipa */
diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h
index 36c05c37f5e9..017b7a0c3dc1 100644
--- a/src/ipa/libipa/awb.h
+++ b/src/ipa/libipa/awb.h
@@ -54,9 +54,13 @@  protected:
 		double ctLo;
 	};
 
-	std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;
+	const std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> &modes() const
+	{
+		return modes_;
+	}
 
 private:
+	std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;
 	ControlInfoMap::Map controls_;
 };
 
diff --git a/src/ipa/libipa/awb_bayes.cpp b/src/ipa/libipa/awb_bayes.cpp
index 9287b884cb95..97bd256526a4 100644
--- a/src/ipa/libipa/awb_bayes.cpp
+++ b/src/ipa/libipa/awb_bayes.cpp
@@ -170,7 +170,7 @@  int AwbBayes::init(const YamlObject &tuningData)
 			<< "Failed to parse mode parameter from tuning file";
 		return ret;
 	}
-	currentMode_ = &modes_[controls::AwbAuto];
+	currentMode_ = &modes().at(controls::AwbAuto);
 
 	transversePos_ = tuningData["transversePos"].get<double>(0.01);
 	transverseNeg_ = tuningData["transverseNeg"].get<double>(0.01);
@@ -256,8 +256,8 @@  void AwbBayes::handleControls(const ControlList &controls)
 {
 	auto mode = controls.get(controls::AwbMode);
 	if (mode) {
-		auto it = modes_.find(static_cast<controls::AwbModeEnum>(*mode));
-		if (it != modes_.end())
+		auto it = modes().find(static_cast<controls::AwbModeEnum>(*mode));
+		if (it != modes().end())
 			currentMode_ = &it->second;
 		else
 			LOG(Awb, Error) << "Unsupported AWB mode " << *mode;
diff --git a/src/ipa/libipa/awb_bayes.h b/src/ipa/libipa/awb_bayes.h
index 23bf88061118..b74201b542fb 100644
--- a/src/ipa/libipa/awb_bayes.h
+++ b/src/ipa/libipa/awb_bayes.h
@@ -59,7 +59,7 @@  private:
 	double transversePos_;
 	double transverseNeg_;
 
-	ModeConfig *currentMode_ = nullptr;
+	const ModeConfig *currentMode_ = nullptr;
 };
 
 } /* namespace ipa */