[v2,3/3] rkisp1: Honor the FrameDurationLimits control
diff mbox series

Message ID 20250221092045.3896021-4-paul.elder@ideasonboard.com
State Accepted
Commit f72c76eb6e06a41d2aaff8c8c4002dea21a7774d
Headers show
Series
  • ipa: rkisp1: Honor FrameDurationLimits
Related show

Commit Message

Paul Elder Feb. 21, 2025, 9:20 a.m. UTC 
Add support to rkisp1 for controlling the framerate via the
FrameDurationLimits control.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

---
Changes in v2:
- recover from bitrot
- fix setting frame duration limits in raw mode
---
 src/ipa/rkisp1/algorithms/agc.cpp        | 60 +++++++++++++++++++-----
 src/ipa/rkisp1/algorithms/agc.h          |  3 ++
 src/ipa/rkisp1/ipa_context.cpp           | 16 ++++++-
 src/ipa/rkisp1/ipa_context.h             |  4 ++
 src/ipa/rkisp1/rkisp1.cpp                |  2 +
 src/libcamera/pipeline/rkisp1/rkisp1.cpp |  1 +
 6 files changed, 74 insertions(+), 12 deletions(-)

Comments

Kieran Bingham Feb. 22, 2025, 2:09 p.m. UTC | #1 
Quoting Paul Elder (2025-02-21 09:20:45)
> Add support to rkisp1 for controlling the framerate via the
> FrameDurationLimits control.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> 
> ---
> Changes in v2:
> - recover from bitrot
> - fix setting frame duration limits in raw mode
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp        | 60 +++++++++++++++++++-----
>  src/ipa/rkisp1/algorithms/agc.h          |  3 ++
>  src/ipa/rkisp1/ipa_context.cpp           | 16 ++++++-
>  src/ipa/rkisp1/ipa_context.h             |  4 ++
>  src/ipa/rkisp1/rkisp1.cpp                |  2 +
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  1 +
>  6 files changed, 74 insertions(+), 12 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 5fece545677e..4e0e3734117e 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -190,6 +190,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>  
>         /* Limit the frame duration to match current initialisation */
>         ControlInfo &frameDurationLimits = context.ctrlMap[&controls::FrameDurationLimits];
> +       context.activeState.agc.minFrameDuration = std::chrono::microseconds(frameDurationLimits.min().get<int64_t>());
>         context.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get<int64_t>());
>  
>         /*
> @@ -307,10 +308,21 @@ void Agc::queueRequest(IPAContext &context,
>  
>         const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
>         if (frameDurationLimits) {
> -               utils::Duration maxFrameDuration =
> -                       std::chrono::milliseconds((*frameDurationLimits).back());
> -               agc.maxFrameDuration = maxFrameDuration;
> +               /* Limit the control value to the limits in ControlInfo */
> +               ControlInfo &limits = context.ctrlMap[&controls::FrameDurationLimits];
> +               int64_t minFrameDuration =
> +                       std::clamp((*frameDurationLimits).front(),
> +                                  limits.min().get<int64_t>(),
> +                                  limits.max().get<int64_t>());
> +               int64_t maxFrameDuration =
> +                       std::clamp((*frameDurationLimits).back(),
> +                                  limits.min().get<int64_t>(),
> +                                  limits.max().get<int64_t>());
> +
> +               agc.minFrameDuration = std::chrono::microseconds(minFrameDuration);
> +               agc.maxFrameDuration = std::chrono::microseconds(maxFrameDuration);
>         }
> +       frameContext.agc.minFrameDuration = agc.minFrameDuration;
>         frameContext.agc.maxFrameDuration = agc.maxFrameDuration;
>  }
>  
> @@ -387,6 +399,7 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>                                      * frameContext.sensor.exposure;
>         metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>         metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> +       metadata.set(controls::FrameDuration, frameContext.agc.frameDuration.get<std::micro>());
>         metadata.set(controls::ExposureTimeMode,
>                      frameContext.agc.autoExposureEnabled
>                      ? controls::ExposureTimeModeAuto
> @@ -396,13 +409,6 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>                      ? controls::AnalogueGainModeAuto
>                      : controls::AnalogueGainModeManual);
>  
> -       /* \todo Use VBlank value calculated from each frame exposure. */
> -       uint32_t vTotal = context.configuration.sensor.size.height
> -                       + context.configuration.sensor.defVBlank;
> -       utils::Duration frameDuration = context.configuration.sensor.lineDuration
> -                                     * vTotal;
> -       metadata.set(controls::FrameDuration, frameDuration.get<std::micro>());

Am I missing where the FrameDuration is now reported in the metadata ?
or has it been removed completely.

I think we should still be reporting the FrameDuration which has been
determined. It should come from
  frameContext.agc.frameDuration.get<std::micro>()

now shouldn't it ?



> -
>         metadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);
>         metadata.set(controls::AeExposureMode, frameContext.agc.exposureMode);
>         metadata.set(controls::AeConstraintMode, frameContext.agc.constraintMode);
> @@ -444,6 +450,27 @@ double Agc::estimateLuminance(double gain) const
>         return ySum / expMeans_.size() / 255;
>  }
>  
> +/**
> + * \brief Process frame duration and compute vblank
> + * \param[in] context The shared IPA context
> + * \param[in] frameContext The current frame context
> + * \param[in] frameDuration The target frame duration
> + *
> + * Compute and populate vblank from the target frame duration.
> + */
> +void Agc::processFrameDuration(IPAContext &context,
> +                              IPAFrameContext &frameContext,
> +                              utils::Duration frameDuration)
> +{
> +       IPACameraSensorInfo &sensorInfo = context.sensorInfo;
> +       utils::Duration lineDuration = context.configuration.sensor.lineDuration;
> +
> +       frameContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;
> +
> +       /* Update frame duration accounting for line length quantization. */
> +       frameContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;
> +}
> +
>  /**
>   * \brief Process RkISP1 statistics, and run AGC operations
>   * \param[in] context The shared IPA context
> @@ -460,6 +487,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>                   ControlList &metadata)
>  {
>         if (!stats) {
> +               processFrameDuration(context, frameContext,
> +                                    frameContext.agc.minFrameDuration);
>                 fillMetadata(context, frameContext, metadata);
>                 return;
>         }
> @@ -497,7 +526,9 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>         double maxAnalogueGain;
>  
>         if (frameContext.agc.autoExposureEnabled) {
> -               minExposureTime = context.configuration.sensor.minExposureTime;
> +               minExposureTime = std::clamp(frameContext.agc.minFrameDuration,
> +                                            context.configuration.sensor.minExposureTime,
> +                                            context.configuration.sensor.maxExposureTime);
>                 maxExposureTime = std::clamp(frameContext.agc.maxFrameDuration,
>                                              context.configuration.sensor.minExposureTime,
>                                              context.configuration.sensor.maxExposureTime);
> @@ -541,6 +572,13 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>         activeState.agc.automatic.exposure = newExposureTime / lineDuration;
>         activeState.agc.automatic.gain = aGain;
>  
> +       /*
> +        * 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));
> +
>         fillMetadata(context, frameContext, metadata);
>         expMeans_ = {};
>  }
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index aa86f2c5bc21..62bcde999fe3 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -50,6 +50,9 @@ private:
>         void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>                           ControlList &metadata);
>         double estimateLuminance(double gain) const override;
> +       void processFrameDuration(IPAContext &context,
> +                                 IPAFrameContext &frameContext,
> +                                 utils::Duration frameDuration);
>  
>         Span<const uint8_t> expMeans_;
>  
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 261c0472a4fc..99611bd5b390 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -180,6 +180,9 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPAActiveState::agc.meteringMode
>   * \brief Metering mode as set by the AeMeteringMode control
>   *
> + * \var IPAActiveState::agc.minFrameDuration
> + * \brief Minimum frame duration as set by the FrameDurationLimits control
> + *
>   * \var IPAActiveState::agc.maxFrameDuration
>   * \brief Maximum frame duration as set by the FrameDurationLimits control
>   */
> @@ -282,7 +285,9 @@ namespace libcamera::ipa::rkisp1 {
>   * \brief Automatic Gain Control parameters for this frame
>   *
>   * The exposure and gain are provided by the AGC algorithm, and are to be
> - * applied to the sensor in order to take effect for this frame.
> + * applied to the sensor in order to take effect for this frame. Additionally
> + * the vertical blanking period is determined to maintain a consistent frame
> + * rate matched to the FrameDurationLimits as set by the user.
>   *
>   * \var IPAFrameContext::agc.exposure
>   * \brief Exposure time expressed as a number of lines computed by the algorithm
> @@ -292,6 +297,9 @@ namespace libcamera::ipa::rkisp1 {
>   *
>   * The gain should be adapted to the sensor specific gain code before applying.
>   *
> + * \var IPAFrameContext::agc.vblank
> + * \brief Vertical blanking parameter computed by the algorithm
> + *
>   * \var IPAFrameContext::agc.autoExposureEnabled
>   * \brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control
>   *
> @@ -307,9 +315,15 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPAFrameContext::agc.meteringMode
>   * \brief Metering mode as set by the AeMeteringMode control
>   *
> + * \var IPAFrameContext::agc.minFrameDuration
> + * \brief Minimum frame duration as set by the FrameDurationLimits control
> + *
>   * \var IPAFrameContext::agc.maxFrameDuration
>   * \brief Maximum frame duration as set by the FrameDurationLimits control
>   *
> + * \var IPAFrameContext::agc.frameDuration
> + * \brief The actual FrameDuration used by the algorithm for the frame
> + *
>   * \var IPAFrameContext::agc.updateMetering
>   * \brief Indicate if new ISP AGC metering parameters need to be applied
>   *
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index c765b928a55f..474f7036f003 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -84,6 +84,7 @@ struct IPAActiveState {
>                 controls::AeConstraintModeEnum constraintMode;
>                 controls::AeExposureModeEnum exposureMode;
>                 controls::AeMeteringModeEnum meteringMode;
> +               utils::Duration minFrameDuration;
>                 utils::Duration maxFrameDuration;
>         } agc;
>  
> @@ -125,12 +126,15 @@ struct IPAFrameContext : public FrameContext {
>         struct {
>                 uint32_t exposure;
>                 double gain;
> +               uint32_t vblank;
>                 bool autoExposureEnabled;
>                 bool autoGainEnabled;
>                 controls::AeConstraintModeEnum constraintMode;
>                 controls::AeExposureModeEnum exposureMode;
>                 controls::AeMeteringModeEnum meteringMode;
> +               utils::Duration minFrameDuration;
>                 utils::Duration maxFrameDuration;
> +               utils::Duration frameDuration;
>                 bool updateMetering;
>                 bool autoExposureModeChange;
>                 bool autoGainModeChange;
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 0e761249d27c..7547d2f274f4 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -453,10 +453,12 @@ void IPARkISP1::setControls(unsigned int frame)
>         IPAFrameContext &frameContext = context_.frameContexts.get(frame);
>         uint32_t exposure = frameContext.agc.exposure;
>         uint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);
> +       uint32_t vblank = frameContext.agc.vblank;
>  
>         ControlList ctrls(sensorControls_);
>         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
>         ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));
> +       ctrls.set(V4L2_CID_VBLANK, static_cast<int32_t>(vblank));
>  
>         setSensorControls.emit(frame, ctrls);
>  }
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 1ac8d8ae7ed9..52633fe3cb85 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1325,6 +1325,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>         std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
>                 { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
>                 { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> +               { V4L2_CID_VBLANK, { 1, false } },
>         };
>  
>         data->delayedCtrls_ =
> -- 
> 2.39.2
>
Paul Elder Feb. 24, 2025, 2:13 a.m. UTC | #2 
On Sat, Feb 22, 2025 at 02:09:01PM +0000, Kieran Bingham wrote:
> Quoting Paul Elder (2025-02-21 09:20:45)
> > Add support to rkisp1 for controlling the framerate via the
> > FrameDurationLimits control.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> > 
> > ---
> > Changes in v2:
> > - recover from bitrot
> > - fix setting frame duration limits in raw mode
> > ---
> >  src/ipa/rkisp1/algorithms/agc.cpp        | 60 +++++++++++++++++++-----
> >  src/ipa/rkisp1/algorithms/agc.h          |  3 ++
> >  src/ipa/rkisp1/ipa_context.cpp           | 16 ++++++-
> >  src/ipa/rkisp1/ipa_context.h             |  4 ++
> >  src/ipa/rkisp1/rkisp1.cpp                |  2 +
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  1 +
> >  6 files changed, 74 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > index 5fece545677e..4e0e3734117e 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > @@ -190,6 +190,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
> >  
> >         /* Limit the frame duration to match current initialisation */
> >         ControlInfo &frameDurationLimits = context.ctrlMap[&controls::FrameDurationLimits];
> > +       context.activeState.agc.minFrameDuration = std::chrono::microseconds(frameDurationLimits.min().get<int64_t>());
> >         context.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get<int64_t>());
> >  
> >         /*
> > @@ -307,10 +308,21 @@ void Agc::queueRequest(IPAContext &context,
> >  
> >         const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
> >         if (frameDurationLimits) {
> > -               utils::Duration maxFrameDuration =
> > -                       std::chrono::milliseconds((*frameDurationLimits).back());
> > -               agc.maxFrameDuration = maxFrameDuration;
> > +               /* Limit the control value to the limits in ControlInfo */
> > +               ControlInfo &limits = context.ctrlMap[&controls::FrameDurationLimits];
> > +               int64_t minFrameDuration =
> > +                       std::clamp((*frameDurationLimits).front(),
> > +                                  limits.min().get<int64_t>(),
> > +                                  limits.max().get<int64_t>());
> > +               int64_t maxFrameDuration =
> > +                       std::clamp((*frameDurationLimits).back(),
> > +                                  limits.min().get<int64_t>(),
> > +                                  limits.max().get<int64_t>());
> > +
> > +               agc.minFrameDuration = std::chrono::microseconds(minFrameDuration);
> > +               agc.maxFrameDuration = std::chrono::microseconds(maxFrameDuration);
> >         }
> > +       frameContext.agc.minFrameDuration = agc.minFrameDuration;
> >         frameContext.agc.maxFrameDuration = agc.maxFrameDuration;
> >  }
> >  
> > @@ -387,6 +399,7 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
> >                                      * frameContext.sensor.exposure;
> >         metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
> >         metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> > +       metadata.set(controls::FrameDuration, frameContext.agc.frameDuration.get<std::micro>());

(It's here!)

> >         metadata.set(controls::ExposureTimeMode,
> >                      frameContext.agc.autoExposureEnabled
> >                      ? controls::ExposureTimeModeAuto
> > @@ -396,13 +409,6 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
> >                      ? controls::AnalogueGainModeAuto
> >                      : controls::AnalogueGainModeManual);
> >  
> > -       /* \todo Use VBlank value calculated from each frame exposure. */
> > -       uint32_t vTotal = context.configuration.sensor.size.height
> > -                       + context.configuration.sensor.defVBlank;
> > -       utils::Duration frameDuration = context.configuration.sensor.lineDuration
> > -                                     * vTotal;
> > -       metadata.set(controls::FrameDuration, frameDuration.get<std::micro>());
> 
> Am I missing where the FrameDuration is now reported in the metadata ?
> or has it been removed completely.
> 
> I think we should still be reporting the FrameDuration which has been
> determined. It should come from
>   frameContext.agc.frameDuration.get<std::micro>()
> 
> now shouldn't it ?

(Yes)


(Paul)

> 
> 
> 
> > -
> >         metadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);
> >         metadata.set(controls::AeExposureMode, frameContext.agc.exposureMode);
> >         metadata.set(controls::AeConstraintMode, frameContext.agc.constraintMode);
> > @@ -444,6 +450,27 @@ double Agc::estimateLuminance(double gain) const
> >         return ySum / expMeans_.size() / 255;
> >  }
> >  
> > +/**
> > + * \brief Process frame duration and compute vblank
> > + * \param[in] context The shared IPA context
> > + * \param[in] frameContext The current frame context
> > + * \param[in] frameDuration The target frame duration
> > + *
> > + * Compute and populate vblank from the target frame duration.
> > + */
> > +void Agc::processFrameDuration(IPAContext &context,
> > +                              IPAFrameContext &frameContext,
> > +                              utils::Duration frameDuration)
> > +{
> > +       IPACameraSensorInfo &sensorInfo = context.sensorInfo;
> > +       utils::Duration lineDuration = context.configuration.sensor.lineDuration;
> > +
> > +       frameContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;
> > +
> > +       /* Update frame duration accounting for line length quantization. */
> > +       frameContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;
> > +}
> > +
> >  /**
> >   * \brief Process RkISP1 statistics, and run AGC operations
> >   * \param[in] context The shared IPA context
> > @@ -460,6 +487,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> >                   ControlList &metadata)
> >  {
> >         if (!stats) {
> > +               processFrameDuration(context, frameContext,
> > +                                    frameContext.agc.minFrameDuration);
> >                 fillMetadata(context, frameContext, metadata);
> >                 return;
> >         }
> > @@ -497,7 +526,9 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> >         double maxAnalogueGain;
> >  
> >         if (frameContext.agc.autoExposureEnabled) {
> > -               minExposureTime = context.configuration.sensor.minExposureTime;
> > +               minExposureTime = std::clamp(frameContext.agc.minFrameDuration,
> > +                                            context.configuration.sensor.minExposureTime,
> > +                                            context.configuration.sensor.maxExposureTime);
> >                 maxExposureTime = std::clamp(frameContext.agc.maxFrameDuration,
> >                                              context.configuration.sensor.minExposureTime,
> >                                              context.configuration.sensor.maxExposureTime);
> > @@ -541,6 +572,13 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> >         activeState.agc.automatic.exposure = newExposureTime / lineDuration;
> >         activeState.agc.automatic.gain = aGain;
> >  
> > +       /*
> > +        * 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));
> > +
> >         fillMetadata(context, frameContext, metadata);
> >         expMeans_ = {};
> >  }
> > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> > index aa86f2c5bc21..62bcde999fe3 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.h
> > +++ b/src/ipa/rkisp1/algorithms/agc.h
> > @@ -50,6 +50,9 @@ private:
> >         void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
> >                           ControlList &metadata);
> >         double estimateLuminance(double gain) const override;
> > +       void processFrameDuration(IPAContext &context,
> > +                                 IPAFrameContext &frameContext,
> > +                                 utils::Duration frameDuration);
> >  
> >         Span<const uint8_t> expMeans_;
> >  
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 261c0472a4fc..99611bd5b390 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -180,6 +180,9 @@ namespace libcamera::ipa::rkisp1 {
> >   * \var IPAActiveState::agc.meteringMode
> >   * \brief Metering mode as set by the AeMeteringMode control
> >   *
> > + * \var IPAActiveState::agc.minFrameDuration
> > + * \brief Minimum frame duration as set by the FrameDurationLimits control
> > + *
> >   * \var IPAActiveState::agc.maxFrameDuration
> >   * \brief Maximum frame duration as set by the FrameDurationLimits control
> >   */
> > @@ -282,7 +285,9 @@ namespace libcamera::ipa::rkisp1 {
> >   * \brief Automatic Gain Control parameters for this frame
> >   *
> >   * The exposure and gain are provided by the AGC algorithm, and are to be
> > - * applied to the sensor in order to take effect for this frame.
> > + * applied to the sensor in order to take effect for this frame. Additionally
> > + * the vertical blanking period is determined to maintain a consistent frame
> > + * rate matched to the FrameDurationLimits as set by the user.
> >   *
> >   * \var IPAFrameContext::agc.exposure
> >   * \brief Exposure time expressed as a number of lines computed by the algorithm
> > @@ -292,6 +297,9 @@ namespace libcamera::ipa::rkisp1 {
> >   *
> >   * The gain should be adapted to the sensor specific gain code before applying.
> >   *
> > + * \var IPAFrameContext::agc.vblank
> > + * \brief Vertical blanking parameter computed by the algorithm
> > + *
> >   * \var IPAFrameContext::agc.autoExposureEnabled
> >   * \brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control
> >   *
> > @@ -307,9 +315,15 @@ namespace libcamera::ipa::rkisp1 {
> >   * \var IPAFrameContext::agc.meteringMode
> >   * \brief Metering mode as set by the AeMeteringMode control
> >   *
> > + * \var IPAFrameContext::agc.minFrameDuration
> > + * \brief Minimum frame duration as set by the FrameDurationLimits control
> > + *
> >   * \var IPAFrameContext::agc.maxFrameDuration
> >   * \brief Maximum frame duration as set by the FrameDurationLimits control
> >   *
> > + * \var IPAFrameContext::agc.frameDuration
> > + * \brief The actual FrameDuration used by the algorithm for the frame
> > + *
> >   * \var IPAFrameContext::agc.updateMetering
> >   * \brief Indicate if new ISP AGC metering parameters need to be applied
> >   *
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index c765b928a55f..474f7036f003 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -84,6 +84,7 @@ struct IPAActiveState {
> >                 controls::AeConstraintModeEnum constraintMode;
> >                 controls::AeExposureModeEnum exposureMode;
> >                 controls::AeMeteringModeEnum meteringMode;
> > +               utils::Duration minFrameDuration;
> >                 utils::Duration maxFrameDuration;
> >         } agc;
> >  
> > @@ -125,12 +126,15 @@ struct IPAFrameContext : public FrameContext {
> >         struct {
> >                 uint32_t exposure;
> >                 double gain;
> > +               uint32_t vblank;
> >                 bool autoExposureEnabled;
> >                 bool autoGainEnabled;
> >                 controls::AeConstraintModeEnum constraintMode;
> >                 controls::AeExposureModeEnum exposureMode;
> >                 controls::AeMeteringModeEnum meteringMode;
> > +               utils::Duration minFrameDuration;
> >                 utils::Duration maxFrameDuration;
> > +               utils::Duration frameDuration;
> >                 bool updateMetering;
> >                 bool autoExposureModeChange;
> >                 bool autoGainModeChange;
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index 0e761249d27c..7547d2f274f4 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -453,10 +453,12 @@ void IPARkISP1::setControls(unsigned int frame)
> >         IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> >         uint32_t exposure = frameContext.agc.exposure;
> >         uint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);
> > +       uint32_t vblank = frameContext.agc.vblank;
> >  
> >         ControlList ctrls(sensorControls_);
> >         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
> >         ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));
> > +       ctrls.set(V4L2_CID_VBLANK, static_cast<int32_t>(vblank));
> >  
> >         setSensorControls.emit(frame, ctrls);
> >  }
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 1ac8d8ae7ed9..52633fe3cb85 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -1325,6 +1325,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >         std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> >                 { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> >                 { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> > +               { V4L2_CID_VBLANK, { 1, false } },
> >         };
> >  
> >         data->delayedCtrls_ =
> > -- 
> > 2.39.2
> >
Kieran Bingham Feb. 24, 2025, 2:53 p.m. UTC | #3 
Quoting Paul Elder (2025-02-24 02:13:39)
> On Sat, Feb 22, 2025 at 02:09:01PM +0000, Kieran Bingham wrote:
> > Quoting Paul Elder (2025-02-21 09:20:45)
> > > Add support to rkisp1 for controlling the framerate via the
> > > FrameDurationLimits control.
> > > 
> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> > > 
> > > ---
> > > Changes in v2:
> > > - recover from bitrot
> > > - fix setting frame duration limits in raw mode
> > > ---
> > >  src/ipa/rkisp1/algorithms/agc.cpp        | 60 +++++++++++++++++++-----
> > >  src/ipa/rkisp1/algorithms/agc.h          |  3 ++
> > >  src/ipa/rkisp1/ipa_context.cpp           | 16 ++++++-
> > >  src/ipa/rkisp1/ipa_context.h             |  4 ++
> > >  src/ipa/rkisp1/rkisp1.cpp                |  2 +
> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  1 +
> > >  6 files changed, 74 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > > index 5fece545677e..4e0e3734117e 100644
> > > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > > @@ -190,6 +190,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
> > >  
> > >         /* Limit the frame duration to match current initialisation */
> > >         ControlInfo &frameDurationLimits = context.ctrlMap[&controls::FrameDurationLimits];
> > > +       context.activeState.agc.minFrameDuration = std::chrono::microseconds(frameDurationLimits.min().get<int64_t>());
> > >         context.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get<int64_t>());
> > >  
> > >         /*
> > > @@ -307,10 +308,21 @@ void Agc::queueRequest(IPAContext &context,
> > >  
> > >         const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
> > >         if (frameDurationLimits) {
> > > -               utils::Duration maxFrameDuration =
> > > -                       std::chrono::milliseconds((*frameDurationLimits).back());
> > > -               agc.maxFrameDuration = maxFrameDuration;
> > > +               /* Limit the control value to the limits in ControlInfo */
> > > +               ControlInfo &limits = context.ctrlMap[&controls::FrameDurationLimits];
> > > +               int64_t minFrameDuration =
> > > +                       std::clamp((*frameDurationLimits).front(),
> > > +                                  limits.min().get<int64_t>(),
> > > +                                  limits.max().get<int64_t>());
> > > +               int64_t maxFrameDuration =
> > > +                       std::clamp((*frameDurationLimits).back(),
> > > +                                  limits.min().get<int64_t>(),
> > > +                                  limits.max().get<int64_t>());
> > > +
> > > +               agc.minFrameDuration = std::chrono::microseconds(minFrameDuration);
> > > +               agc.maxFrameDuration = std::chrono::microseconds(maxFrameDuration);
> > >         }
> > > +       frameContext.agc.minFrameDuration = agc.minFrameDuration;
> > >         frameContext.agc.maxFrameDuration = agc.maxFrameDuration;
> > >  }
> > >  
> > > @@ -387,6 +399,7 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
> > >                                      * frameContext.sensor.exposure;
> > >         metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
> > >         metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> > > +       metadata.set(controls::FrameDuration, frameContext.agc.frameDuration.get<std::micro>());
> 
> (It's here!)
> 
> > >         metadata.set(controls::ExposureTimeMode,
> > >                      frameContext.agc.autoExposureEnabled
> > >                      ? controls::ExposureTimeModeAuto
> > > @@ -396,13 +409,6 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
> > >                      ? controls::AnalogueGainModeAuto
> > >                      : controls::AnalogueGainModeManual);
> > >  
> > > -       /* \todo Use VBlank value calculated from each frame exposure. */
> > > -       uint32_t vTotal = context.configuration.sensor.size.height
> > > -                       + context.configuration.sensor.defVBlank;
> > > -       utils::Duration frameDuration = context.configuration.sensor.lineDuration
> > > -                                     * vTotal;
> > > -       metadata.set(controls::FrameDuration, frameDuration.get<std::micro>());
> > 
> > Am I missing where the FrameDuration is now reported in the metadata ?
> > or has it been removed completely.
> > 
> > I think we should still be reporting the FrameDuration which has been
> > determined. It should come from
> >   frameContext.agc.frameDuration.get<std::micro>()
> > 
> > now shouldn't it ?
> 
> (Yes)

Aha, sorry - I did miss it!


Then, 

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>



> 
> 
> (Paul)
> 
> > 
> > 
> > 
> > > -
> > >         metadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);
> > >         metadata.set(controls::AeExposureMode, frameContext.agc.exposureMode);
> > >         metadata.set(controls::AeConstraintMode, frameContext.agc.constraintMode);
> > > @@ -444,6 +450,27 @@ double Agc::estimateLuminance(double gain) const
> > >         return ySum / expMeans_.size() / 255;
> > >  }
> > >  
> > > +/**
> > > + * \brief Process frame duration and compute vblank
> > > + * \param[in] context The shared IPA context
> > > + * \param[in] frameContext The current frame context
> > > + * \param[in] frameDuration The target frame duration
> > > + *
> > > + * Compute and populate vblank from the target frame duration.
> > > + */
> > > +void Agc::processFrameDuration(IPAContext &context,
> > > +                              IPAFrameContext &frameContext,
> > > +                              utils::Duration frameDuration)
> > > +{
> > > +       IPACameraSensorInfo &sensorInfo = context.sensorInfo;
> > > +       utils::Duration lineDuration = context.configuration.sensor.lineDuration;
> > > +
> > > +       frameContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;
> > > +
> > > +       /* Update frame duration accounting for line length quantization. */
> > > +       frameContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;
> > > +}
> > > +
> > >  /**
> > >   * \brief Process RkISP1 statistics, and run AGC operations
> > >   * \param[in] context The shared IPA context
> > > @@ -460,6 +487,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > >                   ControlList &metadata)
> > >  {
> > >         if (!stats) {
> > > +               processFrameDuration(context, frameContext,
> > > +                                    frameContext.agc.minFrameDuration);
> > >                 fillMetadata(context, frameContext, metadata);
> > >                 return;
> > >         }
> > > @@ -497,7 +526,9 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > >         double maxAnalogueGain;
> > >  
> > >         if (frameContext.agc.autoExposureEnabled) {
> > > -               minExposureTime = context.configuration.sensor.minExposureTime;
> > > +               minExposureTime = std::clamp(frameContext.agc.minFrameDuration,
> > > +                                            context.configuration.sensor.minExposureTime,
> > > +                                            context.configuration.sensor.maxExposureTime);
> > >                 maxExposureTime = std::clamp(frameContext.agc.maxFrameDuration,
> > >                                              context.configuration.sensor.minExposureTime,
> > >                                              context.configuration.sensor.maxExposureTime);
> > > @@ -541,6 +572,13 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > >         activeState.agc.automatic.exposure = newExposureTime / lineDuration;
> > >         activeState.agc.automatic.gain = aGain;
> > >  
> > > +       /*
> > > +        * 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));
> > > +
> > >         fillMetadata(context, frameContext, metadata);
> > >         expMeans_ = {};
> > >  }
> > > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> > > index aa86f2c5bc21..62bcde999fe3 100644
> > > --- a/src/ipa/rkisp1/algorithms/agc.h
> > > +++ b/src/ipa/rkisp1/algorithms/agc.h
> > > @@ -50,6 +50,9 @@ private:
> > >         void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
> > >                           ControlList &metadata);
> > >         double estimateLuminance(double gain) const override;
> > > +       void processFrameDuration(IPAContext &context,
> > > +                                 IPAFrameContext &frameContext,
> > > +                                 utils::Duration frameDuration);
> > >  
> > >         Span<const uint8_t> expMeans_;
> > >  
> > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > > index 261c0472a4fc..99611bd5b390 100644
> > > --- a/src/ipa/rkisp1/ipa_context.cpp
> > > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > > @@ -180,6 +180,9 @@ namespace libcamera::ipa::rkisp1 {
> > >   * \var IPAActiveState::agc.meteringMode
> > >   * \brief Metering mode as set by the AeMeteringMode control
> > >   *
> > > + * \var IPAActiveState::agc.minFrameDuration
> > > + * \brief Minimum frame duration as set by the FrameDurationLimits control
> > > + *
> > >   * \var IPAActiveState::agc.maxFrameDuration
> > >   * \brief Maximum frame duration as set by the FrameDurationLimits control
> > >   */
> > > @@ -282,7 +285,9 @@ namespace libcamera::ipa::rkisp1 {
> > >   * \brief Automatic Gain Control parameters for this frame
> > >   *
> > >   * The exposure and gain are provided by the AGC algorithm, and are to be
> > > - * applied to the sensor in order to take effect for this frame.
> > > + * applied to the sensor in order to take effect for this frame. Additionally
> > > + * the vertical blanking period is determined to maintain a consistent frame
> > > + * rate matched to the FrameDurationLimits as set by the user.
> > >   *
> > >   * \var IPAFrameContext::agc.exposure
> > >   * \brief Exposure time expressed as a number of lines computed by the algorithm
> > > @@ -292,6 +297,9 @@ namespace libcamera::ipa::rkisp1 {
> > >   *
> > >   * The gain should be adapted to the sensor specific gain code before applying.
> > >   *
> > > + * \var IPAFrameContext::agc.vblank
> > > + * \brief Vertical blanking parameter computed by the algorithm
> > > + *
> > >   * \var IPAFrameContext::agc.autoExposureEnabled
> > >   * \brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control
> > >   *
> > > @@ -307,9 +315,15 @@ namespace libcamera::ipa::rkisp1 {
> > >   * \var IPAFrameContext::agc.meteringMode
> > >   * \brief Metering mode as set by the AeMeteringMode control
> > >   *
> > > + * \var IPAFrameContext::agc.minFrameDuration
> > > + * \brief Minimum frame duration as set by the FrameDurationLimits control
> > > + *
> > >   * \var IPAFrameContext::agc.maxFrameDuration
> > >   * \brief Maximum frame duration as set by the FrameDurationLimits control
> > >   *
> > > + * \var IPAFrameContext::agc.frameDuration
> > > + * \brief The actual FrameDuration used by the algorithm for the frame
> > > + *
> > >   * \var IPAFrameContext::agc.updateMetering
> > >   * \brief Indicate if new ISP AGC metering parameters need to be applied
> > >   *
> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > > index c765b928a55f..474f7036f003 100644
> > > --- a/src/ipa/rkisp1/ipa_context.h
> > > +++ b/src/ipa/rkisp1/ipa_context.h
> > > @@ -84,6 +84,7 @@ struct IPAActiveState {
> > >                 controls::AeConstraintModeEnum constraintMode;
> > >                 controls::AeExposureModeEnum exposureMode;
> > >                 controls::AeMeteringModeEnum meteringMode;
> > > +               utils::Duration minFrameDuration;
> > >                 utils::Duration maxFrameDuration;
> > >         } agc;
> > >  
> > > @@ -125,12 +126,15 @@ struct IPAFrameContext : public FrameContext {
> > >         struct {
> > >                 uint32_t exposure;
> > >                 double gain;
> > > +               uint32_t vblank;
> > >                 bool autoExposureEnabled;
> > >                 bool autoGainEnabled;
> > >                 controls::AeConstraintModeEnum constraintMode;
> > >                 controls::AeExposureModeEnum exposureMode;
> > >                 controls::AeMeteringModeEnum meteringMode;
> > > +               utils::Duration minFrameDuration;
> > >                 utils::Duration maxFrameDuration;
> > > +               utils::Duration frameDuration;
> > >                 bool updateMetering;
> > >                 bool autoExposureModeChange;
> > >                 bool autoGainModeChange;
> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > > index 0e761249d27c..7547d2f274f4 100644
> > > --- a/src/ipa/rkisp1/rkisp1.cpp
> > > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > > @@ -453,10 +453,12 @@ void IPARkISP1::setControls(unsigned int frame)
> > >         IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> > >         uint32_t exposure = frameContext.agc.exposure;
> > >         uint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);
> > > +       uint32_t vblank = frameContext.agc.vblank;
> > >  
> > >         ControlList ctrls(sensorControls_);
> > >         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
> > >         ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));
> > > +       ctrls.set(V4L2_CID_VBLANK, static_cast<int32_t>(vblank));
> > >  
> > >         setSensorControls.emit(frame, ctrls);
> > >  }
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index 1ac8d8ae7ed9..52633fe3cb85 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > @@ -1325,6 +1325,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> > >         std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> > >                 { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> > >                 { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> > > +               { V4L2_CID_VBLANK, { 1, false } },
> > >         };
> > >  
> > >         data->delayedCtrls_ =
> > > -- 
> > > 2.39.2
> > >

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
index 5fece545677e..4e0e3734117e 100644
--- a/src/ipa/rkisp1/algorithms/agc.cpp
+++ b/src/ipa/rkisp1/algorithms/agc.cpp
@@ -190,6 +190,7 @@  int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
 
 	/* Limit the frame duration to match current initialisation */
 	ControlInfo &frameDurationLimits = context.ctrlMap[&controls::FrameDurationLimits];
+	context.activeState.agc.minFrameDuration = std::chrono::microseconds(frameDurationLimits.min().get<int64_t>());
 	context.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get<int64_t>());
 
 	/*
@@ -307,10 +308,21 @@  void Agc::queueRequest(IPAContext &context,
 
 	const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
 	if (frameDurationLimits) {
-		utils::Duration maxFrameDuration =
-			std::chrono::milliseconds((*frameDurationLimits).back());
-		agc.maxFrameDuration = maxFrameDuration;
+		/* Limit the control value to the limits in ControlInfo */
+		ControlInfo &limits = context.ctrlMap[&controls::FrameDurationLimits];
+		int64_t minFrameDuration =
+			std::clamp((*frameDurationLimits).front(),
+				   limits.min().get<int64_t>(),
+				   limits.max().get<int64_t>());
+		int64_t maxFrameDuration =
+			std::clamp((*frameDurationLimits).back(),
+				   limits.min().get<int64_t>(),
+				   limits.max().get<int64_t>());
+
+		agc.minFrameDuration = std::chrono::microseconds(minFrameDuration);
+		agc.maxFrameDuration = std::chrono::microseconds(maxFrameDuration);
 	}
+	frameContext.agc.minFrameDuration = agc.minFrameDuration;
 	frameContext.agc.maxFrameDuration = agc.maxFrameDuration;
 }
 
@@ -387,6 +399,7 @@  void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
 				     * frameContext.sensor.exposure;
 	metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
 	metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
+	metadata.set(controls::FrameDuration, frameContext.agc.frameDuration.get<std::micro>());
 	metadata.set(controls::ExposureTimeMode,
 		     frameContext.agc.autoExposureEnabled
 		     ? controls::ExposureTimeModeAuto
@@ -396,13 +409,6 @@  void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
 		     ? controls::AnalogueGainModeAuto
 		     : controls::AnalogueGainModeManual);
 
-	/* \todo Use VBlank value calculated from each frame exposure. */
-	uint32_t vTotal = context.configuration.sensor.size.height
-			+ context.configuration.sensor.defVBlank;
-	utils::Duration frameDuration = context.configuration.sensor.lineDuration
-				      * vTotal;
-	metadata.set(controls::FrameDuration, frameDuration.get<std::micro>());
-
 	metadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);
 	metadata.set(controls::AeExposureMode, frameContext.agc.exposureMode);
 	metadata.set(controls::AeConstraintMode, frameContext.agc.constraintMode);
@@ -444,6 +450,27 @@  double Agc::estimateLuminance(double gain) const
 	return ySum / expMeans_.size() / 255;
 }
 
+/**
+ * \brief Process frame duration and compute vblank
+ * \param[in] context The shared IPA context
+ * \param[in] frameContext The current frame context
+ * \param[in] frameDuration The target frame duration
+ *
+ * Compute and populate vblank from the target frame duration.
+ */
+void Agc::processFrameDuration(IPAContext &context,
+			       IPAFrameContext &frameContext,
+			       utils::Duration frameDuration)
+{
+	IPACameraSensorInfo &sensorInfo = context.sensorInfo;
+	utils::Duration lineDuration = context.configuration.sensor.lineDuration;
+
+	frameContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;
+
+	/* Update frame duration accounting for line length quantization. */
+	frameContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;
+}
+
 /**
  * \brief Process RkISP1 statistics, and run AGC operations
  * \param[in] context The shared IPA context
@@ -460,6 +487,8 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 		  ControlList &metadata)
 {
 	if (!stats) {
+		processFrameDuration(context, frameContext,
+				     frameContext.agc.minFrameDuration);
 		fillMetadata(context, frameContext, metadata);
 		return;
 	}
@@ -497,7 +526,9 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 	double maxAnalogueGain;
 
 	if (frameContext.agc.autoExposureEnabled) {
-		minExposureTime = context.configuration.sensor.minExposureTime;
+		minExposureTime = std::clamp(frameContext.agc.minFrameDuration,
+					     context.configuration.sensor.minExposureTime,
+					     context.configuration.sensor.maxExposureTime);
 		maxExposureTime = std::clamp(frameContext.agc.maxFrameDuration,
 					     context.configuration.sensor.minExposureTime,
 					     context.configuration.sensor.maxExposureTime);
@@ -541,6 +572,13 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 	activeState.agc.automatic.exposure = newExposureTime / lineDuration;
 	activeState.agc.automatic.gain = aGain;
 
+	/*
+	 * 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));
+
 	fillMetadata(context, frameContext, metadata);
 	expMeans_ = {};
 }
diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
index aa86f2c5bc21..62bcde999fe3 100644
--- a/src/ipa/rkisp1/algorithms/agc.h
+++ b/src/ipa/rkisp1/algorithms/agc.h
@@ -50,6 +50,9 @@  private:
 	void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
 			  ControlList &metadata);
 	double estimateLuminance(double gain) const override;
+	void processFrameDuration(IPAContext &context,
+				  IPAFrameContext &frameContext,
+				  utils::Duration frameDuration);
 
 	Span<const uint8_t> expMeans_;
 
diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index 261c0472a4fc..99611bd5b390 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -180,6 +180,9 @@  namespace libcamera::ipa::rkisp1 {
  * \var IPAActiveState::agc.meteringMode
  * \brief Metering mode as set by the AeMeteringMode control
  *
+ * \var IPAActiveState::agc.minFrameDuration
+ * \brief Minimum frame duration as set by the FrameDurationLimits control
+ *
  * \var IPAActiveState::agc.maxFrameDuration
  * \brief Maximum frame duration as set by the FrameDurationLimits control
  */
@@ -282,7 +285,9 @@  namespace libcamera::ipa::rkisp1 {
  * \brief Automatic Gain Control parameters for this frame
  *
  * The exposure and gain are provided by the AGC algorithm, and are to be
- * applied to the sensor in order to take effect for this frame.
+ * applied to the sensor in order to take effect for this frame. Additionally
+ * the vertical blanking period is determined to maintain a consistent frame
+ * rate matched to the FrameDurationLimits as set by the user.
  *
  * \var IPAFrameContext::agc.exposure
  * \brief Exposure time expressed as a number of lines computed by the algorithm
@@ -292,6 +297,9 @@  namespace libcamera::ipa::rkisp1 {
  *
  * The gain should be adapted to the sensor specific gain code before applying.
  *
+ * \var IPAFrameContext::agc.vblank
+ * \brief Vertical blanking parameter computed by the algorithm
+ *
  * \var IPAFrameContext::agc.autoExposureEnabled
  * \brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control
  *
@@ -307,9 +315,15 @@  namespace libcamera::ipa::rkisp1 {
  * \var IPAFrameContext::agc.meteringMode
  * \brief Metering mode as set by the AeMeteringMode control
  *
+ * \var IPAFrameContext::agc.minFrameDuration
+ * \brief Minimum frame duration as set by the FrameDurationLimits control
+ *
  * \var IPAFrameContext::agc.maxFrameDuration
  * \brief Maximum frame duration as set by the FrameDurationLimits control
  *
+ * \var IPAFrameContext::agc.frameDuration
+ * \brief The actual FrameDuration used by the algorithm for the frame
+ *
  * \var IPAFrameContext::agc.updateMetering
  * \brief Indicate if new ISP AGC metering parameters need to be applied
  *
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index c765b928a55f..474f7036f003 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -84,6 +84,7 @@  struct IPAActiveState {
 		controls::AeConstraintModeEnum constraintMode;
 		controls::AeExposureModeEnum exposureMode;
 		controls::AeMeteringModeEnum meteringMode;
+		utils::Duration minFrameDuration;
 		utils::Duration maxFrameDuration;
 	} agc;
 
@@ -125,12 +126,15 @@  struct IPAFrameContext : public FrameContext {
 	struct {
 		uint32_t exposure;
 		double gain;
+		uint32_t vblank;
 		bool autoExposureEnabled;
 		bool autoGainEnabled;
 		controls::AeConstraintModeEnum constraintMode;
 		controls::AeExposureModeEnum exposureMode;
 		controls::AeMeteringModeEnum meteringMode;
+		utils::Duration minFrameDuration;
 		utils::Duration maxFrameDuration;
+		utils::Duration frameDuration;
 		bool updateMetering;
 		bool autoExposureModeChange;
 		bool autoGainModeChange;
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 0e761249d27c..7547d2f274f4 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -453,10 +453,12 @@  void IPARkISP1::setControls(unsigned int frame)
 	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
 	uint32_t exposure = frameContext.agc.exposure;
 	uint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);
+	uint32_t vblank = frameContext.agc.vblank;
 
 	ControlList ctrls(sensorControls_);
 	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
 	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));
+	ctrls.set(V4L2_CID_VBLANK, static_cast<int32_t>(vblank));
 
 	setSensorControls.emit(frame, ctrls);
 }
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 1ac8d8ae7ed9..52633fe3cb85 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1325,6 +1325,7 @@  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 	std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
 		{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
 		{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
+		{ V4L2_CID_VBLANK, { 1, false } },
 	};
 
 	data->delayedCtrls_ =