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

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

Commit Message

Barnabás Pőcze July 23, 2026, 3:42 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 July 24, 2026, 12:49 p.m. UTC | #1
Hi Barnabás

On Thu, Jul 23, 2026 at 05:42:59PM +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>
> ---
>  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({

Doesn't agc_.calculateNewEv() construct a Result before returning it ?
Does using a reference here help ?

>  		.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 e8679c66be..5e12c4b475 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -668,6 +668,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

        \sa ExposureModeHelper::Result

but maybe it's not necessary as doxygen already provides an
inheritance diagram for the two types

> + */
> +
>  /**
>   * \brief Calculate the new exposure value and split it between exposure time
>   * and gain
> @@ -680,7 +685,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)
>  {
>  	/*
> @@ -699,7 +704,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);
> @@ -721,7 +726,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({

Same question on & for this and rkisp1

>  		.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));

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

Thanks
  j

>
>  	fillMetadata(context, frameContext, metadata);
>  }
> --
> 2.55.0
>
Barnabás Pőcze July 24, 2026, 1:05 p.m. UTC | #2
2026. 07. 24. 14:49 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Thu, Jul 23, 2026 at 05:42:59PM +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>
>> ---
>>   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({
> 
> Doesn't agc_.calculateNewEv() construct a Result before returning it ?
> Does using a reference here help ?

It does not change anything, it's just what I usually do in these cases. You could
use `auto`, `auot&&`, or even `AgcMeanLuminance::Result`, all of them would effectively
be the same. This applies at every call-site.


> 
>>   		.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 e8679c66be..5e12c4b475 100644
>> --- a/src/ipa/libipa/agc_mean_luminance.cpp
>> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
>> @@ -668,6 +668,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
> 
>          \sa ExposureModeHelper::Result
> 
> but maybe it's not necessary as doxygen already provides an
> inheritance diagram for the two types

Good point, I did not check what it looks like. But I've just done so, and
there is an inheritance diagram, as well as an option under the direct members:

   Public Attributes inherited from libcamera::ipa::ExposureModeHelper::Result

and clicking on it brings up the members. So I think it's fine.
   

> 
>> + */
>> +
>>   /**
>>    * \brief Calculate the new exposure value and split it between exposure time
>>    * and gain
>> @@ -680,7 +685,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)
>>   {
>>   	/*
>> @@ -699,7 +704,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);
>> @@ -721,7 +726,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({
> 
> Same question on & for this and rkisp1
> 
>>   		.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));
> 
> minors apart
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> Thanks
>    j
> 
>>
>>   	fillMetadata(context, frameContext, metadata);
>>   }
>> --
>> 2.55.0
>>
Jacopo Mondi July 24, 2026, 1:15 p.m. UTC | #3
Hi Barnabás

On Fri, Jul 24, 2026 at 03:05:40PM +0200, Barnabás Pőcze wrote:
> 2026. 07. 24. 14:49 keltezéssel, Jacopo Mondi írta:
> > Hi Barnabás
> >
> > On Thu, Jul 23, 2026 at 05:42:59PM +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>
> > > ---
> > >   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({
> >
> > Doesn't agc_.calculateNewEv() construct a Result before returning it ?
> > Does using a reference here help ?
>
> It does not change anything, it's just what I usually do in these cases. You could
> use `auto`, `auot&&`, or even `AgcMeanLuminance::Result`, all of them would effectively
> be the same. This applies at every call-site.
>

Did I already mention I don't like 'auto' ? :)

>
> >
> > >   		.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 e8679c66be..5e12c4b475 100644
> > > --- a/src/ipa/libipa/agc_mean_luminance.cpp
> > > +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> > > @@ -668,6 +668,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
> >
> >          \sa ExposureModeHelper::Result
> >
> > but maybe it's not necessary as doxygen already provides an
> > inheritance diagram for the two types
>
> Good point, I did not check what it looks like. But I've just done so, and
> there is an inheritance diagram, as well as an option under the direct members:
>
>   Public Attributes inherited from libcamera::ipa::ExposureModeHelper::Result
>
> and clicking on it brings up the members. So I think it's fine.
>

I think so, yes

> >
> > > + */
> > > +
> > >   /**
> > >    * \brief Calculate the new exposure value and split it between exposure time
> > >    * and gain
> > > @@ -680,7 +685,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)
> > >   {
> > >   	/*
> > > @@ -699,7 +704,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);
> > > @@ -721,7 +726,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({
> >
> > Same question on & for this and rkisp1
> >
> > >   		.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));
> >
> > minors apart
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > Thanks
> >    j
> >
> > >
> > >   	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 e8679c66be..5e12c4b475 100644
--- a/src/ipa/libipa/agc_mean_luminance.cpp
+++ b/src/ipa/libipa/agc_mean_luminance.cpp
@@ -668,6 +668,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
@@ -680,7 +685,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)
 {
 	/*
@@ -699,7 +704,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);
@@ -721,7 +726,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);
 }