[v1,2/4] ipa: libipa: agc_mean_luminance: Change luminance target to piecewise linear function

24642 diff mbox series

In some situations it is necessary to specify the target brightness
value depending on the overall lux level. This is a rework [1] to fit
current master.  For backwards compatibility of the tuning files and
easier tuning file handling it is still allowed to specify the luminance
target as plain value.

[1] https://patchwork.libcamera.org/patch/20231/

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/libipa/agc_mean_luminance.cpp | 59 ++++++++++++++++++++++++---
 src/ipa/libipa/agc_mean_luminance.h   | 11 ++++-
 src/ipa/rkisp1/algorithms/agc.cpp     |  1 +
 3 files changed, 63 insertions(+), 8 deletions(-)

Hi Stefan

On 14/10/2025 15:24, Stefan Klug wrote:
> In some situations it is necessary to specify the target brightness
> value depending on the overall lux level. This is a rework [1] to fit
> current master.  For backwards compatibility of the tuning files and
> easier tuning file handling it is still allowed to specify the luminance
> target as plain value.
> 
> [1] https://patchwork.libcamera.org/patch/20231/
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>   src/ipa/libipa/agc_mean_luminance.cpp | 59 ++++++++++++++++++++++++---
>   src/ipa/libipa/agc_mean_luminance.h   | 11 ++++-
>   src/ipa/rkisp1/algorithms/agc.cpp     |  1 +
>   3 files changed, 63 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 64f36bd75dd2..62b1918a45a7 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -54,6 +54,14 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>    */
>   static constexpr double kMaxRelativeLuminanceTarget = 0.95;
>   
> +/*
> + * Default lux level
> + *
> + * If no lux level or a zero lux level is specified, but PWLs are used to
> + * specify luminance targets, this default level is used.
> + */
> +static constexpr unsigned int kDefaultLuxLevel = 500;
> +
>   /**
>    * \struct AgcMeanLuminance::AgcConstraint
>    * \brief The boundaries and target for an AeConstraintMode constraint
> @@ -145,16 +153,32 @@ static constexpr double kMaxRelativeLuminanceTarget = 0.95;
>   
>   AgcMeanLuminance::AgcMeanLuminance()
>   	: exposureCompensation_(1.0), frameCount_(0), filteredExposure_(0s),
> -	  relativeLuminanceTarget_(0)
> +	  lux_(0)
>   {
>   }
>   
>   AgcMeanLuminance::~AgcMeanLuminance() = default;
>   
> -void AgcMeanLuminance::parseRelativeLuminanceTarget(const YamlObject &tuningData)
> +int AgcMeanLuminance::parseRelativeLuminanceTarget(const YamlObject &tuningData)
>   {
> -	relativeLuminanceTarget_ =
> -		tuningData["relativeLuminanceTarget"].get<double>(kDefaultRelativeLuminanceTarget);
> +	relativeLuminanceTarget_.clear();
> +
> +	auto &target = tuningData["relativeLuminanceTarget"];
> +	if (target.isValue()) {
> +		double t = target.get<double>(kDefaultRelativeLuminanceTarget);
> +		relativeLuminanceTarget_.append(0, t);

I think possibly a comment to explain that the intent is to represent a legacy scalar target with a 
flat PWL may spare some confusion in the future...and possibly a warning should shout that it's in 
an inappropriate format here? Otherwise:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

> +		return 0;
> +	}
> +
> +	std::optional<Pwl> pwl = target.get<Pwl>();
> +	if (!pwl) {
> +		LOG(AgcMeanLuminance, Error)
> +			<< "Failed to load relative luminance target.";
> +		return -EINVAL;
> +	}
> +
> +	relativeLuminanceTarget_.swap(*pwl);
> +	return 0;
>   }
>   
>   void AgcMeanLuminance::parseConstraint(const YamlObject &modeDict, int32_t id)
> @@ -385,7 +409,9 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData)
>   {
>   	int ret;
>   
> -	parseRelativeLuminanceTarget(tuningData);
> +	ret = parseRelativeLuminanceTarget(tuningData);
> +	if (ret)
> +		return ret;
>   
>   	ret = parseConstraintModes(tuningData);
>   	if (ret)
> @@ -403,6 +429,16 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData)
>    * AGC calculations. It is expressed as gain instead of EV.
>    */
>   
> +/**
> + * \fn AgcMeanLuminance::setLux(int lux)
> + * \brief Set the lux level
> + * \param[in] lux The lux level
> + *
> + * This function sets the lux level to be used in the AGC calculations. A value
> + * of 0 means no measurement and a default value of \a kDefaultLuxLevel is used
> + * if necessary.
> + */
> +
>   /**
>    * \brief Set the ExposureModeHelper limits for this class
>    * \param[in] minExposureTime Minimum exposure time to allow
> @@ -538,7 +574,18 @@ double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex,
>    */
>   double AgcMeanLuminance::effectiveYTarget() const
>   {
> -	return std::min(relativeLuminanceTarget_ * exposureCompensation_,
> +	double lux = lux_;
> +	if (relativeLuminanceTarget_.size() > 1 && lux_ == 0) {
> +		LOG(AgcMeanLuminance, Debug)
> +			<< "Missing lux value for luminance target calculation, default to "
> +			<< kDefaultLuxLevel;
> +		lux = kDefaultLuxLevel;
> +	}
> +
> +	double luminanceTarget = relativeLuminanceTarget_.eval(
> +		relativeLuminanceTarget_.domain().clamp(lux));
> +
> +	return std::min(luminanceTarget * exposureCompensation_,
>   			kMaxRelativeLuminanceTarget);
>   }
>   
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index d7ec548e3e58..1fbff304a72b 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -20,6 +20,7 @@
>   
>   #include "exposure_mode_helper.h"
>   #include "histogram.h"
> +#include "pwl.h"
>   
>   namespace libcamera {
>   
> @@ -50,6 +51,11 @@ public:
>   		exposureCompensation_ = gain;
>   	}
>   
> +	void setLux(unsigned int lux)
> +	{
> +		lux_ = lux;
> +	}
> +
>   	void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,
>   		       double minGain, double maxGain, std::vector<AgcConstraint> constraints);
>   
> @@ -81,8 +87,8 @@ public:
>   
>   private:
>   	virtual double estimateLuminance(const double gain) const = 0;
> +	int parseRelativeLuminanceTarget(const YamlObject &tuningData);
>   
> -	void parseRelativeLuminanceTarget(const YamlObject &tuningData);
>   	void parseConstraint(const YamlObject &modeDict, int32_t id);
>   	int parseConstraintModes(const YamlObject &tuningData);
>   	int parseExposureModes(const YamlObject &tuningData);
> @@ -95,7 +101,8 @@ private:
>   	double exposureCompensation_;
>   	uint64_t frameCount_;
>   	utils::Duration filteredExposure_;
> -	double relativeLuminanceTarget_;
> +	unsigned int lux_;
> +	Pwl relativeLuminanceTarget_;
>   
>   	std::vector<AgcConstraint> additionalConstraints_;
>   	std::map<int32_t, std::vector<AgcConstraint>> constraintModes_;
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index f5a3c917cb69..1ecaff680978 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -618,6 +618,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>   		effectiveExposureValue *= frameContext.agc.quantizationGain;
>   
>   	setExposureCompensation(pow(2.0, frameContext.agc.exposureValue));
> +	setLux(frameContext.lux.lux);
>   
>   	utils::Duration newExposureTime;
>   	double aGain, qGain, dGain;