[libcamera-devel,2/2] ipa: ipu3: agc: Remove "using" directive from agc.h header
diff mbox series

Message ID 20211015015804.11758-2-laurent.pinchart@ideasonboard.com
State Accepted
Commit 3b07397f0e7d93d54535d9c87e7b9c19d04c97f0
Headers show
Series
  • [libcamera-devel,1/2] ipa: ipu3: awb: Don't pass member variable to member function
Related show

Commit Message

Laurent Pinchart Oct. 15, 2021, 1:58 a.m. UTC 
"using" directives are harmful in headers, as they propagate the
namespace short-circuit to all files that include the header, directly
or indirectly. Drop the directive from agc.h, and use utils::Duration
explicitly. While at it, shorten the namespace qualifier from
libcamera::utils:: to utils:: in agc.cpp for Duration.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/agc.cpp |  6 +++---
 src/ipa/ipu3/algorithms/agc.h   | 14 ++++++--------
 2 files changed, 9 insertions(+), 11 deletions(-)

Comments

Jean-Michel Hautbois Oct. 15, 2021, 7:04 a.m. UTC | #1 
Hi Laurent,

On 15/10/2021 03:58, Laurent Pinchart wrote:
> "using" directives are harmful in headers, as they propagate the
> namespace short-circuit to all files that include the header, directly
> or indirectly. Drop the directive from agc.h, and use utils::Duration
> explicitly. While at it, shorten the namespace qualifier from
> libcamera::utils:: to utils:: in agc.cpp for Duration.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Indeed, it is better that way, thanks !

Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>

> ---
>  src/ipa/ipu3/algorithms/agc.cpp |  6 +++---
>  src/ipa/ipu3/algorithms/agc.h   | 14 ++++++--------
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 5eafe82ca404..970ec42419cc 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)
>  		double newGain = kEvGainTarget * knumHistogramBins / iqMean_;
>  
>  		/* extracted from Rpi::Agc::computeTargetExposure */
> -		libcamera::utils::Duration currentShutter = exposure * lineDuration_;
> +		utils::Duration currentShutter = exposure * lineDuration_;
>  		currentExposureNoDg_ = currentShutter * gain;
>  		LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
>  				    << " Shutter speed " << currentShutter
>  				    << " Gain " << gain;
>  		currentExposure_ = currentExposureNoDg_ * newGain;
> -		libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;
> +		utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;
>  		currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>  		LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_;
>  
>  		/* \todo: estimate if we need to desaturate */
>  		filterExposure();
>  
> -		libcamera::utils::Duration newExposure = 0.0s;
> +		utils::Duration newExposure = 0.0s;
>  		if (currentShutter < maxExposureTime_) {
>  			exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);
>  			newExposure = currentExposure_ / exposure;
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 64b71c65ef00..e0c315fc126c 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -21,8 +21,6 @@ struct IPACameraSensorInfo;
>  
>  namespace ipa::ipu3::algorithms {
>  
> -using utils::Duration;
> -
>  class Agc : public Algorithm
>  {
>  public:
> @@ -43,13 +41,13 @@ private:
>  
>  	double iqMean_;
>  
> -	Duration lineDuration_;
> -	Duration maxExposureTime_;
> +	utils::Duration lineDuration_;
> +	utils::Duration maxExposureTime_;
>  
> -	Duration prevExposure_;
> -	Duration prevExposureNoDg_;
> -	Duration currentExposure_;
> -	Duration currentExposureNoDg_;
> +	utils::Duration prevExposure_;
> +	utils::Duration prevExposureNoDg_;
> +	utils::Duration currentExposure_;
> +	utils::Duration currentExposureNoDg_;
>  
>  	uint32_t stride_;
>  };
>
Kieran Bingham Oct. 15, 2021, 9:06 p.m. UTC | #2 
Quoting Laurent Pinchart (2021-10-15 02:58:04)
> "using" directives are harmful in headers, as they propagate the
> namespace short-circuit to all files that include the header, directly
> or indirectly. Drop the directive from agc.h, and use utils::Duration
> explicitly. While at it, shorten the namespace qualifier from
> libcamera::utils:: to utils:: in agc.cpp for Duration.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/ipa/ipu3/algorithms/agc.cpp |  6 +++---
>  src/ipa/ipu3/algorithms/agc.h   | 14 ++++++--------
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 5eafe82ca404..970ec42419cc 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)
>                 double newGain = kEvGainTarget * knumHistogramBins / iqMean_;
>  
>                 /* extracted from Rpi::Agc::computeTargetExposure */
> -               libcamera::utils::Duration currentShutter = exposure * lineDuration_;
> +               utils::Duration currentShutter = exposure * lineDuration_;
>                 currentExposureNoDg_ = currentShutter * gain;
>                 LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
>                                     << " Shutter speed " << currentShutter
>                                     << " Gain " << gain;
>                 currentExposure_ = currentExposureNoDg_ * newGain;
> -               libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;
> +               utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;
>                 currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>                 LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_;
>  
>                 /* \todo: estimate if we need to desaturate */
>                 filterExposure();
>  
> -               libcamera::utils::Duration newExposure = 0.0s;
> +               utils::Duration newExposure = 0.0s;
>                 if (currentShutter < maxExposureTime_) {
>                         exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);
>                         newExposure = currentExposure_ / exposure;
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 64b71c65ef00..e0c315fc126c 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -21,8 +21,6 @@ struct IPACameraSensorInfo;
>  
>  namespace ipa::ipu3::algorithms {
>  
> -using utils::Duration;
> -
>  class Agc : public Algorithm
>  {
>  public:
> @@ -43,13 +41,13 @@ private:
>  
>         double iqMean_;
>  
> -       Duration lineDuration_;
> -       Duration maxExposureTime_;
> +       utils::Duration lineDuration_;
> +       utils::Duration maxExposureTime_;
>  
> -       Duration prevExposure_;
> -       Duration prevExposureNoDg_;
> -       Duration currentExposure_;
> -       Duration currentExposureNoDg_;
> +       utils::Duration prevExposure_;
> +       utils::Duration prevExposureNoDg_;
> +       utils::Duration currentExposure_;
> +       utils::Duration currentExposureNoDg_;
>  
>         uint32_t stride_;
>  };
> -- 
> Regards,
> 
> Laurent Pinchart
>
Paul Elder Oct. 18, 2021, 4:23 a.m. UTC | #3 
Hi Laurent,

On Fri, Oct 15, 2021 at 04:58:04AM +0300, Laurent Pinchart wrote:
> "using" directives are harmful in headers, as they propagate the
> namespace short-circuit to all files that include the header, directly
> or indirectly. Drop the directive from agc.h, and use utils::Duration
> explicitly. While at it, shorten the namespace qualifier from
> libcamera::utils:: to utils:: in agc.cpp for Duration.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/ipa/ipu3/algorithms/agc.cpp |  6 +++---
>  src/ipa/ipu3/algorithms/agc.h   | 14 ++++++--------
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 5eafe82ca404..970ec42419cc 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)
>  		double newGain = kEvGainTarget * knumHistogramBins / iqMean_;
>  
>  		/* extracted from Rpi::Agc::computeTargetExposure */
> -		libcamera::utils::Duration currentShutter = exposure * lineDuration_;
> +		utils::Duration currentShutter = exposure * lineDuration_;
>  		currentExposureNoDg_ = currentShutter * gain;
>  		LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
>  				    << " Shutter speed " << currentShutter
>  				    << " Gain " << gain;
>  		currentExposure_ = currentExposureNoDg_ * newGain;
> -		libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;
> +		utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;
>  		currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>  		LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_;
>  
>  		/* \todo: estimate if we need to desaturate */
>  		filterExposure();
>  
> -		libcamera::utils::Duration newExposure = 0.0s;
> +		utils::Duration newExposure = 0.0s;
>  		if (currentShutter < maxExposureTime_) {
>  			exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);
>  			newExposure = currentExposure_ / exposure;
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 64b71c65ef00..e0c315fc126c 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -21,8 +21,6 @@ struct IPACameraSensorInfo;
>  
>  namespace ipa::ipu3::algorithms {
>  
> -using utils::Duration;
> -
>  class Agc : public Algorithm
>  {
>  public:
> @@ -43,13 +41,13 @@ private:
>  
>  	double iqMean_;
>  
> -	Duration lineDuration_;
> -	Duration maxExposureTime_;
> +	utils::Duration lineDuration_;
> +	utils::Duration maxExposureTime_;
>  
> -	Duration prevExposure_;
> -	Duration prevExposureNoDg_;
> -	Duration currentExposure_;
> -	Duration currentExposureNoDg_;
> +	utils::Duration prevExposure_;
> +	utils::Duration prevExposureNoDg_;
> +	utils::Duration currentExposure_;
> +	utils::Duration currentExposureNoDg_;
>  
>  	uint32_t stride_;
>  };
> -- 
> Regards,
> 
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
index 5eafe82ca404..970ec42419cc 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -140,20 +140,20 @@  void Agc::lockExposureGain(uint32_t &exposure, double &gain)
 		double newGain = kEvGainTarget * knumHistogramBins / iqMean_;
 
 		/* extracted from Rpi::Agc::computeTargetExposure */
-		libcamera::utils::Duration currentShutter = exposure * lineDuration_;
+		utils::Duration currentShutter = exposure * lineDuration_;
 		currentExposureNoDg_ = currentShutter * gain;
 		LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
 				    << " Shutter speed " << currentShutter
 				    << " Gain " << gain;
 		currentExposure_ = currentExposureNoDg_ * newGain;
-		libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;
+		utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;
 		currentExposure_ = std::min(currentExposure_, maxTotalExposure);
 		LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_;
 
 		/* \todo: estimate if we need to desaturate */
 		filterExposure();
 
-		libcamera::utils::Duration newExposure = 0.0s;
+		utils::Duration newExposure = 0.0s;
 		if (currentShutter < maxExposureTime_) {
 			exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);
 			newExposure = currentExposure_ / exposure;
diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
index 64b71c65ef00..e0c315fc126c 100644
--- a/src/ipa/ipu3/algorithms/agc.h
+++ b/src/ipa/ipu3/algorithms/agc.h
@@ -21,8 +21,6 @@  struct IPACameraSensorInfo;
 
 namespace ipa::ipu3::algorithms {
 
-using utils::Duration;
-
 class Agc : public Algorithm
 {
 public:
@@ -43,13 +41,13 @@  private:
 
 	double iqMean_;
 
-	Duration lineDuration_;
-	Duration maxExposureTime_;
+	utils::Duration lineDuration_;
+	utils::Duration maxExposureTime_;
 
-	Duration prevExposure_;
-	Duration prevExposureNoDg_;
-	Duration currentExposure_;
-	Duration currentExposureNoDg_;
+	utils::Duration prevExposure_;
+	utils::Duration prevExposureNoDg_;
+	utils::Duration currentExposure_;
+	utils::Duration currentExposureNoDg_;
 
 	uint32_t stride_;
 };