[libcamera-devel,v3,13/14] ipa: ipu3: agc: Remove unused variables
diff mbox series

Message ID 20211021164401.110033-14-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: ipu3: Fix AGC bugs
Related show

Commit Message

Jean-Michel Hautbois Oct. 21, 2021, 4:44 p.m. UTC
We currently control the exposure value by the shutter speed and the
analogue gain. We can't use the digital gain to have more than the
maximum exposure value calculated because we are not controlling it.

Remove unused code associated with this digital gain.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/agc.cpp | 19 +++----------------
 src/ipa/ipu3/algorithms/agc.h   |  2 --
 2 files changed, 3 insertions(+), 18 deletions(-)

Comments

Kieran Bingham Oct. 21, 2021, 9:09 p.m. UTC | #1
Quoting Jean-Michel Hautbois (2021-10-21 17:44:00)
> We currently control the exposure value by the shutter speed and the
> analogue gain. We can't use the digital gain to have more than the
> maximum exposure value calculated because we are not controlling it.
> 
> Remove unused code associated with this digital gain.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I still like removing ununsed code ;-)

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

> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 19 +++----------------
>  src/ipa/ipu3/algorithms/agc.h   |  2 --
>  2 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 7ff3029b..be59d0b0 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -42,8 +42,7 @@ static constexpr double kEvGainTarget = 0.5;
>  Agc::Agc()
>         : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
>           minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s),
> -         filteredExposureNoDg_(0s), currentExposure_(0s),
> -         currentExposureNoDg_(0s), prevExposureValue_(0s)
> +         currentExposure_(0s), prevExposureValue_(0s)
>  {
>  }
>  
> @@ -102,7 +101,6 @@ void Agc::filterExposure()
>         if (filteredExposure_ == 0s) {
>                 /* DG stands for digital gain.*/
>                 filteredExposure_ = currentExposure_;
> -               filteredExposureNoDg_ = currentExposureNoDg_;
>         } else {
>                 /*
>                  * If we are close to the desired result, go faster to avoid making
> @@ -115,18 +113,8 @@ void Agc::filterExposure()
>  
>                 filteredExposure_ = speed * currentExposure_ +
>                                 filteredExposure_ * (1.0 - speed);
> -               filteredExposureNoDg_ = speed * currentExposureNoDg_ +
> -                               filteredExposureNoDg_ * (1.0 - speed);
>         }
> -       /*
> -        * We can't let the no_dg exposure deviate too far below the
> -        * total exposure, as there might not be enough digital gain available
> -        * in the ISP to hide it (which will cause nasty oscillation).
> -        */
> -       double fastReduceThreshold = 0.4;
> -       if (filteredExposureNoDg_ <
> -           filteredExposure_ * fastReduceThreshold)
> -               filteredExposureNoDg_ = filteredExposure_ * fastReduceThreshold;
> +
>         LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << filteredExposure_;
>  }
>  
> @@ -145,8 +133,7 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>  
>         /* extracted from Rpi::Agc::computeTargetExposure */
>         utils::Duration currentShutter = exposure * lineDuration_;
> -       currentExposureNoDg_ = currentShutter * analogueGain;
> -       LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
> +       LOG(IPU3Agc, Debug) << "Actual total exposure " << currentShutter * analogueGain
>                             << " Shutter speed " << currentShutter
>                             << " Gain " << analogueGain
>                             << " Needed ev gain " << evGain;
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 2ae88e9f..ad133b98 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -46,9 +46,7 @@ private:
>         uint32_t maxExposureLines_;
>  
>         utils::Duration filteredExposure_;
> -       utils::Duration filteredExposureNoDg_;
>         utils::Duration currentExposure_;
> -       utils::Duration currentExposureNoDg_;
>         utils::Duration prevExposureValue_;
>  
>         uint32_t stride_;
> -- 
> 2.32.0
>

Patch
diff mbox series

diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
index 7ff3029b..be59d0b0 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -42,8 +42,7 @@  static constexpr double kEvGainTarget = 0.5;
 Agc::Agc()
 	: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
 	  minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s),
-	  filteredExposureNoDg_(0s), currentExposure_(0s),
-	  currentExposureNoDg_(0s), prevExposureValue_(0s)
+	  currentExposure_(0s), prevExposureValue_(0s)
 {
 }
 
@@ -102,7 +101,6 @@  void Agc::filterExposure()
 	if (filteredExposure_ == 0s) {
 		/* DG stands for digital gain.*/
 		filteredExposure_ = currentExposure_;
-		filteredExposureNoDg_ = currentExposureNoDg_;
 	} else {
 		/*
 		 * If we are close to the desired result, go faster to avoid making
@@ -115,18 +113,8 @@  void Agc::filterExposure()
 
 		filteredExposure_ = speed * currentExposure_ +
 				filteredExposure_ * (1.0 - speed);
-		filteredExposureNoDg_ = speed * currentExposureNoDg_ +
-				filteredExposureNoDg_ * (1.0 - speed);
 	}
-	/*
-	 * We can't let the no_dg exposure deviate too far below the
-	 * total exposure, as there might not be enough digital gain available
-	 * in the ISP to hide it (which will cause nasty oscillation).
-	 */
-	double fastReduceThreshold = 0.4;
-	if (filteredExposureNoDg_ <
-	    filteredExposure_ * fastReduceThreshold)
-		filteredExposureNoDg_ = filteredExposure_ * fastReduceThreshold;
+
 	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << filteredExposure_;
 }
 
@@ -145,8 +133,7 @@  void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
 
 	/* extracted from Rpi::Agc::computeTargetExposure */
 	utils::Duration currentShutter = exposure * lineDuration_;
-	currentExposureNoDg_ = currentShutter * analogueGain;
-	LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_
+	LOG(IPU3Agc, Debug) << "Actual total exposure " << currentShutter * analogueGain
 			    << " Shutter speed " << currentShutter
 			    << " Gain " << analogueGain
 			    << " Needed ev gain " << evGain;
diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
index 2ae88e9f..ad133b98 100644
--- a/src/ipa/ipu3/algorithms/agc.h
+++ b/src/ipa/ipu3/algorithms/agc.h
@@ -46,9 +46,7 @@  private:
 	uint32_t maxExposureLines_;
 
 	utils::Duration filteredExposure_;
-	utils::Duration filteredExposureNoDg_;
 	utils::Duration currentExposure_;
-	utils::Duration currentExposureNoDg_;
 	utils::Duration prevExposureValue_;
 
 	uint32_t stride_;