[{"id":25020,"web_url":"https://patchwork.libcamera.org/comment/25020/","msgid":"<166368513469.3912877.4529095203848557407@Monstersaurus>","date":"2022-09-20T14:45:34","subject":"Re: [libcamera-devel] [PATCH v4 14/32] 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-09-08 02:41:42)\n> From: Kieran Bingham <kieran.bingham@ideasonboard.com>\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, and will be populated back with per-frame data after\n> converting the RkISP1 IPA module to using a frame context queue.\n> \n> The IPAActiveState structure will slowly morph into a different entity\n> as individual algorithm get later ported to the frame context API.\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> 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> Changes since v3:\n> \n> - Rewrite commit message\n\nPerhaps an SoB too ;-)\n\nThat's fine by me, and this is a key part of converting this IPA.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> ---\n>  src/ipa/rkisp1/algorithms/agc.cpp    | 23 ++++---\n>  src/ipa/rkisp1/algorithms/awb.cpp    | 40 ++++++------\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/dpf.cpp    |  6 +-\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.cpp       | 94 +++++++++++++++-------------\n>  src/ipa/rkisp1/ipa_context.h         |  7 ++-\n>  src/ipa/rkisp1/rkisp1.cpp            | 12 ++--\n>  12 files changed, 105 insertions(+), 93 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 4c246d71a739..606f13e776a3 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,10 @@ 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> +       /*\n> +        * \\todo Use the upcoming per-frame context API that will provide a\n> +        * frame index\n> +        */\n>         frameCount_ = 0;\n>         return 0;\n>  }\n> @@ -140,18 +143,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 +219,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> @@ -328,7 +331,7 @@ void Agc::prepare(IPAContext &context,\n>                   [[maybe_unused]] IPAFrameContext &frameContext,\n>                   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 a23c32015454..2bd9ef779bc3 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -36,10 +36,10 @@ 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.frameContext.awb.autoEnabled = true;\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> +       context.activeState.awb.autoEnabled = true;\n>  \n>         /*\n>          * Define the measurement window for AWB as a centered rectangle\n> @@ -79,16 +79,16 @@ void Awb::prepare(IPAContext &context,\n>                   [[maybe_unused]] IPAFrameContext &frameContext,\n>                   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> @@ -131,7 +131,7 @@ void Awb::queueRequest(IPAContext &context,\n>                        [[maybe_unused]] IPAFrameContext &frameContext,\n>                        const ControlList &controls)\n>  {\n> -       auto &awb = context.frameContext.awb;\n> +       auto &awb = context.activeState.awb;\n>  \n>         const auto &awbEnable = controls.get(controls::AwbEnable);\n>         if (awbEnable && *awbEnable != awb.autoEnabled) {\n> @@ -162,7 +162,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> @@ -194,24 +194,24 @@ 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> -       if (frameContext.awb.autoEnabled) {\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> +       if (activeState.awb.autoEnabled) {\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>         }\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 c45a317ada08..0f7226cf217d 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -70,7 +70,7 @@ void BlackLevelCorrection::prepare(IPAContext &context,\n>                                    [[maybe_unused]] IPAFrameContext &frameContext,\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 bc4a07073706..ea819b2acfcb 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> @@ -41,7 +41,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n>                                    [[maybe_unused]] IPAFrameContext &frameContext,\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> @@ -76,7 +76,7 @@ void ColorProcessing::prepare(IPAContext &context,\n>                               [[maybe_unused]] IPAFrameContext &frameContext,\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 5030526769c7..7c14ace0abee 100644\n> --- a/src/ipa/rkisp1/algorithms/dpcc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> @@ -263,7 +263,7 @@ void DefectPixelClusterCorrection::prepare(IPAContext &context,\n>                                            [[maybe_unused]] IPAFrameContext &frameContext,\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/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index be024fc5fe90..b8c837c21243 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -179,7 +179,7 @@ void Dpf::queueRequest(IPAContext &context,\n>                        [[maybe_unused]] IPAFrameContext &frameContext,\n>                        const ControlList &controls)\n>  {\n> -       auto &dpf = context.frameContext.dpf;\n> +       auto &dpf = context.activeState.dpf;\n>  \n>         const auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n>         if (denoise) {\n> @@ -214,9 +214,9 @@ void Dpf::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>         if (!initialized_)\n>                 return;\n>  \n> -       auto &dpf = context.frameContext.dpf;\n> +       auto &dpf = context.activeState.dpf;\n>  \n> -       if (context.frameContext.frameCount == 0) {\n> +       if (context.activeState.frameCount == 0) {\n>                 params->others.dpf_config = config_;\n>                 params->others.dpf_strength_config = strengthConfig_;\n>  \n> diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp\n> index 6aa5476aa25e..837560eb20a6 100644\n> --- a/src/ipa/rkisp1/algorithms/filter.cpp\n> +++ b/src/ipa/rkisp1/algorithms/filter.cpp\n> @@ -47,7 +47,7 @@ void Filter::queueRequest(IPAContext &context,\n>                           [[maybe_unused]] IPAFrameContext &frameContext,\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> @@ -91,7 +91,7 @@ void Filter::prepare(IPAContext &context,\n>                      [[maybe_unused]] IPAFrameContext &frameContext,\n>                      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 dfc76b3d8ffd..879ca2973e8a 100644\n> --- a/src/ipa/rkisp1/algorithms/gsl.cpp\n> +++ b/src/ipa/rkisp1/algorithms/gsl.cpp\n> @@ -123,7 +123,7 @@ void GammaSensorLinearization::prepare(IPAContext &context,\n>                                        [[maybe_unused]] IPAFrameContext &frameContext,\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 e4b04136dbaf..9c717bc5f99e 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -137,7 +137,7 @@ void LensShadingCorrection::prepare(IPAContext &context,\n>                                     [[maybe_unused]] IPAFrameContext &frameContext,\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.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index cc66bc70ac44..d18f4996aad2 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -85,114 +85,115 @@ namespace libcamera::ipa::rkisp1 {\n>   */\n>  \n>  /**\n> - * \\struct IPAFrameContext\n> - * \\brief Per-frame context for algorithms\n> + * \\struct IPAActiveState\n> + * \\brief Active state for algorithms\n>   *\n> - * The frame context stores data specific to a single frame processed by the\n> - * IPA. Each frame processed by the IPA has a context associated with it,\n> - * accessible through the IPAContext structure.\n> + * The active state stores algorithm-specific data that needs to be shared\n> + * between multiple algorithms and the IPA module. It is accessible through the\n> + * IPAContext structure.\n>   *\n> - * \\todo Detail how to access contexts for a particular frame\n> + * \\todo Split the data contained in this structure between the active state\n> + * and the frame contexts.\n>   *\n> - * Each of the fields in the frame context belongs to either a specific\n> + * Each of the fields in the active state belongs to either a specific\n>   * algorithm, or to the top-level IPA module. A field may be read by any\n>   * algorithm, but should only be written by its owner.\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::agc\n> - * \\brief Context for the Automatic Gain Control algorithm\n> + * \\var IPAActiveState::agc\n> + * \\brief State for the Automatic Gain Control algorithm\n>   *\n>   * The exposure and gain determined are expected to be applied to the sensor\n>   * at the earliest opportunity.\n>   *\n> - * \\var IPAFrameContext::agc.exposure\n> + * \\var IPAActiveState::agc.exposure\n>   * \\brief Exposure time expressed as a number of lines\n>   *\n> - * \\var IPAFrameContext::agc.gain\n> + * \\var IPAActiveState::agc.gain\n>   * \\brief Analogue gain multiplier\n>   *\n>   * The gain should be adapted to the sensor specific gain code before applying.\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::awb\n> - * \\brief Context for the Automatic White Balance algorithm\n> + * \\var IPAActiveState::awb\n> + * \\brief State for the Automatic White Balance algorithm\n>   *\n> - * \\struct IPAFrameContext::awb.gains\n> + * \\struct IPAActiveState::awb.gains\n>   * \\brief White balance gains\n>   *\n> - * \\var IPAFrameContext::awb.gains.red\n> + * \\var IPAActiveState::awb.gains.red\n>   * \\brief White balance gain for R channel\n>   *\n> - * \\var IPAFrameContext::awb.gains.green\n> + * \\var IPAActiveState::awb.gains.green\n>   * \\brief White balance gain for G channel\n>   *\n> - * \\var IPAFrameContext::awb.gains.blue\n> + * \\var IPAActiveState::awb.gains.blue\n>   * \\brief White balance gain for B channel\n>   *\n> - * \\var IPAFrameContext::awb.temperatureK\n> + * \\var IPAActiveState::awb.temperatureK\n>   * \\brief Estimated color temperature\n>   *\n> - * \\var IPAFrameContext::awb.autoEnabled\n> + * \\var IPAActiveState::awb.autoEnabled\n>   * \\brief Whether the Auto White Balance algorithm is enabled\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::cproc\n> - * \\brief Context for the Color Processing algorithm\n> + * \\var IPAActiveState::cproc\n> + * \\brief State for the Color Processing algorithm\n>   *\n> - * \\struct IPAFrameContext::cproc.brightness\n> + * \\struct IPAActiveState::cproc.brightness\n>   * \\brief Brightness level\n>   *\n> - * \\var IPAFrameContext::cproc.contrast\n> + * \\var IPAActiveState::cproc.contrast\n>   * \\brief Contrast level\n>   *\n> - * \\var IPAFrameContext::cproc.saturation\n> + * \\var IPAActiveState::cproc.saturation\n>   * \\brief Saturation level\n>   *\n> - * \\var IPAFrameContext::cproc.updateParams\n> + * \\var IPAActiveState::cproc.updateParams\n>   * \\brief Indicates if ISP parameters need to be updated\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::dpf\n> - * \\brief Context for the Denoise Pre-Filter algorithm\n> + * \\var IPAActiveState::dpf\n> + * \\brief State for the Denoise Pre-Filter algorithm\n>   *\n> - * \\var IPAFrameContext::dpf.denoise\n> + * \\var IPAActiveState::dpf.denoise\n>   * \\brief Indicates if denoise is activated\n>   *\n> - * \\var IPAFrameContext::dpf.updateParams\n> + * \\var IPAActiveState::dpf.updateParams\n>   * \\brief Indicates if ISP parameters need to be updated\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::filter\n> - * \\brief Context for the Filter algorithm\n> + * \\var IPAActiveState::filter\n> + * \\brief State for the Filter algorithm\n>   *\n> - * \\struct IPAFrameContext::filter.denoise\n> + * \\struct IPAActiveState::filter.denoise\n>   * \\brief Denoising level\n>   *\n> - * \\var IPAFrameContext::filter.sharpness\n> + * \\var IPAActiveState::filter.sharpness\n>   * \\brief Sharpness level\n>   *\n> - * \\var IPAFrameContext::filter.updateParams\n> + * \\var IPAActiveState::filter.updateParams\n>   * \\brief Indicates if ISP parameters need to be updated\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::sensor\n> + * \\var IPAActiveState::sensor\n>   * \\brief Effective sensor values\n>   *\n> - * \\var IPAFrameContext::sensor.exposure\n> + * \\var IPAActiveState::sensor.exposure\n>   * \\brief Exposure time expressed as a number of lines\n>   *\n> - * \\var IPAFrameContext::sensor.gain\n> + * \\var IPAActiveState::sensor.gain\n>   * \\brief Analogue gain multiplier\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::frameCount\n> + * \\var IPAActiveState::frameCount\n>   * \\brief Counter of requests queued to the IPA module\n>   *\n>   * The counter is reset to 0 when the IPA module is configured, and is\n> @@ -200,6 +201,14 @@ namespace libcamera::ipa::rkisp1 {\n>   * Algorithm::prepare() function of all algorithms.\n>   */\n>  \n> +/**\n> + * \\struct IPAFrameContext\n> + * \\brief Per-frame context for algorithms\n> + *\n> + * This structure is currently unused and will be replaced by a real per-frame\n> + * context.\n> + */\n> +\n>  /**\n>   * \\struct IPAContext\n>   * \\brief Global IPA context data shared between all algorithms\n> @@ -207,13 +216,10 @@ namespace libcamera::ipa::rkisp1 {\n>   * \\var IPAContext::configuration\n>   * \\brief The IPA session configuration, immutable during the session\n>   *\n> - * \\var IPAContext::frameContext\n> - * \\brief The frame context for the frame being processed\n> + * \\var IPAContext::activeState\n> + * \\brief The IPA active state, storing the latest state for all algorithms\n>   *\n> - * \\todo While the frame context is supposed to be per-frame, this\n> - * single frame context stores data related to both the current frame\n> - * and the previous frames, with fields being updated as the algorithms\n> - * are run. This needs to be turned into real per-frame data storage.\n> + * \\todo Introduce per-frame contexts\n>   */\n>  \n>  } /* namespace libcamera::ipa::rkisp1 */\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 854151f2ca75..5590482c6a8c 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -46,7 +46,7 @@ struct IPASessionConfiguration {\n>         } hw;\n>  };\n>  \n> -struct IPAFrameContext {\n> +struct IPAActiveState {\n>         struct {\n>                 uint32_t exposure;\n>                 double gain;\n> @@ -89,9 +89,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 f7a17459b6c3..24d5b9647838 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -244,7 +244,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> @@ -310,7 +310,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>                 algo->prepare(context_, frame, frameContext, 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> @@ -320,9 +320,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> @@ -340,8 +340,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> Regards,\n> \n> Laurent Pinchart\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 EB422C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 20 Sep 2022 14:45:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 555C96218B;\n\tTue, 20 Sep 2022 16:45:39 +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 ECD6B6218B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 20 Sep 2022 16:45:37 +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 635556BE;\n\tTue, 20 Sep 2022 16:45:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663685139;\n\tbh=K38tyBiYN1AFsHbQwZb8zuBMLWFMDMTlyU2klM/4vx8=;\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=0gSvRQ0OplwADZOZ75O1LIdDSzkyfy1RxR+H83mMJqHRxM5W+D2RoKV339ObOTG8s\n\tZuTDCWEkJJtdZIOPsc5ogKKRGf/6COkXDmCgnjDuB/RNRHDvUh242jV2XOU8ewf1me\n\tWUcGhTRhnPhInoKwLGPDxgtuM3kDFzHgW8SZ2EAlDJ1aJ2komde5XJjz3oBuCSFvme\n\tjqzxttGck9BKJRixRVnCIvzXmdEEM808aUUHMJ+GDR8/gxjWkULY4yS35/SQB5eO6b\n\tOkcf9nZhiT6HnOXrmDb17v8eUvA4D/D2LM+YGxdty2S/rzXdgFF3g6sXwnBzSYQxR/\n\tDg4ljO9ykj22g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663685137;\n\tbh=K38tyBiYN1AFsHbQwZb8zuBMLWFMDMTlyU2klM/4vx8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=KrOoBDb6JbDOGZY5Ym3q9iTXoxvwBW4e9P0Uorc7f9dgrMOyAlhjV3Q8+ABxoAvQg\n\tIGydge+6IXQ7p/guaVbqa4GXSkA51Iirg4frVtZanyyH9rWzZfGKV23bPlqDOOg+ZB\n\tXYbA1lSidZH8BGxxYGv5gFfEsEdGNSGjmiJMP8Ok="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"KrOoBDb6\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220908014200.28728-15-laurent.pinchart@ideasonboard.com>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-15-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 20 Sep 2022 15:45:34 +0100","Message-ID":"<166368513469.3912877.4529095203848557407@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 14/32] 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":25060,"web_url":"https://patchwork.libcamera.org/comment/25060/","msgid":"<20220921183211.hdg3ab6vccw55wub@uno.localdomain>","date":"2022-09-21T18:32:11","subject":"Re: [libcamera-devel] [PATCH v4 14/32] ipa: rkisp1: Rename\n\tframeContext to activeState","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Thu, Sep 08, 2022 at 04:41:42AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> From: Kieran Bingham <kieran.bingham@ideasonboard.com>\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, and will be populated back with per-frame data after\n> converting the RkISP1 IPA module to using a frame context queue.\n>\n> The IPAActiveState structure will slowly morph into a different entity\n> as individual algorithm get later ported to the frame context API.\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> 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\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\n> ---\n> Changes since v3:\n>\n> - Rewrite commit message\n> ---\n>  src/ipa/rkisp1/algorithms/agc.cpp    | 23 ++++---\n>  src/ipa/rkisp1/algorithms/awb.cpp    | 40 ++++++------\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/dpf.cpp    |  6 +-\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.cpp       | 94 +++++++++++++++-------------\n>  src/ipa/rkisp1/ipa_context.h         |  7 ++-\n>  src/ipa/rkisp1/rkisp1.cpp            | 12 ++--\n>  12 files changed, 105 insertions(+), 93 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 4c246d71a739..606f13e776a3 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>  \t/* Configure the default exposure and gain. */\n> -\tcontext.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> -\tcontext.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration;\n> +\tcontext.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> +\tcontext.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration;\n>\n>  \t/*\n>  \t * According to the RkISP1 documentation:\n> @@ -98,7 +98,10 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>  \tcontext.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;\n>  \tcontext.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n>\n> -\t/* \\todo Use actual frame index by populating it in the frameContext. */\n> +\t/*\n> +\t * \\todo Use the upcoming per-frame context API that will provide a\n> +\t * frame index\n> +\t */\n>  \tframeCount_ = 0;\n>  \treturn 0;\n>  }\n> @@ -140,18 +143,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>  \tIPASessionConfiguration &configuration = context.configuration;\n> -\tIPAFrameContext &frameContext = context.frameContext;\n> +\tIPAActiveState &activeState = context.activeState;\n>\n>  \t/* Get the effective exposure and gain applied on the sensor. */\n> -\tuint32_t exposure = frameContext.sensor.exposure;\n> -\tdouble analogueGain = frameContext.sensor.gain;\n> +\tuint32_t exposure = activeState.sensor.exposure;\n> +\tdouble analogueGain = activeState.sensor.gain;\n>\n>  \t/* Use the highest of the two gain estimates. */\n>  \tdouble evGain = std::max(yGain, iqMeanGain);\n> @@ -216,8 +219,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n>  \t\t\t      << stepGain;\n>\n>  \t/* Update the estimated exposure and gain. */\n> -\tframeContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> -\tframeContext.agc.gain = stepGain;\n> +\tactiveState.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> +\tactiveState.agc.gain = stepGain;\n>  }\n>\n>  /**\n> @@ -328,7 +331,7 @@ void Agc::prepare(IPAContext &context,\n>  \t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t  rkisp1_params_cfg *params)\n>  {\n> -\tif (context.frameContext.frameCount > 0)\n> +\tif (context.activeState.frameCount > 0)\n>  \t\treturn;\n>\n>  \t/* Configure the measurement window. */\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index a23c32015454..2bd9ef779bc3 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -36,10 +36,10 @@ LOG_DEFINE_CATEGORY(RkISP1Awb)\n>  int Awb::configure(IPAContext &context,\n>  \t\t   const IPACameraSensorInfo &configInfo)\n>  {\n> -\tcontext.frameContext.awb.gains.red = 1.0;\n> -\tcontext.frameContext.awb.gains.blue = 1.0;\n> -\tcontext.frameContext.awb.gains.green = 1.0;\n> -\tcontext.frameContext.awb.autoEnabled = true;\n> +\tcontext.activeState.awb.gains.red = 1.0;\n> +\tcontext.activeState.awb.gains.blue = 1.0;\n> +\tcontext.activeState.awb.gains.green = 1.0;\n> +\tcontext.activeState.awb.autoEnabled = true;\n>\n>  \t/*\n>  \t * Define the measurement window for AWB as a centered rectangle\n> @@ -79,16 +79,16 @@ void Awb::prepare(IPAContext &context,\n>  \t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t  rkisp1_params_cfg *params)\n>  {\n> -\tparams->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green;\n> -\tparams->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue;\n> -\tparams->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red;\n> -\tparams->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green;\n> +\tparams->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green;\n> +\tparams->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue;\n> +\tparams->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red;\n> +\tparams->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green;\n>\n>  \t/* Update the gains. */\n>  \tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;\n>\n>  \t/* If we already have configured the gains and window, return. */\n> -\tif (context.frameContext.frameCount > 0)\n> +\tif (context.activeState.frameCount > 0)\n>  \t\treturn;\n>\n>  \t/* Configure the gains to apply. */\n> @@ -131,7 +131,7 @@ void Awb::queueRequest(IPAContext &context,\n>  \t\t       [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t       const ControlList &controls)\n>  {\n> -\tauto &awb = context.frameContext.awb;\n> +\tauto &awb = context.activeState.awb;\n>\n>  \tconst auto &awbEnable = controls.get(controls::AwbEnable);\n>  \tif (awbEnable && *awbEnable != awb.autoEnabled) {\n> @@ -162,7 +162,7 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n>  {\n>  \tconst rkisp1_cif_isp_stat *params = &stats->params;\n>  \tconst rkisp1_cif_isp_awb_stat *awb = &params->awb;\n> -\tIPAFrameContext &frameContext = context.frameContext;\n> +\tIPAActiveState &activeState = context.activeState;\n>\n>  \t/* Get the YCbCr mean values */\n>  \tdouble yMean = awb->awb_mean[0].mean_y_or_g;\n> @@ -194,24 +194,24 @@ void Awb::process([[maybe_unused]] IPAContext &context,\n>\n>  \t/* Filter the values to avoid oscillations. */\n>  \tdouble speed = 0.2;\n> -\tredGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red;\n> -\tblueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue;\n> +\tredGain = speed * redGain + (1 - speed) * activeState.awb.gains.red;\n> +\tblueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue;\n>\n>  \t/*\n>  \t * Gain values are unsigned integer value, range 0 to 4 with 8 bit\n>  \t * fractional part.\n>  \t */\n> -\tif (frameContext.awb.autoEnabled) {\n> -\t\tframeContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> -\t\tframeContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n> +\tif (activeState.awb.autoEnabled) {\n> +\t\tactiveState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);\n> +\t\tactiveState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);\n>  \t}\n>  \t/* Hardcode the green gain to 1.0. */\n> -\tframeContext.awb.gains.green = 1.0;\n> +\tactiveState.awb.gains.green = 1.0;\n>\n> -\tframeContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n> +\tactiveState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);\n>\n> -\tLOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.frameContext.awb.gains.red\n> -\t\t\t      << \" and for blue: \" << context.frameContext.awb.gains.blue;\n> +\tLOG(RkISP1Awb, Debug) << \"Gain found for red: \" << context.activeState.awb.gains.red\n> +\t\t\t      << \" 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 c45a317ada08..0f7226cf217d 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -70,7 +70,7 @@ void BlackLevelCorrection::prepare(IPAContext &context,\n>  \t\t\t\t   [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t\t\t   rkisp1_params_cfg *params)\n>  {\n> -\tif (context.frameContext.frameCount > 0)\n> +\tif (context.activeState.frameCount > 0)\n>  \t\treturn;\n>\n>  \tif (!tuningParameters_)\n> diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\n> index bc4a07073706..ea819b2acfcb 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> @@ -41,7 +41,7 @@ void ColorProcessing::queueRequest(IPAContext &context,\n>  \t\t\t\t   [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t\t\t   const ControlList &controls)\n>  {\n> -\tauto &cproc = context.frameContext.cproc;\n> +\tauto &cproc = context.activeState.cproc;\n>\n>  \tconst auto &brightness = controls.get(controls::Brightness);\n>  \tif (brightness) {\n> @@ -76,7 +76,7 @@ void ColorProcessing::prepare(IPAContext &context,\n>  \t\t\t      [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t\t      rkisp1_params_cfg *params)\n>  {\n> -\tauto &cproc = context.frameContext.cproc;\n> +\tauto &cproc = context.activeState.cproc;\n>\n>  \t/* Check if the algorithm configuration has been updated. */\n>  \tif (!cproc.updateParams)\n> diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> index 5030526769c7..7c14ace0abee 100644\n> --- a/src/ipa/rkisp1/algorithms/dpcc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp\n> @@ -263,7 +263,7 @@ void DefectPixelClusterCorrection::prepare(IPAContext &context,\n>  \t\t\t\t\t   [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t\t\t\t   rkisp1_params_cfg *params)\n>  {\n> -\tif (context.frameContext.frameCount > 0)\n> +\tif (context.activeState.frameCount > 0)\n>  \t\treturn;\n>\n>  \tif (!initialized_)\n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index be024fc5fe90..b8c837c21243 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -179,7 +179,7 @@ void Dpf::queueRequest(IPAContext &context,\n>  \t\t       [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t       const ControlList &controls)\n>  {\n> -\tauto &dpf = context.frameContext.dpf;\n> +\tauto &dpf = context.activeState.dpf;\n>\n>  \tconst auto &denoise = controls.get(controls::draft::NoiseReductionMode);\n>  \tif (denoise) {\n> @@ -214,9 +214,9 @@ void Dpf::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \tif (!initialized_)\n>  \t\treturn;\n>\n> -\tauto &dpf = context.frameContext.dpf;\n> +\tauto &dpf = context.activeState.dpf;\n>\n> -\tif (context.frameContext.frameCount == 0) {\n> +\tif (context.activeState.frameCount == 0) {\n>  \t\tparams->others.dpf_config = config_;\n>  \t\tparams->others.dpf_strength_config = strengthConfig_;\n>\n> diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp\n> index 6aa5476aa25e..837560eb20a6 100644\n> --- a/src/ipa/rkisp1/algorithms/filter.cpp\n> +++ b/src/ipa/rkisp1/algorithms/filter.cpp\n> @@ -47,7 +47,7 @@ void Filter::queueRequest(IPAContext &context,\n>  \t\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t\t  const ControlList &controls)\n>  {\n> -\tauto &filter = context.frameContext.filter;\n> +\tauto &filter = context.activeState.filter;\n>\n>  \tconst auto &sharpness = controls.get(controls::Sharpness);\n>  \tif (sharpness) {\n> @@ -91,7 +91,7 @@ void Filter::prepare(IPAContext &context,\n>  \t\t     [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t     rkisp1_params_cfg *params)\n>  {\n> -\tauto &filter = context.frameContext.filter;\n> +\tauto &filter = context.activeState.filter;\n>\n>  \t/* Check if the algorithm configuration has been updated. */\n>  \tif (!filter.updateParams)\n> diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp\n> index dfc76b3d8ffd..879ca2973e8a 100644\n> --- a/src/ipa/rkisp1/algorithms/gsl.cpp\n> +++ b/src/ipa/rkisp1/algorithms/gsl.cpp\n> @@ -123,7 +123,7 @@ void GammaSensorLinearization::prepare(IPAContext &context,\n>  \t\t\t\t       [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t\t\t       rkisp1_params_cfg *params)\n>  {\n> -\tif (context.frameContext.frameCount > 0)\n> +\tif (context.activeState.frameCount > 0)\n>  \t\treturn;\n>\n>  \tif (!initialized_)\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index e4b04136dbaf..9c717bc5f99e 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -137,7 +137,7 @@ void LensShadingCorrection::prepare(IPAContext &context,\n>  \t\t\t\t    [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t\t\t    rkisp1_params_cfg *params)\n>  {\n> -\tif (context.frameContext.frameCount > 0)\n> +\tif (context.activeState.frameCount > 0)\n>  \t\treturn;\n>\n>  \tif (!initialized_)\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index cc66bc70ac44..d18f4996aad2 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -85,114 +85,115 @@ namespace libcamera::ipa::rkisp1 {\n>   */\n>\n>  /**\n> - * \\struct IPAFrameContext\n> - * \\brief Per-frame context for algorithms\n> + * \\struct IPAActiveState\n> + * \\brief Active state for algorithms\n>   *\n> - * The frame context stores data specific to a single frame processed by the\n> - * IPA. Each frame processed by the IPA has a context associated with it,\n> - * accessible through the IPAContext structure.\n> + * The active state stores algorithm-specific data that needs to be shared\n> + * between multiple algorithms and the IPA module. It is accessible through the\n> + * IPAContext structure.\n>   *\n> - * \\todo Detail how to access contexts for a particular frame\n> + * \\todo Split the data contained in this structure between the active state\n> + * and the frame contexts.\n>   *\n> - * Each of the fields in the frame context belongs to either a specific\n> + * Each of the fields in the active state belongs to either a specific\n>   * algorithm, or to the top-level IPA module. A field may be read by any\n>   * algorithm, but should only be written by its owner.\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::agc\n> - * \\brief Context for the Automatic Gain Control algorithm\n> + * \\var IPAActiveState::agc\n> + * \\brief State for the Automatic Gain Control algorithm\n>   *\n>   * The exposure and gain determined are expected to be applied to the sensor\n>   * at the earliest opportunity.\n>   *\n> - * \\var IPAFrameContext::agc.exposure\n> + * \\var IPAActiveState::agc.exposure\n>   * \\brief Exposure time expressed as a number of lines\n>   *\n> - * \\var IPAFrameContext::agc.gain\n> + * \\var IPAActiveState::agc.gain\n>   * \\brief Analogue gain multiplier\n>   *\n>   * The gain should be adapted to the sensor specific gain code before applying.\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::awb\n> - * \\brief Context for the Automatic White Balance algorithm\n> + * \\var IPAActiveState::awb\n> + * \\brief State for the Automatic White Balance algorithm\n>   *\n> - * \\struct IPAFrameContext::awb.gains\n> + * \\struct IPAActiveState::awb.gains\n>   * \\brief White balance gains\n>   *\n> - * \\var IPAFrameContext::awb.gains.red\n> + * \\var IPAActiveState::awb.gains.red\n>   * \\brief White balance gain for R channel\n>   *\n> - * \\var IPAFrameContext::awb.gains.green\n> + * \\var IPAActiveState::awb.gains.green\n>   * \\brief White balance gain for G channel\n>   *\n> - * \\var IPAFrameContext::awb.gains.blue\n> + * \\var IPAActiveState::awb.gains.blue\n>   * \\brief White balance gain for B channel\n>   *\n> - * \\var IPAFrameContext::awb.temperatureK\n> + * \\var IPAActiveState::awb.temperatureK\n>   * \\brief Estimated color temperature\n>   *\n> - * \\var IPAFrameContext::awb.autoEnabled\n> + * \\var IPAActiveState::awb.autoEnabled\n>   * \\brief Whether the Auto White Balance algorithm is enabled\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::cproc\n> - * \\brief Context for the Color Processing algorithm\n> + * \\var IPAActiveState::cproc\n> + * \\brief State for the Color Processing algorithm\n>   *\n> - * \\struct IPAFrameContext::cproc.brightness\n> + * \\struct IPAActiveState::cproc.brightness\n>   * \\brief Brightness level\n>   *\n> - * \\var IPAFrameContext::cproc.contrast\n> + * \\var IPAActiveState::cproc.contrast\n>   * \\brief Contrast level\n>   *\n> - * \\var IPAFrameContext::cproc.saturation\n> + * \\var IPAActiveState::cproc.saturation\n>   * \\brief Saturation level\n>   *\n> - * \\var IPAFrameContext::cproc.updateParams\n> + * \\var IPAActiveState::cproc.updateParams\n>   * \\brief Indicates if ISP parameters need to be updated\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::dpf\n> - * \\brief Context for the Denoise Pre-Filter algorithm\n> + * \\var IPAActiveState::dpf\n> + * \\brief State for the Denoise Pre-Filter algorithm\n>   *\n> - * \\var IPAFrameContext::dpf.denoise\n> + * \\var IPAActiveState::dpf.denoise\n>   * \\brief Indicates if denoise is activated\n>   *\n> - * \\var IPAFrameContext::dpf.updateParams\n> + * \\var IPAActiveState::dpf.updateParams\n>   * \\brief Indicates if ISP parameters need to be updated\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::filter\n> - * \\brief Context for the Filter algorithm\n> + * \\var IPAActiveState::filter\n> + * \\brief State for the Filter algorithm\n>   *\n> - * \\struct IPAFrameContext::filter.denoise\n> + * \\struct IPAActiveState::filter.denoise\n>   * \\brief Denoising level\n>   *\n> - * \\var IPAFrameContext::filter.sharpness\n> + * \\var IPAActiveState::filter.sharpness\n>   * \\brief Sharpness level\n>   *\n> - * \\var IPAFrameContext::filter.updateParams\n> + * \\var IPAActiveState::filter.updateParams\n>   * \\brief Indicates if ISP parameters need to be updated\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::sensor\n> + * \\var IPAActiveState::sensor\n>   * \\brief Effective sensor values\n>   *\n> - * \\var IPAFrameContext::sensor.exposure\n> + * \\var IPAActiveState::sensor.exposure\n>   * \\brief Exposure time expressed as a number of lines\n>   *\n> - * \\var IPAFrameContext::sensor.gain\n> + * \\var IPAActiveState::sensor.gain\n>   * \\brief Analogue gain multiplier\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::frameCount\n> + * \\var IPAActiveState::frameCount\n>   * \\brief Counter of requests queued to the IPA module\n>   *\n>   * The counter is reset to 0 when the IPA module is configured, and is\n> @@ -200,6 +201,14 @@ namespace libcamera::ipa::rkisp1 {\n>   * Algorithm::prepare() function of all algorithms.\n>   */\n>\n> +/**\n> + * \\struct IPAFrameContext\n> + * \\brief Per-frame context for algorithms\n> + *\n> + * This structure is currently unused and will be replaced by a real per-frame\n> + * context.\n> + */\n> +\n>  /**\n>   * \\struct IPAContext\n>   * \\brief Global IPA context data shared between all algorithms\n> @@ -207,13 +216,10 @@ namespace libcamera::ipa::rkisp1 {\n>   * \\var IPAContext::configuration\n>   * \\brief The IPA session configuration, immutable during the session\n>   *\n> - * \\var IPAContext::frameContext\n> - * \\brief The frame context for the frame being processed\n> + * \\var IPAContext::activeState\n> + * \\brief The IPA active state, storing the latest state for all algorithms\n>   *\n> - * \\todo While the frame context is supposed to be per-frame, this\n> - * single frame context stores data related to both the current frame\n> - * and the previous frames, with fields being updated as the algorithms\n> - * are run. This needs to be turned into real per-frame data storage.\n> + * \\todo Introduce per-frame contexts\n>   */\n>\n>  } /* namespace libcamera::ipa::rkisp1 */\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 854151f2ca75..5590482c6a8c 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -46,7 +46,7 @@ struct IPASessionConfiguration {\n>  \t} hw;\n>  };\n>\n> -struct IPAFrameContext {\n> +struct IPAActiveState {\n>  \tstruct {\n>  \t\tuint32_t exposure;\n>  \t\tdouble gain;\n> @@ -89,9 +89,12 @@ struct IPAFrameContext {\n>  \tunsigned int frameCount;\n>  };\n>\n> +struct IPAFrameContext {\n> +};\n> +\n>  struct IPAContext {\n>  \tIPASessionConfiguration configuration;\n> -\tIPAFrameContext frameContext;\n> +\tIPAActiveState activeState;\n>  };\n>\n>  } /* namespace ipa::rkisp1 */\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index f7a17459b6c3..24d5b9647838 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -244,7 +244,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n>  \tcontext_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);\n>  \tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n>\n> -\tcontext_.frameContext.frameCount = 0;\n> +\tcontext_.activeState.frameCount = 0;\n>\n>  \tfor (auto const &algo : algorithms()) {\n>  \t\tint ret = algo->configure(context_, info);\n> @@ -310,7 +310,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>  \t\talgo->prepare(context_, frame, frameContext, params);\n>\n>  \tparamsBufferReady.emit(frame);\n> -\tcontext_.frameContext.frameCount++;\n> +\tcontext_.activeState.frameCount++;\n>  }\n>\n>  void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> @@ -320,9 +320,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n>  \t\treinterpret_cast<rkisp1_stat_buffer *>(\n>  \t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n>\n> -\tcontext_.frameContext.sensor.exposure =\n> +\tcontext_.activeState.sensor.exposure =\n>  \t\tsensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n> -\tcontext_.frameContext.sensor.gain =\n> +\tcontext_.activeState.sensor.gain =\n>  \t\tcamHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());\n>\n>  \tunsigned int aeState = 0;\n> @@ -340,8 +340,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n>\n>  void IPARkISP1::setControls(unsigned int frame)\n>  {\n> -\tuint32_t exposure = context_.frameContext.agc.exposure;\n> -\tuint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n> +\tuint32_t exposure = context_.activeState.agc.exposure;\n> +\tuint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n>\n>  \tControlList ctrls(ctrls_);\n>  \tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));\n> --\n> Regards,\n>\n> Laurent Pinchart\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 56EFCC3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Sep 2022 18:32:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E7583621F1;\n\tWed, 21 Sep 2022 20:32:14 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::222])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B47E600AA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Sep 2022 20:32:13 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 2323940004;\n\tWed, 21 Sep 2022 18:32:12 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663785135;\n\tbh=u5/qwH69MfuhvTxgJjp8w9bOArMsJwsNGyMG5AB7ahw=;\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=l/vbyZBbBd1r01LyAO/2DiDIq1Ziy1Evi2/Vys1r6P+deiMWqAMKY4AtrPKqWTBtj\n\tHcqSOCbBvYLcgN7OT/ygyN/Fa/UUxlooT5YkBXnvdLn1IwZz9A/ppilzMCHcNfGzAV\n\tK5PEsBDVUx3ntqrqRgsdYGAanTOPizgDVJCVe07YHF79WWHdVEGiF9kgEqM/8qLCHN\n\t7WMJRZHdYXLU/G5Coip5smOCWqYBEjhUGl6TYOymCvt0g1VPpbkOOd3Vqhbo2xQuKt\n\tVnidCPjfQ3g6+5gCi2N4MT3rs/NFGeBSOkmmgd3fSrCX7h1mSY2uSLHk7C1zNe70ne\n\tttNAt+Jig671w==","Date":"Wed, 21 Sep 2022 20:32:11 +0200","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20220921183211.hdg3ab6vccw55wub@uno.localdomain>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-15-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220908014200.28728-15-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 14/32] 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":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]