[PATCH 3/3] ipa: libipa: Add flicker controls to AgcMeanLuminance

[Paul Elder]

On Fri, Jan 17, 2025 at 02:34:10PM +0000, Daniel Scally wrote:
> Add controls for AeFlickerMode and AeFlickerPeriod to the
> AgcMeanLuminance class. The controls passed to an algorithm's
> queueRequest() are forwarded to AgcMeanLuminance through a new
> function, and the values then passed to the ExposureModeHelper to be
> used in calculating the new exposure time.
> 
> Take the opportunity to call the new parseControls() function in each
> of the derived class' queueRequest() function.
> 
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp       |  3 +-
>  src/ipa/libipa/agc_mean_luminance.cpp | 47 ++++++++++++++++++++++++++-
>  src/ipa/libipa/agc_mean_luminance.h   |  5 +++
>  src/ipa/mali-c55/algorithms/agc.cpp   |  2 ++
>  src/ipa/rkisp1/algorithms/agc.cpp     |  2 ++
>  5 files changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 383b046c..b993aaa7 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -129,8 +129,9 @@ int Agc::configure(IPAContext &context,
>  void Agc::queueRequest([[maybe_unused]] typename Module::Context &context,
>  		       [[maybe_unused]] const uint32_t frame,
>  		       [[maybe_unused]] typename Module::FrameContext &frameContext,
> -		       [[maybe_unused]] const ControlList &controls)
> +		       const ControlList &controls)
>  {
> +	parseControls(controls);
>  }
>  
>  Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index b5e6afe3..4acf7b55 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -136,6 +136,10 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>  AgcMeanLuminance::AgcMeanLuminance()
>  	: frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
>  {
> +	controls_[&controls::AeFlickerMode] = ControlInfo(static_cast<int>(controls::FlickerOff),
> +							  static_cast<int>(controls::FlickerManual),

As I've been reminded recently, we should use the span constructor for
these.


With that,

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> +							  static_cast<int>(controls::FlickerOff));
> +	controls_[&controls::AeFlickerPeriod] = ControlInfo(100, 1000000);
>  }
>  
>  AgcMeanLuminance::~AgcMeanLuminance() = default;
> @@ -479,6 +483,39 @@ double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex,
>  	return gain;
>  }
>  
> +/**
> + * \brief Parse the controls passed to an algorithm for the ones we need
> + * \param[in] controls the ControlList passed to an algorithm by the IPA
> + *
> + * This function must be called by a derived class in its queueRequest()
> + * function so that we can extract the controls needed by this base class.
> + */
> +void AgcMeanLuminance::parseControls(const ControlList &controls)
> +{
> +	const auto &flickerMode = controls.get(controls::AeFlickerMode);
> +	if (flickerMode) {
> +		switch (*flickerMode) {
> +		case controls::AeFlickerModeEnum::FlickerOff:
> +		case controls::AeFlickerModeEnum::FlickerManual:
> +			flickerMode_ = static_cast<controls::AeFlickerModeEnum>(*flickerMode);
> +			break;
> +		default:
> +			LOG(AgcMeanLuminance, Error)
> +				<< "Flicker mode " << *flickerMode << " is not supported";
> +			break;
> +		}
> +	}
> +
> +	const auto &flickerPeriod = controls.get(controls::AeFlickerPeriod);
> +	if (flickerPeriod) {
> +		/*
> +		 * If at some future point we support automatic flicker
> +		 * mitigation then this will need revision.
> +		 */
> +		flickerPeriod_ = *flickerPeriod * 1.0us;
> +	}
> +}
> +
>  /**
>   * \brief Apply a filter on the exposure value to limit the speed of changes
>   * \param[in] exposureValue The target exposure from the AGC algorithm
> @@ -560,7 +597,15 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,
>  	newExposureValue = filterExposure(newExposureValue);
>  
>  	frameCount_++;
> -	return exposureModeHelper->splitExposure(newExposureValue, 0us);
> +
> +	utils::Duration flickerPeriod;
> +
> +	if (flickerMode_ == controls::AeFlickerModeEnum::FlickerManual)
> +		flickerPeriod = flickerPeriod_;
> +	else
> +		flickerPeriod = 0us;
> +
> +	return exposureModeHelper->splitExposure(newExposureValue, flickerPeriod);
>  }
>  
>  /**
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index c41391cb..b9b36687 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -14,6 +14,7 @@
>  
>  #include <libcamera/base/utils.h>
>  
> +#include <libcamera/control_ids.h>
>  #include <libcamera/controls.h>
>  
>  #include "libcamera/internal/yaml_parser.h"
> @@ -71,6 +72,8 @@ public:
>  		frameCount_ = 0;
>  	}
>  
> +	void parseControls(const ControlList &controls);
> +
>  private:
>  	virtual double estimateLuminance(const double gain) const = 0;
>  
> @@ -87,6 +90,8 @@ private:
>  	uint64_t frameCount_;
>  	utils::Duration filteredExposure_;
>  	double relativeLuminanceTarget_;
> +	utils::Duration flickerPeriod_;
> +	controls::AeFlickerModeEnum flickerMode_;
>  
>  	std::map<int32_t, std::vector<AgcConstraint>> constraintModes_;
>  	std::map<int32_t, std::shared_ptr<ExposureModeHelper>> exposureModeHelpers_;
> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
> index 70667db3..6a80c44f 100644
> --- a/src/ipa/mali-c55/algorithms/agc.cpp
> +++ b/src/ipa/mali-c55/algorithms/agc.cpp
> @@ -238,6 +238,8 @@ void Agc::queueRequest(IPAContext &context, const uint32_t frame,
>  			<< "Digital gain set to " << agc.manual.ispGain
>  			<< " on request sequence " << frame;
>  	}
> +
> +	parseControls(controls);
>  }
>  
>  size_t Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 40e5a8f4..7b1763a2 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -275,6 +275,8 @@ void Agc::queueRequest(IPAContext &context,
>  		agc.maxFrameDuration = maxFrameDuration;
>  	}
>  	frameContext.agc.maxFrameDuration = agc.maxFrameDuration;
> +
> +	parseControls(controls);
>  }
>  
>  /**
> -- 
> 2.34.1
>