[{"id":24676,"web_url":"https://patchwork.libcamera.org/comment/24676/","msgid":"<166083884775.542608.3842593206807831315@Monstersaurus>","date":"2022-08-18T16:07:27","subject":"Re: [libcamera-devel] [PATCH v3 02/17] ipa: rkisp1: Rename\n\tframeContext to activeState","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi via libcamera-devel (2022-08-18 10:43:55)\n> From: Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org>\n> \n> Move the existing frame context structure to the IPAActiveState.\n> This structure should store the most up to date results for the IPA\n> which may be shared to other algorithms that operate on the data.\n> \n\nLaurent queried this patch in v2.\n\nI suggested we add the following to this commit message:\n\n\"\"\"\nAll existing algorithm state is moved to the ActiveState and the\nalgorithms should be updated individually to use the FrameContext\ndirectly where appropriate.\n\"\"\"\n\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/ipa/rkisp1/algorithms/agc.cpp    | 20 ++++++++--------\n>  src/ipa/rkisp1/algorithms/awb.cpp    | 34 ++++++++++++++--------------\n>  src/ipa/rkisp1/algorithms/blc.cpp    |  2 +-\n>  src/ipa/rkisp1/algorithms/cproc.cpp  |  4 ++--\n>  src/ipa/rkisp1/algorithms/dpcc.cpp   |  2 +-\n>  src/ipa/rkisp1/algorithms/filter.cpp |  4 ++--\n>  src/ipa/rkisp1/algorithms/gsl.cpp    |  2 +-\n>  src/ipa/rkisp1/algorithms/lsc.cpp    |  2 +-\n>  src/ipa/rkisp1/ipa_context.h         |  7 ++++--\n>  src/ipa/rkisp1/rkisp1.cpp            | 12 +++++-----\n>  10 files changed, 46 insertions(+), 43 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index a1bb7d972926..483e941fe427 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -73,8 +73,8 @@ Agc::Agc()\n>  int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>  {\n>         /* Configure the default exposure and gain. */\n> -       context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> -       context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration;\n> +       context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> +       context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration;\n>  \n>         /*\n>          * According to the RkISP1 documentation:\n> @@ -98,7 +98,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>         context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;\n>         context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n>  \n> -       /* \\todo Use actual frame index by populating it in the frameContext. */\n> +       /* \\todo Use actual frame index by populating it in the activeState. */\n>         frameCount_ = 0;\n>         return 0;\n>  }\n> @@ -140,18 +140,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n>  \n>  /**\n>   * \\brief Estimate the new exposure and gain values\n> - * \\param[inout] frameContext The shared IPA frame Context\n> + * \\param[inout] context The shared IPA Context\n>   * \\param[in] yGain The gain calculated on the current brightness level\n>   * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n>   */\n>  void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n>  {\n>         IPASessionConfiguration &configuration = context.configuration;\n> -       IPAFrameContext &frameContext = context.frameContext;\n> +       IPAActiveState &activeState = context.activeState;\n>  \n>         /* Get the effective exposure and gain applied on the sensor. */\n> -       uint32_t exposure = frameContext.sensor.exposure;\n> -       double analogueGain = frameContext.sensor.gain;\n> +       uint32_t exposure = activeState.sensor.exposure;\n> +       double analogueGain = activeState.sensor.gain;\n>  \n>         /* Use the highest of the two gain estimates. */\n>         double evGain = std::max(yGain, iqMeanGain);\n> @@ -216,8 +216,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n>                               << stepGain;\n>  \n>         /* Update the estimated exposure and gain. */\n> -       frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> -       frameContext.agc.gain = stepGain;\n> +       activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> +       activeState.agc.gain = stepGain;\n>  }\n>  \n>  /**\n> @@ -324,7 +324,7 @@ void Agc::process(IPAContext &context,\n>   */\n>  void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params)\n>  {\n> -       if (context.frameContext.frameCount > 0)\n> +       if (context.activeState.frameCount > 0)\n>                 return;\n>  \n>         /* Configure the measurement window. */\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 9f00364d12b1..427aaeb1e955 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -35,9 +35,9 @@ LOG_DEFINE_CATEGORY(RkISP1Awb)\n>  int Awb::configure(IPAContext &context,\n>                    const IPACameraSensorInfo &configInfo)\n>  {\n> -       context.frameContext.awb.gains.red = 1.0;\n> -       context.frameContext.awb.gains.blue = 1.0;\n> -       context.frameContext.awb.gains.green = 1.0;\n> +       context.activeState.awb.gains.red = 1.0;\n> +       context.activeState.awb.gains.blue = 1.0;\n> +       context.activeState.awb.gains.green = 1.0;\n>  \n>         /*\n>          * Define the measurement window for AWB as a centered rectangle\n> @@ -72,16 +72,16 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n>   */\n>  void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)\n>  {\n> -       params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green;\n> -       params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue;\n> -       params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red;\n> -       params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green;\n> +       params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green;\n> +       params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue;\n> +       params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red;\n> +       params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green;\n>  \n>         /* Update the gains. */\n>         params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n>  \n>         /* If we already have configured the gains and window, return. */\n> -       if (context.frameContext.frameCount > 0)\n> +       if (context.activeState.frameCount > 0)\n>                 return;\n>  \n>         /* Configure the gains to apply. */\n> @@ -125,7 +125,7 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n>  {\n>         const rkisp1_cif_isp_stat *params = &stats->params;\n>         const rkisp1_cif_isp_awb_stat *awb = &params->awb;\n> -       IPAFrameContext &frameContext = context.frameContext;\n> +       IPAActiveState &activeState = context.activeState;\n>  \n>         /* Get the YCbCr mean values */\n>         double yMean = awb->awb_mean[0].mean_y_or_g;\n> @@ -157,22 +157,22 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n>  \n>         /* Filter the values to avoid oscillations. */\n>         double speed = 0.2;\n> -       redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red;\n> -       blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue;\n> +       redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red;\n> +       blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue;\n>  \n>         /*\n>          * Gain values are unsigned integer value, range 0 to 4 with 8 bit\n>          * fractional part.\n>          */\n> -       frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> -       frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> +       activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> +       activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n>         /* Hardcode the green gain to 1.0. */\n> -       frameContext.awb.gains.green = 1.0;\n> +       activeState.awb.gains.green = 1.0;\n>  \n> -       frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> +       activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n>  \n> -       LOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.frameContext.awb.gains.red\n> -                             << \" and for blue: \" << context.frameContext.awb.gains.blue;\n> +       LOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.activeState.awb.gains.red\n> +                             << \" and for blue: \" << context.activeState.awb.gains.blue;\n>  }\n>  \n>  REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> index a58569fa2dc2..4d55a2d529cb 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -68,7 +68,7 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,\n>  void BlackLevelCorrection::prepare(IPAContext &context,\n>                                    rkisp1_params_cfg *params)\n>  {\n> -       if (context.frameContext.frameCount > 0)\n> +       if (context.activeState.frameCount > 0)\n>                 return;\n>  \n>         if (!tuningParameters_)\n> diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\n> index bca5ab6907d6..a3b778d1c908 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> @@ -40,7 +40,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n>                                    [[maybe_unused]] const uint32_t frame,\n>                                    const ControlList &controls)\n>  {\n> -       auto &cproc = context.frameContext.cproc;\n> +       auto &cproc = context.activeState.cproc;\n>  \n>         const auto &brightness = controls.get(controls::Brightness);\n>         if (brightness) {\n> @@ -73,7 +73,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n>  void ColorProcessing::prepare(IPAContext &context,\n>                               rkisp1_params_cfg *params)\n>  {\n> -       auto &cproc = context.frameContext.cproc;\n> +       auto &cproc = context.activeState.cproc;\n>  \n>         /* Check if the algorithm configuration has been updated. */\n>         if (!cproc.updateParams)\n> diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> index 69bc651eaf08..93d37b1dae44 100644\n> --- a/src/ipa/rkisp1/algorithms/dpcc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> @@ -234,7 +234,7 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context,\n>  void DefectPixelClusterCorrection::prepare(IPAContext &context,\n>                                            rkisp1_params_cfg *params)\n>  {\n> -       if (context.frameContext.frameCount > 0)\n> +       if (context.activeState.frameCount > 0)\n>                 return;\n>  \n>         if (!initialized_)\n> diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp\n> index 8ca10fd1ee9d..bc7fc1f32f34 100644\n> --- a/src/ipa/rkisp1/algorithms/filter.cpp\n> +++ b/src/ipa/rkisp1/algorithms/filter.cpp\n> @@ -46,7 +46,7 @@ void Filter::queueRequest(IPAContext &context,\n>                           [[maybe_unused]] const uint32_t frame,\n>                           const ControlList &controls)\n>  {\n> -       auto &filter = context.frameContext.filter;\n> +       auto &filter = context.activeState.filter;\n>  \n>         const auto &sharpness = controls.get(controls::Sharpness);\n>         if (sharpness) {\n> @@ -87,7 +87,7 @@ void Filter::queueRequest(IPAContext &context,\n>   */\n>  void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params)\n>  {\n> -       auto &filter = context.frameContext.filter;\n> +       auto &filter = context.activeState.filter;\n>  \n>         /* Check if the algorithm configuration has been updated. */\n>         if (!filter.updateParams)\n> diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp\n> index 2fd1a23d3a9b..dd9974627cd2 100644\n> --- a/src/ipa/rkisp1/algorithms/gsl.cpp\n> +++ b/src/ipa/rkisp1/algorithms/gsl.cpp\n> @@ -121,7 +121,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,\n>  void GammaSensorLinearization::prepare(IPAContext &context,\n>                                        rkisp1_params_cfg *params)\n>  {\n> -       if (context.frameContext.frameCount > 0)\n> +       if (context.activeState.frameCount > 0)\n>                 return;\n>  \n>         if (!initialized_)\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 05c8c0dab5c8..4ed467086199 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -125,7 +125,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n>  void LensShadingCorrection::prepare(IPAContext &context,\n>                                     rkisp1_params_cfg *params)\n>  {\n> -       if (context.frameContext.frameCount > 0)\n> +       if (context.activeState.frameCount > 0)\n>                 return;\n>  \n>         if (!initialized_)\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 2bdb6a81d7c9..a8daeca487ae 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -41,7 +41,7 @@ struct IPASessionConfiguration {\n>         } hw;\n>  };\n>  \n> -struct IPAFrameContext {\n> +struct IPAActiveState {\n>         struct {\n>                 uint32_t exposure;\n>                 double gain;\n> @@ -78,9 +78,12 @@ struct IPAFrameContext {\n>         unsigned int frameCount;\n>  };\n>  \n> +struct IPAFrameContext {\n> +};\n> +\n>  struct IPAContext {\n>         IPASessionConfiguration configuration;\n> -       IPAFrameContext frameContext;\n> +       IPAActiveState activeState;\n>  };\n>  \n>  } /* namespace ipa::rkisp1 */\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 17d42d38eb45..88e6a2b23eed 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n>         context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);\n>         context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n>  \n> -       context_.frameContext.frameCount = 0;\n> +       context_.activeState.frameCount = 0;\n>  \n>         for (auto const &algo : algorithms()) {\n>                 int ret = algo->configure(context_, info);\n> @@ -306,7 +306,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>                 algo->prepare(context_, params);\n>  \n>         paramsBufferReady.emit(frame);\n> -       context_.frameContext.frameCount++;\n> +       context_.activeState.frameCount++;\n>  }\n>  \n>  void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> @@ -316,9 +316,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n>                 reinterpret_cast<rkisp1_stat_buffer *>(\n>                         mappedBuffers_.at(bufferId).planes()[0].data());\n>  \n> -       context_.frameContext.sensor.exposure =\n> +       context_.activeState.sensor.exposure =\n>                 sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n> -       context_.frameContext.sensor.gain =\n> +       context_.activeState.sensor.gain =\n>                 camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());\n>  \n>         unsigned int aeState = 0;\n> @@ -333,8 +333,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n>  \n>  void IPARkISP1::setControls(unsigned int frame)\n>  {\n> -       uint32_t exposure = context_.frameContext.agc.exposure;\n> -       uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n> +       uint32_t exposure = context_.activeState.agc.exposure;\n> +       uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n>  \n>         ControlList ctrls(ctrls_);\n>         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));\n> -- \n> 2.37.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 87E67BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 18 Aug 2022 16:07:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D39FE61FC0;\n\tThu, 18 Aug 2022 18:07:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 69DF961FA7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 18 Aug 2022 18:07:30 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 02AC84A8;\n\tThu, 18 Aug 2022 18:07:29 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660838851;\n\tbh=MinHGoU3PLAQTx7QoeKF1xlp2OlTNpNRW64lmiICklE=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=ox+5l1Nuo81Fa3Tm4s1bWU6iEGnAKMBRAApqMvR0wGpNIg+13gpXeOF4A5ha4+5zY\n\tGYdMp3PQLBgW3EsGCngKJ+5NjTQI0sY7IEFArQfaM4HGoSb8cGoEGZ3qFRS8QxTuUC\n\tkoszmYBzVKvpsEA7HIu2bsEi3urbfXpYXkG2L0NobV8x8bxMLkXfuw3BiSQSwd5e6u\n\tHEKOLLkKT/sr+gl6jUGk9EXlX4Vdnc2vjtBl2GDJs7VtVrKmefeVHLVLK/FyNiKeUO\n\ttIc672QqyJYh/hmdvoam/SxEzuO3AdygiaiJOqXlxScJuePCOuOA0m9lKTUKOgIkBR\n\tmCbaVz4QMwOxQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660838850;\n\tbh=MinHGoU3PLAQTx7QoeKF1xlp2OlTNpNRW64lmiICklE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=B7mFSFJAFx/5rTiyPY+gsrVEe9R3NE8Wxi0ieBrXK/dhimwr/SZQuzUmmjQF8O6Us\n\t3mtYBd2d6q8WfiXwBrpLFvxIGNuisxQ7TW57gnl63a1GG3A0NMKgDmrcvMhxhnXMAd\n\txEj+Q9nZskaG8ryryXFrYRqvPKqQXn7i94ZCE6ec="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"B7mFSFJA\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220818094410.1671-3-jacopo@jmondi.org>","References":"<20220818094410.1671-1-jacopo@jmondi.org>\n\t<20220818094410.1671-3-jacopo@jmondi.org>","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Date":"Thu, 18 Aug 2022 17:07:27 +0100","Message-ID":"<166083884775.542608.3842593206807831315@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 02/17] ipa: rkisp1: Rename\n\tframeContext to activeState","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24752,"web_url":"https://patchwork.libcamera.org/comment/24752/","msgid":"<YwVyrhcMZgSMot5T@pendragon.ideasonboard.com>","date":"2022-08-24T00:37:02","subject":"Re: [libcamera-devel] [PATCH v3 02/17] ipa: rkisp1: Rename\n\tframeContext to activeState","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Aug 18, 2022 at 05:07:27PM +0100, Kieran Bingham via libcamera-devel wrote:\n> Quoting Jacopo Mondi via libcamera-devel (2022-08-18 10:43:55)\n> > From: Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org>\n> > \n> > Move the existing frame context structure to the IPAActiveState.\n> > This structure should store the most up to date results for the IPA\n> > which may be shared to other algorithms that operate on the data.\n> \n> Laurent queried this patch in v2.\n> \n> I suggested we add the following to this commit message:\n> \n> \"\"\"\n> All existing algorithm state is moved to the ActiveState and the\n> algorithms should be updated individually to use the FrameContext\n> directly where appropriate.\n> \"\"\"\n\nI'd like to make it clear (partly to check that my understanding is\ncorrect :-)) that this commit is about moving the existing frame context\naside to make space for the new one. How about\n\n--------\nThe RkISP1 IPA module creates a single instance of its IPAFrameContext\nstructure, effectively using it more as an active state than a per-frame\ncontext. To prepare for the introduction of a real per-frame context,\nmove all the members of the IPAFrameContext structure to a new\nIPAActiveState structure. The IPAFrameContext becomes effectively\nunused at runtime, but can't be removed yet as it is still required as a\ntemplate argument to the Module and Algorithm classes.\n\nThe new IPAActiveState structure is not foreseen to stay, its members\nshould move to either the new per-frame context that will be introduced,\nor to the individual algorithm classes.\n--------\n\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/ipa/rkisp1/algorithms/agc.cpp    | 20 ++++++++--------\n> >  src/ipa/rkisp1/algorithms/awb.cpp    | 34 ++++++++++++++--------------\n> >  src/ipa/rkisp1/algorithms/blc.cpp    |  2 +-\n> >  src/ipa/rkisp1/algorithms/cproc.cpp  |  4 ++--\n> >  src/ipa/rkisp1/algorithms/dpcc.cpp   |  2 +-\n> >  src/ipa/rkisp1/algorithms/filter.cpp |  4 ++--\n> >  src/ipa/rkisp1/algorithms/gsl.cpp    |  2 +-\n> >  src/ipa/rkisp1/algorithms/lsc.cpp    |  2 +-\n> >  src/ipa/rkisp1/ipa_context.h         |  7 ++++--\n> >  src/ipa/rkisp1/rkisp1.cpp            | 12 +++++-----\n> >  10 files changed, 46 insertions(+), 43 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > index a1bb7d972926..483e941fe427 100644\n> > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > @@ -73,8 +73,8 @@ Agc::Agc()\n> >  int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> >  {\n> >         /* Configure the default exposure and gain. */\n> > -       context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> > -       context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration;\n> > +       context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> > +       context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration;\n> >  \n> >         /*\n> >          * According to the RkISP1 documentation:\n> > @@ -98,7 +98,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> >         context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;\n> >         context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n> >  \n> > -       /* \\todo Use actual frame index by populating it in the frameContext. */\n> > +       /* \\todo Use actual frame index by populating it in the activeState. */\n\nThat will go later in the series, but we could still keep it accurate in\nthe meantime by writing\n\n\t/*\n\t * \\todo Use the upcoming per-frame context API that will provide a\n\t * frame index\n\t */\n\n> >         frameCount_ = 0;\n> >         return 0;\n> >  }\n> > @@ -140,18 +140,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n> >  \n> >  /**\n> >   * \\brief Estimate the new exposure and gain values\n> > - * \\param[inout] frameContext The shared IPA frame Context\n> > + * \\param[inout] context The shared IPA Context\n\nAnd add to the commit message\n\nWhile at it, fix a typo in the documentation of the\nAgc::computeExposure() function that incorrectly refers to the frame\ncontext instead of the global context.\n\n> >   * \\param[in] yGain The gain calculated on the current brightness level\n> >   * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n> >   */\n> >  void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n> >  {\n> >         IPASessionConfiguration &configuration = context.configuration;\n> > -       IPAFrameContext &frameContext = context.frameContext;\n> > +       IPAActiveState &activeState = context.activeState;\n> >  \n> >         /* Get the effective exposure and gain applied on the sensor. */\n> > -       uint32_t exposure = frameContext.sensor.exposure;\n> > -       double analogueGain = frameContext.sensor.gain;\n> > +       uint32_t exposure = activeState.sensor.exposure;\n> > +       double analogueGain = activeState.sensor.gain;\n> >  \n> >         /* Use the highest of the two gain estimates. */\n> >         double evGain = std::max(yGain, iqMeanGain);\n> > @@ -216,8 +216,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n> >                               << stepGain;\n> >  \n> >         /* Update the estimated exposure and gain. */\n> > -       frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> > -       frameContext.agc.gain = stepGain;\n> > +       activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> > +       activeState.agc.gain = stepGain;\n> >  }\n> >  \n> >  /**\n> > @@ -324,7 +324,7 @@ void Agc::process(IPAContext &context,\n> >   */\n> >  void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> >  {\n> > -       if (context.frameContext.frameCount > 0)\n> > +       if (context.activeState.frameCount > 0)\n> >                 return;\n> >  \n> >         /* Configure the measurement window. */\n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index 9f00364d12b1..427aaeb1e955 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -35,9 +35,9 @@ LOG_DEFINE_CATEGORY(RkISP1Awb)\n> >  int Awb::configure(IPAContext &context,\n> >                    const IPACameraSensorInfo &configInfo)\n> >  {\n> > -       context.frameContext.awb.gains.red = 1.0;\n> > -       context.frameContext.awb.gains.blue = 1.0;\n> > -       context.frameContext.awb.gains.green = 1.0;\n> > +       context.activeState.awb.gains.red = 1.0;\n> > +       context.activeState.awb.gains.blue = 1.0;\n> > +       context.activeState.awb.gains.green = 1.0;\n> >  \n> >         /*\n> >          * Define the measurement window for AWB as a centered rectangle\n> > @@ -72,16 +72,16 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n> >   */\n> >  void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> >  {\n> > -       params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green;\n> > -       params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue;\n> > -       params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red;\n> > -       params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green;\n> > +       params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green;\n> > +       params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue;\n> > +       params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red;\n> > +       params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green;\n> >  \n> >         /* Update the gains. */\n> >         params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n> >  \n> >         /* If we already have configured the gains and window, return. */\n> > -       if (context.frameContext.frameCount > 0)\n> > +       if (context.activeState.frameCount > 0)\n> >                 return;\n> >  \n> >         /* Configure the gains to apply. */\n> > @@ -125,7 +125,7 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n> >  {\n> >         const rkisp1_cif_isp_stat *params = &stats->params;\n> >         const rkisp1_cif_isp_awb_stat *awb = &params->awb;\n> > -       IPAFrameContext &frameContext = context.frameContext;\n> > +       IPAActiveState &activeState = context.activeState;\n> >  \n> >         /* Get the YCbCr mean values */\n> >         double yMean = awb->awb_mean[0].mean_y_or_g;\n> > @@ -157,22 +157,22 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n> >  \n> >         /* Filter the values to avoid oscillations. */\n> >         double speed = 0.2;\n> > -       redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red;\n> > -       blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue;\n> > +       redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red;\n> > +       blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue;\n> >  \n> >         /*\n> >          * Gain values are unsigned integer value, range 0 to 4 with 8 bit\n> >          * fractional part.\n> >          */\n> > -       frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> > -       frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> > +       activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> > +       activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> >         /* Hardcode the green gain to 1.0. */\n> > -       frameContext.awb.gains.green = 1.0;\n> > +       activeState.awb.gains.green = 1.0;\n> >  \n> > -       frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> > +       activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> >  \n> > -       LOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.frameContext.awb.gains.red\n> > -                             << \" and for blue: \" << context.frameContext.awb.gains.blue;\n> > +       LOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.activeState.awb.gains.red\n> > +                             << \" and for blue: \" << context.activeState.awb.gains.blue;\n> >  }\n> >  \n> >  REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> > index a58569fa2dc2..4d55a2d529cb 100644\n> > --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> > @@ -68,7 +68,7 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,\n> >  void BlackLevelCorrection::prepare(IPAContext &context,\n> >                                    rkisp1_params_cfg *params)\n> >  {\n> > -       if (context.frameContext.frameCount > 0)\n> > +       if (context.activeState.frameCount > 0)\n> >                 return;\n> >  \n> >         if (!tuningParameters_)\n> > diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\n> > index bca5ab6907d6..a3b778d1c908 100644\n> > --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> > @@ -40,7 +40,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> >                                    [[maybe_unused]] const uint32_t frame,\n> >                                    const ControlList &controls)\n> >  {\n> > -       auto &cproc = context.frameContext.cproc;\n> > +       auto &cproc = context.activeState.cproc;\n> >  \n> >         const auto &brightness = controls.get(controls::Brightness);\n> >         if (brightness) {\n> > @@ -73,7 +73,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> >  void ColorProcessing::prepare(IPAContext &context,\n> >                               rkisp1_params_cfg *params)\n> >  {\n> > -       auto &cproc = context.frameContext.cproc;\n> > +       auto &cproc = context.activeState.cproc;\n> >  \n> >         /* Check if the algorithm configuration has been updated. */\n> >         if (!cproc.updateParams)\n> > diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> > index 69bc651eaf08..93d37b1dae44 100644\n> > --- a/src/ipa/rkisp1/algorithms/dpcc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> > @@ -234,7 +234,7 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context,\n> >  void DefectPixelClusterCorrection::prepare(IPAContext &context,\n> >                                            rkisp1_params_cfg *params)\n> >  {\n> > -       if (context.frameContext.frameCount > 0)\n> > +       if (context.activeState.frameCount > 0)\n> >                 return;\n> >  \n> >         if (!initialized_)\n> > diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp\n> > index 8ca10fd1ee9d..bc7fc1f32f34 100644\n> > --- a/src/ipa/rkisp1/algorithms/filter.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/filter.cpp\n> > @@ -46,7 +46,7 @@ void Filter::queueRequest(IPAContext &context,\n> >                           [[maybe_unused]] const uint32_t frame,\n> >                           const ControlList &controls)\n> >  {\n> > -       auto &filter = context.frameContext.filter;\n> > +       auto &filter = context.activeState.filter;\n> >  \n> >         const auto &sharpness = controls.get(controls::Sharpness);\n> >         if (sharpness) {\n> > @@ -87,7 +87,7 @@ void Filter::queueRequest(IPAContext &context,\n> >   */\n> >  void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> >  {\n> > -       auto &filter = context.frameContext.filter;\n> > +       auto &filter = context.activeState.filter;\n> >  \n> >         /* Check if the algorithm configuration has been updated. */\n> >         if (!filter.updateParams)\n> > diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp\n> > index 2fd1a23d3a9b..dd9974627cd2 100644\n> > --- a/src/ipa/rkisp1/algorithms/gsl.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/gsl.cpp\n> > @@ -121,7 +121,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,\n> >  void GammaSensorLinearization::prepare(IPAContext &context,\n> >                                        rkisp1_params_cfg *params)\n> >  {\n> > -       if (context.frameContext.frameCount > 0)\n> > +       if (context.activeState.frameCount > 0)\n> >                 return;\n> >  \n> >         if (!initialized_)\n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > index 05c8c0dab5c8..4ed467086199 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -125,7 +125,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> >  void LensShadingCorrection::prepare(IPAContext &context,\n> >                                     rkisp1_params_cfg *params)\n> >  {\n> > -       if (context.frameContext.frameCount > 0)\n> > +       if (context.activeState.frameCount > 0)\n> >                 return;\n> >  \n> >         if (!initialized_)\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 2bdb6a81d7c9..a8daeca487ae 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -41,7 +41,7 @@ struct IPASessionConfiguration {\n> >         } hw;\n> >  };\n> >  \n> > -struct IPAFrameContext {\n> > +struct IPAActiveState {\n> >         struct {\n> >                 uint32_t exposure;\n> >                 double gain;\n> > @@ -78,9 +78,12 @@ struct IPAFrameContext {\n> >         unsigned int frameCount;\n> >  };\n> >  \n> > +struct IPAFrameContext {\n> > +};\n> > +\n> >  struct IPAContext {\n> >         IPASessionConfiguration configuration;\n> > -       IPAFrameContext frameContext;\n> > +       IPAActiveState activeState;\n> >  };\n\nThe documentation in ipa_context.cpp needs to be updated too.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nThis patch could go the very front of the series.\n\n> >  \n> >  } /* namespace ipa::rkisp1 */\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 17d42d38eb45..88e6a2b23eed 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> >         context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);\n> >         context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n> >  \n> > -       context_.frameContext.frameCount = 0;\n> > +       context_.activeState.frameCount = 0;\n> >  \n> >         for (auto const &algo : algorithms()) {\n> >                 int ret = algo->configure(context_, info);\n> > @@ -306,7 +306,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> >                 algo->prepare(context_, params);\n> >  \n> >         paramsBufferReady.emit(frame);\n> > -       context_.frameContext.frameCount++;\n> > +       context_.activeState.frameCount++;\n> >  }\n> >  \n> >  void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> > @@ -316,9 +316,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n> >                 reinterpret_cast<rkisp1_stat_buffer *>(\n> >                         mappedBuffers_.at(bufferId).planes()[0].data());\n> >  \n> > -       context_.frameContext.sensor.exposure =\n> > +       context_.activeState.sensor.exposure =\n> >                 sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n> > -       context_.frameContext.sensor.gain =\n> > +       context_.activeState.sensor.gain =\n> >                 camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());\n> >  \n> >         unsigned int aeState = 0;\n> > @@ -333,8 +333,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n> >  \n> >  void IPARkISP1::setControls(unsigned int frame)\n> >  {\n> > -       uint32_t exposure = context_.frameContext.agc.exposure;\n> > -       uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n> > +       uint32_t exposure = context_.activeState.agc.exposure;\n> > +       uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n> >  \n> >         ControlList ctrls(ctrls_);\n> >         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 74EE9C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Aug 2022 00:37:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9EBA161FBD;\n\tWed, 24 Aug 2022 02:37:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A8FDA61FA0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Aug 2022 02:37:08 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D56C92B3;\n\tWed, 24 Aug 2022 02:37:07 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661301430;\n\tbh=ZTtkR9Eug4LMBvqRPSvPlgvGR2PMWomTN5mU9dKyrrU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Zzo8SzfZjwdnY+Wqd71hXGB7q+0zqYnAEYPrSDARNBtkYEr6jkVydeLlbxZe6wwys\n\tnpk+sZgz73NeUSTWYyDudLqt9haGiTc+5hoso7CizIH91DxOhmLL50t0nm3oiTCUaY\n\tMVK3KbqEmp8qKfBb9tVVmSGN4sSCdibZImbAmPWEIvrlldtn7x7dPTVZ9AeJB/l/z4\n\t5UrEw5C/nlbkPRoi6+IpBbJqAx6eSfXCqv7cskYy7PmigflOg7q/lLkMBSVON+vRtW\n\ts4hAOEabEZWADWF6c38UUyjreC8uk3cmYILYKrdB1kZcIanPT9tkVkZoggo0Ekthq6\n\tizKl3+UkTYuZw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661301428;\n\tbh=ZTtkR9Eug4LMBvqRPSvPlgvGR2PMWomTN5mU9dKyrrU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=k827dF7/n1yg2gzhPO9JvitnEVsbtFpcnQ/VTaL34xWIKoVbLyRaTIx2aJjeJFfzi\n\tr2sjUToekCvS+vSE1SjfoFq+hDNwNNG7BYHvFIyzl8UO9Gnw+9ymvIiSBTs6+RRqeV\n\tqPbzku0FHFKnRK6xngi5s+3tVCoCBrtayZo6vnoM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"k827dF7/\"; dkim-atps=neutral","Date":"Wed, 24 Aug 2022 03:37:02 +0300","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YwVyrhcMZgSMot5T@pendragon.ideasonboard.com>","References":"<20220818094410.1671-1-jacopo@jmondi.org>\n\t<20220818094410.1671-3-jacopo@jmondi.org>\n\t<166083884775.542608.3842593206807831315@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166083884775.542608.3842593206807831315@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 02/17] ipa: rkisp1: Rename\n\tframeContext to activeState","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24757,"web_url":"https://patchwork.libcamera.org/comment/24757/","msgid":"<166133322214.3322205.15855074958123555812@Monstersaurus>","date":"2022-08-24T09:27:02","subject":"Re: [libcamera-devel] [PATCH v3 02/17] ipa: rkisp1: Rename\n\tframeContext to activeState","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-08-24 01:37:02)\n> On Thu, Aug 18, 2022 at 05:07:27PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > Quoting Jacopo Mondi via libcamera-devel (2022-08-18 10:43:55)\n> > > From: Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org>\n> > > \n> > > Move the existing frame context structure to the IPAActiveState.\n> > > This structure should store the most up to date results for the IPA\n> > > which may be shared to other algorithms that operate on the data.\n> > \n> > Laurent queried this patch in v2.\n> > \n> > I suggested we add the following to this commit message:\n> > \n> > \"\"\"\n> > All existing algorithm state is moved to the ActiveState and the\n> > algorithms should be updated individually to use the FrameContext\n> > directly where appropriate.\n> > \"\"\"\n> \n> I'd like to make it clear (partly to check that my understanding is\n> correct :-)) that this commit is about moving the existing frame context\n> aside to make space for the new one. How about\n> \n> --------\n> The RkISP1 IPA module creates a single instance of its IPAFrameContext\n> structure, effectively using it more as an active state than a per-frame\n> context. To prepare for the introduction of a real per-frame context,\n> move all the members of the IPAFrameContext structure to a new\n> IPAActiveState structure. The IPAFrameContext becomes effectively\n> unused at runtime, but can't be removed yet as it is still required as a\n> template argument to the Module and Algorithm classes.\n> \n> The new IPAActiveState structure is not foreseen to stay, its members\n> should move to either the new per-frame context that will be introduced,\n> or to the individual algorithm classes.\n\nNot foreseen might be too strong. I keep saying, I believe there may be\ngood cause for it ActiveState to exist. But until we get progress to\nactually implement something real (blocked effectively by this series) I\ncan't quantify exactly what.\n\nMy expectations are that things like Modes (Is AWB enabled or in manual)\nare going to be in a shared IPAActiveState - as these may need to be\nretrieved or queried (or adjusted in the case of disabling a mode so\nthat AF can operate) based on an active state, not a frame context.\n\nOtherwise, the first paragraph is fine with me.\n\n> --------\n> \n> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > ---\n> > >  src/ipa/rkisp1/algorithms/agc.cpp    | 20 ++++++++--------\n> > >  src/ipa/rkisp1/algorithms/awb.cpp    | 34 ++++++++++++++--------------\n> > >  src/ipa/rkisp1/algorithms/blc.cpp    |  2 +-\n> > >  src/ipa/rkisp1/algorithms/cproc.cpp  |  4 ++--\n> > >  src/ipa/rkisp1/algorithms/dpcc.cpp   |  2 +-\n> > >  src/ipa/rkisp1/algorithms/filter.cpp |  4 ++--\n> > >  src/ipa/rkisp1/algorithms/gsl.cpp    |  2 +-\n> > >  src/ipa/rkisp1/algorithms/lsc.cpp    |  2 +-\n> > >  src/ipa/rkisp1/ipa_context.h         |  7 ++++--\n> > >  src/ipa/rkisp1/rkisp1.cpp            | 12 +++++-----\n> > >  10 files changed, 46 insertions(+), 43 deletions(-)\n> > > \n> > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > > index a1bb7d972926..483e941fe427 100644\n> > > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > > @@ -73,8 +73,8 @@ Agc::Agc()\n> > >  int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> > >  {\n> > >         /* Configure the default exposure and gain. */\n> > > -       context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> > > -       context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration;\n> > > +       context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> > > +       context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration;\n> > >  \n> > >         /*\n> > >          * According to the RkISP1 documentation:\n> > > @@ -98,7 +98,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> > >         context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;\n> > >         context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n> > >  \n> > > -       /* \\todo Use actual frame index by populating it in the frameContext. */\n> > > +       /* \\todo Use actual frame index by populating it in the activeState. */\n> \n> That will go later in the series, but we could still keep it accurate in\n> the meantime by writing\n> \n>         /*\n>          * \\todo Use the upcoming per-frame context API that will provide a\n>          * frame index\n>          */\n> \n> > >         frameCount_ = 0;\n> > >         return 0;\n> > >  }\n> > > @@ -140,18 +140,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n> > >  \n> > >  /**\n> > >   * \\brief Estimate the new exposure and gain values\n> > > - * \\param[inout] frameContext The shared IPA frame Context\n> > > + * \\param[inout] context The shared IPA Context\n> \n> And add to the commit message\n> \n> While at it, fix a typo in the documentation of the\n> Agc::computeExposure() function that incorrectly refers to the frame\n> context instead of the global context.\n> \n> > >   * \\param[in] yGain The gain calculated on the current brightness level\n> > >   * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n> > >   */\n> > >  void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n> > >  {\n> > >         IPASessionConfiguration &configuration = context.configuration;\n> > > -       IPAFrameContext &frameContext = context.frameContext;\n> > > +       IPAActiveState &activeState = context.activeState;\n> > >  \n> > >         /* Get the effective exposure and gain applied on the sensor. */\n> > > -       uint32_t exposure = frameContext.sensor.exposure;\n> > > -       double analogueGain = frameContext.sensor.gain;\n> > > +       uint32_t exposure = activeState.sensor.exposure;\n> > > +       double analogueGain = activeState.sensor.gain;\n> > >  \n> > >         /* Use the highest of the two gain estimates. */\n> > >         double evGain = std::max(yGain, iqMeanGain);\n> > > @@ -216,8 +216,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n> > >                               << stepGain;\n> > >  \n> > >         /* Update the estimated exposure and gain. */\n> > > -       frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> > > -       frameContext.agc.gain = stepGain;\n> > > +       activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> > > +       activeState.agc.gain = stepGain;\n> > >  }\n> > >  \n> > >  /**\n> > > @@ -324,7 +324,7 @@ void Agc::process(IPAContext &context,\n> > >   */\n> > >  void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> > >  {\n> > > -       if (context.frameContext.frameCount > 0)\n> > > +       if (context.activeState.frameCount > 0)\n> > >                 return;\n> > >  \n> > >         /* Configure the measurement window. */\n> > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > index 9f00364d12b1..427aaeb1e955 100644\n> > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > @@ -35,9 +35,9 @@ LOG_DEFINE_CATEGORY(RkISP1Awb)\n> > >  int Awb::configure(IPAContext &context,\n> > >                    const IPACameraSensorInfo &configInfo)\n> > >  {\n> > > -       context.frameContext.awb.gains.red = 1.0;\n> > > -       context.frameContext.awb.gains.blue = 1.0;\n> > > -       context.frameContext.awb.gains.green = 1.0;\n> > > +       context.activeState.awb.gains.red = 1.0;\n> > > +       context.activeState.awb.gains.blue = 1.0;\n> > > +       context.activeState.awb.gains.green = 1.0;\n> > >  \n> > >         /*\n> > >          * Define the measurement window for AWB as a centered rectangle\n> > > @@ -72,16 +72,16 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n> > >   */\n> > >  void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> > >  {\n> > > -       params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green;\n> > > -       params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue;\n> > > -       params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red;\n> > > -       params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green;\n> > > +       params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green;\n> > > +       params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue;\n> > > +       params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red;\n> > > +       params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green;\n> > >  \n> > >         /* Update the gains. */\n> > >         params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n> > >  \n> > >         /* If we already have configured the gains and window, return. */\n> > > -       if (context.frameContext.frameCount > 0)\n> > > +       if (context.activeState.frameCount > 0)\n> > >                 return;\n> > >  \n> > >         /* Configure the gains to apply. */\n> > > @@ -125,7 +125,7 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n> > >  {\n> > >         const rkisp1_cif_isp_stat *params = &stats->params;\n> > >         const rkisp1_cif_isp_awb_stat *awb = &params->awb;\n> > > -       IPAFrameContext &frameContext = context.frameContext;\n> > > +       IPAActiveState &activeState = context.activeState;\n> > >  \n> > >         /* Get the YCbCr mean values */\n> > >         double yMean = awb->awb_mean[0].mean_y_or_g;\n> > > @@ -157,22 +157,22 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n> > >  \n> > >         /* Filter the values to avoid oscillations. */\n> > >         double speed = 0.2;\n> > > -       redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red;\n> > > -       blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue;\n> > > +       redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red;\n> > > +       blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue;\n> > >  \n> > >         /*\n> > >          * Gain values are unsigned integer value, range 0 to 4 with 8 bit\n> > >          * fractional part.\n> > >          */\n> > > -       frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> > > -       frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> > > +       activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> > > +       activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> > >         /* Hardcode the green gain to 1.0. */\n> > > -       frameContext.awb.gains.green = 1.0;\n> > > +       activeState.awb.gains.green = 1.0;\n> > >  \n> > > -       frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> > > +       activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> > >  \n> > > -       LOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.frameContext.awb.gains.red\n> > > -                             << \" and for blue: \" << context.frameContext.awb.gains.blue;\n> > > +       LOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.activeState.awb.gains.red\n> > > +                             << \" and for blue: \" << context.activeState.awb.gains.blue;\n> > >  }\n> > >  \n> > >  REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> > > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> > > index a58569fa2dc2..4d55a2d529cb 100644\n> > > --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> > > @@ -68,7 +68,7 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,\n> > >  void BlackLevelCorrection::prepare(IPAContext &context,\n> > >                                    rkisp1_params_cfg *params)\n> > >  {\n> > > -       if (context.frameContext.frameCount > 0)\n> > > +       if (context.activeState.frameCount > 0)\n> > >                 return;\n> > >  \n> > >         if (!tuningParameters_)\n> > > diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\n> > > index bca5ab6907d6..a3b778d1c908 100644\n> > > --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> > > @@ -40,7 +40,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> > >                                    [[maybe_unused]] const uint32_t frame,\n> > >                                    const ControlList &controls)\n> > >  {\n> > > -       auto &cproc = context.frameContext.cproc;\n> > > +       auto &cproc = context.activeState.cproc;\n> > >  \n> > >         const auto &brightness = controls.get(controls::Brightness);\n> > >         if (brightness) {\n> > > @@ -73,7 +73,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n> > >  void ColorProcessing::prepare(IPAContext &context,\n> > >                               rkisp1_params_cfg *params)\n> > >  {\n> > > -       auto &cproc = context.frameContext.cproc;\n> > > +       auto &cproc = context.activeState.cproc;\n> > >  \n> > >         /* Check if the algorithm configuration has been updated. */\n> > >         if (!cproc.updateParams)\n> > > diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> > > index 69bc651eaf08..93d37b1dae44 100644\n> > > --- a/src/ipa/rkisp1/algorithms/dpcc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> > > @@ -234,7 +234,7 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context,\n> > >  void DefectPixelClusterCorrection::prepare(IPAContext &context,\n> > >                                            rkisp1_params_cfg *params)\n> > >  {\n> > > -       if (context.frameContext.frameCount > 0)\n> > > +       if (context.activeState.frameCount > 0)\n> > >                 return;\n> > >  \n> > >         if (!initialized_)\n> > > diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp\n> > > index 8ca10fd1ee9d..bc7fc1f32f34 100644\n> > > --- a/src/ipa/rkisp1/algorithms/filter.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/filter.cpp\n> > > @@ -46,7 +46,7 @@ void Filter::queueRequest(IPAContext &context,\n> > >                           [[maybe_unused]] const uint32_t frame,\n> > >                           const ControlList &controls)\n> > >  {\n> > > -       auto &filter = context.frameContext.filter;\n> > > +       auto &filter = context.activeState.filter;\n> > >  \n> > >         const auto &sharpness = controls.get(controls::Sharpness);\n> > >         if (sharpness) {\n> > > @@ -87,7 +87,7 @@ void Filter::queueRequest(IPAContext &context,\n> > >   */\n> > >  void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> > >  {\n> > > -       auto &filter = context.frameContext.filter;\n> > > +       auto &filter = context.activeState.filter;\n> > >  \n> > >         /* Check if the algorithm configuration has been updated. */\n> > >         if (!filter.updateParams)\n> > > diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp\n> > > index 2fd1a23d3a9b..dd9974627cd2 100644\n> > > --- a/src/ipa/rkisp1/algorithms/gsl.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/gsl.cpp\n> > > @@ -121,7 +121,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,\n> > >  void GammaSensorLinearization::prepare(IPAContext &context,\n> > >                                        rkisp1_params_cfg *params)\n> > >  {\n> > > -       if (context.frameContext.frameCount > 0)\n> > > +       if (context.activeState.frameCount > 0)\n> > >                 return;\n> > >  \n> > >         if (!initialized_)\n> > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > index 05c8c0dab5c8..4ed467086199 100644\n> > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > @@ -125,7 +125,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > >  void LensShadingCorrection::prepare(IPAContext &context,\n> > >                                     rkisp1_params_cfg *params)\n> > >  {\n> > > -       if (context.frameContext.frameCount > 0)\n> > > +       if (context.activeState.frameCount > 0)\n> > >                 return;\n> > >  \n> > >         if (!initialized_)\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index 2bdb6a81d7c9..a8daeca487ae 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -41,7 +41,7 @@ struct IPASessionConfiguration {\n> > >         } hw;\n> > >  };\n> > >  \n> > > -struct IPAFrameContext {\n> > > +struct IPAActiveState {\n> > >         struct {\n> > >                 uint32_t exposure;\n> > >                 double gain;\n> > > @@ -78,9 +78,12 @@ struct IPAFrameContext {\n> > >         unsigned int frameCount;\n> > >  };\n> > >  \n> > > +struct IPAFrameContext {\n> > > +};\n> > > +\n> > >  struct IPAContext {\n> > >         IPASessionConfiguration configuration;\n> > > -       IPAFrameContext frameContext;\n> > > +       IPAActiveState activeState;\n> > >  };\n> \n> The documentation in ipa_context.cpp needs to be updated too.\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> This patch could go the very front of the series.\n> \n> > >  \n> > >  } /* namespace ipa::rkisp1 */\n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index 17d42d38eb45..88e6a2b23eed 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> > >         context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);\n> > >         context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n> > >  \n> > > -       context_.frameContext.frameCount = 0;\n> > > +       context_.activeState.frameCount = 0;\n> > >  \n> > >         for (auto const &algo : algorithms()) {\n> > >                 int ret = algo->configure(context_, info);\n> > > @@ -306,7 +306,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > >                 algo->prepare(context_, params);\n> > >  \n> > >         paramsBufferReady.emit(frame);\n> > > -       context_.frameContext.frameCount++;\n> > > +       context_.activeState.frameCount++;\n> > >  }\n> > >  \n> > >  void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> > > @@ -316,9 +316,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n> > >                 reinterpret_cast<rkisp1_stat_buffer *>(\n> > >                         mappedBuffers_.at(bufferId).planes()[0].data());\n> > >  \n> > > -       context_.frameContext.sensor.exposure =\n> > > +       context_.activeState.sensor.exposure =\n> > >                 sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n> > > -       context_.frameContext.sensor.gain =\n> > > +       context_.activeState.sensor.gain =\n> > >                 camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());\n> > >  \n> > >         unsigned int aeState = 0;\n> > > @@ -333,8 +333,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n> > >  \n> > >  void IPARkISP1::setControls(unsigned int frame)\n> > >  {\n> > > -       uint32_t exposure = context_.frameContext.agc.exposure;\n> > > -       uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n> > > +       uint32_t exposure = context_.activeState.agc.exposure;\n> > > +       uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n> > >  \n> > >         ControlList ctrls(ctrls_);\n> > >         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EA4AFC0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Aug 2022 09:27:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7889B61FBE;\n\tWed, 24 Aug 2022 11:27:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A94961FA2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Aug 2022 11:27:05 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C41582B3;\n\tWed, 24 Aug 2022 11:27:04 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661333226;\n\tbh=/d/B3xlITB/tYG8OP/+ubvnGYdiMhlGw/a3jKRVnX9Y=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=paEpegChRmCi2T/qylhNLWjFx33J7DEhtqiao5sLMEXXOzU794i27n0BNct14UE1M\n\trq4XPxWUNECNOgJ2ARKG9Z4AkoKfDUwKbh1tp4BOWIrkcyiXxruJ+p0eNP/GzH3xw8\n\tVpk0dJMQOzclovVQWz3Ofkt3hMzyaRqFR553lI3C4ab0aMMN6h1D2nfDOzgbCkTVUt\n\tzzNE//U365bVFgDo8DMe/PmRCz+IjuQGxRzqZ6IqNhLYi0BFmmdGphvag1lgmPiVoq\n\tZq7NkisSnH6TfJWE+WqfY6TZsN2QMKiG38yhPvIb7qmUKuic4/xNYEl0tPo7u406e4\n\t7GRnrVOl+mW6w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661333224;\n\tbh=/d/B3xlITB/tYG8OP/+ubvnGYdiMhlGw/a3jKRVnX9Y=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=GW7aDuWqOb2F5jSFU9CNd1OKEDvX+GKE1QCv6vRn/jHw9URB+oBck7Tt7jjlKpG/O\n\tYslFXJukMnVS9fN2+KYw/QoyJ0x6DWmhEvNcl3z1+KUV4ZvwX61W8CUVkS5dtFhoYO\n\txbZLOaqyCUfSnQYG96pSi0jHv76U6TAfshv5q58o="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GW7aDuWq\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<YwVyrhcMZgSMot5T@pendragon.ideasonboard.com>","References":"<20220818094410.1671-1-jacopo@jmondi.org>\n\t<20220818094410.1671-3-jacopo@jmondi.org>\n\t<166083884775.542608.3842593206807831315@Monstersaurus>\n\t<YwVyrhcMZgSMot5T@pendragon.ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Wed, 24 Aug 2022 10:27:02 +0100","Message-ID":"<166133322214.3322205.15855074958123555812@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 02/17] ipa: rkisp1: Rename\n\tframeContext to activeState","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]