[PATCH v2 3/3] rkisp1: Honor the FrameDurationLimits control
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Feb 24 15:53:26 CET 2025
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 at ideasonboard.com>
> > > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > Reviewed-by: Umang Jain <umang.jain at 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 at 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
> > >
More information about the libcamera-devel
mailing list