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( mappedBuffers_.at(bufferId).planes()[0].data()); - context_.activeState.sensor.exposure = + frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get(); - context_.activeState.sensor.gain = + frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get()); 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(exposure));