[libcamera-devel] [PATCH v3 3/3] ipa: ipu3: Put IPAFrameContext(s) in a ring buffer
Jacopo Mondi
jacopo at jmondi.org
Wed May 18 09:47:26 CEST 2022
Hi Umang,
On Tue, May 17, 2022 at 09:18:33PM +0200, Umang Jain via libcamera-devel wrote:
> Instead of having one frame context constantly being updated,
> this patch aims to introduce per-frame IPAFrameContext which
> are stored in a ring buffer. Whenever a request is queued, a new
> IPAFrameContext is created and inserted into the ring buffer.
>
> The IPAFrameContext structure itself has been slightly extended
> to store a frame id and a ControlList for incoming frame
> controls (sent in by the application). The next step would be to
> read and set these controls whenever the request is actually queued
> to the hardware.
>
> Since now we are working in multiples of IPAFrameContext, the
> Algorithm::process() will actually take in a IPAFrameContext pointer
> (as opposed to a nullptr while preparing for this change).
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/ipa/ipu3/algorithms/agc.cpp | 11 +++++------
> src/ipa/ipu3/algorithms/agc.h | 4 ++--
> src/ipa/ipu3/ipa_context.cpp | 24 ++++++++++++++++++++++--
> src/ipa/ipu3/ipa_context.h | 14 +++++++++++++-
> src/ipa/ipu3/ipu3.cpp | 22 +++++++++++++---------
> 5 files changed, 55 insertions(+), 20 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 383a8deb..f16be534 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -183,14 +183,13 @@ 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(IPAContext &context, double yGain,
> - double iqMeanGain)
> +void Agc::computeExposure(IPAContext &context, IPAFrameContext *frameContext,
> + 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;
> + uint32_t exposure = frameContext->sensor.exposure;
> + double analogueGain = frameContext->sensor.gain;
>
> /* Use the highest of the two gain estimates. */
> double evGain = std::max(yGain, iqMeanGain);
> @@ -360,7 +359,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameCo
> break;
> }
>
> - computeExposure(context, yGain, iqMeanGain);
> + computeExposure(context, frameContext, yGain, iqMeanGain);
> frameCount_++;
> }
>
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 219a1a96..105ae0f2 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -35,8 +35,8 @@ private:
> double measureBrightness(const ipu3_uapi_stats_3a *stats,
> const ipu3_uapi_grid_config &grid) const;
> utils::Duration filterExposure(utils::Duration currentExposure);
> - void computeExposure(IPAContext &context, double yGain,
> - double iqMeanGain);
> + void computeExposure(IPAContext &context, IPAFrameContext *frameContext,
> + double yGain, double iqMeanGain);
> double estimateLuminance(IPAActiveState &activeState,
> const ipu3_uapi_grid_config &grid,
> const ipu3_uapi_stats_3a *stats,
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 06eb2776..81b30600 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -58,8 +58,8 @@ namespace libcamera::ipa::ipu3 {
> * \var IPAContext::configuration
> * \brief The IPA session configuration, immutable during the session
> *
> - * \var IPAContext::frameContext
> - * \brief The frame context for the frame being processed
> + * \var IPAContext::frameContexts
> + * \brief Ring buffer of the IPAFrameContext(s)
> *
> * \var IPAContext::activeState
> * \brief The current state of IPA algorithms
> @@ -183,6 +183,26 @@ namespace libcamera::ipa::ipu3 {
> */
>
> /**
> + * \brief Default constructor for IPAFrameContext
> + */
> +IPAFrameContext::IPAFrameContext() = default;
> +
> +/**
> + * \brief Construct a IPAFrameContext instance
> + */
> +IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)
> + : frame(id), frameControls(reqControls)
> +{
> + sensor = {};
> +}
> +
> +/**
> + * \var IPAFrameContext::frame
> + * \brief The frame number
> + *
> + * \var IPAFrameContext::frameControls
> + * \brief Controls sent in by the application while queuing the request
> + *
> * \var IPAFrameContext::sensor
> * \brief Effective sensor values that were applied for the frame
> *
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 8d681131..42e11141 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -8,16 +8,22 @@
>
> #pragma once
>
> +#include <array>
> +
> #include <linux/intel-ipu3.h>
>
> #include <libcamera/base/utils.h>
>
> +#include <libcamera/controls.h>
> #include <libcamera/geometry.h>
>
> namespace libcamera {
>
> namespace ipa::ipu3 {
>
> +/* Maximum number of frame contexts to be held */
> +static constexpr uint32_t kMaxFrameContexts = 16;
> +
> struct IPASessionConfiguration {
> struct {
> ipu3_uapi_grid_config bdsGrid;
> @@ -71,17 +77,23 @@ struct IPAActiveState {
> };
>
> struct IPAFrameContext {
> + IPAFrameContext();
> + IPAFrameContext(uint32_t id, const ControlList &reqControls);
> +
> struct {
> uint32_t exposure;
> double gain;
> } sensor;
> +
> + uint32_t frame;
> + ControlList frameControls;
> };
>
> struct IPAContext {
> IPASessionConfiguration configuration;
> IPAActiveState activeState;
>
> - IPAFrameContext frameContext;
> + std::array<IPAFrameContext, kMaxFrameContexts> frameContexts;
> };
>
> } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 16e5028f..4322f96b 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -313,7 +313,7 @@ int IPAIPU3::init(const IPASettings &settings,
> }
>
> /* Clean context */
> - context_ = {};
> + context_.configuration = {};
> context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;
>
> /* Construct our Algorithms */
> @@ -456,7 +456,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>
> /* Clean IPAActiveState at each reconfiguration. */
> context_.activeState = {};
> - context_.frameContext = {};
> + IPAFrameContext initFrameContext;
> + context_.frameContexts.fill(initFrameContext);
>
> if (!validateSensorControls()) {
> LOG(IPAIPU3, Error) << "Sensor control validation failed.";
> @@ -568,15 +569,17 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
> const ipu3_uapi_stats_3a *stats =
> reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
>
> - context_.frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> - context_.frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> + IPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];
> +
> + frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> + frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>
> double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
> int32_t vBlank = context_.configuration.sensor.defVBlank;
> ControlList ctrls(controls::controls);
>
> for (auto const &algo : algorithms_)
> - algo->process(context_, nullptr, stats);
> + algo->process(context_, &frameContext, stats);
>
> setControls(frame);
>
> @@ -584,11 +587,11 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
> int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) * lineDuration;
> ctrls.set(controls::FrameDuration, frameDuration);
>
> - ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);
> + ctrls.set(controls::AnalogueGain, frameContext.sensor.gain);
>
> ctrls.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);
>
> - ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);
> + ctrls.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);
>
> /*
> * \todo The Metadata provides a path to getting extended data
> @@ -609,10 +612,11 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
> * Parse the request to handle any IPA-managed controls that were set from the
> * application such as manual sensor settings.
> */
> -void IPAIPU3::queueRequest([[maybe_unused]] const uint32_t frame,
> - [[maybe_unused]] const ControlList &controls)
> +void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)
> {
> /* \todo Start processing for 'frame' based on 'controls'. */
> + IPAFrameContext newFrameContext(frame, controls);
> + context_.frameContexts[frame % kMaxFrameContexts] = newFrameContext;
Just a little note, which now is not that relevant as we have a
limited number of controls, but this goes through at least 2 copies a
the 'controls' list.
Constructor:
IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)
: frame(id), frameControls(reqControls)
And the implicitely defined copy assignment operator.
I wonder if
context_.frameContexts[frame % kMaxFrameContexts] = {frame, controls};
would spare a copy
The rest looks good to me
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> }
>
> /**
> --
> 2.31.0
>
More information about the libcamera-devel
mailing list