[RFC,v3,16/50] ipa: libipa: agc_mean_luminance: calculateNewEv(): Collect results
diff mbox series

Message ID 20260803131435.153927-17-barnabas.pocze@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: libipa: agc rework
Related show

Commit Message

Barnabás Pőcze Aug. 3, 2026, 1:14 p.m. UTC 
Add a new type that gives proper names to the returned quantities
instead of just using an `std::tuple`.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/agc.cpp         | 10 ++++------
 src/ipa/libipa/agc_mean_luminance.cpp   | 11 ++++++++---
 src/ipa/libipa/agc_mean_luminance.h     |  7 ++++---
 src/ipa/libipa/exposure_mode_helper.cpp | 19 ++++++++++++++++++-
 src/ipa/libipa/exposure_mode_helper.h   | 11 ++++++++---
 src/ipa/mali-c55/algorithms/agc.cpp     | 10 ++++------
 src/ipa/rkisp1/algorithms/agc.cpp       | 16 +++++++---------
 7 files changed, 53 insertions(+), 31 deletions(-)

Comments

Jacopo Mondi Aug. 5, 2026, 1:52 p.m. UTC | #1 
Hi Barnabas

On Mon, Aug 03, 2026 at 03:14:01PM +0200, Barnabás Pőcze wrote:
> Add a new type that gives proper names to the returned quantities
> instead of just using an `std::tuple`.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Seems like you forgot the tag from the last version

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

> ---
>  src/ipa/ipu3/algorithms/agc.cpp         | 10 ++++------
>  src/ipa/libipa/agc_mean_luminance.cpp   | 11 ++++++++---
>  src/ipa/libipa/agc_mean_luminance.h     |  7 ++++---
>  src/ipa/libipa/exposure_mode_helper.cpp | 19 ++++++++++++++++++-
>  src/ipa/libipa/exposure_mode_helper.h   | 11 ++++++++---
>  src/ipa/mali-c55/algorithms/agc.cpp     | 10 ++++------
>  src/ipa/rkisp1/algorithms/agc.cpp       | 16 +++++++---------
>  7 files changed, 53 insertions(+), 31 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index ac5b6e7513..e74db62960 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -237,9 +237,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  	double analogueGain = frameContext.sensor.gain;
>  	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
>
> -	utils::Duration newExposureTime;
> -	double aGain, qGain, dGain;
> -	std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({
> +	const auto &newEv = agc_.calculateNewEv({
>  		.traits = AgcTraits{
>  			rgbTriples_,
>  			{{
> @@ -257,12 +255,12 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>
>  	LOG(IPU3Agc, Debug)
>  		<< "Divided up exposure time, analogue gain and digital gain are "
> -		<< newExposureTime << ", " << aGain << " and " << dGain;
> +		<< newEv.exposureTime << ", " << newEv.analogueGain << " and " << newEv.digitalGain;
>
>  	IPAActiveState &activeState = context.activeState;
>  	/* Update the estimated exposure time and gain. */
> -	activeState.agc.exposure = newExposureTime / context.configuration.sensor.lineDuration;
> -	activeState.agc.gain = aGain;
> +	activeState.agc.exposure = newEv.exposureTime / context.configuration.sensor.lineDuration;
> +	activeState.agc.gain = newEv.analogueGain;
>
>  	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>  	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 3a14d778a3..8e6eafd853 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -667,6 +667,11 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
>   * \brief The index of the exposure mode to use
>   */
>
> +/**
> + * \struct AgcMeanLuminance::Result
> + * \brief Collection of results of the mean luminance AGC algorithm
> + */
> +
>  /**
>   * \brief Calculate the new exposure value and split it between exposure time
>   * and gain
> @@ -679,7 +684,7 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
>   * \return Tuple of exposure time, analogue gain, quantization gain and digital
>   * gain
>   */
> -std::tuple<utils::Duration, double, double, double>
> +AgcMeanLuminance::Result
>  AgcMeanLuminance::calculateNewEv(const Params &params)
>  {
>  	/*
> @@ -698,7 +703,7 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>  		 * doesn't get stuck with 0 in case the sensor driver allows a
>  		 * min exposure of 0.
>  		 */
> -		return exposureModeHelper.splitExposure(10ms);
> +		return { exposureModeHelper.splitExposure(10ms) };
>  	}
>
>  	double gain = estimateInitialGain(params.traits);
> @@ -720,7 +725,7 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>  	newExposureValue = filterExposure(newExposureValue);
>
>  	frameCount_++;
> -	return exposureModeHelper.splitExposure(newExposureValue);
> +	return { exposureModeHelper.splitExposure(newExposureValue) };
>  }
>
>  /**
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index 9b82d97ab6..aa4c0369ac 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -9,7 +9,6 @@
>
>  #include <map>
>  #include <memory>
> -#include <tuple>
>  #include <vector>
>
>  #include <libcamera/base/utils.h>
> @@ -87,8 +86,10 @@ public:
>  		uint32_t exposureModeIndex;
>  	};
>
> -	std::tuple<utils::Duration, double, double, double>
> -	calculateNewEv(const Params &params);
> +	struct Result : ExposureModeHelper::Result {
> +	};
> +
> +	[[nodiscard]] Result calculateNewEv(const Params &params);
>
>  	double effectiveYTarget() const;
>
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index 01c5cba8e2..762e27b4fd 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -153,6 +153,23 @@ double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons
>  	return clamped;
>  }
>
> +/**
> + * \struct ExposureModeHelper::Result
> + * \brief Result of splitExposure()
> + *
> + * \var ExposureModeHelper::Result::exposureTime
> + * \brief The applicable exposure time
> + *
> + * \var ExposureModeHelper::Result::analogueGain
> + * \brief The applicable analogue gain
> + *
> + * \var ExposureModeHelper::Result::quantizationGain
> + * \brief The applicable quantization gain
> + *
> + * \var ExposureModeHelper::Result::digitalGain
> + * \brief The applicable digital gain
> + */
> +
>  /**
>   * \brief Split exposure into exposure time and gain
>   * \param[in] exposure Exposure value
> @@ -190,7 +207,7 @@ double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons
>   * \return Tuple of exposure time, analogue gain, quantization gain and digital
>   * gain
>   */
> -std::tuple<utils::Duration, double, double, double>
> +ExposureModeHelper::Result
>  ExposureModeHelper::splitExposure(utils::Duration exposure) const
>  {
>  	ASSERT(maxExposureTime_);
> diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h
> index 968192ddc5..2bab20f974 100644
> --- a/src/ipa/libipa/exposure_mode_helper.h
> +++ b/src/ipa/libipa/exposure_mode_helper.h
> @@ -7,7 +7,6 @@
>
>  #pragma once
>
> -#include <tuple>
>  #include <utility>
>  #include <vector>
>
> @@ -30,8 +29,14 @@ public:
>  	void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,
>  		       double minGain, double maxGain);
>
> -	std::tuple<utils::Duration, double, double, double>
> -	splitExposure(utils::Duration exposure) const;
> +	struct Result {
> +		utils::Duration exposureTime;
> +		double analogueGain;
> +		double quantizationGain;
> +		double digitalGain;
> +	};
> +
> +	[[nodiscard]] Result splitExposure(utils::Duration exposure) const;
>
>  	utils::Duration minExposureTime() const { return minExposureTime_; }
>  	utils::Duration maxExposureTime() const { return maxExposureTime_; }
> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
> index fb97b87c6e..821f43e9fd 100644
> --- a/src/ipa/mali-c55/algorithms/agc.cpp
> +++ b/src/ipa/mali-c55/algorithms/agc.cpp
> @@ -332,9 +332,7 @@ void Agc::process(IPAContext &context,
>  	utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
>  	utils::Duration effectiveExposureValue = currentShutter * analogueGain;
>
> -	utils::Duration shutterTime;
> -	double aGain, qGain, dGain;
> -	std::tie(shutterTime, aGain, qGain, dGain) = agc_.calculateNewEv({
> +	const auto &newEv = agc_.calculateNewEv({
>  		.traits = AgcTraits(statistics_),
>  		.yHist = statistics_.yHist,
>  		.effectiveExposureValue = effectiveExposureValue,
> @@ -344,10 +342,10 @@ void Agc::process(IPAContext &context,
>
>  	LOG(MaliC55Agc, Debug)
>  		<< "Divided up shutter, analogue gain and digital gain are "
> -		<< shutterTime << ", " << aGain << " and " << dGain;
> +		<< newEv.exposureTime << ", " << newEv.analogueGain << " and " << newEv.digitalGain;
>
> -	activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
> -	activeState.agc.automatic.sensorGain = aGain;
> +	activeState.agc.automatic.exposure = newEv.exposureTime / configuration.sensor.lineDuration;
> +	activeState.agc.automatic.sensorGain = newEv.analogueGain;
>
>  	metadata.set(controls::ExposureTime, currentShutter.get<std::micro>());
>  	metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 8b4bcbd71b..a4567c8258 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -720,9 +720,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  	agc_.setExposureCompensation(pow(2.0, frameContext.agc.exposureValue));
>  	agc_.setLux(frameContext.lux.lux);
>
> -	utils::Duration newExposureTime;
> -	double aGain, qGain, dGain;
> -	std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({
> +	const auto &newEv = agc_.calculateNewEv({
>  		.traits = AgcTraits{
>  			{ params->ae.exp_mean, context.hw.numAeCells },
>  			meteringModes_.at(frameContext.agc.meteringMode),
> @@ -735,21 +733,21 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>
>  	LOG(RkISP1Agc, Debug)
>  		<< "Divided up exposure time, analogue gain, quantization gain"
> -		<< " and digital gain are " << newExposureTime << ", " << aGain
> -		<< ", " << qGain << " and " << dGain;
> +		<< " and digital gain are " << newEv.exposureTime << ", " << newEv.analogueGain
> +		<< ", " << newEv.quantizationGain << " and " << newEv.digitalGain;
>
>  	IPAActiveState &activeState = context.activeState;
>  	/* Update the estimated exposure and gain. */
> -	activeState.agc.automatic.exposure = newExposureTime / lineDuration;
> -	activeState.agc.automatic.gain = aGain;
> -	activeState.agc.automatic.quantizationGain = qGain;
> +	activeState.agc.automatic.exposure = newEv.exposureTime / lineDuration;
> +	activeState.agc.automatic.gain = newEv.analogueGain;
> +	activeState.agc.automatic.quantizationGain = newEv.quantizationGain;
>  	activeState.agc.automatic.yTarget = agc_.effectiveYTarget();
>  	/*
>  	 * Expand the target frame duration so that we do not run faster than
>  	 * the minimum frame duration when we have short exposures.
>  	 */
>  	processFrameDuration(context, frameContext,
> -			     std::max(frameContext.agc.minFrameDuration, newExposureTime));
> +			     std::max(frameContext.agc.minFrameDuration, newEv.exposureTime));
>
>  	fillMetadata(context, frameContext, metadata);
>  }
> --
> 2.55.0
>
Stefan Klug Aug. 6, 2026, 10:52 a.m. UTC | #2 
Hi Barnabás,

Quoting Barnabás Pőcze (2026-08-03 15:14:01)
> Add a new type that gives proper names to the returned quantities
> instead of just using an `std::tuple`.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

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

Best regards,
Stefan

> ---
>  src/ipa/ipu3/algorithms/agc.cpp         | 10 ++++------
>  src/ipa/libipa/agc_mean_luminance.cpp   | 11 ++++++++---
>  src/ipa/libipa/agc_mean_luminance.h     |  7 ++++---
>  src/ipa/libipa/exposure_mode_helper.cpp | 19 ++++++++++++++++++-
>  src/ipa/libipa/exposure_mode_helper.h   | 11 ++++++++---
>  src/ipa/mali-c55/algorithms/agc.cpp     | 10 ++++------
>  src/ipa/rkisp1/algorithms/agc.cpp       | 16 +++++++---------
>  7 files changed, 53 insertions(+), 31 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index ac5b6e7513..e74db62960 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -237,9 +237,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>         double analogueGain = frameContext.sensor.gain;
>         utils::Duration effectiveExposureValue = exposureTime * analogueGain;
>  
> -       utils::Duration newExposureTime;
> -       double aGain, qGain, dGain;
> -       std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({
> +       const auto &newEv = agc_.calculateNewEv({
>                 .traits = AgcTraits{
>                         rgbTriples_,
>                         {{
> @@ -257,12 +255,12 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  
>         LOG(IPU3Agc, Debug)
>                 << "Divided up exposure time, analogue gain and digital gain are "
> -               << newExposureTime << ", " << aGain << " and " << dGain;
> +               << newEv.exposureTime << ", " << newEv.analogueGain << " and " << newEv.digitalGain;
>  
>         IPAActiveState &activeState = context.activeState;
>         /* Update the estimated exposure time and gain. */
> -       activeState.agc.exposure = newExposureTime / context.configuration.sensor.lineDuration;
> -       activeState.agc.gain = aGain;
> +       activeState.agc.exposure = newEv.exposureTime / context.configuration.sensor.lineDuration;
> +       activeState.agc.gain = newEv.analogueGain;
>  
>         metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>         metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 3a14d778a3..8e6eafd853 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -667,6 +667,11 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
>   * \brief The index of the exposure mode to use
>   */
>  
> +/**
> + * \struct AgcMeanLuminance::Result
> + * \brief Collection of results of the mean luminance AGC algorithm
> + */
> +
>  /**
>   * \brief Calculate the new exposure value and split it between exposure time
>   * and gain
> @@ -679,7 +684,7 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
>   * \return Tuple of exposure time, analogue gain, quantization gain and digital
>   * gain
>   */
> -std::tuple<utils::Duration, double, double, double>
> +AgcMeanLuminance::Result
>  AgcMeanLuminance::calculateNewEv(const Params &params)
>  {
>         /*
> @@ -698,7 +703,7 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>                  * doesn't get stuck with 0 in case the sensor driver allows a
>                  * min exposure of 0.
>                  */
> -               return exposureModeHelper.splitExposure(10ms);
> +               return { exposureModeHelper.splitExposure(10ms) };
>         }
>  
>         double gain = estimateInitialGain(params.traits);
> @@ -720,7 +725,7 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>         newExposureValue = filterExposure(newExposureValue);
>  
>         frameCount_++;
> -       return exposureModeHelper.splitExposure(newExposureValue);
> +       return { exposureModeHelper.splitExposure(newExposureValue) };
>  }
>  
>  /**
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index 9b82d97ab6..aa4c0369ac 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -9,7 +9,6 @@
>  
>  #include <map>
>  #include <memory>
> -#include <tuple>
>  #include <vector>
>  
>  #include <libcamera/base/utils.h>
> @@ -87,8 +86,10 @@ public:
>                 uint32_t exposureModeIndex;
>         };
>  
> -       std::tuple<utils::Duration, double, double, double>
> -       calculateNewEv(const Params &params);
> +       struct Result : ExposureModeHelper::Result {
> +       };
> +
> +       [[nodiscard]] Result calculateNewEv(const Params &params);
>  
>         double effectiveYTarget() const;
>  
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index 01c5cba8e2..762e27b4fd 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -153,6 +153,23 @@ double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons
>         return clamped;
>  }
>  
> +/**
> + * \struct ExposureModeHelper::Result
> + * \brief Result of splitExposure()
> + *
> + * \var ExposureModeHelper::Result::exposureTime
> + * \brief The applicable exposure time
> + *
> + * \var ExposureModeHelper::Result::analogueGain
> + * \brief The applicable analogue gain
> + *
> + * \var ExposureModeHelper::Result::quantizationGain
> + * \brief The applicable quantization gain
> + *
> + * \var ExposureModeHelper::Result::digitalGain
> + * \brief The applicable digital gain
> + */
> +
>  /**
>   * \brief Split exposure into exposure time and gain
>   * \param[in] exposure Exposure value
> @@ -190,7 +207,7 @@ double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons
>   * \return Tuple of exposure time, analogue gain, quantization gain and digital
>   * gain
>   */
> -std::tuple<utils::Duration, double, double, double>
> +ExposureModeHelper::Result
>  ExposureModeHelper::splitExposure(utils::Duration exposure) const
>  {
>         ASSERT(maxExposureTime_);
> diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h
> index 968192ddc5..2bab20f974 100644
> --- a/src/ipa/libipa/exposure_mode_helper.h
> +++ b/src/ipa/libipa/exposure_mode_helper.h
> @@ -7,7 +7,6 @@
>  
>  #pragma once
>  
> -#include <tuple>
>  #include <utility>
>  #include <vector>
>  
> @@ -30,8 +29,14 @@ public:
>         void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,
>                        double minGain, double maxGain);
>  
> -       std::tuple<utils::Duration, double, double, double>
> -       splitExposure(utils::Duration exposure) const;
> +       struct Result {
> +               utils::Duration exposureTime;
> +               double analogueGain;
> +               double quantizationGain;
> +               double digitalGain;
> +       };
> +
> +       [[nodiscard]] Result splitExposure(utils::Duration exposure) const;
>  
>         utils::Duration minExposureTime() const { return minExposureTime_; }
>         utils::Duration maxExposureTime() const { return maxExposureTime_; }
> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
> index fb97b87c6e..821f43e9fd 100644
> --- a/src/ipa/mali-c55/algorithms/agc.cpp
> +++ b/src/ipa/mali-c55/algorithms/agc.cpp
> @@ -332,9 +332,7 @@ void Agc::process(IPAContext &context,
>         utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
>         utils::Duration effectiveExposureValue = currentShutter * analogueGain;
>  
> -       utils::Duration shutterTime;
> -       double aGain, qGain, dGain;
> -       std::tie(shutterTime, aGain, qGain, dGain) = agc_.calculateNewEv({
> +       const auto &newEv = agc_.calculateNewEv({
>                 .traits = AgcTraits(statistics_),
>                 .yHist = statistics_.yHist,
>                 .effectiveExposureValue = effectiveExposureValue,
> @@ -344,10 +342,10 @@ void Agc::process(IPAContext &context,
>  
>         LOG(MaliC55Agc, Debug)
>                 << "Divided up shutter, analogue gain and digital gain are "
> -               << shutterTime << ", " << aGain << " and " << dGain;
> +               << newEv.exposureTime << ", " << newEv.analogueGain << " and " << newEv.digitalGain;
>  
> -       activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
> -       activeState.agc.automatic.sensorGain = aGain;
> +       activeState.agc.automatic.exposure = newEv.exposureTime / configuration.sensor.lineDuration;
> +       activeState.agc.automatic.sensorGain = newEv.analogueGain;
>  
>         metadata.set(controls::ExposureTime, currentShutter.get<std::micro>());
>         metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 8b4bcbd71b..a4567c8258 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -720,9 +720,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>         agc_.setExposureCompensation(pow(2.0, frameContext.agc.exposureValue));
>         agc_.setLux(frameContext.lux.lux);
>  
> -       utils::Duration newExposureTime;
> -       double aGain, qGain, dGain;
> -       std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({
> +       const auto &newEv = agc_.calculateNewEv({
>                 .traits = AgcTraits{
>                         { params->ae.exp_mean, context.hw.numAeCells },
>                         meteringModes_.at(frameContext.agc.meteringMode),
> @@ -735,21 +733,21 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  
>         LOG(RkISP1Agc, Debug)
>                 << "Divided up exposure time, analogue gain, quantization gain"
> -               << " and digital gain are " << newExposureTime << ", " << aGain
> -               << ", " << qGain << " and " << dGain;
> +               << " and digital gain are " << newEv.exposureTime << ", " << newEv.analogueGain
> +               << ", " << newEv.quantizationGain << " and " << newEv.digitalGain;
>  
>         IPAActiveState &activeState = context.activeState;
>         /* Update the estimated exposure and gain. */
> -       activeState.agc.automatic.exposure = newExposureTime / lineDuration;
> -       activeState.agc.automatic.gain = aGain;
> -       activeState.agc.automatic.quantizationGain = qGain;
> +       activeState.agc.automatic.exposure = newEv.exposureTime / lineDuration;
> +       activeState.agc.automatic.gain = newEv.analogueGain;
> +       activeState.agc.automatic.quantizationGain = newEv.quantizationGain;
>         activeState.agc.automatic.yTarget = agc_.effectiveYTarget();
>         /*
>          * Expand the target frame duration so that we do not run faster than
>          * the minimum frame duration when we have short exposures.
>          */
>         processFrameDuration(context, frameContext,
> -                            std::max(frameContext.agc.minFrameDuration, newExposureTime));
> +                            std::max(frameContext.agc.minFrameDuration, newEv.exposureTime));
>  
>         fillMetadata(context, frameContext, metadata);
>  }
> -- 
> 2.55.0
>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
index ac5b6e7513..e74db62960 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -237,9 +237,7 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 	double analogueGain = frameContext.sensor.gain;
 	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
 
-	utils::Duration newExposureTime;
-	double aGain, qGain, dGain;
-	std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({
+	const auto &newEv = agc_.calculateNewEv({
 		.traits = AgcTraits{
 			rgbTriples_,
 			{{
@@ -257,12 +255,12 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 
 	LOG(IPU3Agc, Debug)
 		<< "Divided up exposure time, analogue gain and digital gain are "
-		<< newExposureTime << ", " << aGain << " and " << dGain;
+		<< newEv.exposureTime << ", " << newEv.analogueGain << " and " << newEv.digitalGain;
 
 	IPAActiveState &activeState = context.activeState;
 	/* Update the estimated exposure time and gain. */
-	activeState.agc.exposure = newExposureTime / context.configuration.sensor.lineDuration;
-	activeState.agc.gain = aGain;
+	activeState.agc.exposure = newEv.exposureTime / context.configuration.sensor.lineDuration;
+	activeState.agc.gain = newEv.analogueGain;
 
 	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
 	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
index 3a14d778a3..8e6eafd853 100644
--- a/src/ipa/libipa/agc_mean_luminance.cpp
+++ b/src/ipa/libipa/agc_mean_luminance.cpp
@@ -667,6 +667,11 @@  utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
  * \brief The index of the exposure mode to use
  */
 
+/**
+ * \struct AgcMeanLuminance::Result
+ * \brief Collection of results of the mean luminance AGC algorithm
+ */
+
 /**
  * \brief Calculate the new exposure value and split it between exposure time
  * and gain
@@ -679,7 +684,7 @@  utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
  * \return Tuple of exposure time, analogue gain, quantization gain and digital
  * gain
  */
-std::tuple<utils::Duration, double, double, double>
+AgcMeanLuminance::Result
 AgcMeanLuminance::calculateNewEv(const Params &params)
 {
 	/*
@@ -698,7 +703,7 @@  AgcMeanLuminance::calculateNewEv(const Params &params)
 		 * doesn't get stuck with 0 in case the sensor driver allows a
 		 * min exposure of 0.
 		 */
-		return exposureModeHelper.splitExposure(10ms);
+		return { exposureModeHelper.splitExposure(10ms) };
 	}
 
 	double gain = estimateInitialGain(params.traits);
@@ -720,7 +725,7 @@  AgcMeanLuminance::calculateNewEv(const Params &params)
 	newExposureValue = filterExposure(newExposureValue);
 
 	frameCount_++;
-	return exposureModeHelper.splitExposure(newExposureValue);
+	return { exposureModeHelper.splitExposure(newExposureValue) };
 }
 
 /**
diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
index 9b82d97ab6..aa4c0369ac 100644
--- a/src/ipa/libipa/agc_mean_luminance.h
+++ b/src/ipa/libipa/agc_mean_luminance.h
@@ -9,7 +9,6 @@ 
 
 #include <map>
 #include <memory>
-#include <tuple>
 #include <vector>
 
 #include <libcamera/base/utils.h>
@@ -87,8 +86,10 @@  public:
 		uint32_t exposureModeIndex;
 	};
 
-	std::tuple<utils::Duration, double, double, double>
-	calculateNewEv(const Params &params);
+	struct Result : ExposureModeHelper::Result {
+	};
+
+	[[nodiscard]] Result calculateNewEv(const Params &params);
 
 	double effectiveYTarget() const;
 
diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
index 01c5cba8e2..762e27b4fd 100644
--- a/src/ipa/libipa/exposure_mode_helper.cpp
+++ b/src/ipa/libipa/exposure_mode_helper.cpp
@@ -153,6 +153,23 @@  double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons
 	return clamped;
 }
 
+/**
+ * \struct ExposureModeHelper::Result
+ * \brief Result of splitExposure()
+ *
+ * \var ExposureModeHelper::Result::exposureTime
+ * \brief The applicable exposure time
+ *
+ * \var ExposureModeHelper::Result::analogueGain
+ * \brief The applicable analogue gain
+ *
+ * \var ExposureModeHelper::Result::quantizationGain
+ * \brief The applicable quantization gain
+ *
+ * \var ExposureModeHelper::Result::digitalGain
+ * \brief The applicable digital gain
+ */
+
 /**
  * \brief Split exposure into exposure time and gain
  * \param[in] exposure Exposure value
@@ -190,7 +207,7 @@  double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons
  * \return Tuple of exposure time, analogue gain, quantization gain and digital
  * gain
  */
-std::tuple<utils::Duration, double, double, double>
+ExposureModeHelper::Result
 ExposureModeHelper::splitExposure(utils::Duration exposure) const
 {
 	ASSERT(maxExposureTime_);
diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h
index 968192ddc5..2bab20f974 100644
--- a/src/ipa/libipa/exposure_mode_helper.h
+++ b/src/ipa/libipa/exposure_mode_helper.h
@@ -7,7 +7,6 @@ 
 
 #pragma once
 
-#include <tuple>
 #include <utility>
 #include <vector>
 
@@ -30,8 +29,14 @@  public:
 	void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,
 		       double minGain, double maxGain);
 
-	std::tuple<utils::Duration, double, double, double>
-	splitExposure(utils::Duration exposure) const;
+	struct Result {
+		utils::Duration exposureTime;
+		double analogueGain;
+		double quantizationGain;
+		double digitalGain;
+	};
+
+	[[nodiscard]] Result splitExposure(utils::Duration exposure) const;
 
 	utils::Duration minExposureTime() const { return minExposureTime_; }
 	utils::Duration maxExposureTime() const { return maxExposureTime_; }
diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
index fb97b87c6e..821f43e9fd 100644
--- a/src/ipa/mali-c55/algorithms/agc.cpp
+++ b/src/ipa/mali-c55/algorithms/agc.cpp
@@ -332,9 +332,7 @@  void Agc::process(IPAContext &context,
 	utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
 	utils::Duration effectiveExposureValue = currentShutter * analogueGain;
 
-	utils::Duration shutterTime;
-	double aGain, qGain, dGain;
-	std::tie(shutterTime, aGain, qGain, dGain) = agc_.calculateNewEv({
+	const auto &newEv = agc_.calculateNewEv({
 		.traits = AgcTraits(statistics_),
 		.yHist = statistics_.yHist,
 		.effectiveExposureValue = effectiveExposureValue,
@@ -344,10 +342,10 @@  void Agc::process(IPAContext &context,
 
 	LOG(MaliC55Agc, Debug)
 		<< "Divided up shutter, analogue gain and digital gain are "
-		<< shutterTime << ", " << aGain << " and " << dGain;
+		<< newEv.exposureTime << ", " << newEv.analogueGain << " and " << newEv.digitalGain;
 
-	activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
-	activeState.agc.automatic.sensorGain = aGain;
+	activeState.agc.automatic.exposure = newEv.exposureTime / configuration.sensor.lineDuration;
+	activeState.agc.automatic.sensorGain = newEv.analogueGain;
 
 	metadata.set(controls::ExposureTime, currentShutter.get<std::micro>());
 	metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);
diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
index 8b4bcbd71b..a4567c8258 100644
--- a/src/ipa/rkisp1/algorithms/agc.cpp
+++ b/src/ipa/rkisp1/algorithms/agc.cpp
@@ -720,9 +720,7 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 	agc_.setExposureCompensation(pow(2.0, frameContext.agc.exposureValue));
 	agc_.setLux(frameContext.lux.lux);
 
-	utils::Duration newExposureTime;
-	double aGain, qGain, dGain;
-	std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({
+	const auto &newEv = agc_.calculateNewEv({
 		.traits = AgcTraits{
 			{ params->ae.exp_mean, context.hw.numAeCells },
 			meteringModes_.at(frameContext.agc.meteringMode),
@@ -735,21 +733,21 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 
 	LOG(RkISP1Agc, Debug)
 		<< "Divided up exposure time, analogue gain, quantization gain"
-		<< " and digital gain are " << newExposureTime << ", " << aGain
-		<< ", " << qGain << " and " << dGain;
+		<< " and digital gain are " << newEv.exposureTime << ", " << newEv.analogueGain
+		<< ", " << newEv.quantizationGain << " and " << newEv.digitalGain;
 
 	IPAActiveState &activeState = context.activeState;
 	/* Update the estimated exposure and gain. */
-	activeState.agc.automatic.exposure = newExposureTime / lineDuration;
-	activeState.agc.automatic.gain = aGain;
-	activeState.agc.automatic.quantizationGain = qGain;
+	activeState.agc.automatic.exposure = newEv.exposureTime / lineDuration;
+	activeState.agc.automatic.gain = newEv.analogueGain;
+	activeState.agc.automatic.quantizationGain = newEv.quantizationGain;
 	activeState.agc.automatic.yTarget = agc_.effectiveYTarget();
 	/*
 	 * Expand the target frame duration so that we do not run faster than
 	 * the minimum frame duration when we have short exposures.
 	 */
 	processFrameDuration(context, frameContext,
-			     std::max(frameContext.agc.minFrameDuration, newExposureTime));
+			     std::max(frameContext.agc.minFrameDuration, newEv.exposureTime));
 
 	fillMetadata(context, frameContext, metadata);
 }