[PATCH 3/3] rkisp1: Honor the FrameDurationLimits control
Umang Jain
umang.jain at ideasonboard.com
Thu Oct 24 05:58:58 CEST 2024
Hi Kieran, Paul
Thank you for the patch.
On 14/10/24 9:17 pm, Kieran Bingham wrote:
> From: Paul Elder <paul.elder at ideasonboard.com>
>
> 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>
> ---
> src/ipa/rkisp1/algorithms/agc.cpp | 52 ++++++++++++++++++------
> 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 | 7 ++++
> 5 files changed, 68 insertions(+), 13 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index e23ab120b3e2..088ecf42c480 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -180,6 +180,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>());
>
> /*
> @@ -267,10 +268,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;
> }
>
> @@ -330,15 +342,8 @@ 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::AeEnable, frameContext.agc.autoEnabled);
> -
> - /* \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);
> @@ -400,6 +405,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> return;
> }
>
> + IPACameraSensorInfo &sensorInfo = context.sensorInfo;
> utils::Duration lineDuration = context.configuration.sensor.lineDuration;
>
> /*
> @@ -418,11 +424,19 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> [](uint32_t x) { return x >> 4; });
> expMeans_ = { params->ae.exp_mean, context.hw->numAeCells };
>
> + /*
> + * Limit the allowed shutter speeds for the exposure helper based on
> + * the frame duration limits.
> + */
> + utils::Duration minShutterSpeed =
> + std::clamp(frameContext.agc.minFrameDuration,
> + context.configuration.sensor.minShutterSpeed,
> + context.configuration.sensor.maxShutterSpeed);
> utils::Duration maxShutterSpeed =
> std::clamp(frameContext.agc.maxFrameDuration,
> context.configuration.sensor.minShutterSpeed,
> context.configuration.sensor.maxShutterSpeed);
> - setLimits(context.configuration.sensor.minShutterSpeed,
> + setLimits(minShutterSpeed,
> maxShutterSpeed,
> context.configuration.sensor.minAnalogueGain,
> context.configuration.sensor.maxAnalogueGain);
> @@ -451,6 +465,20 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> activeState.agc.automatic.exposure = shutterTime / lineDuration;
> activeState.agc.automatic.gain = aGain;
>
> + /*
> + * Determine what our FrameDuration must be and adapt VBLANK to suit
> + * this if we need to expand the shutter time calculated to fill the
> + * remaining time so that we do not run faster than the minimum frame
> + * duration (maximum requested frame rate) when we have short exposures.
> + */
Feels like this should be broken down to atleast two sentences.
+ /*
+ * Determine what our FrameDuration must be and adapt VBLANK to suit
+ * it. If we need to expand the shutter time, calculate VBLANK to fill the
+ * remaining time so that we do not run faster than the minimum frame
+ * duration (maximum requested frame rate) when we have short exposures.
+ */
Other than this,
Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> + utils::Duration frameDuration = std::max(frameContext.agc.minFrameDuration,
> + shutterTime);
> +
> + frameContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;
> +
> + /* Update accounting for line length quantization. */
> + frameContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;
> +
> fillMetadata(context, frameContext, metadata);
> expMeans_ = {};
> }
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 14d0c02a2b32..c5eb0d64ec29 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -177,6 +177,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
> */
> @@ -297,7 +300,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
> @@ -307,6 +312,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.autoEnabled
> * \brief Manual/automatic AGC state as set by the AeEnable control
> *
> @@ -319,9 +327,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 e274d9b01e1c..60c4d647f1ef 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -79,6 +79,7 @@ struct IPAActiveState {
> controls::AeConstraintModeEnum constraintMode;
> controls::AeExposureModeEnum exposureMode;
> controls::AeMeteringModeEnum meteringMode;
> + utils::Duration minFrameDuration;
> utils::Duration maxFrameDuration;
> } agc;
>
> @@ -128,11 +129,14 @@ struct IPAFrameContext : public FrameContext {
> struct {
> uint32_t exposure;
> double gain;
> + uint32_t vblank;
> bool autoEnabled;
> controls::AeConstraintModeEnum constraintMode;
> controls::AeExposureModeEnum exposureMode;
> controls::AeMeteringModeEnum meteringMode;
> + utils::Duration minFrameDuration;
> utils::Duration maxFrameDuration;
> + utils::Duration frameDuration;
> bool updateMetering;
> } agc;
>
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 47777ece783f..17d56fb4e901 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -449,10 +449,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 42961c120036..1ec12185bb78 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -394,6 +394,12 @@ void RkISP1CameraData::paramFilled(unsigned int frame, unsigned int bytesused)
> void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
> const ControlList &sensorControls)
> {
> + if (frame == 0) {
> + ControlList controls = sensorControls;
> + sensor_->setControls(&controls);
> + return;
> + }
> +
> delayedCtrls_->push(sensorControls);
> }
>
> @@ -1138,6 +1144,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> { V4L2_CID_ANALOGUE_GAIN, { 1, false } },
> { V4L2_CID_EXPOSURE, { 2, false } },
> + { V4L2_CID_VBLANK, { 1, false } },
> };
>
> data->delayedCtrls_ =
More information about the libcamera-devel
mailing list