From 906373331f1bc1a0411a48ef5ae3d1d54c19122c Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Thu, 21 Jul 2022 13:13:03 +0100 Subject: ipa: rkisp1: Rename frameContext to activeState The RkISP1 IPA module creates a single instance of its IPAFrameContext structure, effectively using it more as an active state than a per-frame context. To prepare for the introduction of a real per-frame context, move all the members of the IPAFrameContext structure to a new IPAActiveState structure. The IPAFrameContext becomes effectively unused at runtime, and will be populated back with per-frame data after converting the RkISP1 IPA module to using a frame context queue. The IPAActiveState structure will slowly morph into a different entity as individual algorithm get later ported to the frame context API. While at it, fix a typo in the documentation of the Agc::computeExposure() function that incorrectly refers to the frame context instead of the global context. Signed-off-by: Kieran Bingham Reviewed-by: Umang Jain Reviewed-by: Jacopo Mondi Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham Reviewed-by: Jacopo Mondi --- src/ipa/rkisp1/algorithms/agc.cpp | 23 +++++---- src/ipa/rkisp1/algorithms/awb.cpp | 40 +++++++-------- src/ipa/rkisp1/algorithms/blc.cpp | 2 +- src/ipa/rkisp1/algorithms/cproc.cpp | 4 +- src/ipa/rkisp1/algorithms/dpcc.cpp | 2 +- src/ipa/rkisp1/algorithms/dpf.cpp | 6 +-- src/ipa/rkisp1/algorithms/filter.cpp | 4 +- src/ipa/rkisp1/algorithms/gsl.cpp | 2 +- src/ipa/rkisp1/algorithms/lsc.cpp | 2 +- src/ipa/rkisp1/ipa_context.cpp | 94 +++++++++++++++++++----------------- src/ipa/rkisp1/ipa_context.h | 7 ++- src/ipa/rkisp1/rkisp1.cpp | 12 ++--- 12 files changed, 105 insertions(+), 93 deletions(-) (limited to 'src/ipa') diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index fde50835..d50ce495 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -73,8 +73,8 @@ Agc::Agc() int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) { /* Configure the default exposure and gain. */ - context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); - context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration; + context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); + context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration; /* * According to the RkISP1 documentation: @@ -98,7 +98,10 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4; context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4; - /* \todo Use actual frame index by populating it in the frameContext. */ + /* + * \todo Use the upcoming per-frame context API that will provide a + * frame index + */ frameCount_ = 0; return 0; } @@ -140,18 +143,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) /** * \brief Estimate the new exposure and gain values - * \param[inout] frameContext The shared IPA frame Context + * \param[inout] context The shared IPA Context * \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) { IPASessionConfiguration &configuration = context.configuration; - IPAFrameContext &frameContext = context.frameContext; + IPAActiveState &activeState = context.activeState; /* Get the effective exposure and gain applied on the sensor. */ - uint32_t exposure = frameContext.sensor.exposure; - double analogueGain = frameContext.sensor.gain; + uint32_t exposure = activeState.sensor.exposure; + double analogueGain = activeState.sensor.gain; /* Use the highest of the two gain estimates. */ double evGain = std::max(yGain, iqMeanGain); @@ -216,8 +219,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) << stepGain; /* Update the estimated exposure and gain. */ - frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration; - frameContext.agc.gain = stepGain; + activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration; + activeState.agc.gain = stepGain; } /** @@ -329,7 +332,7 @@ void Agc::prepare(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; /* Configure the measurement window. */ diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index a23c3201..2bd9ef77 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -36,10 +36,10 @@ LOG_DEFINE_CATEGORY(RkISP1Awb) int Awb::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) { - context.frameContext.awb.gains.red = 1.0; - context.frameContext.awb.gains.blue = 1.0; - context.frameContext.awb.gains.green = 1.0; - context.frameContext.awb.autoEnabled = true; + context.activeState.awb.gains.red = 1.0; + context.activeState.awb.gains.blue = 1.0; + context.activeState.awb.gains.green = 1.0; + context.activeState.awb.autoEnabled = true; /* * Define the measurement window for AWB as a centered rectangle @@ -79,16 +79,16 @@ void Awb::prepare(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green; - params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue; - params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red; - params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green; + params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green; + params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue; + params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red; + params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green; /* Update the gains. */ params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; /* If we already have configured the gains and window, return. */ - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; /* Configure the gains to apply. */ @@ -131,7 +131,7 @@ void Awb::queueRequest(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, const ControlList &controls) { - auto &awb = context.frameContext.awb; + auto &awb = context.activeState.awb; const auto &awbEnable = controls.get(controls::AwbEnable); if (awbEnable && *awbEnable != awb.autoEnabled) { @@ -162,7 +162,7 @@ void Awb::process([[maybe_unused]] IPAContext &context, { const rkisp1_cif_isp_stat *params = &stats->params; const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; - IPAFrameContext &frameContext = context.frameContext; + IPAActiveState &activeState = context.activeState; /* Get the YCbCr mean values */ double yMean = awb->awb_mean[0].mean_y_or_g; @@ -194,24 +194,24 @@ void Awb::process([[maybe_unused]] IPAContext &context, /* Filter the values to avoid oscillations. */ double speed = 0.2; - redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red; - blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue; + redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red; + blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue; /* * Gain values are unsigned integer value, range 0 to 4 with 8 bit * fractional part. */ - if (frameContext.awb.autoEnabled) { - frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); - frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); + if (activeState.awb.autoEnabled) { + activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); + activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); } /* Hardcode the green gain to 1.0. */ - frameContext.awb.gains.green = 1.0; + activeState.awb.gains.green = 1.0; - frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); + activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); - LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.frameContext.awb.gains.red - << " and for blue: " << context.frameContext.awb.gains.blue; + LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.activeState.awb.gains.red + << " and for blue: " << context.activeState.awb.gains.blue; } REGISTER_IPA_ALGORITHM(Awb, "Awb") diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp index c45a317a..0f7226cf 100644 --- a/src/ipa/rkisp1/algorithms/blc.cpp +++ b/src/ipa/rkisp1/algorithms/blc.cpp @@ -70,7 +70,7 @@ void BlackLevelCorrection::prepare(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; if (!tuningParameters_) diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp index bc4a0707..ea819b2a 100644 --- a/src/ipa/rkisp1/algorithms/cproc.cpp +++ b/src/ipa/rkisp1/algorithms/cproc.cpp @@ -41,7 +41,7 @@ void ColorProcessing::queueRequest(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, const ControlList &controls) { - auto &cproc = context.frameContext.cproc; + auto &cproc = context.activeState.cproc; const auto &brightness = controls.get(controls::Brightness); if (brightness) { @@ -76,7 +76,7 @@ void ColorProcessing::prepare(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - auto &cproc = context.frameContext.cproc; + auto &cproc = context.activeState.cproc; /* Check if the algorithm configuration has been updated. */ if (!cproc.updateParams) diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp index 0c005fe8..cf7de663 100644 --- a/src/ipa/rkisp1/algorithms/dpcc.cpp +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp @@ -236,7 +236,7 @@ void DefectPixelClusterCorrection::prepare(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; if (!initialized_) diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp index be024fc5..b8c837c2 100644 --- a/src/ipa/rkisp1/algorithms/dpf.cpp +++ b/src/ipa/rkisp1/algorithms/dpf.cpp @@ -179,7 +179,7 @@ void Dpf::queueRequest(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, const ControlList &controls) { - auto &dpf = context.frameContext.dpf; + auto &dpf = context.activeState.dpf; const auto &denoise = controls.get(controls::draft::NoiseReductionMode); if (denoise) { @@ -214,9 +214,9 @@ void Dpf::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, if (!initialized_) return; - auto &dpf = context.frameContext.dpf; + auto &dpf = context.activeState.dpf; - if (context.frameContext.frameCount == 0) { + if (context.activeState.frameCount == 0) { params->others.dpf_config = config_; params->others.dpf_strength_config = strengthConfig_; diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp index 6aa5476a..837560eb 100644 --- a/src/ipa/rkisp1/algorithms/filter.cpp +++ b/src/ipa/rkisp1/algorithms/filter.cpp @@ -47,7 +47,7 @@ void Filter::queueRequest(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, const ControlList &controls) { - auto &filter = context.frameContext.filter; + auto &filter = context.activeState.filter; const auto &sharpness = controls.get(controls::Sharpness); if (sharpness) { @@ -91,7 +91,7 @@ void Filter::prepare(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - auto &filter = context.frameContext.filter; + auto &filter = context.activeState.filter; /* Check if the algorithm configuration has been updated. */ if (!filter.updateParams) diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp index dfc76b3d..879ca297 100644 --- a/src/ipa/rkisp1/algorithms/gsl.cpp +++ b/src/ipa/rkisp1/algorithms/gsl.cpp @@ -123,7 +123,7 @@ void GammaSensorLinearization::prepare(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; if (!initialized_) diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp index e4b04136..9c717bc5 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -137,7 +137,7 @@ void LensShadingCorrection::prepare(IPAContext &context, [[maybe_unused]] IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; if (!initialized_) diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index cc66bc70..d18f4996 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -85,114 +85,115 @@ namespace libcamera::ipa::rkisp1 { */ /** - * \struct IPAFrameContext - * \brief Per-frame context for algorithms + * \struct IPAActiveState + * \brief Active state for algorithms * - * The frame context stores data specific to a single frame processed by the - * IPA. Each frame processed by the IPA has a context associated with it, - * accessible through the IPAContext structure. + * The active state stores algorithm-specific data that needs to be shared + * between multiple algorithms and the IPA module. It is accessible through the + * IPAContext structure. * - * \todo Detail how to access contexts for a particular frame + * \todo Split the data contained in this structure between the active state + * and the frame contexts. * - * Each of the fields in the frame context belongs to either a specific + * Each of the fields in the active state belongs to either a specific * algorithm, or to the top-level IPA module. A field may be read by any * algorithm, but should only be written by its owner. */ /** - * \var IPAFrameContext::agc - * \brief Context for the Automatic Gain Control algorithm + * \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. * - * \var IPAFrameContext::agc.exposure + * \var IPAActiveState::agc.exposure * \brief Exposure time expressed as a number of lines * - * \var IPAFrameContext::agc.gain + * \var IPAActiveState::agc.gain * \brief Analogue gain multiplier * * The gain should be adapted to the sensor specific gain code before applying. */ /** - * \var IPAFrameContext::awb - * \brief Context for the Automatic White Balance algorithm + * \var IPAActiveState::awb + * \brief State for the Automatic White Balance algorithm * - * \struct IPAFrameContext::awb.gains + * \struct IPAActiveState::awb.gains * \brief White balance gains * - * \var IPAFrameContext::awb.gains.red + * \var IPAActiveState::awb.gains.red * \brief White balance gain for R channel * - * \var IPAFrameContext::awb.gains.green + * \var IPAActiveState::awb.gains.green * \brief White balance gain for G channel * - * \var IPAFrameContext::awb.gains.blue + * \var IPAActiveState::awb.gains.blue * \brief White balance gain for B channel * - * \var IPAFrameContext::awb.temperatureK + * \var IPAActiveState::awb.temperatureK * \brief Estimated color temperature * - * \var IPAFrameContext::awb.autoEnabled + * \var IPAActiveState::awb.autoEnabled * \brief Whether the Auto White Balance algorithm is enabled */ /** - * \var IPAFrameContext::cproc - * \brief Context for the Color Processing algorithm + * \var IPAActiveState::cproc + * \brief State for the Color Processing algorithm * - * \struct IPAFrameContext::cproc.brightness + * \struct IPAActiveState::cproc.brightness * \brief Brightness level * - * \var IPAFrameContext::cproc.contrast + * \var IPAActiveState::cproc.contrast * \brief Contrast level * - * \var IPAFrameContext::cproc.saturation + * \var IPAActiveState::cproc.saturation * \brief Saturation level * - * \var IPAFrameContext::cproc.updateParams + * \var IPAActiveState::cproc.updateParams * \brief Indicates if ISP parameters need to be updated */ /** - * \var IPAFrameContext::dpf - * \brief Context for the Denoise Pre-Filter algorithm + * \var IPAActiveState::dpf + * \brief State for the Denoise Pre-Filter algorithm * - * \var IPAFrameContext::dpf.denoise + * \var IPAActiveState::dpf.denoise * \brief Indicates if denoise is activated * - * \var IPAFrameContext::dpf.updateParams + * \var IPAActiveState::dpf.updateParams * \brief Indicates if ISP parameters need to be updated */ /** - * \var IPAFrameContext::filter - * \brief Context for the Filter algorithm + * \var IPAActiveState::filter + * \brief State for the Filter algorithm * - * \struct IPAFrameContext::filter.denoise + * \struct IPAActiveState::filter.denoise * \brief Denoising level * - * \var IPAFrameContext::filter.sharpness + * \var IPAActiveState::filter.sharpness * \brief Sharpness level * - * \var IPAFrameContext::filter.updateParams + * \var IPAActiveState::filter.updateParams * \brief Indicates if ISP parameters need to be updated */ /** - * \var IPAFrameContext::sensor + * \var IPAActiveState::sensor * \brief Effective sensor values * - * \var IPAFrameContext::sensor.exposure + * \var IPAActiveState::sensor.exposure * \brief Exposure time expressed as a number of lines * - * \var IPAFrameContext::sensor.gain + * \var IPAActiveState::sensor.gain * \brief Analogue gain multiplier */ /** - * \var IPAFrameContext::frameCount + * \var IPAActiveState::frameCount * \brief Counter of requests queued to the IPA module * * The counter is reset to 0 when the IPA module is configured, and is @@ -200,6 +201,14 @@ namespace libcamera::ipa::rkisp1 { * Algorithm::prepare() function of all algorithms. */ +/** + * \struct IPAFrameContext + * \brief Per-frame context for algorithms + * + * This structure is currently unused and will be replaced by a real per-frame + * context. + */ + /** * \struct IPAContext * \brief Global IPA context data shared between all algorithms @@ -207,13 +216,10 @@ namespace libcamera::ipa::rkisp1 { * \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::activeState + * \brief The IPA active state, storing the latest state for all algorithms * - * \todo While the frame context is supposed to be per-frame, this - * single frame context stores data related to both the current frame - * and the previous frames, with fields being updated as the algorithms - * are run. This needs to be turned into real per-frame data storage. + * \todo Introduce per-frame contexts */ } /* namespace libcamera::ipa::rkisp1 */ diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 854151f2..5590482c 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -46,7 +46,7 @@ struct IPASessionConfiguration { } hw; }; -struct IPAFrameContext { +struct IPAActiveState { struct { uint32_t exposure; double gain; @@ -89,9 +89,12 @@ struct IPAFrameContext { unsigned int frameCount; }; +struct IPAFrameContext { +}; + struct IPAContext { IPASessionConfiguration configuration; - IPAFrameContext frameContext; + IPAActiveState activeState; }; } /* namespace ipa::rkisp1 */ diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 95df8afc..e6aadb56 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -243,7 +243,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain); context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); - context_.frameContext.frameCount = 0; + context_.activeState.frameCount = 0; for (auto const &algo : algorithms()) { int ret = algo->configure(context_, info); @@ -309,7 +309,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) algo->prepare(context_, frame, frameContext, params); paramsBufferReady.emit(frame); - context_.frameContext.frameCount++; + context_.activeState.frameCount++; } void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId, @@ -319,9 +319,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId reinterpret_cast( mappedBuffers_.at(bufferId).planes()[0].data()); - context_.frameContext.sensor.exposure = + context_.activeState.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get(); - context_.frameContext.sensor.gain = + context_.activeState.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get()); unsigned int aeState = 0; @@ -339,8 +339,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId void IPARkISP1::setControls(unsigned int frame) { - uint32_t exposure = context_.frameContext.agc.exposure; - uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain); + uint32_t exposure = context_.activeState.agc.exposure; + uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain); ControlList ctrls(ctrls_); ctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure)); -- cgit v1.2.1