[v2,5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc
diff mbox series

Message ID 20240417131536.484129-6-dan.scally@ideasonboard.com
State New
Headers show
Series
  • Centralise Agc into libipa
Related show

Commit Message

Daniel Scally April 17, 2024, 1:15 p.m. UTC
In preparation for switching to a derivation of MeanLuminanceAgc, add
a function to parse and store the statistics for easy retrieval in an
overriding estimateLuminance() function.

Now that we have a MeanLuminanceAgc class that centralises our AEGC
algorithm, derive the IPU3's Agc class from it and plumb in the
necessary framework to enable it to be used. For simplicity's sake
this commit switches the algorithm to use the derived class, but
does not remove the bespoke functions at this time.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:

	- Squashed the patch adding parseStatistics()
	- Used the first available constraint and exposure modes rather than
	  assuming the "Normal" mode was available
	- Used a vector of RGB tuples in estimateLuminance() rather than 3
	  vectors
	- Stored a copy of the bdsGrid and AWB gains rather than a pointer to
	  the context for use in estimateLuminance()
	- Remembered to report the controls out to IPAIPU3::updateControls()

 src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--
 src/ipa/ipu3/algorithms/agc.h   |  14 +++-
 src/ipa/ipu3/ipa_context.cpp    |   3 +
 src/ipa/ipu3/ipa_context.h      |   5 ++
 src/ipa/ipu3/ipu3.cpp           |   3 +-
 5 files changed, 136 insertions(+), 9 deletions(-)

Comments

Stefan Klug April 18, 2024, 7:56 a.m. UTC | #1
Hi Daniel,

thanks for the patch.

On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:
> In preparation for switching to a derivation of MeanLuminanceAgc, add
> a function to parse and store the statistics for easy retrieval in an
> overriding estimateLuminance() function.
> 
> Now that we have a MeanLuminanceAgc class that centralises our AEGC
> algorithm, derive the IPU3's Agc class from it and plumb in the
> necessary framework to enable it to be used. For simplicity's sake
> this commit switches the algorithm to use the derived class, but
> does not remove the bespoke functions at this time.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v2:
> 
> 	- Squashed the patch adding parseStatistics()
> 	- Used the first available constraint and exposure modes rather than
> 	  assuming the "Normal" mode was available
> 	- Used a vector of RGB tuples in estimateLuminance() rather than 3
> 	  vectors
> 	- Stored a copy of the bdsGrid and AWB gains rather than a pointer to
> 	  the context for use in estimateLuminance()
> 	- Remembered to report the controls out to IPAIPU3::updateControls()
> 
>  src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--
>  src/ipa/ipu3/algorithms/agc.h   |  14 +++-
>  src/ipa/ipu3/ipa_context.cpp    |   3 +
>  src/ipa/ipu3/ipa_context.h      |   5 ++
>  src/ipa/ipu3/ipu3.cpp           |   3 +-
>  5 files changed, 136 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 606a237a..3b9761bd 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -71,11 +71,33 @@ static constexpr uint32_t kNumStartupFrames = 10;
>  static constexpr double kRelativeLuminanceTarget = 0.16;
>  
>  Agc::Agc()
> -	: frameCount_(0), minShutterSpeed_(0s),
> -	  maxShutterSpeed_(0s), filteredExposure_(0s)
> +	: minShutterSpeed_(0s), maxShutterSpeed_(0s)
>  {
>  }
>  
> +/**
> + * \brief Initialise the AGC algorithm from tuning files
> + * \param[in] context The shared IPA context
> + * \param[in] tuningData The YamlObject containing Agc tuning data
> + *
> + * This function calls the base class' tuningData parsers to discover which
> + * control values are supported.
> + *
> + * \return 0 on success or errors from the base class
> + */
> +int Agc::init(IPAContext &context, const YamlObject &tuningData)
> +{
> +	int ret;
> +
> +	ret = parseTuningData(tuningData);
> +	if (ret)
> +		return ret;
> +
> +	context.ctrlMap.merge(controls());
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Configure the AGC given a configInfo
>   * \param[in] context The shared IPA context
> @@ -90,6 +112,7 @@ int Agc::configure(IPAContext &context,
>  	IPAActiveState &activeState = context.activeState;
>  
>  	stride_ = configuration.grid.stride;
> +	bdsGrid_ = configuration.grid.bdsGrid;
>  
>  	minShutterSpeed_ = configuration.agc.minShutterSpeed;
>  	maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,
> @@ -103,6 +126,16 @@ int Agc::configure(IPAContext &context,
>  	activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
>  
>  	frameCount_ = 0;

IMHO That line can be removed as you call resetFrameCount() below.

With that fixed
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

Cheers,
Stefan

> +
> +	context.activeState.agc.constraintMode = constraintModes().begin()->first;
> +	context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;
> +
> +	/* \todo Run this again when FrameDurationLimits is passed in */
> +	configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,
> +				     minAnalogueGain_, maxAnalogueGain_);
> +
> +	resetFrameCount();
> +
>  	return 0;
>  }
>  
> @@ -142,6 +175,39 @@ double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>  	return Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
>  }
>  
> +Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
> +			       const ipu3_uapi_grid_config &grid)
> +{
> +	uint32_t hist[knumHistogramBins] = { 0 };
> +
> +	rgbTriples_.clear();
> +
> +	for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
> +		for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
> +			uint32_t cellPosition = cellY * stride_ + cellX;
> +
> +			const ipu3_uapi_awb_set_item *cell =
> +				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
> +					&stats->awb_raw_buffer.meta_data[cellPosition]);
> +
> +			rgbTriples_.push_back({
> +				cell->R_avg,
> +				(cell->Gr_avg + cell->Gb_avg) / 2,
> +				cell->B_avg
> +			});
> +
> +			/*
> +			 * Store the average green value to estimate the
> +			 * brightness. Even the overexposed pixels are
> +			 * taken into account.
> +			 */
> +			hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;
> +		}
> +	}
> +
> +	return Histogram(Span<uint32_t>(hist));
> +}
> +
>  /**
>   * \brief Apply a filter on the exposure value to limit the speed of changes
>   * \param[in] exposureValue The target exposure from the AGC algorithm
> @@ -247,11 +313,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
>  	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
>  			    << shutterTime << " and "
>  			    << stepGain;
> -
> -	IPAActiveState &activeState = context.activeState;
> -	/* Update the estimated exposure and gain. */
> -	activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
> -	activeState.agc.gain = stepGain;
>  }
>  
>  /**
> @@ -314,6 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,
>  	return ySum / (grid.height * grid.width) / 255;
>  }
>  
> +double Agc::estimateLuminance(double gain)
> +{
> +	double redSum = 0, greenSum = 0, blueSum = 0;
> +
> +	for (unsigned int i = 0; i < rgbTriples_.size(); i++) {
> +		redSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
> +		greenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
> +		blueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
> +	}
> +
> +	double ySum = redSum * rGain_ * 0.299
> +		    + greenSum * gGain_ * 0.587
> +		    + blueSum * bGain_ * 0.114;
> +
> +	return ySum / (bdsGrid_.height * bdsGrid_.width) / 255;
> +}
> +
>  /**
>   * \brief Process IPU3 statistics, and run AGC operations
>   * \param[in] context The shared IPA context
> @@ -366,8 +444,36 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  	computeExposure(context, frameContext, yGain, iqMeanGain);
>  	frameCount_++;
>  
> +	Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
> +	rGain_ = context.activeState.awb.gains.red;
> +	gGain_ = context.activeState.awb.gains.blue;
> +	bGain_ = context.activeState.awb.gains.green;
> +
> +	/*
> +	 * The Agc algorithm needs to know the effective exposure value that was
> +	 * applied to the sensor when the statistics were collected.
> +	 */
>  	utils::Duration exposureTime = context.configuration.sensor.lineDuration
>  				     * frameContext.sensor.exposure;
> +	double analogueGain = frameContext.sensor.gain;
> +	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
> +
> +	utils::Duration shutterTime;
> +	double aGain, dGain;
> +	std::tie(shutterTime, aGain, dGain) =
> +		calculateNewEv(context.activeState.agc.constraintMode,
> +			       context.activeState.agc.exposureMode, hist,
> +			       effectiveExposureValue);
> +
> +	LOG(IPU3Agc, Debug)
> +		<< "Divided up shutter, analogue gain and digital gain are "
> +		<< shutterTime << ", " << aGain << " and " << dGain;
> +
> +	IPAActiveState &activeState = context.activeState;
> +	/* Update the estimated exposure and gain. */
> +	activeState.agc.exposure = shutterTime / context.configuration.sensor.lineDuration;
> +	activeState.agc.gain = aGain;
> +
>  	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>  	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
>  
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 9d6e3ff1..40f32188 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -13,6 +13,9 @@
>  
>  #include <libcamera/geometry.h>
>  
> +#include "libipa/agc_mean_luminance.h"
> +#include "libipa/histogram.h"
> +
>  #include "algorithm.h"
>  
>  namespace libcamera {
> @@ -21,12 +24,13 @@ struct IPACameraSensorInfo;
>  
>  namespace ipa::ipu3::algorithms {
>  
> -class Agc : public Algorithm
> +class Agc : public Algorithm, public AgcMeanLuminance
>  {
>  public:
>  	Agc();
>  	~Agc() = default;
>  
> +	int init(IPAContext &context, const YamlObject &tuningData) override;
>  	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>  	void process(IPAContext &context, const uint32_t frame,
>  		     IPAFrameContext &frameContext,
> @@ -43,6 +47,9 @@ private:
>  				 const ipu3_uapi_grid_config &grid,
>  				 const ipu3_uapi_stats_3a *stats,
>  				 double gain);
> +	double estimateLuminance(double gain) override;
> +	Histogram parseStatistics(const ipu3_uapi_stats_3a *stats,
> +				  const ipu3_uapi_grid_config &grid);
>  
>  	uint64_t frameCount_;
>  
> @@ -55,6 +62,11 @@ private:
>  	utils::Duration filteredExposure_;
>  
>  	uint32_t stride_;
> +	double rGain_;
> +	double gGain_;
> +	double bGain_;
> +	ipu3_uapi_grid_config bdsGrid_;
> +	std::vector<std::tuple<uint8_t, uint8_t, uint8_t>> rgbTriples_;
>  };
>  
>  } /* namespace ipa::ipu3::algorithms */
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 959f314f..c4fb5642 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -47,6 +47,9 @@ namespace libcamera::ipa::ipu3 {
>   *
>   * \var IPAContext::activeState
>   * \brief The current state of IPA algorithms
> + *
> + * \var IPAContext::ctrlMap
> + * \brief A ControlInfoMap::Map of controls populated by the algorithms
>   */
>  
>  /**
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index e9a3863b..a92cb6ce 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -12,6 +12,7 @@
>  
>  #include <libcamera/base/utils.h>
>  
> +#include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
>  
>  #include <libipa/fc_queue.h>
> @@ -55,6 +56,8 @@ struct IPAActiveState {
>  	struct {
>  		uint32_t exposure;
>  		double gain;
> +		uint32_t constraintMode;
> +		uint32_t exposureMode;
>  	} agc;
>  
>  	struct {
> @@ -85,6 +88,8 @@ struct IPAContext {
>  	IPAActiveState activeState;
>  
>  	FCQueue<IPAFrameContext> frameContexts;
> +
> +	ControlInfoMap::Map ctrlMap;
>  };
>  
>  } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 08ee6eb3..4809eb60 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -189,7 +189,7 @@ private:
>  };
>  
>  IPAIPU3::IPAIPU3()
> -	: context_({ {}, {}, { kMaxFrameContexts } })
> +	: context_({ {}, {}, { kMaxFrameContexts }, {} })
>  {
>  }
>  
> @@ -287,6 +287,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>  							       frameDurations[1],
>  							       frameDurations[2]);
>  
> +	controls.merge(context_.ctrlMap);
>  	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
>  }
>  
> -- 
> 2.34.1
>
Jacopo Mondi April 22, 2024, 10:28 a.m. UTC | #2
Hi Dan,
  in $subject and here

s/MeanLuminanceAgc/AgcMeanLuminance

On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:
> In preparation for switching to a derivation of MeanLuminanceAgc, add
> a function to parse and store the statistics for easy retrieval in an
> overriding estimateLuminance() function.
>
> Now that we have a MeanLuminanceAgc class that centralises our AEGC
> algorithm, derive the IPU3's Agc class from it and plumb in the
> necessary framework to enable it to be used. For simplicity's sake
> this commit switches the algorithm to use the derived class, but
> does not remove the bespoke functions at this time.
>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v2:
>
> 	- Squashed the patch adding parseStatistics()
> 	- Used the first available constraint and exposure modes rather than
> 	  assuming the "Normal" mode was available
> 	- Used a vector of RGB tuples in estimateLuminance() rather than 3
> 	  vectors
> 	- Stored a copy of the bdsGrid and AWB gains rather than a pointer to
> 	  the context for use in estimateLuminance()
> 	- Remembered to report the controls out to IPAIPU3::updateControls()
>
>  src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--
>  src/ipa/ipu3/algorithms/agc.h   |  14 +++-
>  src/ipa/ipu3/ipa_context.cpp    |   3 +
>  src/ipa/ipu3/ipa_context.h      |   5 ++
>  src/ipa/ipu3/ipu3.cpp           |   3 +-
>  5 files changed, 136 insertions(+), 9 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 606a237a..3b9761bd 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -71,11 +71,33 @@ static constexpr uint32_t kNumStartupFrames = 10;
>  static constexpr double kRelativeLuminanceTarget = 0.16;
>
>  Agc::Agc()
> -	: frameCount_(0), minShutterSpeed_(0s),
> -	  maxShutterSpeed_(0s), filteredExposure_(0s)
> +	: minShutterSpeed_(0s), maxShutterSpeed_(0s)
>  {
>  }
>
> +/**
> + * \brief Initialise the AGC algorithm from tuning files
> + * \param[in] context The shared IPA context
> + * \param[in] tuningData The YamlObject containing Agc tuning data
> + *
> + * This function calls the base class' tuningData parsers to discover which
> + * control values are supported.
> + *
> + * \return 0 on success or errors from the base class
> + */
> +int Agc::init(IPAContext &context, const YamlObject &tuningData)
> +{
> +	int ret;
> +
> +	ret = parseTuningData(tuningData);

Nit:

        int ret =

> +	if (ret)
> +		return ret;
> +
> +	context.ctrlMap.merge(controls());
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Configure the AGC given a configInfo
>   * \param[in] context The shared IPA context
> @@ -90,6 +112,7 @@ int Agc::configure(IPAContext &context,
>  	IPAActiveState &activeState = context.activeState;
>
>  	stride_ = configuration.grid.stride;
> +	bdsGrid_ = configuration.grid.bdsGrid;

Where is this populated from ?

>
>  	minShutterSpeed_ = configuration.agc.minShutterSpeed;
>  	maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,
> @@ -103,6 +126,16 @@ int Agc::configure(IPAContext &context,
>  	activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
>
>  	frameCount_ = 0;
> +
> +	context.activeState.agc.constraintMode = constraintModes().begin()->first;
> +	context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;
> +
> +	/* \todo Run this again when FrameDurationLimits is passed in */
> +	configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,
> +				     minAnalogueGain_, maxAnalogueGain_);
> +
> +	resetFrameCount();
> +
>  	return 0;
>  }
>
> @@ -142,6 +175,39 @@ double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>  	return Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
>  }
>
> +Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
> +			       const ipu3_uapi_grid_config &grid)
> +{
> +	uint32_t hist[knumHistogramBins] = { 0 };
> +
> +	rgbTriples_.clear();
> +
> +	for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
> +		for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
> +			uint32_t cellPosition = cellY * stride_ + cellX;
> +
> +			const ipu3_uapi_awb_set_item *cell =
> +				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
> +					&stats->awb_raw_buffer.meta_data[cellPosition]);
> +
> +			rgbTriples_.push_back({
> +				cell->R_avg,
> +				(cell->Gr_avg + cell->Gb_avg) / 2,
> +				cell->B_avg
> +			});
> +
> +			/*
> +			 * Store the average green value to estimate the
> +			 * brightness. Even the overexposed pixels are
> +			 * taken into account.
> +			 */
> +			hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;
> +		}
> +	}
> +
> +	return Histogram(Span<uint32_t>(hist));
> +}
> +
>  /**
>   * \brief Apply a filter on the exposure value to limit the speed of changes
>   * \param[in] exposureValue The target exposure from the AGC algorithm
> @@ -247,11 +313,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
>  	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
>  			    << shutterTime << " and "
>  			    << stepGain;
> -
> -	IPAActiveState &activeState = context.activeState;
> -	/* Update the estimated exposure and gain. */
> -	activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
> -	activeState.agc.gain = stepGain;
>  }
>
>  /**
> @@ -314,6 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,
>  	return ySum / (grid.height * grid.width) / 255;
>  }
>
> +double Agc::estimateLuminance(double gain)
> +{
> +	double redSum = 0, greenSum = 0, blueSum = 0;
> +
> +	for (unsigned int i = 0; i < rgbTriples_.size(); i++) {
> +		redSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
> +		greenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
> +		blueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
> +	}
> +
> +	double ySum = redSum * rGain_ * 0.299
> +		    + greenSum * gGain_ * 0.587
> +		    + blueSum * bGain_ * 0.114;
> +
> +	return ySum / (bdsGrid_.height * bdsGrid_.width) / 255;
> +}
> +
>  /**
>   * \brief Process IPU3 statistics, and run AGC operations
>   * \param[in] context The shared IPA context
> @@ -366,8 +444,36 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  	computeExposure(context, frameContext, yGain, iqMeanGain);
>  	frameCount_++;
>
> +	Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
> +	rGain_ = context.activeState.awb.gains.red;
> +	gGain_ = context.activeState.awb.gains.blue;
> +	bGain_ = context.activeState.awb.gains.green;

As a general question: what happens if awb is not active ?

> +
> +	/*
> +	 * The Agc algorithm needs to know the effective exposure value that was
> +	 * applied to the sensor when the statistics were collected.
> +	 */
>  	utils::Duration exposureTime = context.configuration.sensor.lineDuration
>  				     * frameContext.sensor.exposure;
> +	double analogueGain = frameContext.sensor.gain;
> +	utils::Duration effectiveExposureValue = exposureTime * analogueGain;

Is this a Duration ?

> +
> +	utils::Duration shutterTime;
> +	double aGain, dGain;
> +	std::tie(shutterTime, aGain, dGain) =
> +		calculateNewEv(context.activeState.agc.constraintMode,
> +			       context.activeState.agc.exposureMode, hist,
> +			       effectiveExposureValue);
> +
> +	LOG(IPU3Agc, Debug)
> +		<< "Divided up shutter, analogue gain and digital gain are "
> +		<< shutterTime << ", " << aGain << " and " << dGain;
> +
> +	IPAActiveState &activeState = context.activeState;
> +	/* Update the estimated exposure and gain. */
> +	activeState.agc.exposure = shutterTime / context.configuration.sensor.lineDuration;
> +	activeState.agc.gain = aGain;
> +
>  	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>  	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
>
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 9d6e3ff1..40f32188 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -13,6 +13,9 @@
>
>  #include <libcamera/geometry.h>
>
> +#include "libipa/agc_mean_luminance.h"
> +#include "libipa/histogram.h"
> +
>  #include "algorithm.h"
>
>  namespace libcamera {
> @@ -21,12 +24,13 @@ struct IPACameraSensorInfo;
>
>  namespace ipa::ipu3::algorithms {
>
> -class Agc : public Algorithm
> +class Agc : public Algorithm, public AgcMeanLuminance
>  {
>  public:
>  	Agc();
>  	~Agc() = default;
>
> +	int init(IPAContext &context, const YamlObject &tuningData) override;
>  	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>  	void process(IPAContext &context, const uint32_t frame,
>  		     IPAFrameContext &frameContext,
> @@ -43,6 +47,9 @@ private:
>  				 const ipu3_uapi_grid_config &grid,
>  				 const ipu3_uapi_stats_3a *stats,
>  				 double gain);
> +	double estimateLuminance(double gain) override;
> +	Histogram parseStatistics(const ipu3_uapi_stats_3a *stats,
> +				  const ipu3_uapi_grid_config &grid);
>
>  	uint64_t frameCount_;
>
> @@ -55,6 +62,11 @@ private:
>  	utils::Duration filteredExposure_;
>
>  	uint32_t stride_;
> +	double rGain_;
> +	double gGain_;
> +	double bGain_;
> +	ipu3_uapi_grid_config bdsGrid_;
> +	std::vector<std::tuple<uint8_t, uint8_t, uint8_t>> rgbTriples_;
>  };
>
>  } /* namespace ipa::ipu3::algorithms */
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 959f314f..c4fb5642 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -47,6 +47,9 @@ namespace libcamera::ipa::ipu3 {
>   *
>   * \var IPAContext::activeState
>   * \brief The current state of IPA algorithms
> + *
> + * \var IPAContext::ctrlMap
> + * \brief A ControlInfoMap::Map of controls populated by the algorithms
>   */
>
>  /**
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index e9a3863b..a92cb6ce 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -12,6 +12,7 @@
>
>  #include <libcamera/base/utils.h>
>
> +#include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
>
>  #include <libipa/fc_queue.h>
> @@ -55,6 +56,8 @@ struct IPAActiveState {
>  	struct {
>  		uint32_t exposure;
>  		double gain;
> +		uint32_t constraintMode;
> +		uint32_t exposureMode;
>  	} agc;
>
>  	struct {
> @@ -85,6 +88,8 @@ struct IPAContext {
>  	IPAActiveState activeState;
>
>  	FCQueue<IPAFrameContext> frameContexts;
> +
> +	ControlInfoMap::Map ctrlMap;
>  };
>
>  } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 08ee6eb3..4809eb60 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -189,7 +189,7 @@ private:
>  };
>
>  IPAIPU3::IPAIPU3()
> -	: context_({ {}, {}, { kMaxFrameContexts } })
> +	: context_({ {}, {}, { kMaxFrameContexts }, {} })
>  {
>  }
>
> @@ -287,6 +287,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>  							       frameDurations[1],
>  							       frameDurations[2]);
>
> +	controls.merge(context_.ctrlMap);
>  	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
>  }
>
> --
> 2.34.1
>
Daniel Scally April 23, 2024, 9:20 a.m. UTC | #3
Hi Jacopo

On 22/04/2024 11:28, Jacopo Mondi wrote:
> Hi Dan,
>    in $subject and here
>
> s/MeanLuminanceAgc/AgcMeanLuminance
>
> On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:
>> In preparation for switching to a derivation of MeanLuminanceAgc, add
>> a function to parse and store the statistics for easy retrieval in an
>> overriding estimateLuminance() function.
>>
>> Now that we have a MeanLuminanceAgc class that centralises our AEGC
>> algorithm, derive the IPU3's Agc class from it and plumb in the
>> necessary framework to enable it to be used. For simplicity's sake
>> this commit switches the algorithm to use the derived class, but
>> does not remove the bespoke functions at this time.
>>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>> Changes in v2:
>>
>> 	- Squashed the patch adding parseStatistics()
>> 	- Used the first available constraint and exposure modes rather than
>> 	  assuming the "Normal" mode was available
>> 	- Used a vector of RGB tuples in estimateLuminance() rather than 3
>> 	  vectors
>> 	- Stored a copy of the bdsGrid and AWB gains rather than a pointer to
>> 	  the context for use in estimateLuminance()
>> 	- Remembered to report the controls out to IPAIPU3::updateControls()
>>
>>   src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--
>>   src/ipa/ipu3/algorithms/agc.h   |  14 +++-
>>   src/ipa/ipu3/ipa_context.cpp    |   3 +
>>   src/ipa/ipu3/ipa_context.h      |   5 ++
>>   src/ipa/ipu3/ipu3.cpp           |   3 +-
>>   5 files changed, 136 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> index 606a237a..3b9761bd 100644
>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -71,11 +71,33 @@ static constexpr uint32_t kNumStartupFrames = 10;
>>   static constexpr double kRelativeLuminanceTarget = 0.16;
>>
>>   Agc::Agc()
>> -	: frameCount_(0), minShutterSpeed_(0s),
>> -	  maxShutterSpeed_(0s), filteredExposure_(0s)
>> +	: minShutterSpeed_(0s), maxShutterSpeed_(0s)
>>   {
>>   }
>>
>> +/**
>> + * \brief Initialise the AGC algorithm from tuning files
>> + * \param[in] context The shared IPA context
>> + * \param[in] tuningData The YamlObject containing Agc tuning data
>> + *
>> + * This function calls the base class' tuningData parsers to discover which
>> + * control values are supported.
>> + *
>> + * \return 0 on success or errors from the base class
>> + */
>> +int Agc::init(IPAContext &context, const YamlObject &tuningData)
>> +{
>> +	int ret;
>> +
>> +	ret = parseTuningData(tuningData);
> Nit:
>
>          int ret =
>
>> +	if (ret)
>> +		return ret;
>> +
>> +	context.ctrlMap.merge(controls());
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * \brief Configure the AGC given a configInfo
>>    * \param[in] context The shared IPA context
>> @@ -90,6 +112,7 @@ int Agc::configure(IPAContext &context,
>>   	IPAActiveState &activeState = context.activeState;
>>
>>   	stride_ = configuration.grid.stride;
>> +	bdsGrid_ = configuration.grid.bdsGrid;
> Where is this populated from ?


In IPAIPU3::calculateBdsGrid()

>
>>   	minShutterSpeed_ = configuration.agc.minShutterSpeed;
>>   	maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,
>> @@ -103,6 +126,16 @@ int Agc::configure(IPAContext &context,
>>   	activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
>>
>>   	frameCount_ = 0;
>> +
>> +	context.activeState.agc.constraintMode = constraintModes().begin()->first;
>> +	context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;
>> +
>> +	/* \todo Run this again when FrameDurationLimits is passed in */
>> +	configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,
>> +				     minAnalogueGain_, maxAnalogueGain_);
>> +
>> +	resetFrameCount();
>> +
>>   	return 0;
>>   }
>>
>> @@ -142,6 +175,39 @@ double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>>   	return Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
>>   }
>>
>> +Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
>> +			       const ipu3_uapi_grid_config &grid)
>> +{
>> +	uint32_t hist[knumHistogramBins] = { 0 };
>> +
>> +	rgbTriples_.clear();
>> +
>> +	for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
>> +		for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
>> +			uint32_t cellPosition = cellY * stride_ + cellX;
>> +
>> +			const ipu3_uapi_awb_set_item *cell =
>> +				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
>> +					&stats->awb_raw_buffer.meta_data[cellPosition]);
>> +
>> +			rgbTriples_.push_back({
>> +				cell->R_avg,
>> +				(cell->Gr_avg + cell->Gb_avg) / 2,
>> +				cell->B_avg
>> +			});
>> +
>> +			/*
>> +			 * Store the average green value to estimate the
>> +			 * brightness. Even the overexposed pixels are
>> +			 * taken into account.
>> +			 */
>> +			hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;
>> +		}
>> +	}
>> +
>> +	return Histogram(Span<uint32_t>(hist));
>> +}
>> +
>>   /**
>>    * \brief Apply a filter on the exposure value to limit the speed of changes
>>    * \param[in] exposureValue The target exposure from the AGC algorithm
>> @@ -247,11 +313,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
>>   	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
>>   			    << shutterTime << " and "
>>   			    << stepGain;
>> -
>> -	IPAActiveState &activeState = context.activeState;
>> -	/* Update the estimated exposure and gain. */
>> -	activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
>> -	activeState.agc.gain = stepGain;
>>   }
>>
>>   /**
>> @@ -314,6 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,
>>   	return ySum / (grid.height * grid.width) / 255;
>>   }
>>
>> +double Agc::estimateLuminance(double gain)
>> +{
>> +	double redSum = 0, greenSum = 0, blueSum = 0;
>> +
>> +	for (unsigned int i = 0; i < rgbTriples_.size(); i++) {
>> +		redSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
>> +		greenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
>> +		blueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
>> +	}
>> +
>> +	double ySum = redSum * rGain_ * 0.299
>> +		    + greenSum * gGain_ * 0.587
>> +		    + blueSum * bGain_ * 0.114;
>> +
>> +	return ySum / (bdsGrid_.height * bdsGrid_.width) / 255;
>> +}
>> +
>>   /**
>>    * \brief Process IPU3 statistics, and run AGC operations
>>    * \param[in] context The shared IPA context
>> @@ -366,8 +444,36 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>>   	computeExposure(context, frameContext, yGain, iqMeanGain);
>>   	frameCount_++;
>>
>> +	Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
>> +	rGain_ = context.activeState.awb.gains.red;
>> +	gGain_ = context.activeState.awb.gains.blue;
>> +	bGain_ = context.activeState.awb.gains.green;
> As a general question: what happens if awb is not active ?

It's hard-coded active in this IPA.
>
>> +
>> +	/*
>> +	 * The Agc algorithm needs to know the effective exposure value that was
>> +	 * applied to the sensor when the statistics were collected.
>> +	 */
>>   	utils::Duration exposureTime = context.configuration.sensor.lineDuration
>>   				     * frameContext.sensor.exposure;
>> +	double analogueGain = frameContext.sensor.gain;
>> +	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
> Is this a Duration ?


Yes...isn't it? Perhaps the terminology of "Exposure Value" is confusing or wrong? But conceptually 
that variable is meant to be a duration.

>
>> +
>> +	utils::Duration shutterTime;
>> +	double aGain, dGain;
>> +	std::tie(shutterTime, aGain, dGain) =
>> +		calculateNewEv(context.activeState.agc.constraintMode,
>> +			       context.activeState.agc.exposureMode, hist,
>> +			       effectiveExposureValue);
>> +
>> +	LOG(IPU3Agc, Debug)
>> +		<< "Divided up shutter, analogue gain and digital gain are "
>> +		<< shutterTime << ", " << aGain << " and " << dGain;
>> +
>> +	IPAActiveState &activeState = context.activeState;
>> +	/* Update the estimated exposure and gain. */
>> +	activeState.agc.exposure = shutterTime / context.configuration.sensor.lineDuration;
>> +	activeState.agc.gain = aGain;
>> +
>>   	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>>   	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
>> index 9d6e3ff1..40f32188 100644
>> --- a/src/ipa/ipu3/algorithms/agc.h
>> +++ b/src/ipa/ipu3/algorithms/agc.h
>> @@ -13,6 +13,9 @@
>>
>>   #include <libcamera/geometry.h>
>>
>> +#include "libipa/agc_mean_luminance.h"
>> +#include "libipa/histogram.h"
>> +
>>   #include "algorithm.h"
>>
>>   namespace libcamera {
>> @@ -21,12 +24,13 @@ struct IPACameraSensorInfo;
>>
>>   namespace ipa::ipu3::algorithms {
>>
>> -class Agc : public Algorithm
>> +class Agc : public Algorithm, public AgcMeanLuminance
>>   {
>>   public:
>>   	Agc();
>>   	~Agc() = default;
>>
>> +	int init(IPAContext &context, const YamlObject &tuningData) override;
>>   	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>>   	void process(IPAContext &context, const uint32_t frame,
>>   		     IPAFrameContext &frameContext,
>> @@ -43,6 +47,9 @@ private:
>>   				 const ipu3_uapi_grid_config &grid,
>>   				 const ipu3_uapi_stats_3a *stats,
>>   				 double gain);
>> +	double estimateLuminance(double gain) override;
>> +	Histogram parseStatistics(const ipu3_uapi_stats_3a *stats,
>> +				  const ipu3_uapi_grid_config &grid);
>>
>>   	uint64_t frameCount_;
>>
>> @@ -55,6 +62,11 @@ private:
>>   	utils::Duration filteredExposure_;
>>
>>   	uint32_t stride_;
>> +	double rGain_;
>> +	double gGain_;
>> +	double bGain_;
>> +	ipu3_uapi_grid_config bdsGrid_;
>> +	std::vector<std::tuple<uint8_t, uint8_t, uint8_t>> rgbTriples_;
>>   };
>>
>>   } /* namespace ipa::ipu3::algorithms */
>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
>> index 959f314f..c4fb5642 100644
>> --- a/src/ipa/ipu3/ipa_context.cpp
>> +++ b/src/ipa/ipu3/ipa_context.cpp
>> @@ -47,6 +47,9 @@ namespace libcamera::ipa::ipu3 {
>>    *
>>    * \var IPAContext::activeState
>>    * \brief The current state of IPA algorithms
>> + *
>> + * \var IPAContext::ctrlMap
>> + * \brief A ControlInfoMap::Map of controls populated by the algorithms
>>    */
>>
>>   /**
>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>> index e9a3863b..a92cb6ce 100644
>> --- a/src/ipa/ipu3/ipa_context.h
>> +++ b/src/ipa/ipu3/ipa_context.h
>> @@ -12,6 +12,7 @@
>>
>>   #include <libcamera/base/utils.h>
>>
>> +#include <libcamera/controls.h>
>>   #include <libcamera/geometry.h>
>>
>>   #include <libipa/fc_queue.h>
>> @@ -55,6 +56,8 @@ struct IPAActiveState {
>>   	struct {
>>   		uint32_t exposure;
>>   		double gain;
>> +		uint32_t constraintMode;
>> +		uint32_t exposureMode;
>>   	} agc;
>>
>>   	struct {
>> @@ -85,6 +88,8 @@ struct IPAContext {
>>   	IPAActiveState activeState;
>>
>>   	FCQueue<IPAFrameContext> frameContexts;
>> +
>> +	ControlInfoMap::Map ctrlMap;
>>   };
>>
>>   } /* namespace ipa::ipu3 */
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index 08ee6eb3..4809eb60 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -189,7 +189,7 @@ private:
>>   };
>>
>>   IPAIPU3::IPAIPU3()
>> -	: context_({ {}, {}, { kMaxFrameContexts } })
>> +	: context_({ {}, {}, { kMaxFrameContexts }, {} })
>>   {
>>   }
>>
>> @@ -287,6 +287,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>>   							       frameDurations[1],
>>   							       frameDurations[2]);
>>
>> +	controls.merge(context_.ctrlMap);
>>   	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
>>   }
>>
>> --
>> 2.34.1
>>
Daniel Scally April 24, 2024, 9 a.m. UTC | #4
Hi Stefan

On 18/04/2024 08:56, Stefan Klug wrote:
> Hi Daniel,
>
> thanks for the patch.
>
> On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:
>> In preparation for switching to a derivation of MeanLuminanceAgc, add
>> a function to parse and store the statistics for easy retrieval in an
>> overriding estimateLuminance() function.
>>
>> Now that we have a MeanLuminanceAgc class that centralises our AEGC
>> algorithm, derive the IPU3's Agc class from it and plumb in the
>> necessary framework to enable it to be used. For simplicity's sake
>> this commit switches the algorithm to use the derived class, but
>> does not remove the bespoke functions at this time.
>>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>> Changes in v2:
>>
>> 	- Squashed the patch adding parseStatistics()
>> 	- Used the first available constraint and exposure modes rather than
>> 	  assuming the "Normal" mode was available
>> 	- Used a vector of RGB tuples in estimateLuminance() rather than 3
>> 	  vectors
>> 	- Stored a copy of the bdsGrid and AWB gains rather than a pointer to
>> 	  the context for use in estimateLuminance()
>> 	- Remembered to report the controls out to IPAIPU3::updateControls()
>>
>>   src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--
>>   src/ipa/ipu3/algorithms/agc.h   |  14 +++-
>>   src/ipa/ipu3/ipa_context.cpp    |   3 +
>>   src/ipa/ipu3/ipa_context.h      |   5 ++
>>   src/ipa/ipu3/ipu3.cpp           |   3 +-
>>   5 files changed, 136 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> index 606a237a..3b9761bd 100644
>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -71,11 +71,33 @@ static constexpr uint32_t kNumStartupFrames = 10;
>>   static constexpr double kRelativeLuminanceTarget = 0.16;
>>   
>>   Agc::Agc()
>> -	: frameCount_(0), minShutterSpeed_(0s),
>> -	  maxShutterSpeed_(0s), filteredExposure_(0s)
>> +	: minShutterSpeed_(0s), maxShutterSpeed_(0s)
>>   {
>>   }
>>   
>> +/**
>> + * \brief Initialise the AGC algorithm from tuning files
>> + * \param[in] context The shared IPA context
>> + * \param[in] tuningData The YamlObject containing Agc tuning data
>> + *
>> + * This function calls the base class' tuningData parsers to discover which
>> + * control values are supported.
>> + *
>> + * \return 0 on success or errors from the base class
>> + */
>> +int Agc::init(IPAContext &context, const YamlObject &tuningData)
>> +{
>> +	int ret;
>> +
>> +	ret = parseTuningData(tuningData);
>> +	if (ret)
>> +		return ret;
>> +
>> +	context.ctrlMap.merge(controls());
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * \brief Configure the AGC given a configInfo
>>    * \param[in] context The shared IPA context
>> @@ -90,6 +112,7 @@ int Agc::configure(IPAContext &context,
>>   	IPAActiveState &activeState = context.activeState;
>>   
>>   	stride_ = configuration.grid.stride;
>> +	bdsGrid_ = configuration.grid.bdsGrid;
>>   
>>   	minShutterSpeed_ = configuration.agc.minShutterSpeed;
>>   	maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,
>> @@ -103,6 +126,16 @@ int Agc::configure(IPAContext &context,
>>   	activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
>>   
>>   	frameCount_ = 0;
> IMHO That line can be removed as you call resetFrameCount() below.


This is actually libcamera::ipa::ipu3::algorithms::Agc::frameCount_ rather than 
libcamera::ipa::AgcMeanLuminance::frameCount_, which is what resetFrameCount() touches. It's removed 
along with the rest of the bespoke implementation in the next patch...does it still need to be 
removed here?

>
> With that fixed
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>


Thanks!

>
> Cheers,
> Stefan
>
>> +
>> +	context.activeState.agc.constraintMode = constraintModes().begin()->first;
>> +	context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;
>> +
>> +	/* \todo Run this again when FrameDurationLimits is passed in */
>> +	configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,
>> +				     minAnalogueGain_, maxAnalogueGain_);
>> +
>> +	resetFrameCount();
>> +
>>   	return 0;
>>   }
>>   
>> @@ -142,6 +175,39 @@ double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>>   	return Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
>>   }
>>   
>> +Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
>> +			       const ipu3_uapi_grid_config &grid)
>> +{
>> +	uint32_t hist[knumHistogramBins] = { 0 };
>> +
>> +	rgbTriples_.clear();
>> +
>> +	for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
>> +		for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
>> +			uint32_t cellPosition = cellY * stride_ + cellX;
>> +
>> +			const ipu3_uapi_awb_set_item *cell =
>> +				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
>> +					&stats->awb_raw_buffer.meta_data[cellPosition]);
>> +
>> +			rgbTriples_.push_back({
>> +				cell->R_avg,
>> +				(cell->Gr_avg + cell->Gb_avg) / 2,
>> +				cell->B_avg
>> +			});
>> +
>> +			/*
>> +			 * Store the average green value to estimate the
>> +			 * brightness. Even the overexposed pixels are
>> +			 * taken into account.
>> +			 */
>> +			hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;
>> +		}
>> +	}
>> +
>> +	return Histogram(Span<uint32_t>(hist));
>> +}
>> +
>>   /**
>>    * \brief Apply a filter on the exposure value to limit the speed of changes
>>    * \param[in] exposureValue The target exposure from the AGC algorithm
>> @@ -247,11 +313,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
>>   	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
>>   			    << shutterTime << " and "
>>   			    << stepGain;
>> -
>> -	IPAActiveState &activeState = context.activeState;
>> -	/* Update the estimated exposure and gain. */
>> -	activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
>> -	activeState.agc.gain = stepGain;
>>   }
>>   
>>   /**
>> @@ -314,6 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,
>>   	return ySum / (grid.height * grid.width) / 255;
>>   }
>>   
>> +double Agc::estimateLuminance(double gain)
>> +{
>> +	double redSum = 0, greenSum = 0, blueSum = 0;
>> +
>> +	for (unsigned int i = 0; i < rgbTriples_.size(); i++) {
>> +		redSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
>> +		greenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
>> +		blueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
>> +	}
>> +
>> +	double ySum = redSum * rGain_ * 0.299
>> +		    + greenSum * gGain_ * 0.587
>> +		    + blueSum * bGain_ * 0.114;
>> +
>> +	return ySum / (bdsGrid_.height * bdsGrid_.width) / 255;
>> +}
>> +
>>   /**
>>    * \brief Process IPU3 statistics, and run AGC operations
>>    * \param[in] context The shared IPA context
>> @@ -366,8 +444,36 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>>   	computeExposure(context, frameContext, yGain, iqMeanGain);
>>   	frameCount_++;
>>   
>> +	Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
>> +	rGain_ = context.activeState.awb.gains.red;
>> +	gGain_ = context.activeState.awb.gains.blue;
>> +	bGain_ = context.activeState.awb.gains.green;
>> +
>> +	/*
>> +	 * The Agc algorithm needs to know the effective exposure value that was
>> +	 * applied to the sensor when the statistics were collected.
>> +	 */
>>   	utils::Duration exposureTime = context.configuration.sensor.lineDuration
>>   				     * frameContext.sensor.exposure;
>> +	double analogueGain = frameContext.sensor.gain;
>> +	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
>> +
>> +	utils::Duration shutterTime;
>> +	double aGain, dGain;
>> +	std::tie(shutterTime, aGain, dGain) =
>> +		calculateNewEv(context.activeState.agc.constraintMode,
>> +			       context.activeState.agc.exposureMode, hist,
>> +			       effectiveExposureValue);
>> +
>> +	LOG(IPU3Agc, Debug)
>> +		<< "Divided up shutter, analogue gain and digital gain are "
>> +		<< shutterTime << ", " << aGain << " and " << dGain;
>> +
>> +	IPAActiveState &activeState = context.activeState;
>> +	/* Update the estimated exposure and gain. */
>> +	activeState.agc.exposure = shutterTime / context.configuration.sensor.lineDuration;
>> +	activeState.agc.gain = aGain;
>> +
>>   	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>>   	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
>>   
>> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
>> index 9d6e3ff1..40f32188 100644
>> --- a/src/ipa/ipu3/algorithms/agc.h
>> +++ b/src/ipa/ipu3/algorithms/agc.h
>> @@ -13,6 +13,9 @@
>>   
>>   #include <libcamera/geometry.h>
>>   
>> +#include "libipa/agc_mean_luminance.h"
>> +#include "libipa/histogram.h"
>> +
>>   #include "algorithm.h"
>>   
>>   namespace libcamera {
>> @@ -21,12 +24,13 @@ struct IPACameraSensorInfo;
>>   
>>   namespace ipa::ipu3::algorithms {
>>   
>> -class Agc : public Algorithm
>> +class Agc : public Algorithm, public AgcMeanLuminance
>>   {
>>   public:
>>   	Agc();
>>   	~Agc() = default;
>>   
>> +	int init(IPAContext &context, const YamlObject &tuningData) override;
>>   	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>>   	void process(IPAContext &context, const uint32_t frame,
>>   		     IPAFrameContext &frameContext,
>> @@ -43,6 +47,9 @@ private:
>>   				 const ipu3_uapi_grid_config &grid,
>>   				 const ipu3_uapi_stats_3a *stats,
>>   				 double gain);
>> +	double estimateLuminance(double gain) override;
>> +	Histogram parseStatistics(const ipu3_uapi_stats_3a *stats,
>> +				  const ipu3_uapi_grid_config &grid);
>>   
>>   	uint64_t frameCount_;
>>   
>> @@ -55,6 +62,11 @@ private:
>>   	utils::Duration filteredExposure_;
>>   
>>   	uint32_t stride_;
>> +	double rGain_;
>> +	double gGain_;
>> +	double bGain_;
>> +	ipu3_uapi_grid_config bdsGrid_;
>> +	std::vector<std::tuple<uint8_t, uint8_t, uint8_t>> rgbTriples_;
>>   };
>>   
>>   } /* namespace ipa::ipu3::algorithms */
>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
>> index 959f314f..c4fb5642 100644
>> --- a/src/ipa/ipu3/ipa_context.cpp
>> +++ b/src/ipa/ipu3/ipa_context.cpp
>> @@ -47,6 +47,9 @@ namespace libcamera::ipa::ipu3 {
>>    *
>>    * \var IPAContext::activeState
>>    * \brief The current state of IPA algorithms
>> + *
>> + * \var IPAContext::ctrlMap
>> + * \brief A ControlInfoMap::Map of controls populated by the algorithms
>>    */
>>   
>>   /**
>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>> index e9a3863b..a92cb6ce 100644
>> --- a/src/ipa/ipu3/ipa_context.h
>> +++ b/src/ipa/ipu3/ipa_context.h
>> @@ -12,6 +12,7 @@
>>   
>>   #include <libcamera/base/utils.h>
>>   
>> +#include <libcamera/controls.h>
>>   #include <libcamera/geometry.h>
>>   
>>   #include <libipa/fc_queue.h>
>> @@ -55,6 +56,8 @@ struct IPAActiveState {
>>   	struct {
>>   		uint32_t exposure;
>>   		double gain;
>> +		uint32_t constraintMode;
>> +		uint32_t exposureMode;
>>   	} agc;
>>   
>>   	struct {
>> @@ -85,6 +88,8 @@ struct IPAContext {
>>   	IPAActiveState activeState;
>>   
>>   	FCQueue<IPAFrameContext> frameContexts;
>> +
>> +	ControlInfoMap::Map ctrlMap;
>>   };
>>   
>>   } /* namespace ipa::ipu3 */
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index 08ee6eb3..4809eb60 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -189,7 +189,7 @@ private:
>>   };
>>   
>>   IPAIPU3::IPAIPU3()
>> -	: context_({ {}, {}, { kMaxFrameContexts } })
>> +	: context_({ {}, {}, { kMaxFrameContexts }, {} })
>>   {
>>   }
>>   
>> @@ -287,6 +287,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>>   							       frameDurations[1],
>>   							       frameDurations[2]);
>>   
>> +	controls.merge(context_.ctrlMap);
>>   	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
>>   }
>>   
>> -- 
>> 2.34.1
>>
Stefan Klug April 25, 2024, 10:05 a.m. UTC | #5
On Wed, Apr 24, 2024 at 10:00:37AM +0100, Dan Scally wrote:
> Hi Stefan
> 
> On 18/04/2024 08:56, Stefan Klug wrote:
> > Hi Daniel,
> > 
> > thanks for the patch.
> > 
> > On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:
> > > In preparation for switching to a derivation of MeanLuminanceAgc, add
> > > a function to parse and store the statistics for easy retrieval in an
> > > overriding estimateLuminance() function.
> > > 
> > > Now that we have a MeanLuminanceAgc class that centralises our AEGC
> > > algorithm, derive the IPU3's Agc class from it and plumb in the
> > > necessary framework to enable it to be used. For simplicity's sake
> > > this commit switches the algorithm to use the derived class, but
> > > does not remove the bespoke functions at this time.
> > > 
> > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > > ---
> > > Changes in v2:
> > > 
> > > 	- Squashed the patch adding parseStatistics()
> > > 	- Used the first available constraint and exposure modes rather than
> > > 	  assuming the "Normal" mode was available
> > > 	- Used a vector of RGB tuples in estimateLuminance() rather than 3
> > > 	  vectors
> > > 	- Stored a copy of the bdsGrid and AWB gains rather than a pointer to
> > > 	  the context for use in estimateLuminance()
> > > 	- Remembered to report the controls out to IPAIPU3::updateControls()
> > > 
> > >   src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--
> > >   src/ipa/ipu3/algorithms/agc.h   |  14 +++-
> > >   src/ipa/ipu3/ipa_context.cpp    |   3 +
> > >   src/ipa/ipu3/ipa_context.h      |   5 ++
> > >   src/ipa/ipu3/ipu3.cpp           |   3 +-
> > >   5 files changed, 136 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > > index 606a237a..3b9761bd 100644
> > > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > > @@ -71,11 +71,33 @@ static constexpr uint32_t kNumStartupFrames = 10;
> > >   static constexpr double kRelativeLuminanceTarget = 0.16;
> > >   Agc::Agc()
> > > -	: frameCount_(0), minShutterSpeed_(0s),
> > > -	  maxShutterSpeed_(0s), filteredExposure_(0s)
> > > +	: minShutterSpeed_(0s), maxShutterSpeed_(0s)
> > >   {
> > >   }
> > > +/**
> > > + * \brief Initialise the AGC algorithm from tuning files
> > > + * \param[in] context The shared IPA context
> > > + * \param[in] tuningData The YamlObject containing Agc tuning data
> > > + *
> > > + * This function calls the base class' tuningData parsers to discover which
> > > + * control values are supported.
> > > + *
> > > + * \return 0 on success or errors from the base class
> > > + */
> > > +int Agc::init(IPAContext &context, const YamlObject &tuningData)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = parseTuningData(tuningData);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	context.ctrlMap.merge(controls());
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   /**
> > >    * \brief Configure the AGC given a configInfo
> > >    * \param[in] context The shared IPA context
> > > @@ -90,6 +112,7 @@ int Agc::configure(IPAContext &context,
> > >   	IPAActiveState &activeState = context.activeState;
> > >   	stride_ = configuration.grid.stride;
> > > +	bdsGrid_ = configuration.grid.bdsGrid;
> > >   	minShutterSpeed_ = configuration.agc.minShutterSpeed;
> > >   	maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,
> > > @@ -103,6 +126,16 @@ int Agc::configure(IPAContext &context,
> > >   	activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
> > >   	frameCount_ = 0;
> > IMHO That line can be removed as you call resetFrameCount() below.
> 
> 
> This is actually libcamera::ipa::ipu3::algorithms::Agc::frameCount_ rather
> than libcamera::ipa::AgcMeanLuminance::frameCount_, which is what
> resetFrameCount() touches. It's removed along with the rest of the bespoke
> implementation in the next patch...does it still need to be removed here?

Oh I missed that. Sorry for the noise. Then there is no need to remove
it here.

> 
> > 
> > With that fixed
> > Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> 
> Thanks!
> 
> > 
> > Cheers,
> > Stefan
> > 
> > > +
> > > +	context.activeState.agc.constraintMode = constraintModes().begin()->first;
> > > +	context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;
> > > +
> > > +	/* \todo Run this again when FrameDurationLimits is passed in */
> > > +	configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,
> > > +				     minAnalogueGain_, maxAnalogueGain_);
> > > +
> > > +	resetFrameCount();
> > > +
> > >   	return 0;
> > >   }
> > > @@ -142,6 +175,39 @@ double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
> > >   	return Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> > >   }
> > > +Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
> > > +			       const ipu3_uapi_grid_config &grid)
> > > +{
> > > +	uint32_t hist[knumHistogramBins] = { 0 };
> > > +
> > > +	rgbTriples_.clear();
> > > +
> > > +	for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
> > > +		for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
> > > +			uint32_t cellPosition = cellY * stride_ + cellX;
> > > +
> > > +			const ipu3_uapi_awb_set_item *cell =
> > > +				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
> > > +					&stats->awb_raw_buffer.meta_data[cellPosition]);
> > > +
> > > +			rgbTriples_.push_back({
> > > +				cell->R_avg,
> > > +				(cell->Gr_avg + cell->Gb_avg) / 2,
> > > +				cell->B_avg
> > > +			});
> > > +
> > > +			/*
> > > +			 * Store the average green value to estimate the
> > > +			 * brightness. Even the overexposed pixels are
> > > +			 * taken into account.
> > > +			 */
> > > +			hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;
> > > +		}
> > > +	}
> > > +
> > > +	return Histogram(Span<uint32_t>(hist));
> > > +}
> > > +
> > >   /**
> > >    * \brief Apply a filter on the exposure value to limit the speed of changes
> > >    * \param[in] exposureValue The target exposure from the AGC algorithm
> > > @@ -247,11 +313,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
> > >   	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
> > >   			    << shutterTime << " and "
> > >   			    << stepGain;
> > > -
> > > -	IPAActiveState &activeState = context.activeState;
> > > -	/* Update the estimated exposure and gain. */
> > > -	activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
> > > -	activeState.agc.gain = stepGain;
> > >   }
> > >   /**
> > > @@ -314,6 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,
> > >   	return ySum / (grid.height * grid.width) / 255;
> > >   }
> > > +double Agc::estimateLuminance(double gain)
> > > +{
> > > +	double redSum = 0, greenSum = 0, blueSum = 0;
> > > +
> > > +	for (unsigned int i = 0; i < rgbTriples_.size(); i++) {
> > > +		redSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
> > > +		greenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
> > > +		blueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
> > > +	}
> > > +
> > > +	double ySum = redSum * rGain_ * 0.299
> > > +		    + greenSum * gGain_ * 0.587
> > > +		    + blueSum * bGain_ * 0.114;
> > > +
> > > +	return ySum / (bdsGrid_.height * bdsGrid_.width) / 255;
> > > +}
> > > +
> > >   /**
> > >    * \brief Process IPU3 statistics, and run AGC operations
> > >    * \param[in] context The shared IPA context
> > > @@ -366,8 +444,36 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > >   	computeExposure(context, frameContext, yGain, iqMeanGain);
> > >   	frameCount_++;
> > > +	Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
> > > +	rGain_ = context.activeState.awb.gains.red;
> > > +	gGain_ = context.activeState.awb.gains.blue;
> > > +	bGain_ = context.activeState.awb.gains.green;
> > > +
> > > +	/*
> > > +	 * The Agc algorithm needs to know the effective exposure value that was
> > > +	 * applied to the sensor when the statistics were collected.
> > > +	 */
> > >   	utils::Duration exposureTime = context.configuration.sensor.lineDuration
> > >   				     * frameContext.sensor.exposure;
> > > +	double analogueGain = frameContext.sensor.gain;
> > > +	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
> > > +
> > > +	utils::Duration shutterTime;
> > > +	double aGain, dGain;
> > > +	std::tie(shutterTime, aGain, dGain) =
> > > +		calculateNewEv(context.activeState.agc.constraintMode,
> > > +			       context.activeState.agc.exposureMode, hist,
> > > +			       effectiveExposureValue);
> > > +
> > > +	LOG(IPU3Agc, Debug)
> > > +		<< "Divided up shutter, analogue gain and digital gain are "
> > > +		<< shutterTime << ", " << aGain << " and " << dGain;
> > > +
> > > +	IPAActiveState &activeState = context.activeState;
> > > +	/* Update the estimated exposure and gain. */
> > > +	activeState.agc.exposure = shutterTime / context.configuration.sensor.lineDuration;
> > > +	activeState.agc.gain = aGain;
> > > +
> > >   	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
> > >   	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> > > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> > > index 9d6e3ff1..40f32188 100644
> > > --- a/src/ipa/ipu3/algorithms/agc.h
> > > +++ b/src/ipa/ipu3/algorithms/agc.h
> > > @@ -13,6 +13,9 @@
> > >   #include <libcamera/geometry.h>
> > > +#include "libipa/agc_mean_luminance.h"
> > > +#include "libipa/histogram.h"
> > > +
> > >   #include "algorithm.h"
> > >   namespace libcamera {
> > > @@ -21,12 +24,13 @@ struct IPACameraSensorInfo;
> > >   namespace ipa::ipu3::algorithms {
> > > -class Agc : public Algorithm
> > > +class Agc : public Algorithm, public AgcMeanLuminance
> > >   {
> > >   public:
> > >   	Agc();
> > >   	~Agc() = default;
> > > +	int init(IPAContext &context, const YamlObject &tuningData) override;
> > >   	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> > >   	void process(IPAContext &context, const uint32_t frame,
> > >   		     IPAFrameContext &frameContext,
> > > @@ -43,6 +47,9 @@ private:
> > >   				 const ipu3_uapi_grid_config &grid,
> > >   				 const ipu3_uapi_stats_3a *stats,
> > >   				 double gain);
> > > +	double estimateLuminance(double gain) override;
> > > +	Histogram parseStatistics(const ipu3_uapi_stats_3a *stats,
> > > +				  const ipu3_uapi_grid_config &grid);
> > >   	uint64_t frameCount_;
> > > @@ -55,6 +62,11 @@ private:
> > >   	utils::Duration filteredExposure_;
> > >   	uint32_t stride_;
> > > +	double rGain_;
> > > +	double gGain_;
> > > +	double bGain_;
> > > +	ipu3_uapi_grid_config bdsGrid_;
> > > +	std::vector<std::tuple<uint8_t, uint8_t, uint8_t>> rgbTriples_;
> > >   };
> > >   } /* namespace ipa::ipu3::algorithms */
> > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> > > index 959f314f..c4fb5642 100644
> > > --- a/src/ipa/ipu3/ipa_context.cpp
> > > +++ b/src/ipa/ipu3/ipa_context.cpp
> > > @@ -47,6 +47,9 @@ namespace libcamera::ipa::ipu3 {
> > >    *
> > >    * \var IPAContext::activeState
> > >    * \brief The current state of IPA algorithms
> > > + *
> > > + * \var IPAContext::ctrlMap
> > > + * \brief A ControlInfoMap::Map of controls populated by the algorithms
> > >    */
> > >   /**
> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > > index e9a3863b..a92cb6ce 100644
> > > --- a/src/ipa/ipu3/ipa_context.h
> > > +++ b/src/ipa/ipu3/ipa_context.h
> > > @@ -12,6 +12,7 @@
> > >   #include <libcamera/base/utils.h>
> > > +#include <libcamera/controls.h>
> > >   #include <libcamera/geometry.h>
> > >   #include <libipa/fc_queue.h>
> > > @@ -55,6 +56,8 @@ struct IPAActiveState {
> > >   	struct {
> > >   		uint32_t exposure;
> > >   		double gain;
> > > +		uint32_t constraintMode;
> > > +		uint32_t exposureMode;
> > >   	} agc;
> > >   	struct {
> > > @@ -85,6 +88,8 @@ struct IPAContext {
> > >   	IPAActiveState activeState;
> > >   	FCQueue<IPAFrameContext> frameContexts;
> > > +
> > > +	ControlInfoMap::Map ctrlMap;
> > >   };
> > >   } /* namespace ipa::ipu3 */
> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > > index 08ee6eb3..4809eb60 100644
> > > --- a/src/ipa/ipu3/ipu3.cpp
> > > +++ b/src/ipa/ipu3/ipu3.cpp
> > > @@ -189,7 +189,7 @@ private:
> > >   };
> > >   IPAIPU3::IPAIPU3()
> > > -	: context_({ {}, {}, { kMaxFrameContexts } })
> > > +	: context_({ {}, {}, { kMaxFrameContexts }, {} })
> > >   {
> > >   }
> > > @@ -287,6 +287,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
> > >   							       frameDurations[1],
> > >   							       frameDurations[2]);
> > > +	controls.merge(context_.ctrlMap);
> > >   	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
> > >   }
> > > -- 
> > > 2.34.1
> > >

Patch
diff mbox series

diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
index 606a237a..3b9761bd 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -71,11 +71,33 @@  static constexpr uint32_t kNumStartupFrames = 10;
 static constexpr double kRelativeLuminanceTarget = 0.16;
 
 Agc::Agc()
-	: frameCount_(0), minShutterSpeed_(0s),
-	  maxShutterSpeed_(0s), filteredExposure_(0s)
+	: minShutterSpeed_(0s), maxShutterSpeed_(0s)
 {
 }
 
+/**
+ * \brief Initialise the AGC algorithm from tuning files
+ * \param[in] context The shared IPA context
+ * \param[in] tuningData The YamlObject containing Agc tuning data
+ *
+ * This function calls the base class' tuningData parsers to discover which
+ * control values are supported.
+ *
+ * \return 0 on success or errors from the base class
+ */
+int Agc::init(IPAContext &context, const YamlObject &tuningData)
+{
+	int ret;
+
+	ret = parseTuningData(tuningData);
+	if (ret)
+		return ret;
+
+	context.ctrlMap.merge(controls());
+
+	return 0;
+}
+
 /**
  * \brief Configure the AGC given a configInfo
  * \param[in] context The shared IPA context
@@ -90,6 +112,7 @@  int Agc::configure(IPAContext &context,
 	IPAActiveState &activeState = context.activeState;
 
 	stride_ = configuration.grid.stride;
+	bdsGrid_ = configuration.grid.bdsGrid;
 
 	minShutterSpeed_ = configuration.agc.minShutterSpeed;
 	maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,
@@ -103,6 +126,16 @@  int Agc::configure(IPAContext &context,
 	activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
 
 	frameCount_ = 0;
+
+	context.activeState.agc.constraintMode = constraintModes().begin()->first;
+	context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;
+
+	/* \todo Run this again when FrameDurationLimits is passed in */
+	configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,
+				     minAnalogueGain_, maxAnalogueGain_);
+
+	resetFrameCount();
+
 	return 0;
 }
 
@@ -142,6 +175,39 @@  double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
 	return Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
 }
 
+Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
+			       const ipu3_uapi_grid_config &grid)
+{
+	uint32_t hist[knumHistogramBins] = { 0 };
+
+	rgbTriples_.clear();
+
+	for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
+		for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
+			uint32_t cellPosition = cellY * stride_ + cellX;
+
+			const ipu3_uapi_awb_set_item *cell =
+				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
+					&stats->awb_raw_buffer.meta_data[cellPosition]);
+
+			rgbTriples_.push_back({
+				cell->R_avg,
+				(cell->Gr_avg + cell->Gb_avg) / 2,
+				cell->B_avg
+			});
+
+			/*
+			 * Store the average green value to estimate the
+			 * brightness. Even the overexposed pixels are
+			 * taken into account.
+			 */
+			hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;
+		}
+	}
+
+	return Histogram(Span<uint32_t>(hist));
+}
+
 /**
  * \brief Apply a filter on the exposure value to limit the speed of changes
  * \param[in] exposureValue The target exposure from the AGC algorithm
@@ -247,11 +313,6 @@  void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
 	LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
 			    << shutterTime << " and "
 			    << stepGain;
-
-	IPAActiveState &activeState = context.activeState;
-	/* Update the estimated exposure and gain. */
-	activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
-	activeState.agc.gain = stepGain;
 }
 
 /**
@@ -314,6 +375,23 @@  double Agc::estimateLuminance(IPAActiveState &activeState,
 	return ySum / (grid.height * grid.width) / 255;
 }
 
+double Agc::estimateLuminance(double gain)
+{
+	double redSum = 0, greenSum = 0, blueSum = 0;
+
+	for (unsigned int i = 0; i < rgbTriples_.size(); i++) {
+		redSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
+		greenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
+		blueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
+	}
+
+	double ySum = redSum * rGain_ * 0.299
+		    + greenSum * gGain_ * 0.587
+		    + blueSum * bGain_ * 0.114;
+
+	return ySum / (bdsGrid_.height * bdsGrid_.width) / 255;
+}
+
 /**
  * \brief Process IPU3 statistics, and run AGC operations
  * \param[in] context The shared IPA context
@@ -366,8 +444,36 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 	computeExposure(context, frameContext, yGain, iqMeanGain);
 	frameCount_++;
 
+	Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
+	rGain_ = context.activeState.awb.gains.red;
+	gGain_ = context.activeState.awb.gains.blue;
+	bGain_ = context.activeState.awb.gains.green;
+
+	/*
+	 * The Agc algorithm needs to know the effective exposure value that was
+	 * applied to the sensor when the statistics were collected.
+	 */
 	utils::Duration exposureTime = context.configuration.sensor.lineDuration
 				     * frameContext.sensor.exposure;
+	double analogueGain = frameContext.sensor.gain;
+	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
+
+	utils::Duration shutterTime;
+	double aGain, dGain;
+	std::tie(shutterTime, aGain, dGain) =
+		calculateNewEv(context.activeState.agc.constraintMode,
+			       context.activeState.agc.exposureMode, hist,
+			       effectiveExposureValue);
+
+	LOG(IPU3Agc, Debug)
+		<< "Divided up shutter, analogue gain and digital gain are "
+		<< shutterTime << ", " << aGain << " and " << dGain;
+
+	IPAActiveState &activeState = context.activeState;
+	/* Update the estimated exposure and gain. */
+	activeState.agc.exposure = shutterTime / context.configuration.sensor.lineDuration;
+	activeState.agc.gain = aGain;
+
 	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
 	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
 
diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
index 9d6e3ff1..40f32188 100644
--- a/src/ipa/ipu3/algorithms/agc.h
+++ b/src/ipa/ipu3/algorithms/agc.h
@@ -13,6 +13,9 @@ 
 
 #include <libcamera/geometry.h>
 
+#include "libipa/agc_mean_luminance.h"
+#include "libipa/histogram.h"
+
 #include "algorithm.h"
 
 namespace libcamera {
@@ -21,12 +24,13 @@  struct IPACameraSensorInfo;
 
 namespace ipa::ipu3::algorithms {
 
-class Agc : public Algorithm
+class Agc : public Algorithm, public AgcMeanLuminance
 {
 public:
 	Agc();
 	~Agc() = default;
 
+	int init(IPAContext &context, const YamlObject &tuningData) override;
 	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
 	void process(IPAContext &context, const uint32_t frame,
 		     IPAFrameContext &frameContext,
@@ -43,6 +47,9 @@  private:
 				 const ipu3_uapi_grid_config &grid,
 				 const ipu3_uapi_stats_3a *stats,
 				 double gain);
+	double estimateLuminance(double gain) override;
+	Histogram parseStatistics(const ipu3_uapi_stats_3a *stats,
+				  const ipu3_uapi_grid_config &grid);
 
 	uint64_t frameCount_;
 
@@ -55,6 +62,11 @@  private:
 	utils::Duration filteredExposure_;
 
 	uint32_t stride_;
+	double rGain_;
+	double gGain_;
+	double bGain_;
+	ipu3_uapi_grid_config bdsGrid_;
+	std::vector<std::tuple<uint8_t, uint8_t, uint8_t>> rgbTriples_;
 };
 
 } /* namespace ipa::ipu3::algorithms */
diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
index 959f314f..c4fb5642 100644
--- a/src/ipa/ipu3/ipa_context.cpp
+++ b/src/ipa/ipu3/ipa_context.cpp
@@ -47,6 +47,9 @@  namespace libcamera::ipa::ipu3 {
  *
  * \var IPAContext::activeState
  * \brief The current state of IPA algorithms
+ *
+ * \var IPAContext::ctrlMap
+ * \brief A ControlInfoMap::Map of controls populated by the algorithms
  */
 
 /**
diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
index e9a3863b..a92cb6ce 100644
--- a/src/ipa/ipu3/ipa_context.h
+++ b/src/ipa/ipu3/ipa_context.h
@@ -12,6 +12,7 @@ 
 
 #include <libcamera/base/utils.h>
 
+#include <libcamera/controls.h>
 #include <libcamera/geometry.h>
 
 #include <libipa/fc_queue.h>
@@ -55,6 +56,8 @@  struct IPAActiveState {
 	struct {
 		uint32_t exposure;
 		double gain;
+		uint32_t constraintMode;
+		uint32_t exposureMode;
 	} agc;
 
 	struct {
@@ -85,6 +88,8 @@  struct IPAContext {
 	IPAActiveState activeState;
 
 	FCQueue<IPAFrameContext> frameContexts;
+
+	ControlInfoMap::Map ctrlMap;
 };
 
 } /* namespace ipa::ipu3 */
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 08ee6eb3..4809eb60 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -189,7 +189,7 @@  private:
 };
 
 IPAIPU3::IPAIPU3()
-	: context_({ {}, {}, { kMaxFrameContexts } })
+	: context_({ {}, {}, { kMaxFrameContexts }, {} })
 {
 }
 
@@ -287,6 +287,7 @@  void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
 							       frameDurations[1],
 							       frameDurations[2]);
 
+	controls.merge(context_.ctrlMap);
 	*ipaControls = ControlInfoMap(std::move(controls), controls::controls);
 }