[libcamera-devel] [PATCH v5 20/33] ipa: rkisp1: agc: Store per-frame information in frame context
Jacopo Mondi
jacopo at jmondi.org
Tue Sep 27 11:19:52 CEST 2022
On Tue, Sep 27, 2022 at 05:36:29AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Rework the algorithm's usage of the active state to store the value of
> controls for the last queued request in the queueRequest() function, and
> store a copy of the values in the corresponding frame context.
>
> The frame context is used in the prepare() function to populate the ISP
> parameters with values corresponding to the right frame.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> Changes since v4:
>
> - Add todo comments related to sensor controls
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> ---
> src/ipa/rkisp1/algorithms/agc.cpp | 27 +++++++++++++------
> src/ipa/rkisp1/algorithms/agc.h | 3 ++-
> src/ipa/rkisp1/ipa_context.cpp | 43 ++++++++++++++++++++-----------
> src/ipa/rkisp1/ipa_context.h | 14 ++++++----
> src/ipa/rkisp1/rkisp1.cpp | 14 +++++++---
> 5 files changed, 68 insertions(+), 33 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 87fc5d1ffec7..4540bde66db4 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -144,17 +144,19 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
> /**
> * \brief Estimate the new exposure and gain values
> * \param[inout] context The shared IPA Context
> + * \param[in] frameContext The FrameContext for this frame
> * \param[in] yGain The gain calculated on the current brightness level
> * \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)
> {
> IPASessionConfiguration &configuration = context.configuration;
> IPAActiveState &activeState = context.activeState;
>
> /* Get the effective exposure and gain applied on the sensor. */
> - uint32_t exposure = activeState.sensor.exposure;
> - double analogueGain = activeState.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);
> @@ -286,9 +288,16 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const
> * new exposure and gain for the scene.
> */
> void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> - [[maybe_unused]] IPAFrameContext &frameContext,
> - const rkisp1_stat_buffer *stats)
> + IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats)
> {
> + /*
> + * \todo Verify that the exposure and gain applied by the sensor for
> + * this frame match what has been requested. This isn't a hard
> + * requirement for stability of the AGC (the guarantee we need in
> + * automatic mode is a perfect match between the frame and the values
> + * we receive), but is important in manual mode.
> + */
> +
> const rkisp1_cif_isp_stat *params = &stats->params;
> ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);
>
> @@ -320,7 +329,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> break;
> }
>
> - computeExposure(context, yGain, iqMeanGain);
> + computeExposure(context, frameContext, yGain, iqMeanGain);
> frameCount_++;
> }
>
> @@ -328,9 +337,11 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> * \copydoc libcamera::ipa::Algorithm::prepare
> */
> void Agc::prepare(IPAContext &context, const uint32_t frame,
> - [[maybe_unused]] IPAFrameContext &frameContext,
> - rkisp1_params_cfg *params)
> + IPAFrameContext &frameContext, rkisp1_params_cfg *params)
> {
> + frameContext.agc.exposure = context.activeState.agc.exposure;
> + frameContext.agc.gain = context.activeState.agc.gain;
> +
> if (frame > 0)
> return;
>
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index f115ba2ed85c..9ad5c32fd6f6 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -34,7 +34,8 @@ public:
> const rkisp1_stat_buffer *stats) override;
>
> private:
> - void computeExposure(IPAContext &Context, double yGain, double iqMeanGain);
> + void computeExposure(IPAContext &Context, IPAFrameContext &frameContext,
> + double yGain, double iqMeanGain);
> utils::Duration filterExposure(utils::Duration exposureValue);
> double estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);
> double measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const;
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 78a785f5f982..c7d5b1b6ec5a 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -104,16 +104,13 @@ namespace libcamera::ipa::rkisp1 {
> * \var IPAActiveState::agc
> * \brief State for the Automatic Gain Control algorithm
> *
> - * The exposure and gain determined are expected to be applied to the sensor
> - * at the earliest opportunity.
> + * The exposure and gain are the latest values computed by the AGC algorithm.
> *
> * \var IPAActiveState::agc.exposure
> * \brief Exposure time expressed as a number of lines
> *
> * \var IPAActiveState::agc.gain
> * \brief Analogue gain multiplier
> - *
> - * The gain should be adapted to the sensor specific gain code before applying.
> */
>
> /**
> @@ -181,17 +178,6 @@ namespace libcamera::ipa::rkisp1 {
> * \brief Indicates if ISP parameters need to be updated
> */
>
> -/**
> - * \var IPAActiveState::sensor
> - * \brief Effective sensor values
> - *
> - * \var IPAActiveState::sensor.exposure
> - * \brief Exposure time expressed as a number of lines
> - *
> - * \var IPAActiveState::sensor.gain
> - * \brief Analogue gain multiplier
> - */
> -
> /**
> * \struct IPAFrameContext
> * \brief Per-frame context for algorithms
> @@ -199,6 +185,33 @@ namespace libcamera::ipa::rkisp1 {
> * \todo Populate the frame context for all algorithms
> */
>
> +/**
> + * \var IPAFrameContext::agc
> + * \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.
> + *
> + * \var IPAFrameContext::agc.exposure
> + * \brief Exposure time expressed as a number of lines
> + *
> + * \var IPAFrameContext::agc.gain
> + * \brief Analogue gain multiplier
> + *
> + * The gain should be adapted to the sensor specific gain code before applying.
> + */
> +
> +/**
> + * \var IPAFrameContext::sensor
> + * \brief Sensor configuration that used been used for this frame
> + *
> + * \var IPAFrameContext::sensor.exposure
> + * \brief Exposure time expressed as a number of lines
> + *
> + * \var IPAFrameContext::sensor.gain
> + * \brief Analogue gain multiplier
> + */
> +
> /**
> * \struct IPAContext
> * \brief Global IPA context data shared between all algorithms
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index c7041ea2a214..a4d134e700b5 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -83,14 +83,18 @@ struct IPAActiveState {
> uint8_t sharpness;
> bool updateParams;
> } filter;
> -
> - struct {
> - uint32_t exposure;
> - double gain;
> - } sensor;
> };
>
> struct IPAFrameContext : public FrameContext {
> + struct {
> + uint32_t exposure;
> + double gain;
> + } agc;
> +
> + struct {
> + uint32_t exposure;
> + double gain;
> + } sensor;
> };
>
> static constexpr uint32_t kMaxFrameContexts = 16;
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 5409c2d5219e..6297916cf86d 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -332,9 +332,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
> reinterpret_cast<rkisp1_stat_buffer *>(
> mappedBuffers_.at(bufferId).planes()[0].data());
>
> - context_.activeState.sensor.exposure =
> + frameContext.sensor.exposure =
> sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> - context_.activeState.sensor.gain =
> + frameContext.sensor.gain =
> camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>
> unsigned int aeState = 0;
> @@ -349,8 +349,14 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
>
> void IPARkISP1::setControls(unsigned int frame)
> {
> - uint32_t exposure = context_.activeState.agc.exposure;
> - uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);
> + /*
> + * \todo The frame number is most likely wrong here, we need to take
> + * internal sensor delays and other timing parameters into account.
> + */
> +
> + IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> + uint32_t exposure = frameContext.agc.exposure;
> + uint32_t gain = camHelper_->gainCode(frameContext.agc.gain);
>
> ControlList ctrls(ctrls_);
> ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list