[v5,24/24] ipa: libipa: agc: Take exposure margin into account
diff mbox series

Message ID 20261029125844.170324-25-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • ipa: libipa: agc: Take exposure margin into account
Related show

Commit Message

Barnabás Pőcze Oct. 29, 2026, 12:58 p.m. UTC 
The sensor exposure time depends on the vertical blanking amount and the
exposure margin. Instead of relying on the maximum exposure available at
onfiguration, use the frame duration limits (~ vblank) and exposure
margin to dynamically calculate the max available exposure time.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/libipa/agc.cpp | 96 +++++++++++++++++++++++++++++-------------
 src/ipa/libipa/agc.h   |  3 +-
 2 files changed, 69 insertions(+), 30 deletions(-)

Comments

Jacopo Mondi Oct. 29, 2026, 4:07 p.m. UTC | #1 
Hi Barnabás

On Thu, Oct 29, 2026 at 01:58:44PM +0100, Barnabás Pőcze wrote:
> The sensor exposure time depends on the vertical blanking amount and the
> exposure margin. Instead of relying on the maximum exposure available at
> onfiguration, use the frame duration limits (~ vblank) and exposure
> margin to dynamically calculate the max available exposure time.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/ipa/libipa/agc.cpp | 96 +++++++++++++++++++++++++++++-------------
>  src/ipa/libipa/agc.h   |  3 +-
>  2 files changed, 69 insertions(+), 30 deletions(-)
>
> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> index 60e8614777..cc88c99f14 100644
> --- a/src/ipa/libipa/agc.cpp
> +++ b/src/ipa/libipa/agc.cpp
> @@ -72,12 +72,12 @@ namespace agc {
>   * \struct Session
>   * \brief Session configuration for AgcAlgorithm
>   *
> + * \var Session::minExposure
> + * \brief Minimum exposure (in lines) for the streaming session
> + *
>   * \var Session::minExposureTime
>   * \brief Minimum exposure time for the streaming session
>   *
> - * \var Session::maxExposureTime
> - * \brief Maximum exposure time for the streaming session
> - *
>   * \var Session::minAnalogueGain
>   * \brief Minimum analogue gain for the streaming session
>   *
> @@ -102,6 +102,11 @@ namespace agc {
>   * \var Session::sensor.outputSize
>   * \brief Configured output size of the sensor
>   *
> + * \var Session::sensor.exposureMargin
> + * \brief Exposure margin of the sensor
> + *
> + * \sa CameraSensorHelper::exposureMargin()
> + *
>   * \var Session::autoAllowed
>   * \copybrief AgcAlgorithm::ConfigurationParams::autoAllowed
>   * \sa AgcAlgorithm::ConfigurationParams::autoAllowed
> @@ -419,6 +424,16 @@ void addFallbacks(AgcMeanLuminance::Configuration &config)
>  			});
>  }
>
> +[[nodiscard]]
> +uint32_t clampExposure(const agc::Session &session, const agc::ActiveState &state,
> +		       uint32_t exposure)
> +{
> +	const uint32_t maxFrame = state.maxFrameDuration / session.lineDuration;
> +	const uint32_t maxExposure = maxFrame - session.sensor.exposureMargin;
> +
> +	return std::clamp(exposure, session.minExposure, maxExposure);
> +}
> +
>  } /* namespace */
>
>  /**
> @@ -616,7 +631,12 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>  	session.lineDuration = lineLength * 1.0s / config.sensorInfo.pixelRate;
>  	session.sensor.outputSize = config.sensorInfo.outputSize;
>
> -	const double lineDurationUs = session.lineDuration.get<std::micro>();
> +	auto exposureMargin = sensor_ ? sensor_->exposureMargin() : std::nullopt;
> +	session.sensor.exposureMargin = exposureMargin.value_or(4);
> +	if (!exposureMargin)
> +		LOG(Agc, Warning)
> +			<< "Sensor exposure margin not available, using "
> +			<< session.sensor.exposureMargin;
>
>  	/*
>  	 * Compute exposure time limits from the V4L2_CID_EXPOSURE control
> @@ -625,7 +645,6 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>
>  	const ControlInfo &v4l2Exposure = config.sensorControls.find(V4L2_CID_EXPOSURE)->second;
>  	int32_t minExposure = v4l2Exposure.min().get<int32_t>();
> -	int32_t maxExposure = v4l2Exposure.max().get<int32_t>();
>  	int32_t defExposure = v4l2Exposure.def().get<int32_t>();
>
>  	/* Compute the analogue gain limits. */
> @@ -638,28 +657,46 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>  	float maxGain = extractGain(v4l2Gain.max());
>  	float defGain = extractGain(v4l2Gain.def());
>
> -	LOG(Agc, Debug)
> -		<< "exposure: [" << minExposure << ',' << maxExposure << "], "
> -		<< "gain: [" << minGain << ',' << maxGain << "], "
> -		<< "line-duration: " << session.lineDuration << ", "
> -		<< "sensor-output: " << session.sensor.outputSize;
> -
> -
>  	/*
>  	 * When the AGC computes the new exposure values for a frame, it needs
>  	 * to know the limits for exposure time and analogue gain. As it depends
>  	 * on the sensor, update it with the controls.
> -	 *
> -	 * \todo take VBLANK into account for maximum exposure time
>  	 */
> +	session.minExposure = minExposure;
>  	session.minExposureTime = minExposure * session.lineDuration;
> -	session.maxExposureTime = maxExposure * session.lineDuration;
>  	session.minAnalogueGain = minGain;
>  	session.maxAnalogueGain = maxGain;
>  	session.defAnalogueGain = defGain;
>  	session.minFrameDuration = config.sensorInfo.minFrameLength * session.lineDuration;
>  	session.maxFrameDuration = config.sensorInfo.maxFrameLength * session.lineDuration;
>
> +	/*
> +	 * The assumption is that the minimum exposure time is independent of vblank,
> +	 * only the maximum exposure time is affected by it.
> +	 */
> +	const uint32_t maxExposure = config.sensorInfo.maxFrameLength - session.sensor.exposureMargin;
> +	const utils::Duration maxExposureTime = maxExposure * session.lineDuration;
> +
> +	LOG(Agc, Debug)
> +		<< "exposure: [" << session.minExposure << ',' << maxExposure << "], "
> +		<< "exposure-time: [" << session.minExposureTime << ',' << maxExposureTime << "], "
> +		<< "gain: [" << session.minAnalogueGain << ',' << session.maxAnalogueGain << "], "
> +		<< "line-length: " << lineLength << ", "
> +		<< "line-duration: " << session.lineDuration << ", "
> +		<< "frame-height: [" << config.sensorInfo.minFrameLength << ',' << config.sensorInfo.maxFrameLength << "], "
> +		<< "sensor-output: " << session.sensor.outputSize << ", "
> +		<< "sensor-exposure-margin: " << session.sensor.exposureMargin;
> +
> +	/*
> +	 * This is a sanity check. If this fails, something is very likely off.
> +	 * Equality could theoretically be allowed, but it is not so that
> +	 * `(frameDuration / lineDuration) - exposureMargin` is no less
> +	 * than the minimum exposure time even if the division is slightly
> +	 * inaccurate.
> +	 */
> +	if (session.minExposure + session.sensor.exposureMargin >= config.sensorInfo.minFrameLength)
> +		return -EINVAL;
> +

Unlikely, but better check ?

>  	/* Configure the default exposure and gain. */
>  	state = {};
>  	state.automatic.gain = session.minAnalogueGain;
> @@ -755,9 +792,10 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>  		minGain, maxGain, defGain
>  	};
>  	config.ctrlMap[&controls::ExposureTime] = ControlInfo{
> -		static_cast<int32_t>(minExposure * lineDurationUs),
> -		static_cast<int32_t>(maxExposure * lineDurationUs),
> -		static_cast<int32_t>(defExposure * lineDurationUs),
> +		static_cast<int32_t>(session.minExposureTime.get<std::micro>()),
> +		static_cast<int32_t>(maxExposureTime.get<std::micro>()),
> +		static_cast<int32_t>(state.automatic.exposure *
> +				     session.lineDuration.get<std::micro>()),
>  	};
>  	config.ctrlMap[&controls::FrameDurationLimits] = ControlInfo{
>  		static_cast<int64_t>(session.minFrameDuration.get<std::micro>()),
> @@ -880,7 +918,7 @@ void AgcAlgorithm::queueRequest(const agc::Session &session, agc::ActiveState &s
>  	frameContext.autoGainEnabled = state.autoGainEnabled;
>
>  	if (!frameContext.autoExposureEnabled)
> -		frameContext.exposure = state.manual.exposure;
> +		frameContext.exposure = clampExposure(session, state, state.manual.exposure);
>  	if (!frameContext.autoGainEnabled)
>  		frameContext.gain = state.manual.gain;
>
> @@ -995,18 +1033,16 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>  	 * Set the AGC limits using the fixed exposure time and/or gain in
>  	 * manual mode, or the sensor limits in auto mode.
>  	 */
> -	utils::Duration minExposureTime;
> -	utils::Duration maxExposureTime;
> +	utils::Duration minExposureTime = session.minExposureTime;
> +	utils::Duration maxExposureTime = state.maxFrameDuration -
> +					  session.sensor.exposureMargin * lineDuration;
>  	double minAnalogueGain;
>  	double maxAnalogueGain;
>
> -	if (state.autoExposureEnabled) {
> -		minExposureTime = session.minExposureTime;
> -		maxExposureTime = std::clamp(state.maxFrameDuration,
> -					     session.minExposureTime,
> -					     session.maxExposureTime);
> -	} else {
> -		minExposureTime = lineDuration * state.manual.exposure;
> +	if (!state.autoExposureEnabled) {
> +		minExposureTime = std::clamp<utils::Duration>(
> +			state.manual.exposure * lineDuration,
> +			minExposureTime, maxExposureTime);

I think this is a discussion about policy, but I interpret this as we
always limt exposure to the FrameDurationLimits value. I think that's
what we want ?

>  		maxExposureTime = minExposureTime;
>  	}
>
> @@ -1076,6 +1112,8 @@ void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
>  		},
>  	}, impl_);
>
> +	state.automatic.exposure = clampExposure(session, state, state.automatic.exposure);
> +
>  	const utils::Duration newExposureTime = state.automatic.exposure * lineDuration;
>
>  	LOG(Agc, Debug)
> @@ -1106,7 +1144,7 @@ void AgcAlgorithm::processFrameDuration(const agc::Session &session,
>  	const auto frameLength = std::max<uint32_t>(
>  		std::min(std::ceil(frameContext.minFrameDuration / lineDuration),
>  			 frameContext.maxFrameDuration / lineDuration),
> -		frameContext.exposure);
> +		frameContext.exposure + session.sensor.exposureMargin);
>
>  	frameContext.vblank = frameLength - session.sensor.outputSize.height;
>
> diff --git a/src/ipa/libipa/agc.h b/src/ipa/libipa/agc.h
> index d8057720d0..9b4bcf0bed 100644
> --- a/src/ipa/libipa/agc.h
> +++ b/src/ipa/libipa/agc.h
> @@ -36,8 +36,8 @@ class Histogram;
>  namespace agc {
>
>  struct Session {
> +	uint32_t minExposure;
>  	utils::Duration minExposureTime;
> -	utils::Duration maxExposureTime;
>  	double minAnalogueGain;
>  	double maxAnalogueGain;
>  	double defAnalogueGain;
> @@ -47,6 +47,7 @@ struct Session {
>
>  	struct {
>  		Size outputSize;
> +		uint32_t exposureMargin;

Looks good to me
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>  	} sensor;
>
>  	bool autoAllowed;
> --
> 2.56.0
>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
index 60e8614777..cc88c99f14 100644
--- a/src/ipa/libipa/agc.cpp
+++ b/src/ipa/libipa/agc.cpp
@@ -72,12 +72,12 @@  namespace agc {
  * \struct Session
  * \brief Session configuration for AgcAlgorithm
  *
+ * \var Session::minExposure
+ * \brief Minimum exposure (in lines) for the streaming session
+ *
  * \var Session::minExposureTime
  * \brief Minimum exposure time for the streaming session
  *
- * \var Session::maxExposureTime
- * \brief Maximum exposure time for the streaming session
- *
  * \var Session::minAnalogueGain
  * \brief Minimum analogue gain for the streaming session
  *
@@ -102,6 +102,11 @@  namespace agc {
  * \var Session::sensor.outputSize
  * \brief Configured output size of the sensor
  *
+ * \var Session::sensor.exposureMargin
+ * \brief Exposure margin of the sensor
+ *
+ * \sa CameraSensorHelper::exposureMargin()
+ *
  * \var Session::autoAllowed
  * \copybrief AgcAlgorithm::ConfigurationParams::autoAllowed
  * \sa AgcAlgorithm::ConfigurationParams::autoAllowed
@@ -419,6 +424,16 @@  void addFallbacks(AgcMeanLuminance::Configuration &config)
 			});
 }
 
+[[nodiscard]]
+uint32_t clampExposure(const agc::Session &session, const agc::ActiveState &state,
+		       uint32_t exposure)
+{
+	const uint32_t maxFrame = state.maxFrameDuration / session.lineDuration;
+	const uint32_t maxExposure = maxFrame - session.sensor.exposureMargin;
+
+	return std::clamp(exposure, session.minExposure, maxExposure);
+}
+
 } /* namespace */
 
 /**
@@ -616,7 +631,12 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 	session.lineDuration = lineLength * 1.0s / config.sensorInfo.pixelRate;
 	session.sensor.outputSize = config.sensorInfo.outputSize;
 
-	const double lineDurationUs = session.lineDuration.get<std::micro>();
+	auto exposureMargin = sensor_ ? sensor_->exposureMargin() : std::nullopt;
+	session.sensor.exposureMargin = exposureMargin.value_or(4);
+	if (!exposureMargin)
+		LOG(Agc, Warning)
+			<< "Sensor exposure margin not available, using "
+			<< session.sensor.exposureMargin;
 
 	/*
 	 * Compute exposure time limits from the V4L2_CID_EXPOSURE control
@@ -625,7 +645,6 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 
 	const ControlInfo &v4l2Exposure = config.sensorControls.find(V4L2_CID_EXPOSURE)->second;
 	int32_t minExposure = v4l2Exposure.min().get<int32_t>();
-	int32_t maxExposure = v4l2Exposure.max().get<int32_t>();
 	int32_t defExposure = v4l2Exposure.def().get<int32_t>();
 
 	/* Compute the analogue gain limits. */
@@ -638,28 +657,46 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 	float maxGain = extractGain(v4l2Gain.max());
 	float defGain = extractGain(v4l2Gain.def());
 
-	LOG(Agc, Debug)
-		<< "exposure: [" << minExposure << ',' << maxExposure << "], "
-		<< "gain: [" << minGain << ',' << maxGain << "], "
-		<< "line-duration: " << session.lineDuration << ", "
-		<< "sensor-output: " << session.sensor.outputSize;
-
-
 	/*
 	 * When the AGC computes the new exposure values for a frame, it needs
 	 * to know the limits for exposure time and analogue gain. As it depends
 	 * on the sensor, update it with the controls.
-	 *
-	 * \todo take VBLANK into account for maximum exposure time
 	 */
+	session.minExposure = minExposure;
 	session.minExposureTime = minExposure * session.lineDuration;
-	session.maxExposureTime = maxExposure * session.lineDuration;
 	session.minAnalogueGain = minGain;
 	session.maxAnalogueGain = maxGain;
 	session.defAnalogueGain = defGain;
 	session.minFrameDuration = config.sensorInfo.minFrameLength * session.lineDuration;
 	session.maxFrameDuration = config.sensorInfo.maxFrameLength * session.lineDuration;
 
+	/*
+	 * The assumption is that the minimum exposure time is independent of vblank,
+	 * only the maximum exposure time is affected by it.
+	 */
+	const uint32_t maxExposure = config.sensorInfo.maxFrameLength - session.sensor.exposureMargin;
+	const utils::Duration maxExposureTime = maxExposure * session.lineDuration;
+
+	LOG(Agc, Debug)
+		<< "exposure: [" << session.minExposure << ',' << maxExposure << "], "
+		<< "exposure-time: [" << session.minExposureTime << ',' << maxExposureTime << "], "
+		<< "gain: [" << session.minAnalogueGain << ',' << session.maxAnalogueGain << "], "
+		<< "line-length: " << lineLength << ", "
+		<< "line-duration: " << session.lineDuration << ", "
+		<< "frame-height: [" << config.sensorInfo.minFrameLength << ',' << config.sensorInfo.maxFrameLength << "], "
+		<< "sensor-output: " << session.sensor.outputSize << ", "
+		<< "sensor-exposure-margin: " << session.sensor.exposureMargin;
+
+	/*
+	 * This is a sanity check. If this fails, something is very likely off.
+	 * Equality could theoretically be allowed, but it is not so that
+	 * `(frameDuration / lineDuration) - exposureMargin` is no less
+	 * than the minimum exposure time even if the division is slightly
+	 * inaccurate.
+	 */
+	if (session.minExposure + session.sensor.exposureMargin >= config.sensorInfo.minFrameLength)
+		return -EINVAL;
+
 	/* Configure the default exposure and gain. */
 	state = {};
 	state.automatic.gain = session.minAnalogueGain;
@@ -755,9 +792,10 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 		minGain, maxGain, defGain
 	};
 	config.ctrlMap[&controls::ExposureTime] = ControlInfo{
-		static_cast<int32_t>(minExposure * lineDurationUs),
-		static_cast<int32_t>(maxExposure * lineDurationUs),
-		static_cast<int32_t>(defExposure * lineDurationUs),
+		static_cast<int32_t>(session.minExposureTime.get<std::micro>()),
+		static_cast<int32_t>(maxExposureTime.get<std::micro>()),
+		static_cast<int32_t>(state.automatic.exposure *
+				     session.lineDuration.get<std::micro>()),
 	};
 	config.ctrlMap[&controls::FrameDurationLimits] = ControlInfo{
 		static_cast<int64_t>(session.minFrameDuration.get<std::micro>()),
@@ -880,7 +918,7 @@  void AgcAlgorithm::queueRequest(const agc::Session &session, agc::ActiveState &s
 	frameContext.autoGainEnabled = state.autoGainEnabled;
 
 	if (!frameContext.autoExposureEnabled)
-		frameContext.exposure = state.manual.exposure;
+		frameContext.exposure = clampExposure(session, state, state.manual.exposure);
 	if (!frameContext.autoGainEnabled)
 		frameContext.gain = state.manual.gain;
 
@@ -995,18 +1033,16 @@  void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
 	 * Set the AGC limits using the fixed exposure time and/or gain in
 	 * manual mode, or the sensor limits in auto mode.
 	 */
-	utils::Duration minExposureTime;
-	utils::Duration maxExposureTime;
+	utils::Duration minExposureTime = session.minExposureTime;
+	utils::Duration maxExposureTime = state.maxFrameDuration -
+					  session.sensor.exposureMargin * lineDuration;
 	double minAnalogueGain;
 	double maxAnalogueGain;
 
-	if (state.autoExposureEnabled) {
-		minExposureTime = session.minExposureTime;
-		maxExposureTime = std::clamp(state.maxFrameDuration,
-					     session.minExposureTime,
-					     session.maxExposureTime);
-	} else {
-		minExposureTime = lineDuration * state.manual.exposure;
+	if (!state.autoExposureEnabled) {
+		minExposureTime = std::clamp<utils::Duration>(
+			state.manual.exposure * lineDuration,
+			minExposureTime, maxExposureTime);
 		maxExposureTime = minExposureTime;
 	}
 
@@ -1076,6 +1112,8 @@  void AgcAlgorithm::process(const agc::Session &session, agc::ActiveState &state,
 		},
 	}, impl_);
 
+	state.automatic.exposure = clampExposure(session, state, state.automatic.exposure);
+
 	const utils::Duration newExposureTime = state.automatic.exposure * lineDuration;
 
 	LOG(Agc, Debug)
@@ -1106,7 +1144,7 @@  void AgcAlgorithm::processFrameDuration(const agc::Session &session,
 	const auto frameLength = std::max<uint32_t>(
 		std::min(std::ceil(frameContext.minFrameDuration / lineDuration),
 			 frameContext.maxFrameDuration / lineDuration),
-		frameContext.exposure);
+		frameContext.exposure + session.sensor.exposureMargin);
 
 	frameContext.vblank = frameLength - session.sensor.outputSize.height;
 
diff --git a/src/ipa/libipa/agc.h b/src/ipa/libipa/agc.h
index d8057720d0..9b4bcf0bed 100644
--- a/src/ipa/libipa/agc.h
+++ b/src/ipa/libipa/agc.h
@@ -36,8 +36,8 @@  class Histogram;
 namespace agc {
 
 struct Session {
+	uint32_t minExposure;
 	utils::Duration minExposureTime;
-	utils::Duration maxExposureTime;
 	double minAnalogueGain;
 	double maxAnalogueGain;
 	double defAnalogueGain;
@@ -47,6 +47,7 @@  struct Session {
 
 	struct {
 		Size outputSize;
+		uint32_t exposureMargin;
 	} sensor;
 
 	bool autoAllowed;