[PATCH 06/10] ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc

Dan Scally dan.scally at ideasonboard.com
Wed Mar 27 16:04:39 CET 2024


Hi Stefan

On 27/03/2024 15:02, Stefan Klug wrote:
> Hi Daniel,
>
> thanks for the patch.
>
> On Fri, Mar 22, 2024 at 01:14:47PM +0000, Daniel Scally wrote:
>> 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 simplicities 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>
>> ---
>>   src/ipa/ipu3/algorithms/agc.cpp | 98 ++++++++++++++++++++++++++++++---
>>   src/ipa/ipu3/algorithms/agc.h   |  6 +-
>>   src/ipa/ipu3/ipa_context.cpp    |  3 +
>>   src/ipa/ipu3/ipa_context.h      |  5 ++
>>   src/ipa/ipu3/ipu3.cpp           |  2 +-
>>   5 files changed, 105 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> index ee9a42cf..a84534ea 100644
>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -71,11 +71,41 @@ 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), context_(nullptr)
>>   {
>>   }
>>   
>> +/**
>> + * \brief Initialise the AGC algorith 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;
>> +
>> +	parseRelativeLuminanceTarget(tuningData);
>> +
>> +	ret = parseConstraintModes(tuningData);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = parseExposureModes(tuningData);
>> +	if (ret)
>> +		return ret;
>> +
> If I got it right, the agc implementation in libipa requires all
> parseXXX to be called
Correct
>   and even if the tuningData misses the relevant
> entries, dummy entries are generated.
Correct
> So I think these calls should be
> moved to MeanLuminanceAgc::parseTuningData()


OK, will do - thanks for the review!

>
> Cheers,
> Stefan
>
>> +	context.ctrlMap.merge(controls());
>> +	context_ = &context;
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * \brief Configure the AGC given a configInfo
>>    * \param[in] context The shared IPA context
>> @@ -103,6 +133,20 @@ int Agc::configure(IPAContext &context,
>>   	activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
>>   
>>   	frameCount_ = 0;
>> +
>> +	/*
>> +	 * \todo We should use the first available mode rather than assume that
>> +	 * the "Normal" modes are present in tuning data.
>> +	 */
>> +	context.activeState.agc.constraintMode = controls::ConstraintNormal;
>> +	context.activeState.agc.exposureMode = controls::ExposureNormal;
>> +
>> +	for (auto &[id, helper] : exposureModeHelpers()) {
>> +		/* \todo Run this again when FrameDurationLimits is passed in */
>> +		helper->configure(minShutterSpeed_, maxShutterSpeed_,
>> +				  minAnalogueGain_, maxAnalogueGain_);
>> +	}
>> +
>>   	return 0;
>>   }
>>   
>> @@ -280,11 +324,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;
>>   }
>>   
>>   /**
>> @@ -347,6 +386,26 @@ double Agc::estimateLuminance(IPAActiveState &activeState,
>>   	return ySum / (grid.height * grid.width) / 255;
>>   }
>>   
>> +double Agc::estimateLuminance(double gain)
>> +{
>> +	ASSERT(reds_.size() == greens_.size());
>> +	ASSERT(greens_.size() == blues_.size());
>> +	const ipu3_uapi_grid_config &grid = context_->configuration.grid.bdsGrid;
>> +	double redSum = 0, greenSum = 0, blueSum = 0;
>> +
>> +	for (unsigned int i = 0; i < reds_.size(); i++) {
>> +		redSum += std::min(reds_[i] * gain, 255.0);
>> +		greenSum += std::min(greens_[i] * gain, 255.0);
>> +		blueSum += std::min(blues_[i] * gain, 255.0);
>> +	}
>> +
>> +	double ySum = redSum * context_->activeState.awb.gains.red * 0.299
>> +		+ greenSum * context_->activeState.awb.gains.green * 0.587
>> +		+ blueSum * context_->activeState.awb.gains.blue * 0.114;
>> +
>> +	return ySum / (grid.height * grid.width) / 255;
>> +}
>> +
>>   /**
>>    * \brief Process IPU3 statistics, and run AGC operations
>>    * \param[in] context The shared IPA context
>> @@ -399,8 +458,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>>   	computeExposure(context, frameContext, yGain, iqMeanGain);
>>   	frameCount_++;
>>   
>> +	parseStatistics(stats, context.configuration.grid.bdsGrid);
>> +
>> +	/*
>> +	 * 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 7ed0ef7a..8405da9d 100644
>> --- a/src/ipa/ipu3/algorithms/agc.h
>> +++ b/src/ipa/ipu3/algorithms/agc.h
>> @@ -13,6 +13,7 @@
>>   
>>   #include <libcamera/geometry.h>
>>   
>> +#include "libipa/agc.h"
>>   #include "libipa/histogram.h"
>>   
>>   #include "algorithm.h"
>> @@ -23,12 +24,13 @@ struct IPACameraSensorInfo;
>>   
>>   namespace ipa::ipu3::algorithms {
>>   
>> -class Agc : public Algorithm
>> +class Agc : public Algorithm, public MeanLuminanceAgc
>>   {
>>   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,
>> @@ -45,6 +47,7 @@ private:
>>   				 const ipu3_uapi_grid_config &grid,
>>   				 const ipu3_uapi_stats_3a *stats,
>>   				 double gain);
>> +	double estimateLuminance(double gain) override;
>>   	void parseStatistics(const ipu3_uapi_stats_3a *stats,
>>   			     const ipu3_uapi_grid_config &grid);
>>   
>> @@ -59,6 +62,7 @@ private:
>>   	utils::Duration filteredExposure_;
>>   
>>   	uint32_t stride_;
>> +	IPAContext *context_;
>>   	std::vector<uint8_t> reds_;
>>   	std::vector<uint8_t> blues_;
>>   	std::vector<uint8_t> greens_;
>> 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..2fcc0c91 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -189,7 +189,7 @@ private:
>>   };
>>   
>>   IPAIPU3::IPAIPU3()
>> -	: context_({ {}, {}, { kMaxFrameContexts } })
>> +	: context_({ {}, {}, { kMaxFrameContexts }, {} })
>>   {
>>   }
>>   
>> -- 
>> 2.34.1
>>


More information about the libcamera-devel mailing list