[libcamera-devel,v4,3/4] ipa: ipu3: agc: Introduce lineDuration in IPASessionConfiguration
diff mbox series

Message ID 20220224151113.109858-4-jeanmichel.hautbois@ideasonboard.com
State Accepted
Headers show
Series
  • ipa: ipu3: Misc clean up
Related show

Commit Message

Jean-Michel Hautbois Feb. 24, 2022, 3:11 p.m. UTC 
Instead of having a local cached value for line duration, store it in
the IPASessionConfiguration::sensor structure.
While at it, configure the default analogue gain and shutter speed to
controlled fixed values.

The latter is set to be 10ms as it will in most cases be close to the
one needed, making the AGC faster to converge.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
v4: clean only frameContext at configure call.
    [Kieran, I took your R-b if you are ok ?]
---
 src/ipa/ipu3/algorithms/agc.cpp | 26 ++++++++++++++------------
 src/ipa/ipu3/algorithms/agc.h   |  3 +--
 src/ipa/ipu3/ipa_context.cpp    |  8 ++++++++
 src/ipa/ipu3/ipa_context.h      |  4 ++++
 src/ipa/ipu3/ipu3.cpp           | 28 +++++++++++++++-------------
 5 files changed, 42 insertions(+), 27 deletions(-)

Comments

Kieran Bingham Feb. 28, 2022, 2:30 p.m. UTC | #1 
Quoting Jean-Michel Hautbois (2022-02-24 15:11:12)
> Instead of having a local cached value for line duration, store it in
> the IPASessionConfiguration::sensor structure.
> While at it, configure the default analogue gain and shutter speed to
> controlled fixed values.
> 
> The latter is set to be 10ms as it will in most cases be close to the
> one needed, making the AGC faster to converge.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> ---
> v4: clean only frameContext at configure call.
>     [Kieran, I took your R-b if you are ok ?]

Fine with me if I already gave it ;-)

I'll skim down through this to be sure anyway.

I have a worry about the context_ = {}; to clean / initialise it now
that there is a utils::Duration in it.

I think it can possibly be fine, but we probably need to validate it.


> ---
>  src/ipa/ipu3/algorithms/agc.cpp | 26 ++++++++++++++------------
>  src/ipa/ipu3/algorithms/agc.h   |  3 +--
>  src/ipa/ipu3/ipa_context.cpp    |  8 ++++++++
>  src/ipa/ipu3/ipa_context.h      |  4 ++++
>  src/ipa/ipu3/ipu3.cpp           | 28 +++++++++++++++-------------
>  5 files changed, 42 insertions(+), 27 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index a929085c..1eb1bcef 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10;
>  static constexpr double kRelativeLuminanceTarget = 0.16;
>  
>  Agc::Agc()
> -       : frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s),
> +       : frameCount_(0), minShutterSpeed_(0s),
>           maxShutterSpeed_(0s), filteredExposure_(0s)
>  {
>  }
> @@ -83,13 +83,13 @@ Agc::Agc()
>   *
>   * \return 0
>   */
> -int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> +int Agc::configure(IPAContext &context,
> +                  [[maybe_unused]] const IPAConfigInfo &configInfo)
>  {
> -       stride_ = context.configuration.grid.stride;
> +       IPASessionConfiguration &configuration = context.configuration;
> +       IPAFrameContext &frameContext = context.frameContext;
>  
> -       /* \todo use the IPAContext to provide the limits */
> -       lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
> -                     / configInfo.sensorInfo.pixelRate;
> +       stride_ = configuration.grid.stride;
>  
>         minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
>         maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
> @@ -99,8 +99,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>         maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>  
>         /* Configure the default exposure and gain. */
> -       context.frameContext.agc.gain = minAnalogueGain_;
> -       context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
> +       frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);
> +       frameContext.agc.exposure = 10ms / configuration.sensor.lineDuration;
>  
>         return 0;
>  }
> @@ -182,9 +182,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
>   * \param[in] yGain The gain calculated based on the relative luminance target
>   * \param[in] iqMeanGain The gain calculated based on the relative luminance target
>   */
> -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
> +void Agc::computeExposure(IPAContext &context, double yGain,
>                           double iqMeanGain)
>  {
> +       const IPASessionConfiguration &configuration = context.configuration;
> +       IPAFrameContext &frameContext = context.frameContext;
>         /* Get the effective exposure and gain applied on the sensor. */
>         uint32_t exposure = frameContext.sensor.exposure;
>         double analogueGain = frameContext.sensor.gain;
> @@ -200,7 +202,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>         /* extracted from Rpi::Agc::computeTargetExposure */
>  
>         /* Calculate the shutter time in seconds */
> -       utils::Duration currentShutter = exposure * lineDuration_;
> +       utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
>  
>         /*
>          * Update the exposure value for the next computation using the values
> @@ -247,7 +249,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>                             << stepGain;
>  
>         /* Update the estimated exposure and gain. */
> -       frameContext.agc.exposure = shutterTime / lineDuration_;
> +       frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;
>         frameContext.agc.gain = stepGain;
>  }
>  
> @@ -354,7 +356,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>                         break;
>         }
>  
> -       computeExposure(context.frameContext, yGain, iqMeanGain);
> +       computeExposure(context, yGain, iqMeanGain);
>         frameCount_++;
>  }
>  
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 84bfe045..ad705605 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -34,7 +34,7 @@ private:
>         double measureBrightness(const ipu3_uapi_stats_3a *stats,
>                                  const ipu3_uapi_grid_config &grid) const;
>         utils::Duration filterExposure(utils::Duration currentExposure);
> -       void computeExposure(IPAFrameContext &frameContext, double yGain,
> +       void computeExposure(IPAContext &context, double yGain,
>                              double iqMeanGain);
>         double estimateLuminance(IPAFrameContext &frameContext,
>                                  const ipu3_uapi_grid_config &grid,
> @@ -43,7 +43,6 @@ private:
>  
>         uint64_t frameCount_;
>  
> -       utils::Duration lineDuration_;
>         utils::Duration minShutterSpeed_;
>         utils::Duration maxShutterSpeed_;
>  
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 86794ac1..9c4ec936 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -86,6 +86,14 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Maximum analogue gain supported with the configured sensor
>   */
>  
> +/**
> + * \var IPASessionConfiguration::sensor
> + * \brief Sensor-specific configuration of the IPA
> + *
> + * \var IPASessionConfiguration::sensor.lineDuration
> + * \brief Line duration in microseconds
> + */
> +
>  /**
>   * \var IPAFrameContext::agc
>   * \brief Context for the Automatic Gain Control algorithm
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index c6dc0814..e7c49828 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -31,6 +31,10 @@ struct IPASessionConfiguration {
>                 double minAnalogueGain;
>                 double maxAnalogueGain;
>         } agc;
> +
> +       struct {
> +               utils::Duration lineDuration;
> +       } sensor;
>  };
>  
>  struct IPAFrameContext {
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index c0c29416..17324616 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -173,8 +173,6 @@ private:
>         uint32_t minGain_;
>         uint32_t maxGain_;
>  
> -       utils::Duration lineDuration_;
> -
>         /* Interface to the Camera Helper */
>         std::unique_ptr<CameraSensorHelper> camHelper_;
>  
> @@ -206,8 +204,8 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
>          *
>          * \todo take VBLANK into account for maximum shutter speed
>          */
> -       context_.configuration.agc.minShutterSpeed = minExposure * lineDuration_;
> -       context_.configuration.agc.maxShutterSpeed = maxExposure * lineDuration_;
> +       context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
> +       context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;

Some long lines, but those might only be easier to shorten if we had a
local reference to the configuration structure or sensor configuration?

>         context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
>         context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
>  }
> @@ -229,6 +227,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>                              ControlInfoMap *ipaControls)
>  {
>         ControlInfoMap::Map controls{};
> +       double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
>  
>         /*
>          * Compute exposure time limits by using line length and pixel rate
> @@ -237,9 +236,9 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>          * microseconds.
>          */
>         const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
> -       int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration_.get<std::micro>();
> -       int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration_.get<std::micro>();
> -       int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration_.get<std::micro>();
> +       int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;
> +       int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;
> +       int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;
>         controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,
>                                                         defExposure);
>  
> @@ -293,6 +292,10 @@ int IPAIPU3::init(const IPASettings &settings,
>                 return -ENODEV;
>         }
>  
> +       /* Clean context */
> +       context_ = {};

Context_ now contains a utils::Duration which is a std::chrono::duration
type.

What happens to that /object/ when this context is cleaned? Does it lose
any state? Or does it correctly get initialised successfully?

We might want to validate that this is safe C++ ... I'm quite fearful
that it might not be.



> +       context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;
> +
>         /* Construct our Algorithms */
>         algorithms_.push_back(std::make_unique<algorithms::Agc>());
>         algorithms_.push_back(std::make_unique<algorithms::Awb>());
> @@ -454,12 +457,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>  
>         defVBlank_ = itVBlank->second.def().get<int32_t>();
>  
> -       /* Clean context at configuration */
> -       context_ = {};
> -
>         calculateBdsGrid(configInfo.bdsOutputSize);
>  
> -       lineDuration_ = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;
> +       /* Clean frameContext at each reconfiguration. */
> +       context_.frameContext = {};

Yes, I think this is likely required for now, but should go when we get
a per-frame frameContext.

>         /* Update the camera controls using the new sensor settings. */
>         updateControls(sensorInfo_, ctrls_, ipaControls);
> @@ -620,6 +621,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>                               [[maybe_unused]] int64_t frameTimestamp,
>                               const ipu3_uapi_stats_3a *stats)
>  {
> +       double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
>         ControlList ctrls(controls::controls);
>  
>         for (auto const &algo : algorithms_)
> @@ -628,14 +630,14 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>         setControls(frame);
>  
>         /* \todo Use VBlank value calculated from each frame exposure. */
> -       int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration_.get<std::micro>();
> +       int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration;
>         ctrls.set(controls::FrameDuration, frameDuration);
>  
>         ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);
>  
>         ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
>  
> -       ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration_.get<std::micro>());
> +       ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);
>  
>         /*
>          * \todo The Metadata provides a path to getting extended data
> -- 
> 2.32.0
>
Umang Jain Feb. 28, 2022, 4:54 p.m. UTC | #2 
Hi JM,


On 2/24/22 20:41, Jean-Michel Hautbois wrote:
> Instead of having a local cached value for line duration, store it in
> the IPASessionConfiguration::sensor structure.
> While at it, configure the default analogue gain and shutter speed to
> controlled fixed values.
>
> The latter is set to be 10ms as it will in most cases be close to the
> one needed, making the AGC faster to converge.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> ---
> v4: clean only frameContext at configure call.
>      [Kieran, I took your R-b if you are ok ?]
> ---
>   src/ipa/ipu3/algorithms/agc.cpp | 26 ++++++++++++++------------
>   src/ipa/ipu3/algorithms/agc.h   |  3 +--
>   src/ipa/ipu3/ipa_context.cpp    |  8 ++++++++
>   src/ipa/ipu3/ipa_context.h      |  4 ++++
>   src/ipa/ipu3/ipu3.cpp           | 28 +++++++++++++++-------------
>   5 files changed, 42 insertions(+), 27 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index a929085c..1eb1bcef 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10;
>   static constexpr double kRelativeLuminanceTarget = 0.16;
>   
>   Agc::Agc()
> -	: frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s),
> +	: frameCount_(0), minShutterSpeed_(0s),
>   	  maxShutterSpeed_(0s), filteredExposure_(0s)
>   {
>   }
> @@ -83,13 +83,13 @@ Agc::Agc()
>    *
>    * \return 0
>    */
> -int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
> +int Agc::configure(IPAContext &context,
> +		   [[maybe_unused]] const IPAConfigInfo &configInfo)
>   {
> -	stride_ = context.configuration.grid.stride;
> +	IPASessionConfiguration &configuration = context.configuration;
> +	IPAFrameContext &frameContext = context.frameContext;
>   
> -	/* \todo use the IPAContext to provide the limits */
> -	lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
> -		      / configInfo.sensorInfo.pixelRate;
> +	stride_ = configuration.grid.stride;
>   
>   	minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
>   	maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
> @@ -99,8 +99,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>   	maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>   
>   	/* Configure the default exposure and gain. */
> -	context.frameContext.agc.gain = minAnalogueGain_;
> -	context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
> +	frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);
> +	frameContext.agc.exposure = 10ms / configuration.sensor.lineDuration;


Can 10ms be assigned to something like kminShutterSpeed_ or something?

I am satisfied with the changes so,

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

>   
>   	return 0;
>   }
> @@ -182,9 +182,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
>    * \param[in] yGain The gain calculated based on the relative luminance target
>    * \param[in] iqMeanGain The gain calculated based on the relative luminance target
>    */
> -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
> +void Agc::computeExposure(IPAContext &context, double yGain,
>   			  double iqMeanGain)
>   {
> +	const IPASessionConfiguration &configuration = context.configuration;
> +	IPAFrameContext &frameContext = context.frameContext;
>   	/* Get the effective exposure and gain applied on the sensor. */
>   	uint32_t exposure = frameContext.sensor.exposure;
>   	double analogueGain = frameContext.sensor.gain;
> @@ -200,7 +202,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>   	/* extracted from Rpi::Agc::computeTargetExposure */
>   
>   	/* Calculate the shutter time in seconds */
> -	utils::Duration currentShutter = exposure * lineDuration_;
> +	utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
>   
>   	/*
>   	 * Update the exposure value for the next computation using the values
> @@ -247,7 +249,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>   			    << stepGain;
>   
>   	/* Update the estimated exposure and gain. */
> -	frameContext.agc.exposure = shutterTime / lineDuration_;
> +	frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;
>   	frameContext.agc.gain = stepGain;
>   }
>   
> @@ -354,7 +356,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>   			break;
>   	}
>   
> -	computeExposure(context.frameContext, yGain, iqMeanGain);
> +	computeExposure(context, yGain, iqMeanGain);
>   	frameCount_++;
>   }
>   
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 84bfe045..ad705605 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -34,7 +34,7 @@ private:
>   	double measureBrightness(const ipu3_uapi_stats_3a *stats,
>   				 const ipu3_uapi_grid_config &grid) const;
>   	utils::Duration filterExposure(utils::Duration currentExposure);
> -	void computeExposure(IPAFrameContext &frameContext, double yGain,
> +	void computeExposure(IPAContext &context, double yGain,
>   			     double iqMeanGain);
>   	double estimateLuminance(IPAFrameContext &frameContext,
>   				 const ipu3_uapi_grid_config &grid,
> @@ -43,7 +43,6 @@ private:
>   
>   	uint64_t frameCount_;
>   
> -	utils::Duration lineDuration_;
>   	utils::Duration minShutterSpeed_;
>   	utils::Duration maxShutterSpeed_;
>   
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 86794ac1..9c4ec936 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -86,6 +86,14 @@ namespace libcamera::ipa::ipu3 {
>    * \brief Maximum analogue gain supported with the configured sensor
>    */
>   
> +/**
> + * \var IPASessionConfiguration::sensor
> + * \brief Sensor-specific configuration of the IPA
> + *
> + * \var IPASessionConfiguration::sensor.lineDuration
> + * \brief Line duration in microseconds
> + */
> +
>   /**
>    * \var IPAFrameContext::agc
>    * \brief Context for the Automatic Gain Control algorithm
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index c6dc0814..e7c49828 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -31,6 +31,10 @@ struct IPASessionConfiguration {
>   		double minAnalogueGain;
>   		double maxAnalogueGain;
>   	} agc;
> +
> +	struct {
> +		utils::Duration lineDuration;
> +	} sensor;
>   };
>   
>   struct IPAFrameContext {
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index c0c29416..17324616 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -173,8 +173,6 @@ private:
>   	uint32_t minGain_;
>   	uint32_t maxGain_;
>   
> -	utils::Duration lineDuration_;
> -
>   	/* Interface to the Camera Helper */
>   	std::unique_ptr<CameraSensorHelper> camHelper_;
>   
> @@ -206,8 +204,8 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
>   	 *
>   	 * \todo take VBLANK into account for maximum shutter speed
>   	 */
> -	context_.configuration.agc.minShutterSpeed = minExposure * lineDuration_;
> -	context_.configuration.agc.maxShutterSpeed = maxExposure * lineDuration_;
> +	context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
> +	context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
>   	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
>   	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
>   }
> @@ -229,6 +227,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>   			     ControlInfoMap *ipaControls)
>   {
>   	ControlInfoMap::Map controls{};
> +	double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
>   
>   	/*
>   	 * Compute exposure time limits by using line length and pixel rate
> @@ -237,9 +236,9 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>   	 * microseconds.
>   	 */
>   	const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
> -	int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration_.get<std::micro>();
> -	int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration_.get<std::micro>();
> -	int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration_.get<std::micro>();
> +	int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;
> +	int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;
> +	int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;
>   	controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,
>   							defExposure);
>   
> @@ -293,6 +292,10 @@ int IPAIPU3::init(const IPASettings &settings,
>   		return -ENODEV;
>   	}
>   
> +	/* Clean context */
> +	context_ = {};
> +	context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;
> +
>   	/* Construct our Algorithms */
>   	algorithms_.push_back(std::make_unique<algorithms::Agc>());
>   	algorithms_.push_back(std::make_unique<algorithms::Awb>());
> @@ -454,12 +457,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>   
>   	defVBlank_ = itVBlank->second.def().get<int32_t>();
>   
> -	/* Clean context at configuration */
> -	context_ = {};
> -
>   	calculateBdsGrid(configInfo.bdsOutputSize);
>   
> -	lineDuration_ = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;
> +	/* Clean frameContext at each reconfiguration. */
> +	context_.frameContext = {};
>   
>   	/* Update the camera controls using the new sensor settings. */
>   	updateControls(sensorInfo_, ctrls_, ipaControls);
> @@ -620,6 +621,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>   			      [[maybe_unused]] int64_t frameTimestamp,
>   			      const ipu3_uapi_stats_3a *stats)
>   {
> +	double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
>   	ControlList ctrls(controls::controls);
>   
>   	for (auto const &algo : algorithms_)
> @@ -628,14 +630,14 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>   	setControls(frame);
>   
>   	/* \todo Use VBlank value calculated from each frame exposure. */
> -	int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration_.get<std::micro>();
> +	int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration;
>   	ctrls.set(controls::FrameDuration, frameDuration);
>   
>   	ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);
>   
>   	ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
>   
> -	ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration_.get<std::micro>());
> +	ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);
>   
>   	/*
>   	 * \todo The Metadata provides a path to getting extended data
Nicolas Dufresne via libcamera-devel March 11, 2022, 1:49 p.m. UTC | #3 
Hi Kieran,

On 2/28/22 20:00, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2022-02-24 15:11:12)
>> Instead of having a local cached value for line duration, store it in
>> the IPASessionConfiguration::sensor structure.
>> While at it, configure the default analogue gain and shutter speed to
>> controlled fixed values.
>>
>> The latter is set to be 10ms as it will in most cases be close to the
>> one needed, making the AGC faster to converge.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>> ---
>> v4: clean only frameContext at configure call.
>>      [Kieran, I took your R-b if you are ok ?]
> Fine with me if I already gave it ;-)
>
> I'll skim down through this to be sure anyway.
>
> I have a worry about the context_ = {}; to clean / initialise it now
> that there is a utils::Duration in it.
>
> I think it can possibly be fine, but we probably need to validate it.


Good spotting!

I validated with,

--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -293,7 +293,9 @@ int IPAIPU3::init(const IPASettings &settings,
         }

         /* Clean context */
+       context_.configuration.sensor.lineDuration = 345ms;
         context_ = {};
+       LOG(IPAIPU3, Info) << "lineduration after reset : " << 
context_.configuration.sensor.lineDuration.get<std::milli>();
         context_.configuration.sensor.lineDuration = 
sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;

gives an o/p:

[8:20:50.953925353] [29439]  INFO IPAIPU3 ipu3.cpp:298 lineduration 
after reset : 0

so it seems it should be fine.

>
>
>> ---
>>   src/ipa/ipu3/algorithms/agc.cpp | 26 ++++++++++++++------------
>>   src/ipa/ipu3/algorithms/agc.h   |  3 +--
>>   src/ipa/ipu3/ipa_context.cpp    |  8 ++++++++
>>   src/ipa/ipu3/ipa_context.h      |  4 ++++
>>   src/ipa/ipu3/ipu3.cpp           | 28 +++++++++++++++-------------
>>   5 files changed, 42 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> index a929085c..1eb1bcef 100644
>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10;
>>   static constexpr double kRelativeLuminanceTarget = 0.16;
>>   
>>   Agc::Agc()
>> -       : frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s),
>> +       : frameCount_(0), minShutterSpeed_(0s),
>>            maxShutterSpeed_(0s), filteredExposure_(0s)
>>   {
>>   }
>> @@ -83,13 +83,13 @@ Agc::Agc()
>>    *
>>    * \return 0
>>    */
>> -int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>> +int Agc::configure(IPAContext &context,
>> +                  [[maybe_unused]] const IPAConfigInfo &configInfo)
>>   {
>> -       stride_ = context.configuration.grid.stride;
>> +       IPASessionConfiguration &configuration = context.configuration;
>> +       IPAFrameContext &frameContext = context.frameContext;
>>   
>> -       /* \todo use the IPAContext to provide the limits */
>> -       lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
>> -                     / configInfo.sensorInfo.pixelRate;
>> +       stride_ = configuration.grid.stride;
>>   
>>          minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
>>          maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
>> @@ -99,8 +99,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>>          maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>>   
>>          /* Configure the default exposure and gain. */
>> -       context.frameContext.agc.gain = minAnalogueGain_;
>> -       context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
>> +       frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);
>> +       frameContext.agc.exposure = 10ms / configuration.sensor.lineDuration;
>>   
>>          return 0;
>>   }
>> @@ -182,9 +182,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
>>    * \param[in] yGain The gain calculated based on the relative luminance target
>>    * \param[in] iqMeanGain The gain calculated based on the relative luminance target
>>    */
>> -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>> +void Agc::computeExposure(IPAContext &context, double yGain,
>>                            double iqMeanGain)
>>   {
>> +       const IPASessionConfiguration &configuration = context.configuration;
>> +       IPAFrameContext &frameContext = context.frameContext;
>>          /* Get the effective exposure and gain applied on the sensor. */
>>          uint32_t exposure = frameContext.sensor.exposure;
>>          double analogueGain = frameContext.sensor.gain;
>> @@ -200,7 +202,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>>          /* extracted from Rpi::Agc::computeTargetExposure */
>>   
>>          /* Calculate the shutter time in seconds */
>> -       utils::Duration currentShutter = exposure * lineDuration_;
>> +       utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
>>   
>>          /*
>>           * Update the exposure value for the next computation using the values
>> @@ -247,7 +249,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>>                              << stepGain;
>>   
>>          /* Update the estimated exposure and gain. */
>> -       frameContext.agc.exposure = shutterTime / lineDuration_;
>> +       frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;
>>          frameContext.agc.gain = stepGain;
>>   }
>>   
>> @@ -354,7 +356,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>>                          break;
>>          }
>>   
>> -       computeExposure(context.frameContext, yGain, iqMeanGain);
>> +       computeExposure(context, yGain, iqMeanGain);
>>          frameCount_++;
>>   }
>>   
>> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
>> index 84bfe045..ad705605 100644
>> --- a/src/ipa/ipu3/algorithms/agc.h
>> +++ b/src/ipa/ipu3/algorithms/agc.h
>> @@ -34,7 +34,7 @@ private:
>>          double measureBrightness(const ipu3_uapi_stats_3a *stats,
>>                                   const ipu3_uapi_grid_config &grid) const;
>>          utils::Duration filterExposure(utils::Duration currentExposure);
>> -       void computeExposure(IPAFrameContext &frameContext, double yGain,
>> +       void computeExposure(IPAContext &context, double yGain,
>>                               double iqMeanGain);
>>          double estimateLuminance(IPAFrameContext &frameContext,
>>                                   const ipu3_uapi_grid_config &grid,
>> @@ -43,7 +43,6 @@ private:
>>   
>>          uint64_t frameCount_;
>>   
>> -       utils::Duration lineDuration_;
>>          utils::Duration minShutterSpeed_;
>>          utils::Duration maxShutterSpeed_;
>>   
>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
>> index 86794ac1..9c4ec936 100644
>> --- a/src/ipa/ipu3/ipa_context.cpp
>> +++ b/src/ipa/ipu3/ipa_context.cpp
>> @@ -86,6 +86,14 @@ namespace libcamera::ipa::ipu3 {
>>    * \brief Maximum analogue gain supported with the configured sensor
>>    */
>>   
>> +/**
>> + * \var IPASessionConfiguration::sensor
>> + * \brief Sensor-specific configuration of the IPA
>> + *
>> + * \var IPASessionConfiguration::sensor.lineDuration
>> + * \brief Line duration in microseconds
>> + */
>> +
>>   /**
>>    * \var IPAFrameContext::agc
>>    * \brief Context for the Automatic Gain Control algorithm
>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>> index c6dc0814..e7c49828 100644
>> --- a/src/ipa/ipu3/ipa_context.h
>> +++ b/src/ipa/ipu3/ipa_context.h
>> @@ -31,6 +31,10 @@ struct IPASessionConfiguration {
>>                  double minAnalogueGain;
>>                  double maxAnalogueGain;
>>          } agc;
>> +
>> +       struct {
>> +               utils::Duration lineDuration;
>> +       } sensor;
>>   };
>>   
>>   struct IPAFrameContext {
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index c0c29416..17324616 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -173,8 +173,6 @@ private:
>>          uint32_t minGain_;
>>          uint32_t maxGain_;
>>   
>> -       utils::Duration lineDuration_;
>> -
>>          /* Interface to the Camera Helper */
>>          std::unique_ptr<CameraSensorHelper> camHelper_;
>>   
>> @@ -206,8 +204,8 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
>>           *
>>           * \todo take VBLANK into account for maximum shutter speed
>>           */
>> -       context_.configuration.agc.minShutterSpeed = minExposure * lineDuration_;
>> -       context_.configuration.agc.maxShutterSpeed = maxExposure * lineDuration_;
>> +       context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
>> +       context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
> Some long lines, but those might only be easier to shorten if we had a
> local reference to the configuration structure or sensor configuration?


yes, there are few as well in algorithms too. Those need to trimmed as 
well I think.

Local reference seems a solution, but then would be not be neat when the 
context itself becomes large and we are using local references all the IPA.

I see some upside while code reading - that the 'x value' comes the 
context - which will be a hidden when using local references. Just my 
preference.

>
>>          context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
>>          context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
>>   }
>> @@ -229,6 +227,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>>                               ControlInfoMap *ipaControls)
>>   {
>>          ControlInfoMap::Map controls{};
>> +       double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
>>   
>>          /*
>>           * Compute exposure time limits by using line length and pixel rate
>> @@ -237,9 +236,9 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
>>           * microseconds.
>>           */
>>          const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
>> -       int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration_.get<std::micro>();
>> -       int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration_.get<std::micro>();
>> -       int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration_.get<std::micro>();
>> +       int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;
>> +       int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;
>> +       int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;
>>          controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,
>>                                                          defExposure);
>>   
>> @@ -293,6 +292,10 @@ int IPAIPU3::init(const IPASettings &settings,
>>                  return -ENODEV;
>>          }
>>   
>> +       /* Clean context */
>> +       context_ = {};
> Context_ now contains a utils::Duration which is a std::chrono::duration
> type.
>
> What happens to that /object/ when this context is cleaned? Does it lose
> any state? Or does it correctly get initialised successfully?
>
> We might want to validate that this is safe C++ ... I'm quite fearful
> that it might not be.


Answered and validated above. Should be okay!

>
>
>
>> +       context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;
>> +
>>          /* Construct our Algorithms */
>>          algorithms_.push_back(std::make_unique<algorithms::Agc>());
>>          algorithms_.push_back(std::make_unique<algorithms::Awb>());
>> @@ -454,12 +457,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>>   
>>          defVBlank_ = itVBlank->second.def().get<int32_t>();
>>   
>> -       /* Clean context at configuration */
>> -       context_ = {};
>> -
>>          calculateBdsGrid(configInfo.bdsOutputSize);
>>   
>> -       lineDuration_ = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;
>> +       /* Clean frameContext at each reconfiguration. */
>> +       context_.frameContext = {};
> Yes, I think this is likely required for now, but should go when we get
> a per-frame frameContext.


Ah something for me to take note of for my (already submitted) 
IPAFrameContext queue series o_O

>
>>          /* Update the camera controls using the new sensor settings. */
>>          updateControls(sensorInfo_, ctrls_, ipaControls);
>> @@ -620,6 +621,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>>                                [[maybe_unused]] int64_t frameTimestamp,
>>                                const ipu3_uapi_stats_3a *stats)
>>   {
>> +       double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
>>          ControlList ctrls(controls::controls);
>>   
>>          for (auto const &algo : algorithms_)
>> @@ -628,14 +630,14 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>>          setControls(frame);
>>   
>>          /* \todo Use VBlank value calculated from each frame exposure. */
>> -       int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration_.get<std::micro>();
>> +       int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration;
>>          ctrls.set(controls::FrameDuration, frameDuration);
>>   
>>          ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);
>>   
>>          ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
>>   
>> -       ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration_.get<std::micro>());
>> +       ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);
>>   
>>          /*
>>           * \todo The Metadata provides a path to getting extended data
>> -- 
>> 2.32.0
>>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
index a929085c..1eb1bcef 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -71,7 +71,7 @@  static constexpr uint32_t kNumStartupFrames = 10;
 static constexpr double kRelativeLuminanceTarget = 0.16;
 
 Agc::Agc()
-	: frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s),
+	: frameCount_(0), minShutterSpeed_(0s),
 	  maxShutterSpeed_(0s), filteredExposure_(0s)
 {
 }
@@ -83,13 +83,13 @@  Agc::Agc()
  *
  * \return 0
  */
-int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
+int Agc::configure(IPAContext &context,
+		   [[maybe_unused]] const IPAConfigInfo &configInfo)
 {
-	stride_ = context.configuration.grid.stride;
+	IPASessionConfiguration &configuration = context.configuration;
+	IPAFrameContext &frameContext = context.frameContext;
 
-	/* \todo use the IPAContext to provide the limits */
-	lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
-		      / configInfo.sensorInfo.pixelRate;
+	stride_ = configuration.grid.stride;
 
 	minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
 	maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
@@ -99,8 +99,8 @@  int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
 	maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
 
 	/* Configure the default exposure and gain. */
-	context.frameContext.agc.gain = minAnalogueGain_;
-	context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
+	frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);
+	frameContext.agc.exposure = 10ms / configuration.sensor.lineDuration;
 
 	return 0;
 }
@@ -182,9 +182,11 @@  utils::Duration Agc::filterExposure(utils::Duration exposureValue)
  * \param[in] yGain The gain calculated based on the relative luminance target
  * \param[in] iqMeanGain The gain calculated based on the relative luminance target
  */
-void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
+void Agc::computeExposure(IPAContext &context, double yGain,
 			  double iqMeanGain)
 {
+	const IPASessionConfiguration &configuration = context.configuration;
+	IPAFrameContext &frameContext = context.frameContext;
 	/* Get the effective exposure and gain applied on the sensor. */
 	uint32_t exposure = frameContext.sensor.exposure;
 	double analogueGain = frameContext.sensor.gain;
@@ -200,7 +202,7 @@  void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
 	/* extracted from Rpi::Agc::computeTargetExposure */
 
 	/* Calculate the shutter time in seconds */
-	utils::Duration currentShutter = exposure * lineDuration_;
+	utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
 
 	/*
 	 * Update the exposure value for the next computation using the values
@@ -247,7 +249,7 @@  void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
 			    << stepGain;
 
 	/* Update the estimated exposure and gain. */
-	frameContext.agc.exposure = shutterTime / lineDuration_;
+	frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;
 	frameContext.agc.gain = stepGain;
 }
 
@@ -354,7 +356,7 @@  void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
 			break;
 	}
 
-	computeExposure(context.frameContext, yGain, iqMeanGain);
+	computeExposure(context, yGain, iqMeanGain);
 	frameCount_++;
 }
 
diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
index 84bfe045..ad705605 100644
--- a/src/ipa/ipu3/algorithms/agc.h
+++ b/src/ipa/ipu3/algorithms/agc.h
@@ -34,7 +34,7 @@  private:
 	double measureBrightness(const ipu3_uapi_stats_3a *stats,
 				 const ipu3_uapi_grid_config &grid) const;
 	utils::Duration filterExposure(utils::Duration currentExposure);
-	void computeExposure(IPAFrameContext &frameContext, double yGain,
+	void computeExposure(IPAContext &context, double yGain,
 			     double iqMeanGain);
 	double estimateLuminance(IPAFrameContext &frameContext,
 				 const ipu3_uapi_grid_config &grid,
@@ -43,7 +43,6 @@  private:
 
 	uint64_t frameCount_;
 
-	utils::Duration lineDuration_;
 	utils::Duration minShutterSpeed_;
 	utils::Duration maxShutterSpeed_;
 
diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
index 86794ac1..9c4ec936 100644
--- a/src/ipa/ipu3/ipa_context.cpp
+++ b/src/ipa/ipu3/ipa_context.cpp
@@ -86,6 +86,14 @@  namespace libcamera::ipa::ipu3 {
  * \brief Maximum analogue gain supported with the configured sensor
  */
 
+/**
+ * \var IPASessionConfiguration::sensor
+ * \brief Sensor-specific configuration of the IPA
+ *
+ * \var IPASessionConfiguration::sensor.lineDuration
+ * \brief Line duration in microseconds
+ */
+
 /**
  * \var IPAFrameContext::agc
  * \brief Context for the Automatic Gain Control algorithm
diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
index c6dc0814..e7c49828 100644
--- a/src/ipa/ipu3/ipa_context.h
+++ b/src/ipa/ipu3/ipa_context.h
@@ -31,6 +31,10 @@  struct IPASessionConfiguration {
 		double minAnalogueGain;
 		double maxAnalogueGain;
 	} agc;
+
+	struct {
+		utils::Duration lineDuration;
+	} sensor;
 };
 
 struct IPAFrameContext {
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index c0c29416..17324616 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -173,8 +173,6 @@  private:
 	uint32_t minGain_;
 	uint32_t maxGain_;
 
-	utils::Duration lineDuration_;
-
 	/* Interface to the Camera Helper */
 	std::unique_ptr<CameraSensorHelper> camHelper_;
 
@@ -206,8 +204,8 @@  void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
 	 *
 	 * \todo take VBLANK into account for maximum shutter speed
 	 */
-	context_.configuration.agc.minShutterSpeed = minExposure * lineDuration_;
-	context_.configuration.agc.maxShutterSpeed = maxExposure * lineDuration_;
+	context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
+	context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
 	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
 	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
 }
@@ -229,6 +227,7 @@  void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
 			     ControlInfoMap *ipaControls)
 {
 	ControlInfoMap::Map controls{};
+	double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
 
 	/*
 	 * Compute exposure time limits by using line length and pixel rate
@@ -237,9 +236,9 @@  void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
 	 * microseconds.
 	 */
 	const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
-	int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration_.get<std::micro>();
-	int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration_.get<std::micro>();
-	int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration_.get<std::micro>();
+	int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;
+	int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;
+	int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;
 	controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,
 							defExposure);
 
@@ -293,6 +292,10 @@  int IPAIPU3::init(const IPASettings &settings,
 		return -ENODEV;
 	}
 
+	/* Clean context */
+	context_ = {};
+	context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;
+
 	/* Construct our Algorithms */
 	algorithms_.push_back(std::make_unique<algorithms::Agc>());
 	algorithms_.push_back(std::make_unique<algorithms::Awb>());
@@ -454,12 +457,10 @@  int IPAIPU3::configure(const IPAConfigInfo &configInfo,
 
 	defVBlank_ = itVBlank->second.def().get<int32_t>();
 
-	/* Clean context at configuration */
-	context_ = {};
-
 	calculateBdsGrid(configInfo.bdsOutputSize);
 
-	lineDuration_ = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;
+	/* Clean frameContext at each reconfiguration. */
+	context_.frameContext = {};
 
 	/* Update the camera controls using the new sensor settings. */
 	updateControls(sensorInfo_, ctrls_, ipaControls);
@@ -620,6 +621,7 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 			      [[maybe_unused]] int64_t frameTimestamp,
 			      const ipu3_uapi_stats_3a *stats)
 {
+	double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
 	ControlList ctrls(controls::controls);
 
 	for (auto const &algo : algorithms_)
@@ -628,14 +630,14 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 	setControls(frame);
 
 	/* \todo Use VBlank value calculated from each frame exposure. */
-	int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration_.get<std::micro>();
+	int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration;
 	ctrls.set(controls::FrameDuration, frameDuration);
 
 	ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);
 
 	ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
 
-	ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration_.get<std::micro>());
+	ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);
 
 	/*
 	 * \todo The Metadata provides a path to getting extended data