[{"id":23031,"web_url":"https://patchwork.libcamera.org/comment/23031/","msgid":"<20220518072040.lejd456tsbjxzm2x@uno.localdomain>","date":"2022-05-18T07:20:40","subject":"Re: [libcamera-devel] [PATCH v3 1/3] ipa: ipu3: Rework\n\tIPAFrameContext","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Umang\n\nOn Tue, May 17, 2022 at 09:18:31PM +0200, Umang Jain via libcamera-devel wrote:\n> Currently, IPAFrameContext consolidates the values computed by the\n> active state of the algorithms, along with the values applied on\n> the sensor.\n>\n> Moving ahead, we want to have a frame context associated with each\n> incoming request (or frame to be captured). This not necessarily should\n> be tied to the \"active state\" of the algorithms hence:\n> \t- Rename current IPAFrameContext -> IPAActiveState\n> \t  This will now reflect the latest active state of the\n> \t  algorithms and has nothing to do with any frame-related\n> \t  ops/values.\n> \t- Re-instate IPAFrameContext with a sub-structure 'sensor'\n> \t  currently storing the exposure and gain value.\n>\n> Adapt the various access to the frame context to the new changes\n> as described above.\n>\n> Subsequently, the re-instated IPAFrameContext will be extended to\n> contain a frame number and ControlList to remember the incoming\n> request controls provided by the application. A ring-buffer will\n> be introduced to store these frame contexts for a certain number\n> of frames.\n>\n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>  src/ipa/ipu3/algorithms/af.cpp           | 42 ++++++------\n>  src/ipa/ipu3/algorithms/agc.cpp          | 21 +++---\n>  src/ipa/ipu3/algorithms/agc.h            |  2 +-\n>  src/ipa/ipu3/algorithms/awb.cpp          | 16 ++---\n>  src/ipa/ipu3/algorithms/tone_mapping.cpp | 10 +--\n>  src/ipa/ipu3/ipa_context.cpp             | 84 ++++++++++++++----------\n>  src/ipa/ipu3/ipa_context.h               | 16 +++--\n>  src/ipa/ipu3/ipu3.cpp                    | 11 ++--\n>  8 files changed, 110 insertions(+), 92 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index f700b01f..8a5a6b1a 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -185,11 +185,11 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>  \tafIgnoreFrameReset();\n>\n>  \t/* Initial focus value */\n> -\tcontext.frameContext.af.focus = 0;\n> +\tcontext.activeState.af.focus = 0;\n>  \t/* Maximum variance of the AF statistics */\n> -\tcontext.frameContext.af.maxVariance = 0;\n> +\tcontext.activeState.af.maxVariance = 0;\n>  \t/* The stable AF value flag. if it is true, the AF should be in a stable state. */\n> -\tcontext.frameContext.af.stable = false;\n> +\tcontext.activeState.af.stable = false;\n>\n>  \treturn 0;\n>  }\n> @@ -211,10 +211,10 @@ void Af::afCoarseScan(IPAContext &context)\n>\n>  \tif (afScan(context, kCoarseSearchStep)) {\n>  \t\tcoarseCompleted_ = true;\n> -\t\tcontext.frameContext.af.maxVariance = 0;\n> -\t\tfocus_ = context.frameContext.af.focus -\n> -\t\t\t (context.frameContext.af.focus * kFineRange);\n> -\t\tcontext.frameContext.af.focus = focus_;\n> +\t\tcontext.activeState.af.maxVariance = 0;\n> +\t\tfocus_ = context.activeState.af.focus -\n> +\t\t\t (context.activeState.af.focus * kFineRange);\n> +\t\tcontext.activeState.af.focus = focus_;\n>  \t\tpreviousVariance_ = 0;\n>  \t\tmaxStep_ = std::clamp(focus_ + static_cast<uint32_t>((focus_ * kFineRange)),\n>  \t\t\t\t      0U, kMaxFocusSteps);\n> @@ -237,7 +237,7 @@ void Af::afFineScan(IPAContext &context)\n>  \t\treturn;\n>\n>  \tif (afScan(context, kFineSearchStep)) {\n> -\t\tcontext.frameContext.af.stable = true;\n> +\t\tcontext.activeState.af.stable = true;\n>  \t\tfineCompleted_ = true;\n>  \t}\n>  }\n> @@ -254,10 +254,10 @@ void Af::afReset(IPAContext &context)\n>  \tif (afNeedIgnoreFrame())\n>  \t\treturn;\n>\n> -\tcontext.frameContext.af.maxVariance = 0;\n> -\tcontext.frameContext.af.focus = 0;\n> +\tcontext.activeState.af.maxVariance = 0;\n> +\tcontext.activeState.af.focus = 0;\n>  \tfocus_ = 0;\n> -\tcontext.frameContext.af.stable = false;\n> +\tcontext.activeState.af.stable = false;\n>  \tignoreCounter_ = kIgnoreFrame;\n>  \tpreviousVariance_ = 0.0;\n>  \tcoarseCompleted_ = false;\n> @@ -280,7 +280,7 @@ bool Af::afScan(IPAContext &context, int min_step)\n>  {\n>  \tif (focus_ > maxStep_) {\n>  \t\t/* If reach the max step, move lens to the position. */\n> -\t\tcontext.frameContext.af.focus = bestFocus_;\n> +\t\tcontext.activeState.af.focus = bestFocus_;\n>  \t\treturn true;\n>  \t} else {\n>  \t\t/*\n> @@ -288,8 +288,8 @@ bool Af::afScan(IPAContext &context, int min_step)\n>  \t\t * derivative. If the direction changes, it means we have\n>  \t\t * passed a maximum one step before.\n>  \t\t */\n> -\t\tif ((currentVariance_ - context.frameContext.af.maxVariance) >=\n> -\t\t    -(context.frameContext.af.maxVariance * 0.1)) {\n> +\t\tif ((currentVariance_ - context.activeState.af.maxVariance) >=\n> +\t\t    -(context.activeState.af.maxVariance * 0.1)) {\n>  \t\t\t/*\n>  \t\t\t * Positive and zero derivative:\n>  \t\t\t * The variance is still increasing. The focus could be\n> @@ -298,8 +298,8 @@ bool Af::afScan(IPAContext &context, int min_step)\n>  \t\t\t */\n>  \t\t\tbestFocus_ = focus_;\n>  \t\t\tfocus_ += min_step;\n> -\t\t\tcontext.frameContext.af.focus = focus_;\n> -\t\t\tcontext.frameContext.af.maxVariance = currentVariance_;\n> +\t\t\tcontext.activeState.af.focus = focus_;\n> +\t\t\tcontext.activeState.af.maxVariance = currentVariance_;\n>  \t\t} else {\n>  \t\t\t/*\n>  \t\t\t * Negative derivative:\n> @@ -307,7 +307,7 @@ bool Af::afScan(IPAContext &context, int min_step)\n>  \t\t\t * variance is found. Set focus step to previous good one\n>  \t\t\t * then return immediately.\n>  \t\t\t */\n> -\t\t\tcontext.frameContext.af.focus = bestFocus_;\n> +\t\t\tcontext.activeState.af.focus = bestFocus_;\n>  \t\t\treturn true;\n>  \t\t}\n>  \t}\n> @@ -389,13 +389,13 @@ double Af::afEstimateVariance(Span<const y_table_item_t> y_items, bool isY1)\n>  bool Af::afIsOutOfFocus(IPAContext context)\n>  {\n>  \tconst uint32_t diff_var = std::abs(currentVariance_ -\n> -\t\t\t\t\t   context.frameContext.af.maxVariance);\n> -\tconst double var_ratio = diff_var / context.frameContext.af.maxVariance;\n> +\t\t\t\t\t   context.activeState.af.maxVariance);\n> +\tconst double var_ratio = diff_var / context.activeState.af.maxVariance;\n>\n>  \tLOG(IPU3Af, Debug) << \"Variance change rate: \"\n>  \t\t\t   << var_ratio\n>  \t\t\t   << \" Current VCM step: \"\n> -\t\t\t   << context.frameContext.af.focus;\n> +\t\t\t   << context.activeState.af.focus;\n>\n>  \tif (var_ratio > kMaxChange)\n>  \t\treturn true;\n> @@ -437,7 +437,7 @@ void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>  \t */\n>  \tcurrentVariance_ = afEstimateVariance(y_items, !coarseCompleted_);\n>\n> -\tif (!context.frameContext.af.stable) {\n> +\tif (!context.activeState.af.stable) {\n>  \t\tafCoarseScan(context);\n>  \t\tafFineScan(context);\n>  \t} else {\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 7d4b3503..fdeec09d 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -87,7 +87,7 @@ int Agc::configure(IPAContext &context,\n>  \t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>  {\n>  \tconst IPASessionConfiguration &configuration = context.configuration;\n> -\tIPAFrameContext &frameContext = context.frameContext;\n> +\tIPAActiveState &activeState = context.activeState;\n>\n>  \tstride_ = configuration.grid.stride;\n>\n> @@ -99,8 +99,8 @@ int Agc::configure(IPAContext &context,\n>  \tmaxAnalogueGain_ = std::min(configuration.agc.maxAnalogueGain, kMaxAnalogueGain);\n>\n>  \t/* Configure the default exposure and gain. */\n> -\tframeContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n> -\tframeContext.agc.exposure = 10ms / configuration.sensor.lineDuration;\n> +\tactiveState.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n> +\tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>\n>  \tframeCount_ = 0;\n>  \treturn 0;\n> @@ -249,9 +249,10 @@ void Agc::computeExposure(IPAContext &context, double yGain,\n>  \t\t\t    << shutterTime << \" and \"\n>  \t\t\t    << stepGain;\n>\n> +\tIPAActiveState &activeState = context.activeState;\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> @@ -279,7 +280,7 @@ void Agc::computeExposure(IPAContext &context, double yGain,\n>   * More detailed information can be found in:\n>   * https://en.wikipedia.org/wiki/Relative_luminance\n>   */\n> -double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> +double Agc::estimateLuminance(IPAActiveState &activeState,\n>  \t\t\t      const ipu3_uapi_grid_config &grid,\n>  \t\t\t      const ipu3_uapi_stats_3a *stats,\n>  \t\t\t      double gain)\n> @@ -307,9 +308,9 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n>  \t * Apply the AWB gains to approximate colours correctly, use the Rec.\n>  \t * 601 formula to calculate the relative luminance, and normalize it.\n>  \t */\n> -\tdouble ySum = redSum * frameContext.awb.gains.red * 0.299\n> -\t\t    + greenSum * frameContext.awb.gains.green * 0.587\n> -\t\t    + blueSum * frameContext.awb.gains.blue * 0.114;\n> +\tdouble ySum = redSum * activeState.awb.gains.red * 0.299\n> +\t\t    + greenSum * activeState.awb.gains.green * 0.587\n> +\t\t    + blueSum * activeState.awb.gains.blue * 0.114;\n>\n>  \treturn ySum / (grid.height * grid.width) / 255;\n>  }\n> @@ -344,7 +345,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>  \tdouble yTarget = kRelativeLuminanceTarget;\n>\n>  \tfor (unsigned int i = 0; i < 8; i++) {\n> -\t\tdouble yValue = estimateLuminance(context.frameContext,\n> +\t\tdouble yValue = estimateLuminance(context.activeState,\n>  \t\t\t\t\t\t  context.configuration.grid.bdsGrid,\n>  \t\t\t\t\t\t  stats, yGain);\n>  \t\tdouble extraGain = std::min(10.0, yTarget / (yValue + .001));\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index ad705605..31420841 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -36,7 +36,7 @@ private:\n>  \tutils::Duration filterExposure(utils::Duration currentExposure);\n>  \tvoid computeExposure(IPAContext &context, double yGain,\n>  \t\t\t     double iqMeanGain);\n> -\tdouble estimateLuminance(IPAFrameContext &frameContext,\n> +\tdouble estimateLuminance(IPAActiveState &activeState,\n>  \t\t\t\t const ipu3_uapi_grid_config &grid,\n>  \t\t\t\t const ipu3_uapi_stats_3a *stats,\n>  \t\t\t\t double gain);\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index 87a6cc7a..ab6924eb 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -396,10 +396,10 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>  \t * The results are cached, so if no results were calculated, we set the\n>  \t * cached values from asyncResults_ here.\n>  \t */\n> -\tcontext.frameContext.awb.gains.blue = asyncResults_.blueGain;\n> -\tcontext.frameContext.awb.gains.green = asyncResults_.greenGain;\n> -\tcontext.frameContext.awb.gains.red = asyncResults_.redGain;\n> -\tcontext.frameContext.awb.temperatureK = asyncResults_.temperatureK;\n> +\tcontext.activeState.awb.gains.blue = asyncResults_.blueGain;\n> +\tcontext.activeState.awb.gains.green = asyncResults_.greenGain;\n> +\tcontext.activeState.awb.gains.red = asyncResults_.redGain;\n> +\tcontext.activeState.awb.temperatureK = asyncResults_.temperatureK;\n>  }\n>\n>  constexpr uint16_t Awb::threshold(float value)\n> @@ -450,10 +450,10 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>  \tparams->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset\n>  \t\t\t\t\t\t\t* params->acc_param.bnr.opt_center.y_reset;\n>  \t/* Convert to u3.13 fixed point values */\n> -\tparams->acc_param.bnr.wb_gains.gr = 8192 * context.frameContext.awb.gains.green;\n> -\tparams->acc_param.bnr.wb_gains.r  = 8192 * context.frameContext.awb.gains.red;\n> -\tparams->acc_param.bnr.wb_gains.b  = 8192 * context.frameContext.awb.gains.blue;\n> -\tparams->acc_param.bnr.wb_gains.gb = 8192 * context.frameContext.awb.gains.green;\n> +\tparams->acc_param.bnr.wb_gains.gr = 8192 * context.activeState.awb.gains.green;\n> +\tparams->acc_param.bnr.wb_gains.r  = 8192 * context.activeState.awb.gains.red;\n> +\tparams->acc_param.bnr.wb_gains.b  = 8192 * context.activeState.awb.gains.blue;\n> +\tparams->acc_param.bnr.wb_gains.gb = 8192 * context.activeState.awb.gains.green;\n>\n>  \tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n>\n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> index 2040eda5..7c78d0d9 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> @@ -42,7 +42,7 @@ int ToneMapping::configure(IPAContext &context,\n>  \t\t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>  {\n>  \t/* Initialise tone mapping gamma value. */\n> -\tcontext.frameContext.toneMapping.gamma = 0.0;\n> +\tcontext.activeState.toneMapping.gamma = 0.0;\n>\n>  \treturn 0;\n>  }\n> @@ -60,7 +60,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,\n>  {\n>  \t/* Copy the calculated LUT into the parameters buffer. */\n>  \tmemcpy(params->acc_param.gamma.gc_lut.lut,\n> -\t       context.frameContext.toneMapping.gammaCorrection.lut,\n> +\t       context.activeState.toneMapping.gammaCorrection.lut,\n>  \t       IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES *\n>  \t       sizeof(params->acc_param.gamma.gc_lut.lut[0]));\n>\n> @@ -87,11 +87,11 @@ void ToneMapping::process(IPAContext &context,\n>  \t */\n>  \tgamma_ = 1.1;\n>\n> -\tif (context.frameContext.toneMapping.gamma == gamma_)\n> +\tif (context.activeState.toneMapping.gamma == gamma_)\n>  \t\treturn;\n>\n>  \tstruct ipu3_uapi_gamma_corr_lut &lut =\n> -\t\tcontext.frameContext.toneMapping.gammaCorrection;\n> +\t\tcontext.activeState.toneMapping.gammaCorrection;\n>\n>  \tfor (uint32_t i = 0; i < std::size(lut.lut); i++) {\n>  \t\tdouble j = static_cast<double>(i) / (std::size(lut.lut) - 1);\n> @@ -101,7 +101,7 @@ void ToneMapping::process(IPAContext &context,\n>  \t\tlut.lut[i] = gamma * 8191;\n>  \t}\n>\n> -\tcontext.frameContext.toneMapping.gamma = gamma_;\n> +\tcontext.activeState.toneMapping.gamma = gamma_;\n>  }\n>\n>  } /* namespace ipa::ipu3::algorithms */\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index b1570dde..06eb2776 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -24,9 +24,20 @@ namespace libcamera::ipa::ipu3 {\n>   * may also be updated in the start() operation.\n>   */\n>\n> +/**\n> + * \\struct IPAActiveState\n> + * \\brief The active state of the IPA algorithms\n> + *\n> + * The IPA is fed with the statistics generated from the latest frame captured\n> + * by the hardware. The statistics are then processed by the IPA algorithms to\n> + * compute ISP parameters required for the next frame capture. The current state\n> + * of the algorithms is reflected through IPAActiveState. The latest computed\n> + * values by the IPA algorithms are stored in this structure.\n> + */\n> +\n>  /**\n>   * \\struct IPAFrameContext\n> - * \\brief Per-frame context for algorithms\n> + * \\brief Context for a frame\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> @@ -34,9 +45,10 @@ namespace libcamera::ipa::ipu3 {\n>   *\n>   * \\todo Detail how to access contexts for a particular frame\n>   *\n> - * Each of the fields in the frame context 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> + * Fields in the frame context should reflect the values with which the frame\n> + * was processed on the hardware ISP. A field may be read by any algorithm as\n\nIsn't the context storing the sensor parameters rather than the ISP\nconfiguration ?\n\nRogue 'a' at the end of the phrase\n\n> + * and when required, or can be used to prepare the metadata container of the\n\nAlso \"and\" seems not required ?\n\nJust \"metadata for the frame\" ?\n\n\n> + * frame.\n>   */\n>\n>  /**\n> @@ -49,10 +61,10 @@ namespace libcamera::ipa::ipu3 {\n>   * \\var IPAContext::frameContext\n>   * \\brief The frame context for the frame being processed\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> + * \\var IPAContext::activeState\n> + * \\brief The current state of IPA algorithms\n> + *\n> + * \\todo The frame context needs to be turned into real per-frame data storage.\n>   */\n>\n>  /**\n> @@ -78,17 +90,17 @@ namespace libcamera::ipa::ipu3 {\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::af\n> + * \\var IPAActiveState::af\n>   * \\brief Context for the Automatic Focus algorithm\n>   *\n> - * \\struct  IPAFrameContext::af\n> - * \\var IPAFrameContext::af.focus\n> + * \\struct IPAActiveState::af\n> + * \\var IPAActiveState::af.focus\n>   * \\brief Current position of the lens\n>   *\n> - * \\var IPAFrameContext::af.maxVariance\n> + * \\var IPAActiveState::af.maxVariance\n>   * \\brief The maximum variance of the current image.\n>   *\n> - * \\var IPAFrameContext::af.stable\n> + * \\var IPAActiveState::af.stable\n>   * \\brief It is set to true, if the best focus is found.\n>   */\n>\n> @@ -121,64 +133,64 @@ namespace libcamera::ipa::ipu3 {\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::agc\n> + * \\var IPAActiveState::agc\n>   * \\brief Context 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> + * \\var IPAActiveState::awb\n>   * \\brief Context 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>\n>  /**\n> - * \\var IPAFrameContext::sensor\n> - * \\brief Effective sensor values\n> - *\n> - * \\var IPAFrameContext::sensor.exposure\n> - * \\brief Exposure time expressed as a number of lines\n> - *\n> - * \\var IPAFrameContext::sensor.gain\n> - * \\brief Analogue gain multiplier\n> - */\n> -\n> -/**\n> - * \\var IPAFrameContext::toneMapping\n> + * \\var IPAActiveState::toneMapping\n>   * \\brief Context for ToneMapping and Gamma control\n>   *\n> - * \\var IPAFrameContext::toneMapping.gamma\n> + * \\var IPAActiveState::toneMapping.gamma\n>   * \\brief Gamma value for the LUT\n>   *\n> - * \\var IPAFrameContext::toneMapping.gammaCorrection\n> + * \\var IPAActiveState::toneMapping.gammaCorrection\n>   * \\brief Per-pixel tone mapping implemented as a LUT\n>   *\n>   * The LUT structure is defined by the IPU3 kernel interface. See\n>   * <linux/intel-ipu3.h> struct ipu3_uapi_gamma_corr_lut for further details.\n>   */\n>\n> +/**\n> + * \\var IPAFrameContext::sensor\n> + * \\brief Effective sensor values that were applied for the frame\n> + *\n> + * \\var IPAFrameContext::sensor.exposure\n> + * \\brief Exposure time expressed as a number of lines\n> + *\n> + * \\var IPAFrameContext::sensor.gain\n> + * \\brief Analogue gain multiplier\n\nIs this the gain code as the V4L2 controls expects it ?\nIs it worth mentioning a unit here ?\n\nAll minors:\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n> + */\n> +\n>  } /* namespace libcamera::ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 103498ef..8d681131 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -42,7 +42,7 @@ struct IPASessionConfiguration {\n>  \t} sensor;\n>  };\n>\n> -struct IPAFrameContext {\n> +struct IPAActiveState {\n>  \tstruct {\n>  \t\tuint32_t focus;\n>  \t\tdouble maxVariance;\n> @@ -64,19 +64,23 @@ struct IPAFrameContext {\n>  \t\tdouble temperatureK;\n>  \t} awb;\n>\n> -\tstruct {\n> -\t\tuint32_t exposure;\n> -\t\tdouble gain;\n> -\t} sensor;\n> -\n>  \tstruct {\n>  \t\tdouble gamma;\n>  \t\tstruct ipu3_uapi_gamma_corr_lut gammaCorrection;\n>  \t} toneMapping;\n>  };\n>\n> +struct IPAFrameContext {\n> +\tstruct {\n> +\t\tuint32_t exposure;\n> +\t\tdouble gain;\n> +\t} sensor;\n> +};\n> +\n>  struct IPAContext {\n>  \tIPASessionConfiguration configuration;\n> +\tIPAActiveState activeState;\n> +\n>  \tIPAFrameContext frameContext;\n>  };\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index dd6cfd79..3b4fc911 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -454,7 +454,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n>\n>  \tcalculateBdsGrid(configInfo.bdsOutputSize);\n>\n> -\t/* Clean frameContext at each reconfiguration. */\n> +\t/* Clean IPAActiveState at each reconfiguration. */\n> +\tcontext_.activeState = {};\n>  \tcontext_.frameContext = {};\n>\n>  \tif (!validateSensorControls()) {\n> @@ -585,7 +586,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>\n>  \tctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);\n>\n> -\tctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);\n> +\tctrls.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n>\n>  \tctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);\n>\n> @@ -623,8 +624,8 @@ void IPAIPU3::queueRequest([[maybe_unused]] const uint32_t frame,\n>   */\n>  void IPAIPU3::setControls(unsigned int frame)\n>  {\n> -\tint32_t exposure = context_.frameContext.agc.exposure;\n> -\tint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n> +\tint32_t exposure = context_.activeState.agc.exposure;\n> +\tint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n>\n>  \tControlList ctrls(sensorCtrls_);\n>  \tctrls.set(V4L2_CID_EXPOSURE, exposure);\n> @@ -632,7 +633,7 @@ void IPAIPU3::setControls(unsigned int frame)\n>\n>  \tControlList lensCtrls(lensCtrls_);\n>  \tlensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE,\n> -\t\t      static_cast<int32_t>(context_.frameContext.af.focus));\n> +\t\t      static_cast<int32_t>(context_.activeState.af.focus));\n>\n>  \tsetSensorControls.emit(frame, ctrls, lensCtrls);\n>  }\n> --\n> 2.31.0\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 3915BC0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 May 2022 07:20:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82DBA65657;\n\tWed, 18 May 2022 09:20:44 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C2D761FB9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 May 2022 09:20:43 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 3AD0C1C0003;\n\tWed, 18 May 2022 07:20:41 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1652858444;\n\tbh=APEe2U6+BcMY/njGWjDPT8Q1SbwQm9S7np5Zlr7st5A=;\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=WKmFbz/xeIKYb0Lph5OtYnj7QuZPQSye6FvALW316n4XidhZDXvBuN4VcPZVuBhBF\n\tFRKK6J4WLUrzM4y7XuT+7prkTNlnR4ZO8ow/1qvLxyoJOTBNlDfWwRTDxx/r70tQ/9\n\tu7W21xHJXevpocDxgSI+fYRNntO9aKD+0yWoaHm5ZgZYFFo/bMkNHF/y4miaiDC5GG\n\tWKbSZcIO8c0rPZEuwrDmDhyWmYiSngRZ+XrablOfsPPJdQCUpv+qxFVR5m7TQWQ0Np\n\tKvbtXTmS8EdEUBb11+K8z9+/tcWXU+rjNanqJWBRfWpiPhoRcfyMi802bdXuxGjP5f\n\tj/TCtGrRdWwZw==","Date":"Wed, 18 May 2022 09:20:40 +0200","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<20220518072040.lejd456tsbjxzm2x@uno.localdomain>","References":"<20220517191833.333122-1-umang.jain@ideasonboard.com>\n\t<20220517191833.333122-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220517191833.333122-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] ipa: ipu3: Rework\n\tIPAFrameContext","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>"}},{"id":23034,"web_url":"https://patchwork.libcamera.org/comment/23034/","msgid":"<e2786d47-2fe5-679f-7de3-92dc5fb6b123@ideasonboard.com>","date":"2022-05-18T08:58:11","subject":"Re: [libcamera-devel] [PATCH v3 1/3] ipa: ipu3: Rework\n\tIPAFrameContext","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Umang,\n\nOn 18/05/2022 09:20, Jacopo Mondi via libcamera-devel wrote:\n> Hi Umang\n> \n> On Tue, May 17, 2022 at 09:18:31PM +0200, Umang Jain via libcamera-devel wrote:\n>> Currently, IPAFrameContext consolidates the values computed by the\n>> active state of the algorithms, along with the values applied on\n>> the sensor.\n>>\n>> Moving ahead, we want to have a frame context associated with each\n>> incoming request (or frame to be captured). This not necessarily should\n>> be tied to the \"active state\" of the algorithms hence:\n>> \t- Rename current IPAFrameContext -> IPAActiveState\n>> \t  This will now reflect the latest active state of the\n>> \t  algorithms and has nothing to do with any frame-related\n>> \t  ops/values.\n>> \t- Re-instate IPAFrameContext with a sub-structure 'sensor'\n>> \t  currently storing the exposure and gain value.\n>>\n>> Adapt the various access to the frame context to the new changes\n>> as described above.\n>>\n>> Subsequently, the re-instated IPAFrameContext will be extended to\n>> contain a frame number and ControlList to remember the incoming\n>> request controls provided by the application. A ring-buffer will\n>> be introduced to store these frame contexts for a certain number\n>> of frames.\n>>\n>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n\nWith Jacopo comments applied:\nReviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n\n>> ---\n>>   src/ipa/ipu3/algorithms/af.cpp           | 42 ++++++------\n>>   src/ipa/ipu3/algorithms/agc.cpp          | 21 +++---\n>>   src/ipa/ipu3/algorithms/agc.h            |  2 +-\n>>   src/ipa/ipu3/algorithms/awb.cpp          | 16 ++---\n>>   src/ipa/ipu3/algorithms/tone_mapping.cpp | 10 +--\n>>   src/ipa/ipu3/ipa_context.cpp             | 84 ++++++++++++++----------\n>>   src/ipa/ipu3/ipa_context.h               | 16 +++--\n>>   src/ipa/ipu3/ipu3.cpp                    | 11 ++--\n>>   8 files changed, 110 insertions(+), 92 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n>> index f700b01f..8a5a6b1a 100644\n>> --- a/src/ipa/ipu3/algorithms/af.cpp\n>> +++ b/src/ipa/ipu3/algorithms/af.cpp\n>> @@ -185,11 +185,11 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>>   \tafIgnoreFrameReset();\n>>\n>>   \t/* Initial focus value */\n>> -\tcontext.frameContext.af.focus = 0;\n>> +\tcontext.activeState.af.focus = 0;\n>>   \t/* Maximum variance of the AF statistics */\n>> -\tcontext.frameContext.af.maxVariance = 0;\n>> +\tcontext.activeState.af.maxVariance = 0;\n>>   \t/* The stable AF value flag. if it is true, the AF should be in a stable state. */\n>> -\tcontext.frameContext.af.stable = false;\n>> +\tcontext.activeState.af.stable = false;\n>>\n>>   \treturn 0;\n>>   }\n>> @@ -211,10 +211,10 @@ void Af::afCoarseScan(IPAContext &context)\n>>\n>>   \tif (afScan(context, kCoarseSearchStep)) {\n>>   \t\tcoarseCompleted_ = true;\n>> -\t\tcontext.frameContext.af.maxVariance = 0;\n>> -\t\tfocus_ = context.frameContext.af.focus -\n>> -\t\t\t (context.frameContext.af.focus * kFineRange);\n>> -\t\tcontext.frameContext.af.focus = focus_;\n>> +\t\tcontext.activeState.af.maxVariance = 0;\n>> +\t\tfocus_ = context.activeState.af.focus -\n>> +\t\t\t (context.activeState.af.focus * kFineRange);\n>> +\t\tcontext.activeState.af.focus = focus_;\n>>   \t\tpreviousVariance_ = 0;\n>>   \t\tmaxStep_ = std::clamp(focus_ + static_cast<uint32_t>((focus_ * kFineRange)),\n>>   \t\t\t\t      0U, kMaxFocusSteps);\n>> @@ -237,7 +237,7 @@ void Af::afFineScan(IPAContext &context)\n>>   \t\treturn;\n>>\n>>   \tif (afScan(context, kFineSearchStep)) {\n>> -\t\tcontext.frameContext.af.stable = true;\n>> +\t\tcontext.activeState.af.stable = true;\n>>   \t\tfineCompleted_ = true;\n>>   \t}\n>>   }\n>> @@ -254,10 +254,10 @@ void Af::afReset(IPAContext &context)\n>>   \tif (afNeedIgnoreFrame())\n>>   \t\treturn;\n>>\n>> -\tcontext.frameContext.af.maxVariance = 0;\n>> -\tcontext.frameContext.af.focus = 0;\n>> +\tcontext.activeState.af.maxVariance = 0;\n>> +\tcontext.activeState.af.focus = 0;\n>>   \tfocus_ = 0;\n>> -\tcontext.frameContext.af.stable = false;\n>> +\tcontext.activeState.af.stable = false;\n>>   \tignoreCounter_ = kIgnoreFrame;\n>>   \tpreviousVariance_ = 0.0;\n>>   \tcoarseCompleted_ = false;\n>> @@ -280,7 +280,7 @@ bool Af::afScan(IPAContext &context, int min_step)\n>>   {\n>>   \tif (focus_ > maxStep_) {\n>>   \t\t/* If reach the max step, move lens to the position. */\n>> -\t\tcontext.frameContext.af.focus = bestFocus_;\n>> +\t\tcontext.activeState.af.focus = bestFocus_;\n>>   \t\treturn true;\n>>   \t} else {\n>>   \t\t/*\n>> @@ -288,8 +288,8 @@ bool Af::afScan(IPAContext &context, int min_step)\n>>   \t\t * derivative. If the direction changes, it means we have\n>>   \t\t * passed a maximum one step before.\n>>   \t\t */\n>> -\t\tif ((currentVariance_ - context.frameContext.af.maxVariance) >=\n>> -\t\t    -(context.frameContext.af.maxVariance * 0.1)) {\n>> +\t\tif ((currentVariance_ - context.activeState.af.maxVariance) >=\n>> +\t\t    -(context.activeState.af.maxVariance * 0.1)) {\n>>   \t\t\t/*\n>>   \t\t\t * Positive and zero derivative:\n>>   \t\t\t * The variance is still increasing. The focus could be\n>> @@ -298,8 +298,8 @@ bool Af::afScan(IPAContext &context, int min_step)\n>>   \t\t\t */\n>>   \t\t\tbestFocus_ = focus_;\n>>   \t\t\tfocus_ += min_step;\n>> -\t\t\tcontext.frameContext.af.focus = focus_;\n>> -\t\t\tcontext.frameContext.af.maxVariance = currentVariance_;\n>> +\t\t\tcontext.activeState.af.focus = focus_;\n>> +\t\t\tcontext.activeState.af.maxVariance = currentVariance_;\n>>   \t\t} else {\n>>   \t\t\t/*\n>>   \t\t\t * Negative derivative:\n>> @@ -307,7 +307,7 @@ bool Af::afScan(IPAContext &context, int min_step)\n>>   \t\t\t * variance is found. Set focus step to previous good one\n>>   \t\t\t * then return immediately.\n>>   \t\t\t */\n>> -\t\t\tcontext.frameContext.af.focus = bestFocus_;\n>> +\t\t\tcontext.activeState.af.focus = bestFocus_;\n>>   \t\t\treturn true;\n>>   \t\t}\n>>   \t}\n>> @@ -389,13 +389,13 @@ double Af::afEstimateVariance(Span<const y_table_item_t> y_items, bool isY1)\n>>   bool Af::afIsOutOfFocus(IPAContext context)\n>>   {\n>>   \tconst uint32_t diff_var = std::abs(currentVariance_ -\n>> -\t\t\t\t\t   context.frameContext.af.maxVariance);\n>> -\tconst double var_ratio = diff_var / context.frameContext.af.maxVariance;\n>> +\t\t\t\t\t   context.activeState.af.maxVariance);\n>> +\tconst double var_ratio = diff_var / context.activeState.af.maxVariance;\n>>\n>>   \tLOG(IPU3Af, Debug) << \"Variance change rate: \"\n>>   \t\t\t   << var_ratio\n>>   \t\t\t   << \" Current VCM step: \"\n>> -\t\t\t   << context.frameContext.af.focus;\n>> +\t\t\t   << context.activeState.af.focus;\n>>\n>>   \tif (var_ratio > kMaxChange)\n>>   \t\treturn true;\n>> @@ -437,7 +437,7 @@ void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>   \t */\n>>   \tcurrentVariance_ = afEstimateVariance(y_items, !coarseCompleted_);\n>>\n>> -\tif (!context.frameContext.af.stable) {\n>> +\tif (!context.activeState.af.stable) {\n>>   \t\tafCoarseScan(context);\n>>   \t\tafFineScan(context);\n>>   \t} else {\n>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n>> index 7d4b3503..fdeec09d 100644\n>> --- a/src/ipa/ipu3/algorithms/agc.cpp\n>> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n>> @@ -87,7 +87,7 @@ int Agc::configure(IPAContext &context,\n>>   \t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>   {\n>>   \tconst IPASessionConfiguration &configuration = context.configuration;\n>> -\tIPAFrameContext &frameContext = context.frameContext;\n>> +\tIPAActiveState &activeState = context.activeState;\n>>\n>>   \tstride_ = configuration.grid.stride;\n>>\n>> @@ -99,8 +99,8 @@ int Agc::configure(IPAContext &context,\n>>   \tmaxAnalogueGain_ = std::min(configuration.agc.maxAnalogueGain, kMaxAnalogueGain);\n>>\n>>   \t/* Configure the default exposure and gain. */\n>> -\tframeContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n>> -\tframeContext.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>> +\tactiveState.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n>> +\tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>>\n>>   \tframeCount_ = 0;\n>>   \treturn 0;\n>> @@ -249,9 +249,10 @@ void Agc::computeExposure(IPAContext &context, double yGain,\n>>   \t\t\t    << shutterTime << \" and \"\n>>   \t\t\t    << stepGain;\n>>\n>> +\tIPAActiveState &activeState = context.activeState;\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>> @@ -279,7 +280,7 @@ void Agc::computeExposure(IPAContext &context, double yGain,\n>>    * More detailed information can be found in:\n>>    * https://en.wikipedia.org/wiki/Relative_luminance\n>>    */\n>> -double Agc::estimateLuminance(IPAFrameContext &frameContext,\n>> +double Agc::estimateLuminance(IPAActiveState &activeState,\n>>   \t\t\t      const ipu3_uapi_grid_config &grid,\n>>   \t\t\t      const ipu3_uapi_stats_3a *stats,\n>>   \t\t\t      double gain)\n>> @@ -307,9 +308,9 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n>>   \t * Apply the AWB gains to approximate colours correctly, use the Rec.\n>>   \t * 601 formula to calculate the relative luminance, and normalize it.\n>>   \t */\n>> -\tdouble ySum = redSum * frameContext.awb.gains.red * 0.299\n>> -\t\t    + greenSum * frameContext.awb.gains.green * 0.587\n>> -\t\t    + blueSum * frameContext.awb.gains.blue * 0.114;\n>> +\tdouble ySum = redSum * activeState.awb.gains.red * 0.299\n>> +\t\t    + greenSum * activeState.awb.gains.green * 0.587\n>> +\t\t    + blueSum * activeState.awb.gains.blue * 0.114;\n>>\n>>   \treturn ySum / (grid.height * grid.width) / 255;\n>>   }\n>> @@ -344,7 +345,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>   \tdouble yTarget = kRelativeLuminanceTarget;\n>>\n>>   \tfor (unsigned int i = 0; i < 8; i++) {\n>> -\t\tdouble yValue = estimateLuminance(context.frameContext,\n>> +\t\tdouble yValue = estimateLuminance(context.activeState,\n>>   \t\t\t\t\t\t  context.configuration.grid.bdsGrid,\n>>   \t\t\t\t\t\t  stats, yGain);\n>>   \t\tdouble extraGain = std::min(10.0, yTarget / (yValue + .001));\n>> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n>> index ad705605..31420841 100644\n>> --- a/src/ipa/ipu3/algorithms/agc.h\n>> +++ b/src/ipa/ipu3/algorithms/agc.h\n>> @@ -36,7 +36,7 @@ private:\n>>   \tutils::Duration filterExposure(utils::Duration currentExposure);\n>>   \tvoid computeExposure(IPAContext &context, double yGain,\n>>   \t\t\t     double iqMeanGain);\n>> -\tdouble estimateLuminance(IPAFrameContext &frameContext,\n>> +\tdouble estimateLuminance(IPAActiveState &activeState,\n>>   \t\t\t\t const ipu3_uapi_grid_config &grid,\n>>   \t\t\t\t const ipu3_uapi_stats_3a *stats,\n>>   \t\t\t\t double gain);\n>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n>> index 87a6cc7a..ab6924eb 100644\n>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n>> @@ -396,10 +396,10 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>   \t * The results are cached, so if no results were calculated, we set the\n>>   \t * cached values from asyncResults_ here.\n>>   \t */\n>> -\tcontext.frameContext.awb.gains.blue = asyncResults_.blueGain;\n>> -\tcontext.frameContext.awb.gains.green = asyncResults_.greenGain;\n>> -\tcontext.frameContext.awb.gains.red = asyncResults_.redGain;\n>> -\tcontext.frameContext.awb.temperatureK = asyncResults_.temperatureK;\n>> +\tcontext.activeState.awb.gains.blue = asyncResults_.blueGain;\n>> +\tcontext.activeState.awb.gains.green = asyncResults_.greenGain;\n>> +\tcontext.activeState.awb.gains.red = asyncResults_.redGain;\n>> +\tcontext.activeState.awb.temperatureK = asyncResults_.temperatureK;\n>>   }\n>>\n>>   constexpr uint16_t Awb::threshold(float value)\n>> @@ -450,10 +450,10 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>>   \tparams->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset\n>>   \t\t\t\t\t\t\t* params->acc_param.bnr.opt_center.y_reset;\n>>   \t/* Convert to u3.13 fixed point values */\n>> -\tparams->acc_param.bnr.wb_gains.gr = 8192 * context.frameContext.awb.gains.green;\n>> -\tparams->acc_param.bnr.wb_gains.r  = 8192 * context.frameContext.awb.gains.red;\n>> -\tparams->acc_param.bnr.wb_gains.b  = 8192 * context.frameContext.awb.gains.blue;\n>> -\tparams->acc_param.bnr.wb_gains.gb = 8192 * context.frameContext.awb.gains.green;\n>> +\tparams->acc_param.bnr.wb_gains.gr = 8192 * context.activeState.awb.gains.green;\n>> +\tparams->acc_param.bnr.wb_gains.r  = 8192 * context.activeState.awb.gains.red;\n>> +\tparams->acc_param.bnr.wb_gains.b  = 8192 * context.activeState.awb.gains.blue;\n>> +\tparams->acc_param.bnr.wb_gains.gb = 8192 * context.activeState.awb.gains.green;\n>>\n>>   \tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n>> index 2040eda5..7c78d0d9 100644\n>> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n>> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n>> @@ -42,7 +42,7 @@ int ToneMapping::configure(IPAContext &context,\n>>   \t\t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>   {\n>>   \t/* Initialise tone mapping gamma value. */\n>> -\tcontext.frameContext.toneMapping.gamma = 0.0;\n>> +\tcontext.activeState.toneMapping.gamma = 0.0;\n>>\n>>   \treturn 0;\n>>   }\n>> @@ -60,7 +60,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,\n>>   {\n>>   \t/* Copy the calculated LUT into the parameters buffer. */\n>>   \tmemcpy(params->acc_param.gamma.gc_lut.lut,\n>> -\t       context.frameContext.toneMapping.gammaCorrection.lut,\n>> +\t       context.activeState.toneMapping.gammaCorrection.lut,\n>>   \t       IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES *\n>>   \t       sizeof(params->acc_param.gamma.gc_lut.lut[0]));\n>>\n>> @@ -87,11 +87,11 @@ void ToneMapping::process(IPAContext &context,\n>>   \t */\n>>   \tgamma_ = 1.1;\n>>\n>> -\tif (context.frameContext.toneMapping.gamma == gamma_)\n>> +\tif (context.activeState.toneMapping.gamma == gamma_)\n>>   \t\treturn;\n>>\n>>   \tstruct ipu3_uapi_gamma_corr_lut &lut =\n>> -\t\tcontext.frameContext.toneMapping.gammaCorrection;\n>> +\t\tcontext.activeState.toneMapping.gammaCorrection;\n>>\n>>   \tfor (uint32_t i = 0; i < std::size(lut.lut); i++) {\n>>   \t\tdouble j = static_cast<double>(i) / (std::size(lut.lut) - 1);\n>> @@ -101,7 +101,7 @@ void ToneMapping::process(IPAContext &context,\n>>   \t\tlut.lut[i] = gamma * 8191;\n>>   \t}\n>>\n>> -\tcontext.frameContext.toneMapping.gamma = gamma_;\n>> +\tcontext.activeState.toneMapping.gamma = gamma_;\n>>   }\n>>\n>>   } /* namespace ipa::ipu3::algorithms */\n>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n>> index b1570dde..06eb2776 100644\n>> --- a/src/ipa/ipu3/ipa_context.cpp\n>> +++ b/src/ipa/ipu3/ipa_context.cpp\n>> @@ -24,9 +24,20 @@ namespace libcamera::ipa::ipu3 {\n>>    * may also be updated in the start() operation.\n>>    */\n>>\n>> +/**\n>> + * \\struct IPAActiveState\n>> + * \\brief The active state of the IPA algorithms\n>> + *\n>> + * The IPA is fed with the statistics generated from the latest frame captured\n>> + * by the hardware. The statistics are then processed by the IPA algorithms to\n>> + * compute ISP parameters required for the next frame capture. The current state\n>> + * of the algorithms is reflected through IPAActiveState. The latest computed\n>> + * values by the IPA algorithms are stored in this structure.\n>> + */\n>> +\n>>   /**\n>>    * \\struct IPAFrameContext\n>> - * \\brief Per-frame context for algorithms\n>> + * \\brief Context for a frame\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>> @@ -34,9 +45,10 @@ namespace libcamera::ipa::ipu3 {\n>>    *\n>>    * \\todo Detail how to access contexts for a particular frame\n>>    *\n>> - * Each of the fields in the frame context 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>> + * Fields in the frame context should reflect the values with which the frame\n>> + * was processed on the hardware ISP. A field may be read by any algorithm as\n> \n> Isn't the context storing the sensor parameters rather than the ISP\n> configuration ?\n> \n> Rogue 'a' at the end of the phrase\n> \n>> + * and when required, or can be used to prepare the metadata container of the\n> \n> Also \"and\" seems not required ?\n> \n> Just \"metadata for the frame\" ?\n> \n> \n>> + * frame.\n>>    */\n>>\n>>   /**\n>> @@ -49,10 +61,10 @@ namespace libcamera::ipa::ipu3 {\n>>    * \\var IPAContext::frameContext\n>>    * \\brief The frame context for the frame being processed\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>> + * \\var IPAContext::activeState\n>> + * \\brief The current state of IPA algorithms\n>> + *\n>> + * \\todo The frame context needs to be turned into real per-frame data storage.\n>>    */\n>>\n>>   /**\n>> @@ -78,17 +90,17 @@ namespace libcamera::ipa::ipu3 {\n>>    */\n>>\n>>   /**\n>> - * \\var IPAFrameContext::af\n>> + * \\var IPAActiveState::af\n>>    * \\brief Context for the Automatic Focus algorithm\n>>    *\n>> - * \\struct  IPAFrameContext::af\n>> - * \\var IPAFrameContext::af.focus\n>> + * \\struct IPAActiveState::af\n>> + * \\var IPAActiveState::af.focus\n>>    * \\brief Current position of the lens\n>>    *\n>> - * \\var IPAFrameContext::af.maxVariance\n>> + * \\var IPAActiveState::af.maxVariance\n>>    * \\brief The maximum variance of the current image.\n>>    *\n>> - * \\var IPAFrameContext::af.stable\n>> + * \\var IPAActiveState::af.stable\n>>    * \\brief It is set to true, if the best focus is found.\n>>    */\n>>\n>> @@ -121,64 +133,64 @@ namespace libcamera::ipa::ipu3 {\n>>    */\n>>\n>>   /**\n>> - * \\var IPAFrameContext::agc\n>> + * \\var IPAActiveState::agc\n>>    * \\brief Context 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>> + * \\var IPAActiveState::awb\n>>    * \\brief Context 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>>\n>>   /**\n>> - * \\var IPAFrameContext::sensor\n>> - * \\brief Effective sensor values\n>> - *\n>> - * \\var IPAFrameContext::sensor.exposure\n>> - * \\brief Exposure time expressed as a number of lines\n>> - *\n>> - * \\var IPAFrameContext::sensor.gain\n>> - * \\brief Analogue gain multiplier\n>> - */\n>> -\n>> -/**\n>> - * \\var IPAFrameContext::toneMapping\n>> + * \\var IPAActiveState::toneMapping\n>>    * \\brief Context for ToneMapping and Gamma control\n>>    *\n>> - * \\var IPAFrameContext::toneMapping.gamma\n>> + * \\var IPAActiveState::toneMapping.gamma\n>>    * \\brief Gamma value for the LUT\n>>    *\n>> - * \\var IPAFrameContext::toneMapping.gammaCorrection\n>> + * \\var IPAActiveState::toneMapping.gammaCorrection\n>>    * \\brief Per-pixel tone mapping implemented as a LUT\n>>    *\n>>    * The LUT structure is defined by the IPU3 kernel interface. See\n>>    * <linux/intel-ipu3.h> struct ipu3_uapi_gamma_corr_lut for further details.\n>>    */\n>>\n>> +/**\n>> + * \\var IPAFrameContext::sensor\n>> + * \\brief Effective sensor values that were applied for the frame\n>> + *\n>> + * \\var IPAFrameContext::sensor.exposure\n>> + * \\brief Exposure time expressed as a number of lines\n>> + *\n>> + * \\var IPAFrameContext::sensor.gain\n>> + * \\brief Analogue gain multiplier\n> \n> Is this the gain code as the V4L2 controls expects it ?\n> Is it worth mentioning a unit here ?\n> \n> All minors:\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> Thanks\n>     j\n> \n>> + */\n>> +\n>>   } /* namespace libcamera::ipa::ipu3 */\n>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n>> index 103498ef..8d681131 100644\n>> --- a/src/ipa/ipu3/ipa_context.h\n>> +++ b/src/ipa/ipu3/ipa_context.h\n>> @@ -42,7 +42,7 @@ struct IPASessionConfiguration {\n>>   \t} sensor;\n>>   };\n>>\n>> -struct IPAFrameContext {\n>> +struct IPAActiveState {\n>>   \tstruct {\n>>   \t\tuint32_t focus;\n>>   \t\tdouble maxVariance;\n>> @@ -64,19 +64,23 @@ struct IPAFrameContext {\n>>   \t\tdouble temperatureK;\n>>   \t} awb;\n>>\n>> -\tstruct {\n>> -\t\tuint32_t exposure;\n>> -\t\tdouble gain;\n>> -\t} sensor;\n>> -\n>>   \tstruct {\n>>   \t\tdouble gamma;\n>>   \t\tstruct ipu3_uapi_gamma_corr_lut gammaCorrection;\n>>   \t} toneMapping;\n>>   };\n>>\n>> +struct IPAFrameContext {\n>> +\tstruct {\n>> +\t\tuint32_t exposure;\n>> +\t\tdouble gain;\n>> +\t} sensor;\n>> +};\n>> +\n>>   struct IPAContext {\n>>   \tIPASessionConfiguration configuration;\n>> +\tIPAActiveState activeState;\n>> +\n>>   \tIPAFrameContext frameContext;\n>>   };\n>>\n>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>> index dd6cfd79..3b4fc911 100644\n>> --- a/src/ipa/ipu3/ipu3.cpp\n>> +++ b/src/ipa/ipu3/ipu3.cpp\n>> @@ -454,7 +454,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n>>\n>>   \tcalculateBdsGrid(configInfo.bdsOutputSize);\n>>\n>> -\t/* Clean frameContext at each reconfiguration. */\n>> +\t/* Clean IPAActiveState at each reconfiguration. */\n>> +\tcontext_.activeState = {};\n>>   \tcontext_.frameContext = {};\n>>\n>>   \tif (!validateSensorControls()) {\n>> @@ -585,7 +586,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>>\n>>   \tctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);\n>>\n>> -\tctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);\n>> +\tctrls.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n>>\n>>   \tctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);\n>>\n>> @@ -623,8 +624,8 @@ void IPAIPU3::queueRequest([[maybe_unused]] const uint32_t frame,\n>>    */\n>>   void IPAIPU3::setControls(unsigned int frame)\n>>   {\n>> -\tint32_t exposure = context_.frameContext.agc.exposure;\n>> -\tint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n>> +\tint32_t exposure = context_.activeState.agc.exposure;\n>> +\tint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n>>\n>>   \tControlList ctrls(sensorCtrls_);\n>>   \tctrls.set(V4L2_CID_EXPOSURE, exposure);\n>> @@ -632,7 +633,7 @@ void IPAIPU3::setControls(unsigned int frame)\n>>\n>>   \tControlList lensCtrls(lensCtrls_);\n>>   \tlensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE,\n>> -\t\t      static_cast<int32_t>(context_.frameContext.af.focus));\n>> +\t\t      static_cast<int32_t>(context_.activeState.af.focus));\n>>\n>>   \tsetSensorControls.emit(frame, ctrls, lensCtrls);\n>>   }\n>> --\n>> 2.31.0\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 1308FC0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 May 2022 08:58:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 094206565B;\n\tWed, 18 May 2022 10:58:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0AAC461FB9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 May 2022 10:58:17 +0200 (CEST)","from [192.168.106.15] (unknown [37.171.4.20])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AD47F48F;\n\tWed, 18 May 2022 10:58:15 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1652864298;\n\tbh=OOHB0ENXFk3r02wowWuzguW6VrBFYonUdK7yB3m7GZo=;\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=FXK4td7xAMWib1m95rYOgkgPvS6OGxOI4O0w5ZRGooYknavM3Oq+NDOC6SM9Q85Hq\n\tv7PFy8EPUeUapieWiHisHRoS+ZHTBwgEHMt8MxtSvMgtemMRDIYvXIwqNFm8/RNuVO\n\tb0zKp99kL7d8xyyShyPUZ2tEDeDk2ac4MLUWUBhYsmMyGIzAT1CelbF3bfuHAI6ale\n\tscOeCbIBH/V7CAsrmdIVhRhjz+957AuSYpOqBBykdnz/NG4ghOzn6vHZY4NQB1Wgnp\n\tMsdbQ6tBRuDWh7UC0qmoMDPnbXO2qstrVZ0n3JBRxlFDkZfhFXhsKBvQX8cM+utd87\n\tA0k51tb0uaaVA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1652864296;\n\tbh=OOHB0ENXFk3r02wowWuzguW6VrBFYonUdK7yB3m7GZo=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=ZAvfaMCMyseCRx2LdqBt49bLj/yDyVeJU6aGy4eG32f6K7nj6ryivwVfbS+gSdRDM\n\tF7/Rk6zDTQQaMxL8I1HKZYkQbUxdjCbIckhlYD29fnDlaFdsf/pUCvGV4T3SivME/G\n\tZs13PgDmWOhqmoyhvn/AuMBweLGrsEhgjqzexWOs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ZAvfaMCM\"; dkim-atps=neutral","Message-ID":"<e2786d47-2fe5-679f-7de3-92dc5fb6b123@ideasonboard.com>","Date":"Wed, 18 May 2022 10:58:11 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.8.1","Content-Language":"en-US","To":"Jacopo Mondi <jacopo@jmondi.org>,\n\tUmang Jain <umang.jain@ideasonboard.com>","References":"<20220517191833.333122-1-umang.jain@ideasonboard.com>\n\t<20220517191833.333122-2-umang.jain@ideasonboard.com>\n\t<20220518072040.lejd456tsbjxzm2x@uno.localdomain>","In-Reply-To":"<20220518072040.lejd456tsbjxzm2x@uno.localdomain>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] ipa: ipu3: Rework\n\tIPAFrameContext","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":"Jean-Michel Hautbois via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Jean-Michel Hautbois <jeanmichel.hautbois@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":23037,"web_url":"https://patchwork.libcamera.org/comment/23037/","msgid":"<3179500f-ef57-0caa-ae31-b97401c7a585@ideasonboard.com>","date":"2022-05-18T09:31:04","subject":"Re: [libcamera-devel] [PATCH v3 1/3] ipa: ipu3: Rework\n\tIPAFrameContext","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 5/18/22 09:20, Jacopo Mondi wrote:\n> Hi Umang\n>\n> On Tue, May 17, 2022 at 09:18:31PM +0200, Umang Jain via libcamera-devel wrote:\n>> Currently, IPAFrameContext consolidates the values computed by the\n>> active state of the algorithms, along with the values applied on\n>> the sensor.\n>>\n>> Moving ahead, we want to have a frame context associated with each\n>> incoming request (or frame to be captured). This not necessarily should\n>> be tied to the \"active state\" of the algorithms hence:\n>> \t- Rename current IPAFrameContext -> IPAActiveState\n>> \t  This will now reflect the latest active state of the\n>> \t  algorithms and has nothing to do with any frame-related\n>> \t  ops/values.\n>> \t- Re-instate IPAFrameContext with a sub-structure 'sensor'\n>> \t  currently storing the exposure and gain value.\n>>\n>> Adapt the various access to the frame context to the new changes\n>> as described above.\n>>\n>> Subsequently, the re-instated IPAFrameContext will be extended to\n>> contain a frame number and ControlList to remember the incoming\n>> request controls provided by the application. A ring-buffer will\n>> be introduced to store these frame contexts for a certain number\n>> of frames.\n>>\n>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n>> ---\n>>   src/ipa/ipu3/algorithms/af.cpp           | 42 ++++++------\n>>   src/ipa/ipu3/algorithms/agc.cpp          | 21 +++---\n>>   src/ipa/ipu3/algorithms/agc.h            |  2 +-\n>>   src/ipa/ipu3/algorithms/awb.cpp          | 16 ++---\n>>   src/ipa/ipu3/algorithms/tone_mapping.cpp | 10 +--\n>>   src/ipa/ipu3/ipa_context.cpp             | 84 ++++++++++++++----------\n>>   src/ipa/ipu3/ipa_context.h               | 16 +++--\n>>   src/ipa/ipu3/ipu3.cpp                    | 11 ++--\n>>   8 files changed, 110 insertions(+), 92 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n>> index f700b01f..8a5a6b1a 100644\n>> --- a/src/ipa/ipu3/algorithms/af.cpp\n>> +++ b/src/ipa/ipu3/algorithms/af.cpp\n>> @@ -185,11 +185,11 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>>   \tafIgnoreFrameReset();\n>>\n>>   \t/* Initial focus value */\n>> -\tcontext.frameContext.af.focus = 0;\n>> +\tcontext.activeState.af.focus = 0;\n>>   \t/* Maximum variance of the AF statistics */\n>> -\tcontext.frameContext.af.maxVariance = 0;\n>> +\tcontext.activeState.af.maxVariance = 0;\n>>   \t/* The stable AF value flag. if it is true, the AF should be in a stable state. */\n>> -\tcontext.frameContext.af.stable = false;\n>> +\tcontext.activeState.af.stable = false;\n>>\n>>   \treturn 0;\n>>   }\n>> @@ -211,10 +211,10 @@ void Af::afCoarseScan(IPAContext &context)\n>>\n>>   \tif (afScan(context, kCoarseSearchStep)) {\n>>   \t\tcoarseCompleted_ = true;\n>> -\t\tcontext.frameContext.af.maxVariance = 0;\n>> -\t\tfocus_ = context.frameContext.af.focus -\n>> -\t\t\t (context.frameContext.af.focus * kFineRange);\n>> -\t\tcontext.frameContext.af.focus = focus_;\n>> +\t\tcontext.activeState.af.maxVariance = 0;\n>> +\t\tfocus_ = context.activeState.af.focus -\n>> +\t\t\t (context.activeState.af.focus * kFineRange);\n>> +\t\tcontext.activeState.af.focus = focus_;\n>>   \t\tpreviousVariance_ = 0;\n>>   \t\tmaxStep_ = std::clamp(focus_ + static_cast<uint32_t>((focus_ * kFineRange)),\n>>   \t\t\t\t      0U, kMaxFocusSteps);\n>> @@ -237,7 +237,7 @@ void Af::afFineScan(IPAContext &context)\n>>   \t\treturn;\n>>\n>>   \tif (afScan(context, kFineSearchStep)) {\n>> -\t\tcontext.frameContext.af.stable = true;\n>> +\t\tcontext.activeState.af.stable = true;\n>>   \t\tfineCompleted_ = true;\n>>   \t}\n>>   }\n>> @@ -254,10 +254,10 @@ void Af::afReset(IPAContext &context)\n>>   \tif (afNeedIgnoreFrame())\n>>   \t\treturn;\n>>\n>> -\tcontext.frameContext.af.maxVariance = 0;\n>> -\tcontext.frameContext.af.focus = 0;\n>> +\tcontext.activeState.af.maxVariance = 0;\n>> +\tcontext.activeState.af.focus = 0;\n>>   \tfocus_ = 0;\n>> -\tcontext.frameContext.af.stable = false;\n>> +\tcontext.activeState.af.stable = false;\n>>   \tignoreCounter_ = kIgnoreFrame;\n>>   \tpreviousVariance_ = 0.0;\n>>   \tcoarseCompleted_ = false;\n>> @@ -280,7 +280,7 @@ bool Af::afScan(IPAContext &context, int min_step)\n>>   {\n>>   \tif (focus_ > maxStep_) {\n>>   \t\t/* If reach the max step, move lens to the position. */\n>> -\t\tcontext.frameContext.af.focus = bestFocus_;\n>> +\t\tcontext.activeState.af.focus = bestFocus_;\n>>   \t\treturn true;\n>>   \t} else {\n>>   \t\t/*\n>> @@ -288,8 +288,8 @@ bool Af::afScan(IPAContext &context, int min_step)\n>>   \t\t * derivative. If the direction changes, it means we have\n>>   \t\t * passed a maximum one step before.\n>>   \t\t */\n>> -\t\tif ((currentVariance_ - context.frameContext.af.maxVariance) >=\n>> -\t\t    -(context.frameContext.af.maxVariance * 0.1)) {\n>> +\t\tif ((currentVariance_ - context.activeState.af.maxVariance) >=\n>> +\t\t    -(context.activeState.af.maxVariance * 0.1)) {\n>>   \t\t\t/*\n>>   \t\t\t * Positive and zero derivative:\n>>   \t\t\t * The variance is still increasing. The focus could be\n>> @@ -298,8 +298,8 @@ bool Af::afScan(IPAContext &context, int min_step)\n>>   \t\t\t */\n>>   \t\t\tbestFocus_ = focus_;\n>>   \t\t\tfocus_ += min_step;\n>> -\t\t\tcontext.frameContext.af.focus = focus_;\n>> -\t\t\tcontext.frameContext.af.maxVariance = currentVariance_;\n>> +\t\t\tcontext.activeState.af.focus = focus_;\n>> +\t\t\tcontext.activeState.af.maxVariance = currentVariance_;\n>>   \t\t} else {\n>>   \t\t\t/*\n>>   \t\t\t * Negative derivative:\n>> @@ -307,7 +307,7 @@ bool Af::afScan(IPAContext &context, int min_step)\n>>   \t\t\t * variance is found. Set focus step to previous good one\n>>   \t\t\t * then return immediately.\n>>   \t\t\t */\n>> -\t\t\tcontext.frameContext.af.focus = bestFocus_;\n>> +\t\t\tcontext.activeState.af.focus = bestFocus_;\n>>   \t\t\treturn true;\n>>   \t\t}\n>>   \t}\n>> @@ -389,13 +389,13 @@ double Af::afEstimateVariance(Span<const y_table_item_t> y_items, bool isY1)\n>>   bool Af::afIsOutOfFocus(IPAContext context)\n>>   {\n>>   \tconst uint32_t diff_var = std::abs(currentVariance_ -\n>> -\t\t\t\t\t   context.frameContext.af.maxVariance);\n>> -\tconst double var_ratio = diff_var / context.frameContext.af.maxVariance;\n>> +\t\t\t\t\t   context.activeState.af.maxVariance);\n>> +\tconst double var_ratio = diff_var / context.activeState.af.maxVariance;\n>>\n>>   \tLOG(IPU3Af, Debug) << \"Variance change rate: \"\n>>   \t\t\t   << var_ratio\n>>   \t\t\t   << \" Current VCM step: \"\n>> -\t\t\t   << context.frameContext.af.focus;\n>> +\t\t\t   << context.activeState.af.focus;\n>>\n>>   \tif (var_ratio > kMaxChange)\n>>   \t\treturn true;\n>> @@ -437,7 +437,7 @@ void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>   \t */\n>>   \tcurrentVariance_ = afEstimateVariance(y_items, !coarseCompleted_);\n>>\n>> -\tif (!context.frameContext.af.stable) {\n>> +\tif (!context.activeState.af.stable) {\n>>   \t\tafCoarseScan(context);\n>>   \t\tafFineScan(context);\n>>   \t} else {\n>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n>> index 7d4b3503..fdeec09d 100644\n>> --- a/src/ipa/ipu3/algorithms/agc.cpp\n>> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n>> @@ -87,7 +87,7 @@ int Agc::configure(IPAContext &context,\n>>   \t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>   {\n>>   \tconst IPASessionConfiguration &configuration = context.configuration;\n>> -\tIPAFrameContext &frameContext = context.frameContext;\n>> +\tIPAActiveState &activeState = context.activeState;\n>>\n>>   \tstride_ = configuration.grid.stride;\n>>\n>> @@ -99,8 +99,8 @@ int Agc::configure(IPAContext &context,\n>>   \tmaxAnalogueGain_ = std::min(configuration.agc.maxAnalogueGain, kMaxAnalogueGain);\n>>\n>>   \t/* Configure the default exposure and gain. */\n>> -\tframeContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n>> -\tframeContext.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>> +\tactiveState.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n>> +\tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>>\n>>   \tframeCount_ = 0;\n>>   \treturn 0;\n>> @@ -249,9 +249,10 @@ void Agc::computeExposure(IPAContext &context, double yGain,\n>>   \t\t\t    << shutterTime << \" and \"\n>>   \t\t\t    << stepGain;\n>>\n>> +\tIPAActiveState &activeState = context.activeState;\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>> @@ -279,7 +280,7 @@ void Agc::computeExposure(IPAContext &context, double yGain,\n>>    * More detailed information can be found in:\n>>    * https://en.wikipedia.org/wiki/Relative_luminance\n>>    */\n>> -double Agc::estimateLuminance(IPAFrameContext &frameContext,\n>> +double Agc::estimateLuminance(IPAActiveState &activeState,\n>>   \t\t\t      const ipu3_uapi_grid_config &grid,\n>>   \t\t\t      const ipu3_uapi_stats_3a *stats,\n>>   \t\t\t      double gain)\n>> @@ -307,9 +308,9 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n>>   \t * Apply the AWB gains to approximate colours correctly, use the Rec.\n>>   \t * 601 formula to calculate the relative luminance, and normalize it.\n>>   \t */\n>> -\tdouble ySum = redSum * frameContext.awb.gains.red * 0.299\n>> -\t\t    + greenSum * frameContext.awb.gains.green * 0.587\n>> -\t\t    + blueSum * frameContext.awb.gains.blue * 0.114;\n>> +\tdouble ySum = redSum * activeState.awb.gains.red * 0.299\n>> +\t\t    + greenSum * activeState.awb.gains.green * 0.587\n>> +\t\t    + blueSum * activeState.awb.gains.blue * 0.114;\n>>\n>>   \treturn ySum / (grid.height * grid.width) / 255;\n>>   }\n>> @@ -344,7 +345,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>   \tdouble yTarget = kRelativeLuminanceTarget;\n>>\n>>   \tfor (unsigned int i = 0; i < 8; i++) {\n>> -\t\tdouble yValue = estimateLuminance(context.frameContext,\n>> +\t\tdouble yValue = estimateLuminance(context.activeState,\n>>   \t\t\t\t\t\t  context.configuration.grid.bdsGrid,\n>>   \t\t\t\t\t\t  stats, yGain);\n>>   \t\tdouble extraGain = std::min(10.0, yTarget / (yValue + .001));\n>> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n>> index ad705605..31420841 100644\n>> --- a/src/ipa/ipu3/algorithms/agc.h\n>> +++ b/src/ipa/ipu3/algorithms/agc.h\n>> @@ -36,7 +36,7 @@ private:\n>>   \tutils::Duration filterExposure(utils::Duration currentExposure);\n>>   \tvoid computeExposure(IPAContext &context, double yGain,\n>>   \t\t\t     double iqMeanGain);\n>> -\tdouble estimateLuminance(IPAFrameContext &frameContext,\n>> +\tdouble estimateLuminance(IPAActiveState &activeState,\n>>   \t\t\t\t const ipu3_uapi_grid_config &grid,\n>>   \t\t\t\t const ipu3_uapi_stats_3a *stats,\n>>   \t\t\t\t double gain);\n>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n>> index 87a6cc7a..ab6924eb 100644\n>> --- a/src/ipa/ipu3/algorithms/awb.cpp\n>> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n>> @@ -396,10 +396,10 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>   \t * The results are cached, so if no results were calculated, we set the\n>>   \t * cached values from asyncResults_ here.\n>>   \t */\n>> -\tcontext.frameContext.awb.gains.blue = asyncResults_.blueGain;\n>> -\tcontext.frameContext.awb.gains.green = asyncResults_.greenGain;\n>> -\tcontext.frameContext.awb.gains.red = asyncResults_.redGain;\n>> -\tcontext.frameContext.awb.temperatureK = asyncResults_.temperatureK;\n>> +\tcontext.activeState.awb.gains.blue = asyncResults_.blueGain;\n>> +\tcontext.activeState.awb.gains.green = asyncResults_.greenGain;\n>> +\tcontext.activeState.awb.gains.red = asyncResults_.redGain;\n>> +\tcontext.activeState.awb.temperatureK = asyncResults_.temperatureK;\n>>   }\n>>\n>>   constexpr uint16_t Awb::threshold(float value)\n>> @@ -450,10 +450,10 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>>   \tparams->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset\n>>   \t\t\t\t\t\t\t* params->acc_param.bnr.opt_center.y_reset;\n>>   \t/* Convert to u3.13 fixed point values */\n>> -\tparams->acc_param.bnr.wb_gains.gr = 8192 * context.frameContext.awb.gains.green;\n>> -\tparams->acc_param.bnr.wb_gains.r  = 8192 * context.frameContext.awb.gains.red;\n>> -\tparams->acc_param.bnr.wb_gains.b  = 8192 * context.frameContext.awb.gains.blue;\n>> -\tparams->acc_param.bnr.wb_gains.gb = 8192 * context.frameContext.awb.gains.green;\n>> +\tparams->acc_param.bnr.wb_gains.gr = 8192 * context.activeState.awb.gains.green;\n>> +\tparams->acc_param.bnr.wb_gains.r  = 8192 * context.activeState.awb.gains.red;\n>> +\tparams->acc_param.bnr.wb_gains.b  = 8192 * context.activeState.awb.gains.blue;\n>> +\tparams->acc_param.bnr.wb_gains.gb = 8192 * context.activeState.awb.gains.green;\n>>\n>>   \tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n>> index 2040eda5..7c78d0d9 100644\n>> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n>> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n>> @@ -42,7 +42,7 @@ int ToneMapping::configure(IPAContext &context,\n>>   \t\t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>   {\n>>   \t/* Initialise tone mapping gamma value. */\n>> -\tcontext.frameContext.toneMapping.gamma = 0.0;\n>> +\tcontext.activeState.toneMapping.gamma = 0.0;\n>>\n>>   \treturn 0;\n>>   }\n>> @@ -60,7 +60,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,\n>>   {\n>>   \t/* Copy the calculated LUT into the parameters buffer. */\n>>   \tmemcpy(params->acc_param.gamma.gc_lut.lut,\n>> -\t       context.frameContext.toneMapping.gammaCorrection.lut,\n>> +\t       context.activeState.toneMapping.gammaCorrection.lut,\n>>   \t       IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES *\n>>   \t       sizeof(params->acc_param.gamma.gc_lut.lut[0]));\n>>\n>> @@ -87,11 +87,11 @@ void ToneMapping::process(IPAContext &context,\n>>   \t */\n>>   \tgamma_ = 1.1;\n>>\n>> -\tif (context.frameContext.toneMapping.gamma == gamma_)\n>> +\tif (context.activeState.toneMapping.gamma == gamma_)\n>>   \t\treturn;\n>>\n>>   \tstruct ipu3_uapi_gamma_corr_lut &lut =\n>> -\t\tcontext.frameContext.toneMapping.gammaCorrection;\n>> +\t\tcontext.activeState.toneMapping.gammaCorrection;\n>>\n>>   \tfor (uint32_t i = 0; i < std::size(lut.lut); i++) {\n>>   \t\tdouble j = static_cast<double>(i) / (std::size(lut.lut) - 1);\n>> @@ -101,7 +101,7 @@ void ToneMapping::process(IPAContext &context,\n>>   \t\tlut.lut[i] = gamma * 8191;\n>>   \t}\n>>\n>> -\tcontext.frameContext.toneMapping.gamma = gamma_;\n>> +\tcontext.activeState.toneMapping.gamma = gamma_;\n>>   }\n>>\n>>   } /* namespace ipa::ipu3::algorithms */\n>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n>> index b1570dde..06eb2776 100644\n>> --- a/src/ipa/ipu3/ipa_context.cpp\n>> +++ b/src/ipa/ipu3/ipa_context.cpp\n>> @@ -24,9 +24,20 @@ namespace libcamera::ipa::ipu3 {\n>>    * may also be updated in the start() operation.\n>>    */\n>>\n>> +/**\n>> + * \\struct IPAActiveState\n>> + * \\brief The active state of the IPA algorithms\n>> + *\n>> + * The IPA is fed with the statistics generated from the latest frame captured\n>> + * by the hardware. The statistics are then processed by the IPA algorithms to\n>> + * compute ISP parameters required for the next frame capture. The current state\n>> + * of the algorithms is reflected through IPAActiveState. The latest computed\n>> + * values by the IPA algorithms are stored in this structure.\n>> + */\n>> +\n>>   /**\n>>    * \\struct IPAFrameContext\n>> - * \\brief Per-frame context for algorithms\n>> + * \\brief Context for a frame\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>> @@ -34,9 +45,10 @@ namespace libcamera::ipa::ipu3 {\n>>    *\n>>    * \\todo Detail how to access contexts for a particular frame\n>>    *\n>> - * Each of the fields in the frame context 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>> + * Fields in the frame context should reflect the values with which the frame\n>> + * was processed on the hardware ISP. A field may be read by any algorithm as\n> Isn't the context storing the sensor parameters rather than the ISP\n> configuration ?\n\n\nYou are correct! I'll update the doc to replace this with sensor controls\n\n>\n> Rogue 'a' at the end of the phrase\n\n\nAre you pointing the - \" 'A' field may be read by any algorithm... \" ? \nor Something else (sorry I can't notice the rogue 'a' here...)\n\n>\n>> + * and when required, or can be used to prepare the metadata container of the\n> Also \"and\" seems not required ?\n>\n> Just \"metadata for the frame\" ?\n\n\nAck.\n\n>\n>\n>> + * frame.\n>>    */\n>>\n>>   /**\n>> @@ -49,10 +61,10 @@ namespace libcamera::ipa::ipu3 {\n>>    * \\var IPAContext::frameContext\n>>    * \\brief The frame context for the frame being processed\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>> + * \\var IPAContext::activeState\n>> + * \\brief The current state of IPA algorithms\n>> + *\n>> + * \\todo The frame context needs to be turned into real per-frame data storage.\n>>    */\n>>\n>>   /**\n>> @@ -78,17 +90,17 @@ namespace libcamera::ipa::ipu3 {\n>>    */\n>>\n>>   /**\n>> - * \\var IPAFrameContext::af\n>> + * \\var IPAActiveState::af\n>>    * \\brief Context for the Automatic Focus algorithm\n>>    *\n>> - * \\struct  IPAFrameContext::af\n>> - * \\var IPAFrameContext::af.focus\n>> + * \\struct IPAActiveState::af\n>> + * \\var IPAActiveState::af.focus\n>>    * \\brief Current position of the lens\n>>    *\n>> - * \\var IPAFrameContext::af.maxVariance\n>> + * \\var IPAActiveState::af.maxVariance\n>>    * \\brief The maximum variance of the current image.\n>>    *\n>> - * \\var IPAFrameContext::af.stable\n>> + * \\var IPAActiveState::af.stable\n>>    * \\brief It is set to true, if the best focus is found.\n>>    */\n>>\n>> @@ -121,64 +133,64 @@ namespace libcamera::ipa::ipu3 {\n>>    */\n>>\n>>   /**\n>> - * \\var IPAFrameContext::agc\n>> + * \\var IPAActiveState::agc\n>>    * \\brief Context 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>> + * \\var IPAActiveState::awb\n>>    * \\brief Context 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>>\n>>   /**\n>> - * \\var IPAFrameContext::sensor\n>> - * \\brief Effective sensor values\n>> - *\n>> - * \\var IPAFrameContext::sensor.exposure\n>> - * \\brief Exposure time expressed as a number of lines\n>> - *\n>> - * \\var IPAFrameContext::sensor.gain\n>> - * \\brief Analogue gain multiplier\n>> - */\n>> -\n>> -/**\n>> - * \\var IPAFrameContext::toneMapping\n>> + * \\var IPAActiveState::toneMapping\n>>    * \\brief Context for ToneMapping and Gamma control\n>>    *\n>> - * \\var IPAFrameContext::toneMapping.gamma\n>> + * \\var IPAActiveState::toneMapping.gamma\n>>    * \\brief Gamma value for the LUT\n>>    *\n>> - * \\var IPAFrameContext::toneMapping.gammaCorrection\n>> + * \\var IPAActiveState::toneMapping.gammaCorrection\n>>    * \\brief Per-pixel tone mapping implemented as a LUT\n>>    *\n>>    * The LUT structure is defined by the IPU3 kernel interface. See\n>>    * <linux/intel-ipu3.h> struct ipu3_uapi_gamma_corr_lut for further details.\n>>    */\n>>\n>> +/**\n>> + * \\var IPAFrameContext::sensor\n>> + * \\brief Effective sensor values that were applied for the frame\n>> + *\n>> + * \\var IPAFrameContext::sensor.exposure\n>> + * \\brief Exposure time expressed as a number of lines\n>> + *\n>> + * \\var IPAFrameContext::sensor.gain\n>> + * \\brief Analogue gain multiplier\n> Is this the gain code as the V4L2 controls expects it ?\n> Is it worth mentioning a unit here ?\n\nWell, I am using the already documented sensor.gain (just moving it here)...\n\nNevermind, I'll check for correct-ness\n\nNo, this is not the gainCode, but the gain value returned by \nCameraSensorHelper::gain(gainCode).\n\nSo, we query the gainCode from the sensorControls (that were applied on \nthe sensor when frame was captured) - ran it through the camera sensor \nhelper and set it as:\n\nIPAFrameContext.sensor.gain = camHelper->gain(gainCode)\n\nSo, discussing with Jean-Michel, this is the \"decoded\" gain.\n\nI am now starting to think we should align the definition around gain. \nOne is coded gain (aka gainCode) and another is just gain.\nWhat we have right now is: \"gain code\", \"real gain\", \"gain multiplier\" \n... I think it has been confusing to follow. Probably can I sort this \nout in a out-of-series patch on top?\n\n\n>\n> All minors:\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\n\nThanks!\n\n>\n> Thanks\n>     j\n>\n>> + */\n>> +\n>>   } /* namespace libcamera::ipa::ipu3 */\n>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n>> index 103498ef..8d681131 100644\n>> --- a/src/ipa/ipu3/ipa_context.h\n>> +++ b/src/ipa/ipu3/ipa_context.h\n>> @@ -42,7 +42,7 @@ struct IPASessionConfiguration {\n>>   \t} sensor;\n>>   };\n>>\n>> -struct IPAFrameContext {\n>> +struct IPAActiveState {\n>>   \tstruct {\n>>   \t\tuint32_t focus;\n>>   \t\tdouble maxVariance;\n>> @@ -64,19 +64,23 @@ struct IPAFrameContext {\n>>   \t\tdouble temperatureK;\n>>   \t} awb;\n>>\n>> -\tstruct {\n>> -\t\tuint32_t exposure;\n>> -\t\tdouble gain;\n>> -\t} sensor;\n>> -\n>>   \tstruct {\n>>   \t\tdouble gamma;\n>>   \t\tstruct ipu3_uapi_gamma_corr_lut gammaCorrection;\n>>   \t} toneMapping;\n>>   };\n>>\n>> +struct IPAFrameContext {\n>> +\tstruct {\n>> +\t\tuint32_t exposure;\n>> +\t\tdouble gain;\n>> +\t} sensor;\n>> +};\n>> +\n>>   struct IPAContext {\n>>   \tIPASessionConfiguration configuration;\n>> +\tIPAActiveState activeState;\n>> +\n>>   \tIPAFrameContext frameContext;\n>>   };\n>>\n>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>> index dd6cfd79..3b4fc911 100644\n>> --- a/src/ipa/ipu3/ipu3.cpp\n>> +++ b/src/ipa/ipu3/ipu3.cpp\n>> @@ -454,7 +454,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n>>\n>>   \tcalculateBdsGrid(configInfo.bdsOutputSize);\n>>\n>> -\t/* Clean frameContext at each reconfiguration. */\n>> +\t/* Clean IPAActiveState at each reconfiguration. */\n>> +\tcontext_.activeState = {};\n>>   \tcontext_.frameContext = {};\n>>\n>>   \tif (!validateSensorControls()) {\n>> @@ -585,7 +586,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>>\n>>   \tctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);\n>>\n>> -\tctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);\n>> +\tctrls.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n>>\n>>   \tctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);\n>>\n>> @@ -623,8 +624,8 @@ void IPAIPU3::queueRequest([[maybe_unused]] const uint32_t frame,\n>>    */\n>>   void IPAIPU3::setControls(unsigned int frame)\n>>   {\n>> -\tint32_t exposure = context_.frameContext.agc.exposure;\n>> -\tint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n>> +\tint32_t exposure = context_.activeState.agc.exposure;\n>> +\tint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n>>\n>>   \tControlList ctrls(sensorCtrls_);\n>>   \tctrls.set(V4L2_CID_EXPOSURE, exposure);\n>> @@ -632,7 +633,7 @@ void IPAIPU3::setControls(unsigned int frame)\n>>\n>>   \tControlList lensCtrls(lensCtrls_);\n>>   \tlensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE,\n>> -\t\t      static_cast<int32_t>(context_.frameContext.af.focus));\n>> +\t\t      static_cast<int32_t>(context_.activeState.af.focus));\n>>\n>>   \tsetSensorControls.emit(frame, ctrls, lensCtrls);\n>>   }\n>> --\n>> 2.31.0\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 8A059C3256\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 May 2022 09:31:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CFD3D65655;\n\tWed, 18 May 2022 11:31:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7584961FB9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 May 2022 11:31:08 +0200 (CEST)","from [192.168.1.164] (unknown [45.131.31.124])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C7FB2475;\n\tWed, 18 May 2022 11:31:07 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1652866269;\n\tbh=KyEM1iirZFwonXL5k3YVW6SeyV7mMbObvCXv/zD2QDk=;\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=lZQZtIGx8yGqXFoBJF/MCBZFZOJ0KqkXmQAxwbGJPh1qtBGTUtI5/PNKdygysrlNR\n\tQcpzgw1nRUxQZ5N1loO/OQojkSkws0pHIUi6A5nTXufwyOeoczyICq9EKcL1TWb8Oc\n\ttlG6fuwuqG1nq00QobztD+Vtb7cSZWNvVTyJiCmKeoG91MRCH2CB6tVWbQxBmmcgtJ\n\tQ54QfM348TjALCYwaSqv0TNAwsZTPYIunIFIMnd4aOznHOf51tRlcMT65C21AkWo28\n\tcr4qP4qZhCEDVVnRPz3hNtoqYiDhH7zw2trocA8LROoT3i2wcg8YiU+zoKxe9vuplu\n\t7eCrurlQE7asQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1652866268;\n\tbh=KyEM1iirZFwonXL5k3YVW6SeyV7mMbObvCXv/zD2QDk=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=LIb1FPTpdEoy53521h5EKMxtaxiHep8GDmvMJv9TjufcM9XQkk+5KHmE46gkJxO5o\n\tjJj/S+wa/OIonS1lbsA5mpXMhdYZEDt3fs/oJK3BSndFifNio6iNS90wAwUbG5txXi\n\tecH6cUgcrXWu7UN0LGDfz+unME+CxGJR66zEhW2s="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"LIb1FPTp\"; dkim-atps=neutral","Message-ID":"<3179500f-ef57-0caa-ae31-b97401c7a585@ideasonboard.com>","Date":"Wed, 18 May 2022 11:31:04 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20220517191833.333122-1-umang.jain@ideasonboard.com>\n\t<20220517191833.333122-2-umang.jain@ideasonboard.com>\n\t<20220518072040.lejd456tsbjxzm2x@uno.localdomain>","In-Reply-To":"<20220518072040.lejd456tsbjxzm2x@uno.localdomain>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] ipa: ipu3: Rework\n\tIPAFrameContext","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":"Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Umang Jain <umang.jain@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":23040,"web_url":"https://patchwork.libcamera.org/comment/23040/","msgid":"<165286942548.368702.175982003173968929@Monstersaurus>","date":"2022-05-18T10:23:45","subject":"Re: [libcamera-devel] [PATCH v3 1/3] ipa: ipu3: Rework\n\tIPAFrameContext","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain via libcamera-devel (2022-05-17 20:18:31)\n> Currently, IPAFrameContext consolidates the values computed by the\n> active state of the algorithms, along with the values applied on\n> the sensor.\n> \n> Moving ahead, we want to have a frame context associated with each\n> incoming request (or frame to be captured). This not necessarily should\n> be tied to the \"active state\" of the algorithms hence:\n\n'This shouldn't necessarily be tied to'\n\n>         - Rename current IPAFrameContext -> IPAActiveState\n>           This will now reflect the latest active state of the\n>           algorithms and has nothing to do with any frame-related\n>           ops/values.\n>         - Re-instate IPAFrameContext with a sub-structure 'sensor'\n>           currently storing the exposure and gain value.\n> \n> Adapt the various access to the frame context to the new changes\n> as described above.\n> \n> Subsequently, the re-instated IPAFrameContext will be extended to\n> contain a frame number and ControlList to remember the incoming\n> request controls provided by the application. A ring-buffer will\n> be introduced to store these frame contexts for a certain number\n> of frames.\n> \n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>  src/ipa/ipu3/algorithms/af.cpp           | 42 ++++++------\n>  src/ipa/ipu3/algorithms/agc.cpp          | 21 +++---\n>  src/ipa/ipu3/algorithms/agc.h            |  2 +-\n>  src/ipa/ipu3/algorithms/awb.cpp          | 16 ++---\n>  src/ipa/ipu3/algorithms/tone_mapping.cpp | 10 +--\n>  src/ipa/ipu3/ipa_context.cpp             | 84 ++++++++++++++----------\n>  src/ipa/ipu3/ipa_context.h               | 16 +++--\n>  src/ipa/ipu3/ipu3.cpp                    | 11 ++--\n>  8 files changed, 110 insertions(+), 92 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index f700b01f..8a5a6b1a 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -185,11 +185,11 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>         afIgnoreFrameReset();\n>  \n>         /* Initial focus value */\n> -       context.frameContext.af.focus = 0;\n> +       context.activeState.af.focus = 0;\n>         /* Maximum variance of the AF statistics */\n> -       context.frameContext.af.maxVariance = 0;\n> +       context.activeState.af.maxVariance = 0;\n>         /* The stable AF value flag. if it is true, the AF should be in a stable state. */\n> -       context.frameContext.af.stable = false;\n> +       context.activeState.af.stable = false;\n>  \n>         return 0;\n>  }\n> @@ -211,10 +211,10 @@ void Af::afCoarseScan(IPAContext &context)\n>  \n>         if (afScan(context, kCoarseSearchStep)) {\n>                 coarseCompleted_ = true;\n> -               context.frameContext.af.maxVariance = 0;\n> -               focus_ = context.frameContext.af.focus -\n> -                        (context.frameContext.af.focus * kFineRange);\n> -               context.frameContext.af.focus = focus_;\n> +               context.activeState.af.maxVariance = 0;\n> +               focus_ = context.activeState.af.focus -\n> +                        (context.activeState.af.focus * kFineRange);\n> +               context.activeState.af.focus = focus_;\n>                 previousVariance_ = 0;\n>                 maxStep_ = std::clamp(focus_ + static_cast<uint32_t>((focus_ * kFineRange)),\n>                                       0U, kMaxFocusSteps);\n> @@ -237,7 +237,7 @@ void Af::afFineScan(IPAContext &context)\n>                 return;\n>  \n>         if (afScan(context, kFineSearchStep)) {\n> -               context.frameContext.af.stable = true;\n> +               context.activeState.af.stable = true;\n>                 fineCompleted_ = true;\n>         }\n>  }\n> @@ -254,10 +254,10 @@ void Af::afReset(IPAContext &context)\n>         if (afNeedIgnoreFrame())\n>                 return;\n>  \n> -       context.frameContext.af.maxVariance = 0;\n> -       context.frameContext.af.focus = 0;\n> +       context.activeState.af.maxVariance = 0;\n> +       context.activeState.af.focus = 0;\n>         focus_ = 0;\n> -       context.frameContext.af.stable = false;\n> +       context.activeState.af.stable = false;\n>         ignoreCounter_ = kIgnoreFrame;\n>         previousVariance_ = 0.0;\n>         coarseCompleted_ = false;\n> @@ -280,7 +280,7 @@ bool Af::afScan(IPAContext &context, int min_step)\n>  {\n>         if (focus_ > maxStep_) {\n>                 /* If reach the max step, move lens to the position. */\n> -               context.frameContext.af.focus = bestFocus_;\n> +               context.activeState.af.focus = bestFocus_;\n>                 return true;\n>         } else {\n>                 /*\n> @@ -288,8 +288,8 @@ bool Af::afScan(IPAContext &context, int min_step)\n>                  * derivative. If the direction changes, it means we have\n>                  * passed a maximum one step before.\n>                  */\n> -               if ((currentVariance_ - context.frameContext.af.maxVariance) >=\n> -                   -(context.frameContext.af.maxVariance * 0.1)) {\n> +               if ((currentVariance_ - context.activeState.af.maxVariance) >=\n> +                   -(context.activeState.af.maxVariance * 0.1)) {\n>                         /*\n>                          * Positive and zero derivative:\n>                          * The variance is still increasing. The focus could be\n> @@ -298,8 +298,8 @@ bool Af::afScan(IPAContext &context, int min_step)\n>                          */\n>                         bestFocus_ = focus_;\n>                         focus_ += min_step;\n> -                       context.frameContext.af.focus = focus_;\n> -                       context.frameContext.af.maxVariance = currentVariance_;\n> +                       context.activeState.af.focus = focus_;\n> +                       context.activeState.af.maxVariance = currentVariance_;\n>                 } else {\n>                         /*\n>                          * Negative derivative:\n> @@ -307,7 +307,7 @@ bool Af::afScan(IPAContext &context, int min_step)\n>                          * variance is found. Set focus step to previous good one\n>                          * then return immediately.\n>                          */\n> -                       context.frameContext.af.focus = bestFocus_;\n> +                       context.activeState.af.focus = bestFocus_;\n>                         return true;\n>                 }\n>         }\n> @@ -389,13 +389,13 @@ double Af::afEstimateVariance(Span<const y_table_item_t> y_items, bool isY1)\n>  bool Af::afIsOutOfFocus(IPAContext context)\n>  {\n>         const uint32_t diff_var = std::abs(currentVariance_ -\n> -                                          context.frameContext.af.maxVariance);\n> -       const double var_ratio = diff_var / context.frameContext.af.maxVariance;\n> +                                          context.activeState.af.maxVariance);\n> +       const double var_ratio = diff_var / context.activeState.af.maxVariance;\n>  \n>         LOG(IPU3Af, Debug) << \"Variance change rate: \"\n>                            << var_ratio\n>                            << \" Current VCM step: \"\n> -                          << context.frameContext.af.focus;\n> +                          << context.activeState.af.focus;\n>  \n>         if (var_ratio > kMaxChange)\n>                 return true;\n> @@ -437,7 +437,7 @@ void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>          */\n>         currentVariance_ = afEstimateVariance(y_items, !coarseCompleted_);\n>  \n> -       if (!context.frameContext.af.stable) {\n> +       if (!context.activeState.af.stable) {\n>                 afCoarseScan(context);\n>                 afFineScan(context);\n>         } else {\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 7d4b3503..fdeec09d 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -87,7 +87,7 @@ int Agc::configure(IPAContext &context,\n>                    [[maybe_unused]] const IPAConfigInfo &configInfo)\n>  {\n>         const IPASessionConfiguration &configuration = context.configuration;\n> -       IPAFrameContext &frameContext = context.frameContext;\n> +       IPAActiveState &activeState = context.activeState;\n>  \n>         stride_ = configuration.grid.stride;\n>  \n> @@ -99,8 +99,8 @@ int Agc::configure(IPAContext &context,\n>         maxAnalogueGain_ = std::min(configuration.agc.maxAnalogueGain, kMaxAnalogueGain);\n>  \n>         /* Configure the default exposure and gain. */\n> -       frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n> -       frameContext.agc.exposure = 10ms / configuration.sensor.lineDuration;\n> +       activeState.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n> +       activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>  \n>         frameCount_ = 0;\n>         return 0;\n> @@ -249,9 +249,10 @@ void Agc::computeExposure(IPAContext &context, double yGain,\n>                             << shutterTime << \" and \"\n>                             << stepGain;\n>  \n> +       IPAActiveState &activeState = context.activeState;\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> @@ -279,7 +280,7 @@ void Agc::computeExposure(IPAContext &context, double yGain,\n>   * More detailed information can be found in:\n>   * https://en.wikipedia.org/wiki/Relative_luminance\n>   */\n> -double Agc::estimateLuminance(IPAFrameContext &frameContext,\n> +double Agc::estimateLuminance(IPAActiveState &activeState,\n>                               const ipu3_uapi_grid_config &grid,\n>                               const ipu3_uapi_stats_3a *stats,\n>                               double gain)\n> @@ -307,9 +308,9 @@ double Agc::estimateLuminance(IPAFrameContext &frameContext,\n>          * Apply the AWB gains to approximate colours correctly, use the Rec.\n>          * 601 formula to calculate the relative luminance, and normalize it.\n>          */\n> -       double ySum = redSum * frameContext.awb.gains.red * 0.299\n> -                   + greenSum * frameContext.awb.gains.green * 0.587\n> -                   + blueSum * frameContext.awb.gains.blue * 0.114;\n> +       double ySum = redSum * activeState.awb.gains.red * 0.299\n> +                   + greenSum * activeState.awb.gains.green * 0.587\n> +                   + blueSum * activeState.awb.gains.blue * 0.114;\n>  \n>         return ySum / (grid.height * grid.width) / 255;\n>  }\n> @@ -344,7 +345,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>         double yTarget = kRelativeLuminanceTarget;\n>  \n>         for (unsigned int i = 0; i < 8; i++) {\n> -               double yValue = estimateLuminance(context.frameContext,\n> +               double yValue = estimateLuminance(context.activeState,\n>                                                   context.configuration.grid.bdsGrid,\n>                                                   stats, yGain);\n>                 double extraGain = std::min(10.0, yTarget / (yValue + .001));\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index ad705605..31420841 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -36,7 +36,7 @@ private:\n>         utils::Duration filterExposure(utils::Duration currentExposure);\n>         void computeExposure(IPAContext &context, double yGain,\n>                              double iqMeanGain);\n> -       double estimateLuminance(IPAFrameContext &frameContext,\n> +       double estimateLuminance(IPAActiveState &activeState,\n>                                  const ipu3_uapi_grid_config &grid,\n>                                  const ipu3_uapi_stats_3a *stats,\n>                                  double gain);\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index 87a6cc7a..ab6924eb 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -396,10 +396,10 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>          * The results are cached, so if no results were calculated, we set the\n>          * cached values from asyncResults_ here.\n>          */\n> -       context.frameContext.awb.gains.blue = asyncResults_.blueGain;\n> -       context.frameContext.awb.gains.green = asyncResults_.greenGain;\n> -       context.frameContext.awb.gains.red = asyncResults_.redGain;\n> -       context.frameContext.awb.temperatureK = asyncResults_.temperatureK;\n> +       context.activeState.awb.gains.blue = asyncResults_.blueGain;\n> +       context.activeState.awb.gains.green = asyncResults_.greenGain;\n> +       context.activeState.awb.gains.red = asyncResults_.redGain;\n> +       context.activeState.awb.temperatureK = asyncResults_.temperatureK;\n>  }\n>  \n>  constexpr uint16_t Awb::threshold(float value)\n> @@ -450,10 +450,10 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n>         params->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset\n>                                                         * params->acc_param.bnr.opt_center.y_reset;\n>         /* Convert to u3.13 fixed point values */\n> -       params->acc_param.bnr.wb_gains.gr = 8192 * context.frameContext.awb.gains.green;\n> -       params->acc_param.bnr.wb_gains.r  = 8192 * context.frameContext.awb.gains.red;\n> -       params->acc_param.bnr.wb_gains.b  = 8192 * context.frameContext.awb.gains.blue;\n> -       params->acc_param.bnr.wb_gains.gb = 8192 * context.frameContext.awb.gains.green;\n> +       params->acc_param.bnr.wb_gains.gr = 8192 * context.activeState.awb.gains.green;\n> +       params->acc_param.bnr.wb_gains.r  = 8192 * context.activeState.awb.gains.red;\n> +       params->acc_param.bnr.wb_gains.b  = 8192 * context.activeState.awb.gains.blue;\n> +       params->acc_param.bnr.wb_gains.gb = 8192 * context.activeState.awb.gains.green;\n>  \n>         LOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n>  \n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> index 2040eda5..7c78d0d9 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> @@ -42,7 +42,7 @@ int ToneMapping::configure(IPAContext &context,\n>                            [[maybe_unused]] const IPAConfigInfo &configInfo)\n>  {\n>         /* Initialise tone mapping gamma value. */\n> -       context.frameContext.toneMapping.gamma = 0.0;\n> +       context.activeState.toneMapping.gamma = 0.0;\n>  \n>         return 0;\n>  }\n> @@ -60,7 +60,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,\n>  {\n>         /* Copy the calculated LUT into the parameters buffer. */\n>         memcpy(params->acc_param.gamma.gc_lut.lut,\n> -              context.frameContext.toneMapping.gammaCorrection.lut,\n> +              context.activeState.toneMapping.gammaCorrection.lut,\n>                IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES *\n>                sizeof(params->acc_param.gamma.gc_lut.lut[0]));\n>  \n> @@ -87,11 +87,11 @@ void ToneMapping::process(IPAContext &context,\n>          */\n>         gamma_ = 1.1;\n>  \n> -       if (context.frameContext.toneMapping.gamma == gamma_)\n> +       if (context.activeState.toneMapping.gamma == gamma_)\n>                 return;\n>  \n>         struct ipu3_uapi_gamma_corr_lut &lut =\n> -               context.frameContext.toneMapping.gammaCorrection;\n> +               context.activeState.toneMapping.gammaCorrection;\n>  \n>         for (uint32_t i = 0; i < std::size(lut.lut); i++) {\n>                 double j = static_cast<double>(i) / (std::size(lut.lut) - 1);\n> @@ -101,7 +101,7 @@ void ToneMapping::process(IPAContext &context,\n>                 lut.lut[i] = gamma * 8191;\n>         }\n>  \n> -       context.frameContext.toneMapping.gamma = gamma_;\n> +       context.activeState.toneMapping.gamma = gamma_;\n>  }\n>  \n>  } /* namespace ipa::ipu3::algorithms */\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index b1570dde..06eb2776 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -24,9 +24,20 @@ namespace libcamera::ipa::ipu3 {\n>   * may also be updated in the start() operation.\n>   */\n>  \n> +/**\n> + * \\struct IPAActiveState\n> + * \\brief The active state of the IPA algorithms\n> + *\n> + * The IPA is fed with the statistics generated from the latest frame captured\n> + * by the hardware. The statistics are then processed by the IPA algorithms to\n> + * compute ISP parameters required for the next frame capture. The current state\n\nI am almost tempted to suggest 'desired' for the next frame capture, as\nit's what the algorithms have decided they \"want\" ... but it's trivial\nand 'required' is fine too.\n\n> + * of the algorithms is reflected through IPAActiveState. The latest computed\n> + * values by the IPA algorithms are stored in this structure.\n\nPossible re-word of that last couple of sentences.\n\n\"\"\"\nThe current state of the algorithms is reflected through the\nIPAActiveState to store the values most recently computed by the IPA\nalgorithms.\n\"\"\"\n\n\n> + */\n> +\n>  /**\n>   * \\struct IPAFrameContext\n> - * \\brief Per-frame context for algorithms\n> + * \\brief Context for a frame\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> @@ -34,9 +45,10 @@ namespace libcamera::ipa::ipu3 {\n>   *\n>   * \\todo Detail how to access contexts for a particular frame\n\nI think this todo should probably be removed in the patch that\nexplicitly gives frame contexts to the algorithm calls.\n\n\n>   *\n> - * Each of the fields in the frame context 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> + * Fields in the frame context should reflect the values with which the frame\n> + * was processed on the hardware ISP. A field may be read by any algorithm as\n> + * and when required, or can be used to prepare the metadata container of the\n> + * frame.\n\nI think I'd like to expand on this a bit: (can still be reworked) to\nstate that this storage is about keeping track of what an applciation\nwants to do with this specific frame (both before and after it's\ncaptured):\n\n\"\"\"\nFields in the frame context should reflect values and controls\nassociated with the specific frame as requested by the application, and\nas configured by the hardware. Fields can be read by algorithms to\ndetermine if they should update any specific action for this frame, and\nfinally to update the metadata control lists when the frame is fully\ncompleted.\n\n\"\"\"\n\n\n>   */\n>  \n>  /**\n> @@ -49,10 +61,10 @@ namespace libcamera::ipa::ipu3 {\n>   * \\var IPAContext::frameContext\n>   * \\brief The frame context for the frame being processed\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> + * \\var IPAContext::activeState\n> + * \\brief The current state of IPA algorithms\n> + *\n> + * \\todo The frame context needs to be turned into real per-frame data storage.\n\nI think that todo belongs above IPAContext::activeState, and should be\nassociated with the IPAContext::frameContext. But I expect it's removed\nin the next patch ....\n\nAnyway, I think this takes us in the right direction to track per frame\ncontrols so:\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n>   */\n>  \n>  /**\n> @@ -78,17 +90,17 @@ namespace libcamera::ipa::ipu3 {\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::af\n> + * \\var IPAActiveState::af\n>   * \\brief Context for the Automatic Focus algorithm\n>   *\n> - * \\struct  IPAFrameContext::af\n> - * \\var IPAFrameContext::af.focus\n> + * \\struct IPAActiveState::af\n> + * \\var IPAActiveState::af.focus\n>   * \\brief Current position of the lens\n>   *\n> - * \\var IPAFrameContext::af.maxVariance\n> + * \\var IPAActiveState::af.maxVariance\n>   * \\brief The maximum variance of the current image.\n>   *\n> - * \\var IPAFrameContext::af.stable\n> + * \\var IPAActiveState::af.stable\n>   * \\brief It is set to true, if the best focus is found.\n>   */\n>  \n> @@ -121,64 +133,64 @@ namespace libcamera::ipa::ipu3 {\n>   */\n>  \n>  /**\n> - * \\var IPAFrameContext::agc\n> + * \\var IPAActiveState::agc\n>   * \\brief Context 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> + * \\var IPAActiveState::awb\n>   * \\brief Context 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>  \n>  /**\n> - * \\var IPAFrameContext::sensor\n> - * \\brief Effective sensor values\n> - *\n> - * \\var IPAFrameContext::sensor.exposure\n> - * \\brief Exposure time expressed as a number of lines\n> - *\n> - * \\var IPAFrameContext::sensor.gain\n> - * \\brief Analogue gain multiplier\n> - */\n> -\n> -/**\n> - * \\var IPAFrameContext::toneMapping\n> + * \\var IPAActiveState::toneMapping\n>   * \\brief Context for ToneMapping and Gamma control\n>   *\n> - * \\var IPAFrameContext::toneMapping.gamma\n> + * \\var IPAActiveState::toneMapping.gamma\n>   * \\brief Gamma value for the LUT\n>   *\n> - * \\var IPAFrameContext::toneMapping.gammaCorrection\n> + * \\var IPAActiveState::toneMapping.gammaCorrection\n>   * \\brief Per-pixel tone mapping implemented as a LUT\n>   *\n>   * The LUT structure is defined by the IPU3 kernel interface. See\n>   * <linux/intel-ipu3.h> struct ipu3_uapi_gamma_corr_lut for further details.\n>   */\n>  \n> +/**\n> + * \\var IPAFrameContext::sensor\n> + * \\brief Effective sensor values that were applied for the frame\n> + *\n> + * \\var IPAFrameContext::sensor.exposure\n> + * \\brief Exposure time expressed as a number of lines\n> + *\n> + * \\var IPAFrameContext::sensor.gain\n> + * \\brief Analogue gain multiplier\n> + */\n> +\n>  } /* namespace libcamera::ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 103498ef..8d681131 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -42,7 +42,7 @@ struct IPASessionConfiguration {\n>         } sensor;\n>  };\n>  \n> -struct IPAFrameContext {\n> +struct IPAActiveState {\n>         struct {\n>                 uint32_t focus;\n>                 double maxVariance;\n> @@ -64,19 +64,23 @@ struct IPAFrameContext {\n>                 double temperatureK;\n>         } awb;\n>  \n> -       struct {\n> -               uint32_t exposure;\n> -               double gain;\n> -       } sensor;\n> -\n>         struct {\n>                 double gamma;\n>                 struct ipu3_uapi_gamma_corr_lut gammaCorrection;\n>         } toneMapping;\n>  };\n>  \n> +struct IPAFrameContext {\n> +       struct {\n> +               uint32_t exposure;\n> +               double gain;\n> +       } sensor;\n> +};\n> +\n>  struct IPAContext {\n>         IPASessionConfiguration configuration;\n> +       IPAActiveState activeState;\n> +\n>         IPAFrameContext frameContext;\n>  };\n>  \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index dd6cfd79..3b4fc911 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -454,7 +454,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n>  \n>         calculateBdsGrid(configInfo.bdsOutputSize);\n>  \n> -       /* Clean frameContext at each reconfiguration. */\n> +       /* Clean IPAActiveState at each reconfiguration. */\n> +       context_.activeState = {};\n>         context_.frameContext = {};\n>  \n>         if (!validateSensorControls()) {\n> @@ -585,7 +586,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>  \n>         ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);\n>  \n> -       ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);\n> +       ctrls.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n>  \n>         ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration);\n>  \n> @@ -623,8 +624,8 @@ void IPAIPU3::queueRequest([[maybe_unused]] const uint32_t frame,\n>   */\n>  void IPAIPU3::setControls(unsigned int frame)\n>  {\n> -       int32_t exposure = context_.frameContext.agc.exposure;\n> -       int32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n> +       int32_t exposure = context_.activeState.agc.exposure;\n> +       int32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n>  \n>         ControlList ctrls(sensorCtrls_);\n>         ctrls.set(V4L2_CID_EXPOSURE, exposure);\n> @@ -632,7 +633,7 @@ void IPAIPU3::setControls(unsigned int frame)\n>  \n>         ControlList lensCtrls(lensCtrls_);\n>         lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE,\n> -                     static_cast<int32_t>(context_.frameContext.af.focus));\n> +                     static_cast<int32_t>(context_.activeState.af.focus));\n>  \n>         setSensorControls.emit(frame, ctrls, lensCtrls);\n>  }\n> -- \n> 2.31.0\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 BFB30C0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 May 2022 10:23:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1838E6565B;\n\tWed, 18 May 2022 12:23:50 +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 8EC6165656\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 May 2022 12:23:48 +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 147F2484;\n\tWed, 18 May 2022 12:23:48 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1652869430;\n\tbh=GFB91puFULrdEZjp76U7w3A49Fsk+vGe0CaSOH+b5+g=;\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=SMcrp6NI3dCn7XeJHI/ZpK+GNS/lwj28yRA35Yrro/ShZoh4u8CcclJlP5wAzhat1\n\t7WCPomFUPriq3HsApkSjg1+NQu2wthZhqvxoLIj29gHFvaiUhH/D6TXPR11fcmVPJG\n\teQHSibz8PLKfnK9KKZ3gNx3EEF3yEc9lQvs5WiArah6xdtWR14SJM2PbCEH3LL5cRO\n\tha1cn+uaYCaevHkTCdXPOw3KcY5Uu/4//2Vn3YCO2b7qwXYE6yM9e4DrAjb+RibG8A\n\tOLQ1U8h1H0sjYcYSwK0WXINrsqXjSsor58XkQvQkyqwTbHWTtFBADwd+0LQO+bEVFr\n\t3svrTiuGoJGLQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1652869428;\n\tbh=GFB91puFULrdEZjp76U7w3A49Fsk+vGe0CaSOH+b5+g=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=AesKOP9NY7NpPuNp9oar4KlJvnZ07d6GvksEKniYMshfMVMNX0AWHBUJIuKy2Nt2Q\n\tXa8TjhKuGTTydJ2wAzp24qFSmsTJmu2DOT2cZWUWwDpmctv2wsCjJtdvLD+SIeZhfg\n\tVj1VNd/3P6hGNYAkomFCTJre37VtsnmVsLOb3rb0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AesKOP9N\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220517191833.333122-2-umang.jain@ideasonboard.com>","References":"<20220517191833.333122-1-umang.jain@ideasonboard.com>\n\t<20220517191833.333122-2-umang.jain@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 18 May 2022 11:23:45 +0100","Message-ID":"<165286942548.368702.175982003173968929@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] ipa: ipu3: Rework\n\tIPAFrameContext","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>"}}]