[libcamera-devel] [PATCH 17/22] ipa: ipu3: Use a IPAFrameContext pointer from the per-frame queue
Jacopo Mondi
jacopo at jmondi.org
Mon Nov 8 15:10:43 CET 2021
Hi Jean-Michel
On Mon, Nov 08, 2021 at 02:13:45PM +0100, Jean-Michel Hautbois wrote:
> We have one frame context shared between the algorithms thanks to a
> local cached context_ variable in IPAIPU3. Now that we can store the
> frame contexts in a queue, implement all the needed functions for that
> and convert the frame context to a pointer.
>
> The algorithm are now using the values applied on the frame they are
> processing, and not the latest one.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
> src/ipa/ipu3/algorithms/agc.cpp | 12 ++++----
> src/ipa/ipu3/algorithms/agc.h | 2 +-
> src/ipa/ipu3/algorithms/awb.cpp | 14 ++++-----
> src/ipa/ipu3/algorithms/tone_mapping.cpp | 8 ++---
> src/ipa/ipu3/ipa_context.h | 2 +-
> src/ipa/ipu3/ipu3.cpp | 38 +++++++++++++++++++-----
> 6 files changed, 49 insertions(+), 27 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 52cf2753..2eee5b6b 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -245,7 +245,7 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain, double curre
> * \param[in] stats The IPU3 statistics and ISP results
> * \param[in] currentYGain The gain calculated on the current brightness level
> */
> -double Agc::computeInitialY(IPAFrameContext &frameContext,
> +double Agc::computeInitialY(IPAFrameContext *frameContext,
> const ipu3_uapi_grid_config &grid,
> const ipu3_uapi_stats_3a *stats,
> double currentYGain)
> @@ -271,9 +271,9 @@ double Agc::computeInitialY(IPAFrameContext &frameContext,
> * Estimate the sum of the brightness values, weighted with the gains
> * applied on the channels in AWB.
> */
> - double Y_sum = redSum * frameContext.awb.gains.red * .299 +
> - greenSum * frameContext.awb.gains.green * .587 +
> - blueSum * frameContext.awb.gains.blue * .114;
> + double Y_sum = redSum * frameContext->awb.gains.red * .299 +
> + greenSum * frameContext->awb.gains.green * .587 +
> + blueSum * frameContext->awb.gains.blue * .114;
>
> /* And return the average brightness */
> return Y_sum / (grid.height * grid.width);
> @@ -291,8 +291,8 @@ double Agc::computeInitialY(IPAFrameContext &frameContext,
> void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> {
> /* Get the latest exposure and gain applied */
> - uint32_t &exposure = context.frameContext.agc.exposure;
> - double &analogueGain = context.frameContext.agc.gain;
> + uint32_t &exposure = context.frameContext->agc.exposure;
> + double &analogueGain = context.frameContext->agc.gain;
> measureBrightness(stats, context.configuration.grid.bdsGrid);
>
> double currentYGain = 1.0;
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 6f5d71e0..5d6bef9d 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -35,7 +35,7 @@ private:
> const ipu3_uapi_grid_config &grid);
> void filterExposure();
> void computeExposure(uint32_t &exposure, double &gain, double currentYGain);
> - double computeInitialY(IPAFrameContext &frameContext,
> + double computeInitialY(IPAFrameContext *frameContext,
> const ipu3_uapi_grid_config &grid,
> const ipu3_uapi_stats_3a *stats,
> double currentYGain);
> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> index a4114659..bd55d377 100644
> --- a/src/ipa/ipu3/algorithms/awb.cpp
> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> @@ -382,9 +382,9 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> * The results are cached, so if no results were calculated, we set the
> * cached values from asyncResults_ here.
> */
> - context.frameContext.awb.gains.blue = asyncResults_.blueGain;
> - context.frameContext.awb.gains.green = asyncResults_.greenGain;
> - context.frameContext.awb.gains.red = asyncResults_.redGain;
> + context.frameContext->awb.gains.blue = asyncResults_.blueGain;
> + context.frameContext->awb.gains.green = asyncResults_.greenGain;
> + context.frameContext->awb.gains.red = asyncResults_.redGain;
> }
>
> constexpr uint16_t Awb::threshold(float value)
> @@ -432,10 +432,10 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)
> params->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset
> * params->acc_param.bnr.opt_center.y_reset;
> /* Convert to u3.13 fixed point values */
> - params->acc_param.bnr.wb_gains.gr = 8192 * context.frameContext.awb.gains.green;
> - params->acc_param.bnr.wb_gains.r = 8192 * context.frameContext.awb.gains.red;
> - params->acc_param.bnr.wb_gains.b = 8192 * context.frameContext.awb.gains.blue;
> - params->acc_param.bnr.wb_gains.gb = 8192 * context.frameContext.awb.gains.green;
> + params->acc_param.bnr.wb_gains.gr = 8192 * context.frameContext->awb.gains.green;
> + params->acc_param.bnr.wb_gains.r = 8192 * context.frameContext->awb.gains.red;
> + params->acc_param.bnr.wb_gains.b = 8192 * context.frameContext->awb.gains.blue;
> + params->acc_param.bnr.wb_gains.gb = 8192 * context.frameContext->awb.gains.green;
>
> LOG(IPU3Awb, Debug) << "Color temperature estimated: " << asyncResults_.temperatureK;
>
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> index 5d74c552..50498f41 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -57,7 +57,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,
> {
> /* Copy the calculated LUT into the parameters buffer. */
> memcpy(params->acc_param.gamma.gc_lut.lut,
> - context.frameContext.toneMapping.gammaCorrection.lut,
> + context.frameContext->toneMapping.gammaCorrection.lut,
> IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES *
> sizeof(params->acc_param.gamma.gc_lut.lut[0]));
>
> @@ -84,11 +84,11 @@ void ToneMapping::process(IPAContext &context,
> */
> gamma_ = 1.1;
>
> - if (context.frameContext.toneMapping.gamma == gamma_)
> + if (context.frameContext->toneMapping.gamma == gamma_)
> return;
>
> struct ipu3_uapi_gamma_corr_lut &lut =
> - context.frameContext.toneMapping.gammaCorrection;
> + context.frameContext->toneMapping.gammaCorrection;
>
> for (uint32_t i = 0; i < std::size(lut.lut); i++) {
> double j = static_cast<double>(i) / (std::size(lut.lut) - 1);
> @@ -98,7 +98,7 @@ void ToneMapping::process(IPAContext &context,
> lut.lut[i] = gamma * 8191;
> }
>
> - context.frameContext.toneMapping.gamma = gamma_;
> + context.frameContext->toneMapping.gamma = gamma_;
> }
>
> } /* namespace ipa::ipu3::algorithms */
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index ee8f7b55..69780915 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -57,7 +57,7 @@ struct IPAFrameContext {
>
> struct IPAContext {
> IPASessionConfiguration configuration;
> - IPAFrameContext frameContext;
> + IPAFrameContext *frameContext;
> };
>
> } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 177c5c2f..b60ab7e7 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -522,10 +522,26 @@ void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
>
> void IPAIPU3::frameStarted([[maybe_unused]] unsigned int frame)
> {
> + /*
> + * Instantiate a new IPAFrameContext to push to the queue. The lifetime
> + * of this pointer is indirectly controlled by the
> + * ActionSetSensorControls event, as this is where we will delete it.
> + */
> + struct IPAFrameContext *frameContext = new IPAFrameContext;
> + frameContext->frameId = frame;
> + frameContextQueue.push(frameContext);
Maybe a detail:
Wouldn't it be safer to store unique_ptr<IPAFrameContext> in the queue
instead of manually allocate/free ?
> }
>
> void IPAIPU3::frameCompleted([[maybe_unused]] unsigned int frame)
> {
> + /*
> + * Remove the pointer from the queue, it should not be accessed
> + * anymore and delete it.
> + */
> + struct IPAFrameContext *frameContext = frameContextQueue.front();
> + ASSERT(frameContext->frameId == frame);
> + frameContextQueue.pop();
> + delete frameContext;
> }
>
> /**
> @@ -579,10 +595,6 @@ void IPAIPU3::processEvent(const IPU3Event &event)
> const ipu3_uapi_stats_3a *stats =
> reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
>
> - /* \todo move those into processControls */
> - context_.frameContext.agc.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> - context_.frameContext.agc.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> -
> parseStatistics(event.frame, event.frameTimestamp, stats);
> /*
> * To save incurring extra IPC calls, we do not send explicit events
> @@ -610,9 +622,12 @@ void IPAIPU3::processEvent(const IPU3Event &event)
> */
> void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,
> [[maybe_unused]] const ControlList &controls,
> - [[maybe_unused]] const ControlList &sensorCtrls)
> + const ControlList &sensorCtrls)
> {
> - /* \todo Start processing for 'frame' based on 'controls'. */
> + struct IPAFrameContext *frameContext = frameContextQueue.back();
> + ASSERT(frameContext->frameId == frame);
> + frameContext->agc.exposure = sensorCtrls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> + frameContext->agc.gain = camHelper_->gain(sensorCtrls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> }
>
> /**
> @@ -636,6 +651,9 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> */
> params->use = {};
>
> + /* Set the reference to the current frame context */
> + context_.frameContext = frameContextQueue.front();
> +
> for (auto const &algo : algorithms_)
> algo->prepare(context_, params);
>
> @@ -661,6 +679,9 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> {
> ControlList ctrls(controls::controls);
>
> + /* Set the reference to the current frame context */
> + context_.frameContext = frameContextQueue.front();
> +
> for (auto const &algo : algorithms_)
> algo->process(context_, stats);
>
> @@ -690,8 +711,9 @@ void IPAIPU3::setControls(unsigned int frame)
> IPU3Action op;
> op.op = ActionSetSensorControls;
>
> - exposure_ = context_.frameContext.agc.exposure;
> - gain_ = camHelper_->gainCode(context_.frameContext.agc.gain);
> + /* Apply the exposure and gain updated values */
> + exposure_ = context_.frameContext->agc.exposure;
> + gain_ = camHelper_->gainCode(context_.frameContext->agc.gain);
>
> ControlList ctrls(ctrls_);
> ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
> --
> 2.32.0
>
More information about the libcamera-devel
mailing list