[PATCH v2 3/8] ipa: libipa: Add ExposureModeHelper

[Dan Scally]

Hi Jacopo

On 19/04/2024 15:10, Jacopo Mondi wrote:
> Hi Dan
>
> On Wed, Apr 17, 2024 at 02:15:31PM +0100, Daniel Scally wrote:
>> From: Paul Elder <paul.elder at ideasonboard.com>
>>
>> Add a helper for managing exposure modes and splitting exposure times
>> into shutter and gain values.
>>
>> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
>> ---
>> Changes in v2:
>>
>> 	- Expanded the documentation
>> 	- Dropped the overloads for fixed shutter / gain - the same
>> 	  functionality is instead done by setting min and max shutter and gain
>> 	  to the same value
>> 	- Changed ::init() to consume a vector of pairs instead of two separate
>> 	  vectors
>> 	- Reworked splitExposure()
>>
>>   src/ipa/libipa/exposure_mode_helper.cpp | 257 ++++++++++++++++++++++++
>>   src/ipa/libipa/exposure_mode_helper.h   |  53 +++++
>>   src/ipa/libipa/meson.build              |   2 +
>>   3 files changed, 312 insertions(+)
>>   create mode 100644 src/ipa/libipa/exposure_mode_helper.cpp
>>   create mode 100644 src/ipa/libipa/exposure_mode_helper.h
>>
>> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
>> new file mode 100644
>> index 00000000..6463de18
>> --- /dev/null
>> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
>> @@ -0,0 +1,257 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
>> + *
>> + * exposure_mode_helper.cpp - Helper class that performs computations relating to exposure
>> + */
>> +#include "exposure_mode_helper.h"
>> +
>> +#include <algorithm>
>> +
>> +#include <libcamera/base/log.h>
>> +
>> +/**
>> + * \file exposure_mode_helper.h
>> + * \brief Helper class that performs computations relating to exposure
>> + *
>> + * AEGC algorithms have a need to split exposure between shutter and gain.
> s/shutter/shutter time ?
>
> This and in the other occurences
>
> also, "gain" which gain ? Analogue, digital or a combination of the
> two ?


Both

>
>> + * Multiple implementations do so based on paired sets of shutter and gain
>> + * limits; provide a helper to avoid duplicating the code.
>> + */
>> +
>> +namespace libcamera {
>> +
>> +LOG_DEFINE_CATEGORY(ExposureModeHelper)
>> +
>> +namespace ipa {
>> +
>> +/**
>> + * \class ExposureModeHelper
>> + * \brief Class for splitting exposure into shutter and gain
>> + *
>> + * The ExposureModeHelper class provides a standard interface through which an
>> + * AEGC algorithm can divide exposure between shutter and gain. It is configured
>> + * with a set of shutter and gain pairs and works by initially fixing gain at
>> + * 1.0 and increasing shutter up to the shutter value from the first pair in the
>> + * set in an attempt to meet the required exposure value.
>> + *
>> + * If the required exposure is not achievable by the first shutter value alone
>> + * it ramps gain up to the value from the first pair in the set. If the required
>> + * exposure is still not met it then allows shutter to ramp up to the shutter
>> + * value from the second pair in the set, and continues in this vein until
>> + * either the required exposure value is met, or else the hardware's shutter or
>> + * gain limits are reached.
>> + *
>> + * This method allows users to strike a balance between a well-exposed image and
>> + * an acceptable frame-rate, as opposed to simply maximising shutter followed by
>> + * gain. The same helpers can be used to perform the latter operation if needed
>> + * by passing an empty set of pairs to the initialisation function.
>> + *
>> + * The gain values may exceed a camera sensor's analogue gain limits if either
>> + * it or the IPA is also capable of digital gain. The configure() function must
>> + * be called with the hardware's limits to inform the helper of those
>> + * constraints. Any gain that is needed will be applied as analogue gain first
>> + * until the hardware's limit is reached, following which digital gain will be
>> + * used.
>> + */
>> +
>> +/**
>> + * \brief Construct an ExposureModeHelper instance
>> + */
>> +ExposureModeHelper::ExposureModeHelper()
>> +	: minShutter_(0), maxShutter_(0), minGain_(0), maxGain_(0)
>> +{
>> +}
>> +
>> +/**
>> + * \brief Initialize an ExposureModeHelper instance
>> + * \param[in] stages The vector of paired shutter time and gain limits
>> + *
>> + * This function is expected to be called a single time once the algorithm that
>> + * is using these helpers has built the necessary list of shutter and gain pairs
>> + * to use (archetypically by parsing them from a tuning file during the
> Is archetypically intentional ? Isn't "typically" enough ?


It's intentional yes; "archetypically" is like "in the first implementation" whereas "typically" is 
like "in most implementations"...but I can switch to typically so it's less confusing, either works.

>
>> + * algorithm's .init() call).
>> + *
>> + * The input steps are shutter and _total_ gain pairs; the gain encompasses both
>> + * analogue and digital gain.
>> + *
> Shutter -time- ?
>
> And yes, now gain is better explained.
>
>> + * The vector of stages may be empty. In that case, the helper will simply use
>> + * the runtime limits set through setShutterGainLimits() instead.
>> + */
>> +void ExposureModeHelper::init(const std::vector<std::pair<utils::Duration, double>> stages)
>> +{
>> +	/* We only need to check shutters_, as gains_ is filled alongside it */
>> +	if (!shutters_.empty()) {
>> +		LOG(ExposureModeHelper, Warning)
>> +			<< "Initialization attempted multiple times";
>> +		return;
> As we do this in a function instead of a constructor, we can return a
> value. Is this useful for users of this class ?


I don't think there's any need to return anything in this function if that's what you mean

>
>> +	}
>> +
>> +	for (auto stage : stages) {
> auto const &
>
>> +		shutters_.push_back(stage.first);
>> +		gains_.push_back(stage.second);
>> +	}
>> +}
>> +
>> +/**
>> + * \brief Set the shutter and gain limits
>> + * \param[in] minShutter The minimum shutter time supported
>> + * \param[in] maxShutter The maximum shutter time supported
>> + * \param[in] minGain The minimum analogue gain supported
>> + * \param[in] maxGain The maximum analogue gain supported
> Ah so this is only analogue ? but the steps are the total gain ?


Correct

>
>> + *
>> + * This function configures the shutter and analogue gain limits that need to be
>> + * adhered to as the helper divides up exposure. Note that these function *must*
>> + * be called whenever those limits change and before splitExposure() is used.
>> + *
>> + * If the algorithm using the helpers needs to indicate that either shutter,
>> + * analogue gain or both should be fixed it can do so by setting both the minima
>> + * and maxima to the same value.
> Are minima and maxima intentional ? Isn't this minimum and maximum ?


Minima/maxima is the plural, meaning to refer to both "minShutter" and "minGain" for example

>
>> + */
>> +void ExposureModeHelper::setShutterGainLimits(utils::Duration minShutter,
> Or just setLimits()
>
>> +					     utils::Duration maxShutter,
>> +					     double minGain,
>> +					     double maxGain)
>> +{
>> +	minShutter_ = minShutter;
>> +	maxShutter_ = maxShutter;
>> +	minGain_ = minGain;
>> +	maxGain_ = maxGain;
>> +}
>> +
>> +utils::Duration ExposureModeHelper::clampShutter(utils::Duration shutter) const
>> +{
>> +	return std::clamp(shutter, minShutter_, maxShutter_);
>> +}
>> +
>> +double ExposureModeHelper::clampGain(double gain) const
>> +{
>> +	return std::clamp(gain, minGain_, maxGain_);
>> +}
>> +
>> +/**
>> + * \brief Split exposure time into shutter and gain
>> + * \param[in] exposure Exposure time
>> + *
>> + * This function divides a given exposure value into shutter time, analogue and
> exposure 'value' or exposure 'time' as in the function's brief ?


It's Exposure time rather than value, if by Exposure value you mean: 
https://en.wikipedia.org/wiki/Exposure_value



>
>> + * digital gain by iterating through sets of shutter and gain limits. At each
> does this iterate 'limits' or the stages populated at init() time ?


The limits parsed from tuning data

>
>> + * stage the current stage's shutter limit is multiplied by the previous stage's
>> + * gain limit (or 1.0 initially) to see if the combination of the two can meet
>> + * the required exposure value. If they cannot then splitExpothe current stage's shutter
> Exposure definitely seems to be a value then (and this matches my
> understanding)


I just mean the value of the exposure parameter; the input is the exposure time needed to attain the 
target

>
> 'splitExposthe' ?
>
>
>> + * limit is multiplied by the same stage's gain limit to see if that combination
>> + * can meet the required exposure value. If they cannot then the function moves
>> + * to consider the next stage.
> I think something went wrong with formatting here
>
>> + *
>> + * When a combination of shutter and gain _stage_ limits are found that are
> why '_stage_' ? and why limits ?
>
> Can't it be simply:
>
>   * When a combination of shutter time and gain are found to be sufficient


I was trying to make it clear that it's working on the limits from the sets parsed from tuning data 
rather than the runtime limits set through setShutterGainLimits.

>
>> + * sufficient to meet the required exposure value, the function attempts to
>> + * reduce shutter time as much as possible whilst fixing gain and still meeting
>> + * the exposure value. If a _runtime_ limit prevents shutter time from being
> Again, why _runtime_ ?


And here trying to emphasis that it's the limits from setShutterGainLimits rather than those parsed 
from Yaml

>
>> + * lowered enough to meet the exposure value with gain fixed at the stage limit,
>> + * gain is also lowered to compensate.
>> + *
>> + * Once the shutter time and gain values are ascertained, gain is assigned as
>> + * analogue gain as much as possible, with digital gain only in use if the
>> + * maximum analogue gain runtime limit is unable to accomodate the exposure
>> + * value.
>> + *
>> + * If no combination of shutter and gain limits is found that meets the required
>> + * exposure value, the helper falls-back to simply maximising the shutter time
>> + * first, followed by analogue gain, followed by digital gain.
>> + *
>> + * @return Tuple of shutter time, analogue gain, and digital gain
>> + */
>> +std::tuple<utils::Duration, double, double>
>> +ExposureModeHelper::splitExposure(utils::Duration exposure) const
>> +{
>> +	ASSERT(maxShutter_);
>> +	ASSERT(maxGain_);
>> +
>> +	bool gainFixed = minGain_ == maxGain_;
>> +	bool shutterFixed = minShutter_ == maxShutter_;
>> +
>> +	/*
>> +	 * There's no point entering the loop if we cannot change either gain
>> +	 * nor shutter anyway.
>> +	 */
>> +	if (shutterFixed && gainFixed)
>> +		return { minShutter_, minGain_, exposure / (minShutter_ * minGain_) };
>> +
>> +	utils::Duration shutter;
>> +	double stageGain;
>> +	double gain;
>> +
>> +	for (unsigned int stage = 0; stage < gains_.size(); stage++) {
>> +		double lastStageGain = stage == 0 ? 1.0 : clampGain(gains_[stage - 1]);
>> +		utils::Duration stageShutter = clampShutter(shutters_[stage]);
>> +		stageGain = clampGain(gains_[stage]);
>> +
>> +		/*
>> +		 * We perform the clamping on both shutter and gain in case the
>> +		 * helper has had limits set that prevent those values being
>> +		 * lowered beyond a certain minimum...this can happen at runtime
>> +		 * for various reasons and so would not be known when the stage
>> +		 * limits are initialised.
>> +		 */
>> +
>> +		if (stageShutter * lastStageGain >= exposure) {
>> +			shutter = clampShutter(exposure / clampGain(lastStageGain));
>> +			gain = clampGain(exposure / shutter);
>> +
>> +			return { shutter, gain, exposure / (shutter * gain) };
>> +		}
>> +
>> +		if (stageShutter * stageGain >= exposure) {
>> +			shutter = clampShutter(exposure / clampGain(stageGain));
>> +			gain = clampGain(exposure / shutter);
>> +
>> +			return { shutter, gain, exposure / (shutter * gain) };
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * From here on all we can do is max out the shutter, followed by the
>> +	 * analogue gain. If we still haven't achieved the target we send the
>> +	 * rest of the exposure time to digital gain. If we were given no stages
>> +	 * to use then set stageGain to 1.0 so that shutter is maxed before gain
>> +	 * touched at all.
>> +	 */
>> +	if (gains_.empty())
>> +		stageGain = 1.0;
>> +
>> +	shutter = clampShutter(exposure / clampGain(stageGain));
>> +	gain = clampGain(exposure / shutter);
>> +
>> +	return { shutter, gain, exposure / (shutter * gain) };
>> +}
>> +
>> +/**
>> + * \fn ExposureModeHelper::minShutter()
>> + * \brief Retrieve the configured minimum shutter time limit set through
>> + *        setShutterGainLimits()
> No alignement in briefs
>
>> + * \return The minShutter_ value
>> + */
>> +
>> +/**
>> + * \fn ExposureModeHelper::maxShutter()
>> + * \brief Retrieve the configured maximum shutter time set through
>> + *        setShutterGainLimits()
> ditto
>
>> + * \return The maxShutter_ value
>> + */
>> +
>> +/**
>> + * \fn ExposureModeHelper::minGain()
>> + * \brief Retrieve the configured minimum gain set through
>> + *        setShutterGainLimits()
>> + * \return The minGain_ value
>> + */
>> +
>> +/**
>> + * \fn ExposureModeHelper::maxGain()
>> + * \brief Retrieve the configured maximum gain set through
>> + *        setShutterGainLimits()
>> + * \return The maxGain_ value
>> + */
>> +
>> +} /* namespace ipa */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h
>> new file mode 100644
>> index 00000000..1f672135
>> --- /dev/null
>> +++ b/src/ipa/libipa/exposure_mode_helper.h
>> @@ -0,0 +1,53 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
>> + *
>> + * exposure_mode_helper.h - Helper class that performs computations relating to exposure
>> + */
>> +
>> +#pragma once
>> +
>> +#include <tuple>
> Missing <pair>
>
>> +#include <vector>
>> +
>> +#include <libcamera/base/utils.h>
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa {
>> +
>> +class ExposureModeHelper
>> +{
>> +public:
>> +	ExposureModeHelper();
>> +	~ExposureModeHelper() = default;
>> +
>> +	void init(const std::vector<std::pair<utils::Duration, double>> stages);
>> +	void setShutterGainLimits(utils::Duration minShutter,
>> +				  utils::Duration maxShutter,
>> +				  double minGain, double maxGain);
>> +
>> +	std::tuple<utils::Duration, double, double>
>> +	splitExposure(utils::Duration exposure) const;
>> +
>> +	utils::Duration minShutter() const { return minShutter_; }
>> +	utils::Duration maxShutter() const { return maxShutter_; }
>> +	double minGain() const { return minGain_; }
>> +	double maxGain() const { return maxGain_; }
>> +
>> +private:
>> +	utils::Duration clampShutter(utils::Duration shutter) const;
>> +	double clampGain(double gain) const;
>> +
>> +	std::vector<utils::Duration> shutters_;
>> +	std::vector<double> gains_;
>> +
>> +	utils::Duration minShutter_;
>> +	utils::Duration maxShutter_;
>> +	double minGain_;
>> +	double maxGain_;
>> +};
>> +
>> +} /* namespace ipa */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
>> index 016b8e0e..37fbd177 100644
>> --- a/src/ipa/libipa/meson.build
>> +++ b/src/ipa/libipa/meson.build
>> @@ -3,6 +3,7 @@
>>   libipa_headers = files([
>>       'algorithm.h',
>>       'camera_sensor_helper.h',
>> +    'exposure_mode_helper.h',
>>       'fc_queue.h',
>>       'histogram.h',
>>       'module.h',
>> @@ -11,6 +12,7 @@ libipa_headers = files([
>>   libipa_sources = files([
>>       'algorithm.cpp',
>>       'camera_sensor_helper.cpp',
>> +    'exposure_mode_helper.cpp',
>>       'fc_queue.cpp',
>>       'histogram.cpp',
>>       'module.cpp',
>> --
>> 2.34.1
>>