[libcamera-devel,v3,9/9] ipa: ipu3: Move IPU3 agc into algorithms
diff mbox series

Message ID 20210818155403.268694-10-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • IPU3: Introduce modularity for algorithms
Related show

Commit Message

Jean-Michel Hautbois Aug. 18, 2021, 3:54 p.m. UTC 
Now that the interface is properly used by the AGC class, move it into
ipa::ipu3::algorithms and let the loops do the calls.
As we need to exchange the exposure_ and gain_ by passing them through the
FrameContext, use the calculated values in setControls() function to
ease the reading.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 21 +++++++++----------
 src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 20 +++++++++---------
 src/ipa/ipu3/algorithms/meson.build           |  1 +
 src/ipa/ipu3/ipu3.cpp                         | 15 +++++--------
 src/ipa/ipu3/meson.build                      |  1 -
 5 files changed, 26 insertions(+), 32 deletions(-)
 rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (93%)
 rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (73%)

Comments

Laurent Pinchart Aug. 18, 2021, 10:05 p.m. UTC | #1 
Hi Jean-Michel,

Thank you for the patch.

On Wed, Aug 18, 2021 at 05:54:03PM +0200, Jean-Michel Hautbois wrote:
> Now that the interface is properly used by the AGC class, move it into
> ipa::ipu3::algorithms and let the loops do the calls.
> As we need to exchange the exposure_ and gain_ by passing them through the
> FrameContext, use the calculated values in setControls() function to
> ease the reading.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 21 +++++++++----------
>  src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 20 +++++++++---------
>  src/ipa/ipu3/algorithms/meson.build           |  1 +
>  src/ipa/ipu3/ipu3.cpp                         | 15 +++++--------
>  src/ipa/ipu3/meson.build                      |  1 -
>  5 files changed, 26 insertions(+), 32 deletions(-)
>  rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (93%)
>  rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (73%)
> 
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> similarity index 93%
> rename from src/ipa/ipu3/ipu3_agc.cpp
> rename to src/ipa/ipu3/algorithms/agc.cpp
> index 1c1f5fb5..bb119cb4 100644
> --- a/src/ipa/ipu3/ipu3_agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -5,7 +5,7 @@
>   * ipu3_agc.cpp - AGC/AEC control algorithm
>   */
>  
> -#include "ipu3_agc.h"
> +#include "agc.h"
>  
>  #include <algorithm>
>  #include <chrono>
> @@ -21,7 +21,7 @@ namespace libcamera {
>  
>  using namespace std::literals::chrono_literals;
>  
> -namespace ipa::ipu3 {
> +namespace ipa::ipu3::algorithms {
>  
>  LOG_DEFINE_CATEGORY(IPU3Agc)
>  
> @@ -50,14 +50,13 @@ static constexpr double kEvGainTarget = 0.5;
>  /* A cell is 8 bytes and contains averages for RGB values and saturation ratio */
>  static constexpr uint8_t kCellSize = 8;
>  
> -IPU3Agc::IPU3Agc()
> +Agc::Agc()
>  	: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
> -	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),
> -	  currentExposure_(0s), currentExposureNoDg_(0s)

Why can those two fields not be initialized anymore ?

> +	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s)
>  {
>  }
>  
> -int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> +int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>  {
>  	aeGrid_ = context.configuration.grid.bdsGrid;
>  
> @@ -67,7 +66,7 @@ int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>  	return 0;
>  }
>  
> -void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
> +void Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
>  {
>  	const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;
>  	Rectangle aeRegion = { statsAeGrid.x_start,
> @@ -108,7 +107,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
>  	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
>  }
>  
> -void IPU3Agc::filterExposure()
> +void Agc::filterExposure()
>  {
>  	double speed = 0.2;
>  	if (prevExposure_ == 0s) {
> @@ -142,7 +141,7 @@ void IPU3Agc::filterExposure()
>  	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << prevExposure_;
>  }
>  
> -void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
> +void Agc::lockExposureGain(uint32_t &exposure, double &gain)
>  {
>  	/* Algorithm initialization should wait for first valid frames */
>  	/* \todo - have a number of frames given by DelayedControls ?
> @@ -185,7 +184,7 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
>  	lastFrame_ = frameCount_;
>  }
>  
> -void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> +void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>  {
>  	uint32_t &exposure = context.frameContext.agc.exposure;
>  	double &gain = context.frameContext.agc.gain;
> @@ -194,6 +193,6 @@ void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>  	frameCount_++;
>  }
>  
> -} /* namespace ipa::ipu3 */
> +} /* namespace ipa::ipu3::algorithms */
>  
>  } /* namespace libcamera */
> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/algorithms/agc.h
> similarity index 73%
> rename from src/ipa/ipu3/ipu3_agc.h
> rename to src/ipa/ipu3/algorithms/agc.h
> index 0e922664..1739d2fd 100644
> --- a/src/ipa/ipu3/ipu3_agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -2,10 +2,10 @@
>  /*
>   * Copyright (C) 2021, Ideas On Board
>   *
> - * ipu3_agc.h - IPU3 AGC/AEC control algorithm
> + * agc.h - IPU3 AGC/AEC control algorithm
>   */
> -#ifndef __LIBCAMERA_IPU3_AGC_H__
> -#define __LIBCAMERA_IPU3_AGC_H__
> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
> +#define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
>  
>  #include <linux/intel-ipu3.h>
>  
> @@ -13,21 +13,21 @@
>  
>  #include <libcamera/geometry.h>
>  
> -#include "algorithms/algorithm.h"
> +#include "algorithm.h"
>  
>  namespace libcamera {
>  
>  struct IPACameraSensorInfo;
>  
> -namespace ipa::ipu3 {
> +namespace ipa::ipu3::algorithms {
>  
>  using utils::Duration;
>  
> -class IPU3Agc : public Algorithm
> +class Agc : public Algorithm
>  {
>  public:
> -	IPU3Agc();
> -	~IPU3Agc() = default;
> +	Agc();
> +	~Agc() = default;
>  
>  	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>  	void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> @@ -53,8 +53,8 @@ private:
>  	Duration currentExposureNoDg_;
>  };
>  
> -} /* namespace ipa::ipu3 */
> +} /* namespace ipa::ipu3::algorithms */
>  
>  } /* namespace libcamera */
>  
> -#endif /* __LIBCAMERA_IPU3_AGC_H__ */
> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__ */
> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> index 87377f4e..acaadd90 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -2,6 +2,7 @@
>  
>  ipu3_ipa_algorithms = files([
>      'algorithm.cpp',
> +    'agc.cpp',

Wrong alphabetical order.

>      'awb.cpp',
>      'contrast.cpp',
>  ])
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index f4f49025..d73ec79d 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -30,9 +30,9 @@
>  #include "libcamera/internal/mapped_framebuffer.h"
>  
>  #include "algorithms/algorithm.h"
> +#include "algorithms/agc.h"

Here too.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  #include "algorithms/awb.h"
>  #include "algorithms/contrast.h"
> -#include "ipu3_agc.h"
>  #include "libipa/camera_sensor_helper.h"
>  
>  static constexpr uint32_t kMaxCellWidthPerSet = 160;
> @@ -136,8 +136,6 @@ private:
>  	uint32_t minGain_;
>  	uint32_t maxGain_;
>  
> -	/* Interface to the AEC/AGC algorithm */
> -	std::unique_ptr<IPU3Agc> agcAlgo_;
>  	/* Interface to the Camera Helper */
>  	std::unique_ptr<CameraSensorHelper> camHelper_;
>  
> @@ -218,6 +216,7 @@ int IPAIPU3::init(const IPASettings &settings,
>  	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
>  
>  	/* Construct our Algorithms */
> +	algorithms_.emplace_back(new algorithms::Agc());
>  	algorithms_.emplace_back(new algorithms::Awb());
>  	algorithms_.emplace_back(new algorithms::Contrast());
>  
> @@ -337,10 +336,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
>  			return ret;
>  	}
>  
> -
> -	agcAlgo_ = std::make_unique<IPU3Agc>();
> -	agcAlgo_->configure(context_, configInfo);
> -
>  	return 0;
>  }
>  
> @@ -436,9 +431,6 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>  	for (auto const &algo : algorithms_)
>  		algo->process(context_, stats);
>  
> -	agcAlgo_->process(context_, stats);
> -	exposure_ = context_.frameContext.agc.exposure;
> -	gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
>  	setControls(frame);
>  
>  	/* \todo Use VBlank value calculated from each frame exposure. */
> @@ -458,6 +450,9 @@ void IPAIPU3::setControls(unsigned int frame)
>  	IPU3Action op;
>  	op.op = ActionSetSensorControls;
>  
> +	exposure_ = context_.frameContext.agc.exposure;
> +	gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
> +
>  	ControlList ctrls(ctrls_);
>  	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
> index d1126947..39320980 100644
> --- a/src/ipa/ipu3/meson.build
> +++ b/src/ipa/ipu3/meson.build
> @@ -6,7 +6,6 @@ ipa_name = 'ipa_ipu3'
>  
>  ipu3_ipa_sources = files([
>      'ipu3.cpp',
> -    'ipu3_agc.cpp',
>  ])
>  
>  ipu3_ipa_sources += ipu3_ipa_algorithms
Kieran Bingham Aug. 18, 2021, 10:09 p.m. UTC | #2 
On 18/08/2021 23:05, Laurent Pinchart wrote:
> Hi Jean-Michel,
> 
> Thank you for the patch.
> 
> On Wed, Aug 18, 2021 at 05:54:03PM +0200, Jean-Michel Hautbois wrote:
>> Now that the interface is properly used by the AGC class, move it into
>> ipa::ipu3::algorithms and let the loops do the calls.
>> As we need to exchange the exposure_ and gain_ by passing them through the
>> FrameContext, use the calculated values in setControls() function to
>> ease the reading.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>  .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 21 +++++++++----------
>>  src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 20 +++++++++---------
>>  src/ipa/ipu3/algorithms/meson.build           |  1 +
>>  src/ipa/ipu3/ipu3.cpp                         | 15 +++++--------
>>  src/ipa/ipu3/meson.build                      |  1 -
>>  5 files changed, 26 insertions(+), 32 deletions(-)
>>  rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (93%)
>>  rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (73%)
>>
>> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> similarity index 93%
>> rename from src/ipa/ipu3/ipu3_agc.cpp
>> rename to src/ipa/ipu3/algorithms/agc.cpp
>> index 1c1f5fb5..bb119cb4 100644
>> --- a/src/ipa/ipu3/ipu3_agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -5,7 +5,7 @@
>>   * ipu3_agc.cpp - AGC/AEC control algorithm
>>   */
>>  
>> -#include "ipu3_agc.h"
>> +#include "agc.h"
>>  
>>  #include <algorithm>
>>  #include <chrono>
>> @@ -21,7 +21,7 @@ namespace libcamera {
>>  
>>  using namespace std::literals::chrono_literals;
>>  
>> -namespace ipa::ipu3 {
>> +namespace ipa::ipu3::algorithms {
>>  
>>  LOG_DEFINE_CATEGORY(IPU3Agc)
>>  
>> @@ -50,14 +50,13 @@ static constexpr double kEvGainTarget = 0.5;
>>  /* A cell is 8 bytes and contains averages for RGB values and saturation ratio */
>>  static constexpr uint8_t kCellSize = 8;
>>  
>> -IPU3Agc::IPU3Agc()
>> +Agc::Agc()
>>  	: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
>> -	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),
>> -	  currentExposure_(0s), currentExposureNoDg_(0s)
> 
> Why can those two fields not be initialized anymore ?
> 
>> +	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s)
>>  {
>>  }
>>  
>> -int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>> +int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>>  {
>>  	aeGrid_ = context.configuration.grid.bdsGrid;
>>  
>> @@ -67,7 +66,7 @@ int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>>  	return 0;
>>  }
>>  
>> -void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
>> +void Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
>>  {
>>  	const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;
>>  	Rectangle aeRegion = { statsAeGrid.x_start,
>> @@ -108,7 +107,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
>>  	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
>>  }
>>  
>> -void IPU3Agc::filterExposure()
>> +void Agc::filterExposure()
>>  {
>>  	double speed = 0.2;
>>  	if (prevExposure_ == 0s) {
>> @@ -142,7 +141,7 @@ void IPU3Agc::filterExposure()
>>  	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << prevExposure_;
>>  }
>>  
>> -void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
>> +void Agc::lockExposureGain(uint32_t &exposure, double &gain)
>>  {
>>  	/* Algorithm initialization should wait for first valid frames */
>>  	/* \todo - have a number of frames given by DelayedControls ?
>> @@ -185,7 +184,7 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
>>  	lastFrame_ = frameCount_;
>>  }
>>  
>> -void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>> +void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>>  {
>>  	uint32_t &exposure = context.frameContext.agc.exposure;
>>  	double &gain = context.frameContext.agc.gain;
>> @@ -194,6 +193,6 @@ void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>>  	frameCount_++;
>>  }
>>  
>> -} /* namespace ipa::ipu3 */
>> +} /* namespace ipa::ipu3::algorithms */
>>  
>>  } /* namespace libcamera */
>> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/algorithms/agc.h
>> similarity index 73%
>> rename from src/ipa/ipu3/ipu3_agc.h
>> rename to src/ipa/ipu3/algorithms/agc.h
>> index 0e922664..1739d2fd 100644
>> --- a/src/ipa/ipu3/ipu3_agc.h
>> +++ b/src/ipa/ipu3/algorithms/agc.h
>> @@ -2,10 +2,10 @@
>>  /*
>>   * Copyright (C) 2021, Ideas On Board
>>   *
>> - * ipu3_agc.h - IPU3 AGC/AEC control algorithm
>> + * agc.h - IPU3 AGC/AEC control algorithm
>>   */
>> -#ifndef __LIBCAMERA_IPU3_AGC_H__
>> -#define __LIBCAMERA_IPU3_AGC_H__
>> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
>> +#define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
>>  
>>  #include <linux/intel-ipu3.h>
>>  
>> @@ -13,21 +13,21 @@
>>  
>>  #include <libcamera/geometry.h>
>>  
>> -#include "algorithms/algorithm.h"
>> +#include "algorithm.h"
>>  
>>  namespace libcamera {
>>  
>>  struct IPACameraSensorInfo;
>>  
>> -namespace ipa::ipu3 {
>> +namespace ipa::ipu3::algorithms {
>>  
>>  using utils::Duration;
>>  
>> -class IPU3Agc : public Algorithm
>> +class Agc : public Algorithm
>>  {
>>  public:
>> -	IPU3Agc();
>> -	~IPU3Agc() = default;
>> +	Agc();
>> +	~Agc() = default;
>>  
>>  	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>>  	void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
>> @@ -53,8 +53,8 @@ private:
>>  	Duration currentExposureNoDg_;
>>  };
>>  
>> -} /* namespace ipa::ipu3 */
>> +} /* namespace ipa::ipu3::algorithms */
>>  
>>  } /* namespace libcamera */
>>  
>> -#endif /* __LIBCAMERA_IPU3_AGC_H__ */
>> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__ */
>> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
>> index 87377f4e..acaadd90 100644
>> --- a/src/ipa/ipu3/algorithms/meson.build
>> +++ b/src/ipa/ipu3/algorithms/meson.build
>> @@ -2,6 +2,7 @@
>>  
>>  ipu3_ipa_algorithms = files([
>>      'algorithm.cpp',
>> +    'agc.cpp',
> 
> Wrong alphabetical order.

This one doesn't worry me too much if it's sorted alphabetically...

> 
>>      'awb.cpp',
>>      'contrast.cpp',
>>  ])
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index f4f49025..d73ec79d 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -30,9 +30,9 @@
>>  #include "libcamera/internal/mapped_framebuffer.h"
>>  
>>  #include "algorithms/algorithm.h"
>> +#include "algorithms/agc.h"
> 
> Here too.
> 

But this one does.

If agc.h comes before algorithm.h, then by definition we're relying upon
the inclusion of algorithm.h from agc.h, so there's no point it being
here at all.

But I'd personally keep it, and keep it at the beginning of the
inclusions, as an exception to the alphabetical ordering.

The other algorithms can be sorted alphabetically though

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
>>  #include "algorithms/awb.h"
>>  #include "algorithms/contrast.h"
>> -#include "ipu3_agc.h"
>>  #include "libipa/camera_sensor_helper.h"
>>  
>>  static constexpr uint32_t kMaxCellWidthPerSet = 160;
>> @@ -136,8 +136,6 @@ private:
>>  	uint32_t minGain_;
>>  	uint32_t maxGain_;
>>  
>> -	/* Interface to the AEC/AGC algorithm */
>> -	std::unique_ptr<IPU3Agc> agcAlgo_;
>>  	/* Interface to the Camera Helper */
>>  	std::unique_ptr<CameraSensorHelper> camHelper_;
>>  
>> @@ -218,6 +216,7 @@ int IPAIPU3::init(const IPASettings &settings,
>>  	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
>>  
>>  	/* Construct our Algorithms */
>> +	algorithms_.emplace_back(new algorithms::Agc());
>>  	algorithms_.emplace_back(new algorithms::Awb());
>>  	algorithms_.emplace_back(new algorithms::Contrast());
>>  
>> @@ -337,10 +336,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
>>  			return ret;
>>  	}
>>  
>> -
>> -	agcAlgo_ = std::make_unique<IPU3Agc>();
>> -	agcAlgo_->configure(context_, configInfo);
>> -
>>  	return 0;
>>  }
>>  
>> @@ -436,9 +431,6 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>>  	for (auto const &algo : algorithms_)
>>  		algo->process(context_, stats);
>>  
>> -	agcAlgo_->process(context_, stats);
>> -	exposure_ = context_.frameContext.agc.exposure;
>> -	gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
>>  	setControls(frame);
>>  
>>  	/* \todo Use VBlank value calculated from each frame exposure. */
>> @@ -458,6 +450,9 @@ void IPAIPU3::setControls(unsigned int frame)
>>  	IPU3Action op;
>>  	op.op = ActionSetSensorControls;
>>  
>> +	exposure_ = context_.frameContext.agc.exposure;
>> +	gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
>> +
>>  	ControlList ctrls(ctrls_);
>>  	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
>>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
>> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
>> index d1126947..39320980 100644
>> --- a/src/ipa/ipu3/meson.build
>> +++ b/src/ipa/ipu3/meson.build
>> @@ -6,7 +6,6 @@ ipa_name = 'ipa_ipu3'
>>  
>>  ipu3_ipa_sources = files([
>>      'ipu3.cpp',
>> -    'ipu3_agc.cpp',
>>  ])
>>  
>>  ipu3_ipa_sources += ipu3_ipa_algorithms
>
Laurent Pinchart Aug. 18, 2021, 10:18 p.m. UTC | #3 
Hi Kieran,

On Wed, Aug 18, 2021 at 11:09:03PM +0100, Kieran Bingham wrote:
> On 18/08/2021 23:05, Laurent Pinchart wrote:
> > On Wed, Aug 18, 2021 at 05:54:03PM +0200, Jean-Michel Hautbois wrote:
> >> Now that the interface is properly used by the AGC class, move it into
> >> ipa::ipu3::algorithms and let the loops do the calls.
> >> As we need to exchange the exposure_ and gain_ by passing them through the
> >> FrameContext, use the calculated values in setControls() function to
> >> ease the reading.
> >>
> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >> ---
> >>  .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 21 +++++++++----------
> >>  src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 20 +++++++++---------
> >>  src/ipa/ipu3/algorithms/meson.build           |  1 +
> >>  src/ipa/ipu3/ipu3.cpp                         | 15 +++++--------
> >>  src/ipa/ipu3/meson.build                      |  1 -
> >>  5 files changed, 26 insertions(+), 32 deletions(-)
> >>  rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (93%)
> >>  rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (73%)
> >>
> >> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> >> similarity index 93%
> >> rename from src/ipa/ipu3/ipu3_agc.cpp
> >> rename to src/ipa/ipu3/algorithms/agc.cpp
> >> index 1c1f5fb5..bb119cb4 100644
> >> --- a/src/ipa/ipu3/ipu3_agc.cpp
> >> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> >> @@ -5,7 +5,7 @@
> >>   * ipu3_agc.cpp - AGC/AEC control algorithm
> >>   */
> >>  
> >> -#include "ipu3_agc.h"
> >> +#include "agc.h"
> >>  
> >>  #include <algorithm>
> >>  #include <chrono>
> >> @@ -21,7 +21,7 @@ namespace libcamera {
> >>  
> >>  using namespace std::literals::chrono_literals;
> >>  
> >> -namespace ipa::ipu3 {
> >> +namespace ipa::ipu3::algorithms {
> >>  
> >>  LOG_DEFINE_CATEGORY(IPU3Agc)
> >>  
> >> @@ -50,14 +50,13 @@ static constexpr double kEvGainTarget = 0.5;
> >>  /* A cell is 8 bytes and contains averages for RGB values and saturation ratio */
> >>  static constexpr uint8_t kCellSize = 8;
> >>  
> >> -IPU3Agc::IPU3Agc()
> >> +Agc::Agc()
> >>  	: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
> >> -	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),
> >> -	  currentExposure_(0s), currentExposureNoDg_(0s)
> > 
> > Why can those two fields not be initialized anymore ?
> > 
> >> +	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s)
> >>  {
> >>  }
> >>  
> >> -int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> >> +int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> >>  {
> >>  	aeGrid_ = context.configuration.grid.bdsGrid;
> >>  
> >> @@ -67,7 +66,7 @@ int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> >>  	return 0;
> >>  }
> >>  
> >> -void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
> >> +void Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
> >>  {
> >>  	const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;
> >>  	Rectangle aeRegion = { statsAeGrid.x_start,
> >> @@ -108,7 +107,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
> >>  	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> >>  }
> >>  
> >> -void IPU3Agc::filterExposure()
> >> +void Agc::filterExposure()
> >>  {
> >>  	double speed = 0.2;
> >>  	if (prevExposure_ == 0s) {
> >> @@ -142,7 +141,7 @@ void IPU3Agc::filterExposure()
> >>  	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << prevExposure_;
> >>  }
> >>  
> >> -void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
> >> +void Agc::lockExposureGain(uint32_t &exposure, double &gain)
> >>  {
> >>  	/* Algorithm initialization should wait for first valid frames */
> >>  	/* \todo - have a number of frames given by DelayedControls ?
> >> @@ -185,7 +184,7 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
> >>  	lastFrame_ = frameCount_;
> >>  }
> >>  
> >> -void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> >> +void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> >>  {
> >>  	uint32_t &exposure = context.frameContext.agc.exposure;
> >>  	double &gain = context.frameContext.agc.gain;
> >> @@ -194,6 +193,6 @@ void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> >>  	frameCount_++;
> >>  }
> >>  
> >> -} /* namespace ipa::ipu3 */
> >> +} /* namespace ipa::ipu3::algorithms */
> >>  
> >>  } /* namespace libcamera */
> >> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/algorithms/agc.h
> >> similarity index 73%
> >> rename from src/ipa/ipu3/ipu3_agc.h
> >> rename to src/ipa/ipu3/algorithms/agc.h
> >> index 0e922664..1739d2fd 100644
> >> --- a/src/ipa/ipu3/ipu3_agc.h
> >> +++ b/src/ipa/ipu3/algorithms/agc.h
> >> @@ -2,10 +2,10 @@
> >>  /*
> >>   * Copyright (C) 2021, Ideas On Board
> >>   *
> >> - * ipu3_agc.h - IPU3 AGC/AEC control algorithm
> >> + * agc.h - IPU3 AGC/AEC control algorithm
> >>   */
> >> -#ifndef __LIBCAMERA_IPU3_AGC_H__
> >> -#define __LIBCAMERA_IPU3_AGC_H__
> >> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
> >> +#define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
> >>  
> >>  #include <linux/intel-ipu3.h>
> >>  
> >> @@ -13,21 +13,21 @@
> >>  
> >>  #include <libcamera/geometry.h>
> >>  
> >> -#include "algorithms/algorithm.h"
> >> +#include "algorithm.h"
> >>  
> >>  namespace libcamera {
> >>  
> >>  struct IPACameraSensorInfo;
> >>  
> >> -namespace ipa::ipu3 {
> >> +namespace ipa::ipu3::algorithms {
> >>  
> >>  using utils::Duration;
> >>  
> >> -class IPU3Agc : public Algorithm
> >> +class Agc : public Algorithm
> >>  {
> >>  public:
> >> -	IPU3Agc();
> >> -	~IPU3Agc() = default;
> >> +	Agc();
> >> +	~Agc() = default;
> >>  
> >>  	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> >>  	void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> >> @@ -53,8 +53,8 @@ private:
> >>  	Duration currentExposureNoDg_;
> >>  };
> >>  
> >> -} /* namespace ipa::ipu3 */
> >> +} /* namespace ipa::ipu3::algorithms */
> >>  
> >>  } /* namespace libcamera */
> >>  
> >> -#endif /* __LIBCAMERA_IPU3_AGC_H__ */
> >> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__ */
> >> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> >> index 87377f4e..acaadd90 100644
> >> --- a/src/ipa/ipu3/algorithms/meson.build
> >> +++ b/src/ipa/ipu3/algorithms/meson.build
> >> @@ -2,6 +2,7 @@
> >>  
> >>  ipu3_ipa_algorithms = files([
> >>      'algorithm.cpp',
> >> +    'agc.cpp',
> > 
> > Wrong alphabetical order.
> 
> This one doesn't worry me too much if it's sorted alphabetically...
> 
> >>      'awb.cpp',
> >>      'contrast.cpp',
> >>  ])
> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> >> index f4f49025..d73ec79d 100644
> >> --- a/src/ipa/ipu3/ipu3.cpp
> >> +++ b/src/ipa/ipu3/ipu3.cpp
> >> @@ -30,9 +30,9 @@
> >>  #include "libcamera/internal/mapped_framebuffer.h"
> >>  
> >>  #include "algorithms/algorithm.h"
> >> +#include "algorithms/agc.h"
> > 
> > Here too.
> 
> But this one does.
> 
> If agc.h comes before algorithm.h, then by definition we're relying upon
> the inclusion of algorithm.h from agc.h, so there's no point it being
> here at all.

I've thought the same, but every exception will be flagged repeatedly by
checkstyle.py. If it's not clear why the exception is there, someone
will probably submit a patch. That's why I'd rather sort headers
alphabetically here too, and then forget about it :-) I wouldn't be
against dropping algorithm.h if it bothers you. Or algorithm.h could
move to the parent directory, with only the algorithm implementation
stored in algorithms/. It's really nitpicking :-)

> But I'd personally keep it, and keep it at the beginning of the
> inclusions, as an exception to the alphabetical ordering.
> 
> The other algorithms can be sorted alphabetically though
> 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> >>  #include "algorithms/awb.h"
> >>  #include "algorithms/contrast.h"
> >> -#include "ipu3_agc.h"
> >>  #include "libipa/camera_sensor_helper.h"
> >>  
> >>  static constexpr uint32_t kMaxCellWidthPerSet = 160;
> >> @@ -136,8 +136,6 @@ private:
> >>  	uint32_t minGain_;
> >>  	uint32_t maxGain_;
> >>  
> >> -	/* Interface to the AEC/AGC algorithm */
> >> -	std::unique_ptr<IPU3Agc> agcAlgo_;
> >>  	/* Interface to the Camera Helper */
> >>  	std::unique_ptr<CameraSensorHelper> camHelper_;
> >>  
> >> @@ -218,6 +216,7 @@ int IPAIPU3::init(const IPASettings &settings,
> >>  	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
> >>  
> >>  	/* Construct our Algorithms */
> >> +	algorithms_.emplace_back(new algorithms::Agc());
> >>  	algorithms_.emplace_back(new algorithms::Awb());
> >>  	algorithms_.emplace_back(new algorithms::Contrast());
> >>  
> >> @@ -337,10 +336,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
> >>  			return ret;
> >>  	}
> >>  
> >> -
> >> -	agcAlgo_ = std::make_unique<IPU3Agc>();
> >> -	agcAlgo_->configure(context_, configInfo);
> >> -
> >>  	return 0;
> >>  }
> >>  
> >> @@ -436,9 +431,6 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> >>  	for (auto const &algo : algorithms_)
> >>  		algo->process(context_, stats);
> >>  
> >> -	agcAlgo_->process(context_, stats);
> >> -	exposure_ = context_.frameContext.agc.exposure;
> >> -	gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
> >>  	setControls(frame);
> >>  
> >>  	/* \todo Use VBlank value calculated from each frame exposure. */
> >> @@ -458,6 +450,9 @@ void IPAIPU3::setControls(unsigned int frame)
> >>  	IPU3Action op;
> >>  	op.op = ActionSetSensorControls;
> >>  
> >> +	exposure_ = context_.frameContext.agc.exposure;
> >> +	gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
> >> +
> >>  	ControlList ctrls(ctrls_);
> >>  	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
> >>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> >> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
> >> index d1126947..39320980 100644
> >> --- a/src/ipa/ipu3/meson.build
> >> +++ b/src/ipa/ipu3/meson.build
> >> @@ -6,7 +6,6 @@ ipa_name = 'ipa_ipu3'
> >>  
> >>  ipu3_ipa_sources = files([
> >>      'ipu3.cpp',
> >> -    'ipu3_agc.cpp',
> >>  ])
> >>  
> >>  ipu3_ipa_sources += ipu3_ipa_algorithms

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
similarity index 93%
rename from src/ipa/ipu3/ipu3_agc.cpp
rename to src/ipa/ipu3/algorithms/agc.cpp
index 1c1f5fb5..bb119cb4 100644
--- a/src/ipa/ipu3/ipu3_agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -5,7 +5,7 @@ 
  * ipu3_agc.cpp - AGC/AEC control algorithm
  */
 
-#include "ipu3_agc.h"
+#include "agc.h"
 
 #include <algorithm>
 #include <chrono>
@@ -21,7 +21,7 @@  namespace libcamera {
 
 using namespace std::literals::chrono_literals;
 
-namespace ipa::ipu3 {
+namespace ipa::ipu3::algorithms {
 
 LOG_DEFINE_CATEGORY(IPU3Agc)
 
@@ -50,14 +50,13 @@  static constexpr double kEvGainTarget = 0.5;
 /* A cell is 8 bytes and contains averages for RGB values and saturation ratio */
 static constexpr uint8_t kCellSize = 8;
 
-IPU3Agc::IPU3Agc()
+Agc::Agc()
 	: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),
-	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),
-	  currentExposure_(0s), currentExposureNoDg_(0s)
+	  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s)
 {
 }
 
-int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
+int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
 {
 	aeGrid_ = context.configuration.grid.bdsGrid;
 
@@ -67,7 +66,7 @@  int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
 	return 0;
 }
 
-void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
+void Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
 {
 	const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;
 	Rectangle aeRegion = { statsAeGrid.x_start,
@@ -108,7 +107,7 @@  void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)
 	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
 }
 
-void IPU3Agc::filterExposure()
+void Agc::filterExposure()
 {
 	double speed = 0.2;
 	if (prevExposure_ == 0s) {
@@ -142,7 +141,7 @@  void IPU3Agc::filterExposure()
 	LOG(IPU3Agc, Debug) << "After filtering, total_exposure " << prevExposure_;
 }
 
-void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
+void Agc::lockExposureGain(uint32_t &exposure, double &gain)
 {
 	/* Algorithm initialization should wait for first valid frames */
 	/* \todo - have a number of frames given by DelayedControls ?
@@ -185,7 +184,7 @@  void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)
 	lastFrame_ = frameCount_;
 }
 
-void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
+void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
 {
 	uint32_t &exposure = context.frameContext.agc.exposure;
 	double &gain = context.frameContext.agc.gain;
@@ -194,6 +193,6 @@  void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
 	frameCount_++;
 }
 
-} /* namespace ipa::ipu3 */
+} /* namespace ipa::ipu3::algorithms */
 
 } /* namespace libcamera */
diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/algorithms/agc.h
similarity index 73%
rename from src/ipa/ipu3/ipu3_agc.h
rename to src/ipa/ipu3/algorithms/agc.h
index 0e922664..1739d2fd 100644
--- a/src/ipa/ipu3/ipu3_agc.h
+++ b/src/ipa/ipu3/algorithms/agc.h
@@ -2,10 +2,10 @@ 
 /*
  * Copyright (C) 2021, Ideas On Board
  *
- * ipu3_agc.h - IPU3 AGC/AEC control algorithm
+ * agc.h - IPU3 AGC/AEC control algorithm
  */
-#ifndef __LIBCAMERA_IPU3_AGC_H__
-#define __LIBCAMERA_IPU3_AGC_H__
+#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
+#define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__
 
 #include <linux/intel-ipu3.h>
 
@@ -13,21 +13,21 @@ 
 
 #include <libcamera/geometry.h>
 
-#include "algorithms/algorithm.h"
+#include "algorithm.h"
 
 namespace libcamera {
 
 struct IPACameraSensorInfo;
 
-namespace ipa::ipu3 {
+namespace ipa::ipu3::algorithms {
 
 using utils::Duration;
 
-class IPU3Agc : public Algorithm
+class Agc : public Algorithm
 {
 public:
-	IPU3Agc();
-	~IPU3Agc() = default;
+	Agc();
+	~Agc() = default;
 
 	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
 	void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
@@ -53,8 +53,8 @@  private:
 	Duration currentExposureNoDg_;
 };
 
-} /* namespace ipa::ipu3 */
+} /* namespace ipa::ipu3::algorithms */
 
 } /* namespace libcamera */
 
-#endif /* __LIBCAMERA_IPU3_AGC_H__ */
+#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__ */
diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
index 87377f4e..acaadd90 100644
--- a/src/ipa/ipu3/algorithms/meson.build
+++ b/src/ipa/ipu3/algorithms/meson.build
@@ -2,6 +2,7 @@ 
 
 ipu3_ipa_algorithms = files([
     'algorithm.cpp',
+    'agc.cpp',
     'awb.cpp',
     'contrast.cpp',
 ])
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index f4f49025..d73ec79d 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -30,9 +30,9 @@ 
 #include "libcamera/internal/mapped_framebuffer.h"
 
 #include "algorithms/algorithm.h"
+#include "algorithms/agc.h"
 #include "algorithms/awb.h"
 #include "algorithms/contrast.h"
-#include "ipu3_agc.h"
 #include "libipa/camera_sensor_helper.h"
 
 static constexpr uint32_t kMaxCellWidthPerSet = 160;
@@ -136,8 +136,6 @@  private:
 	uint32_t minGain_;
 	uint32_t maxGain_;
 
-	/* Interface to the AEC/AGC algorithm */
-	std::unique_ptr<IPU3Agc> agcAlgo_;
 	/* Interface to the Camera Helper */
 	std::unique_ptr<CameraSensorHelper> camHelper_;
 
@@ -218,6 +216,7 @@  int IPAIPU3::init(const IPASettings &settings,
 	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
 
 	/* Construct our Algorithms */
+	algorithms_.emplace_back(new algorithms::Agc());
 	algorithms_.emplace_back(new algorithms::Awb());
 	algorithms_.emplace_back(new algorithms::Contrast());
 
@@ -337,10 +336,6 @@  int IPAIPU3::configure(const IPAConfigInfo &configInfo)
 			return ret;
 	}
 
-
-	agcAlgo_ = std::make_unique<IPU3Agc>();
-	agcAlgo_->configure(context_, configInfo);
-
 	return 0;
 }
 
@@ -436,9 +431,6 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 	for (auto const &algo : algorithms_)
 		algo->process(context_, stats);
 
-	agcAlgo_->process(context_, stats);
-	exposure_ = context_.frameContext.agc.exposure;
-	gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
 	setControls(frame);
 
 	/* \todo Use VBlank value calculated from each frame exposure. */
@@ -458,6 +450,9 @@  void IPAIPU3::setControls(unsigned int frame)
 	IPU3Action op;
 	op.op = ActionSetSensorControls;
 
+	exposure_ = context_.frameContext.agc.exposure;
+	gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
+
 	ControlList ctrls(ctrls_);
 	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
 	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
index d1126947..39320980 100644
--- a/src/ipa/ipu3/meson.build
+++ b/src/ipa/ipu3/meson.build
@@ -6,7 +6,6 @@  ipa_name = 'ipa_ipu3'
 
 ipu3_ipa_sources = files([
     'ipu3.cpp',
-    'ipu3_agc.cpp',
 ])
 
 ipu3_ipa_sources += ipu3_ipa_algorithms