[PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc

Stefan Klug stefan.klug at ideasonboard.com
Thu Apr 18 09:56:37 CEST 2024


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 at 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 at 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
> 


More information about the libcamera-devel mailing list