[{"id":25525,"web_url":"https://patchwork.libcamera.org/comment/25525/","msgid":"<20221024012907.GS3874866@pyrite.rasen.tech>","date":"2022-10-24T01:29:07","subject":"Re: [libcamera-devel] [PATCH v3 01/13] ipa: Sort algorithm\n\toperations based on calling order","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"On Mon, Oct 24, 2022 at 03:03:44AM +0300, Laurent Pinchart wrote:\n> Reorder functions in the base ipa::Algorithm and its derived classes to\n> match the calling order: queueRequest(), prepare() and process(). This\n> makes the code flow easier to read. No functional change intended.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> ---\n>  src/ipa/ipu3/algorithms/af.cpp    |  32 +++---\n>  src/ipa/ipu3/algorithms/awb.cpp   | 166 +++++++++++++++---------------\n>  src/ipa/libipa/algorithm.cpp      |  34 +++---\n>  src/ipa/libipa/algorithm.h        |  14 +--\n>  src/ipa/rkisp1/algorithms/agc.cpp |  80 +++++++-------\n>  src/ipa/rkisp1/algorithms/awb.cpp |  94 ++++++++---------\n>  src/ipa/rkisp1/algorithms/awb.h   |   6 +-\n>  7 files changed, 213 insertions(+), 213 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index 5a0452a5719b..12927eecf613 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -113,22 +113,6 @@ Af::Af()\n>  {\n>  }\n>  \n> -/**\n> - * \\copydoc libcamera::ipa::Algorithm::prepare\n> - */\n> -void Af::prepare(IPAContext &context,\n> -\t\t [[maybe_unused]] const uint32_t frame,\n> -\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> -\t\t ipu3_uapi_params *params)\n> -{\n> -\tconst struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> -\tparams->acc_param.af.grid_cfg = grid;\n> -\tparams->acc_param.af.filter_config = afFilterConfigDefault;\n> -\n> -\t/* Enable AF processing block */\n> -\tparams->use.acc_af = 1;\n> -}\n> -\n>  /**\n>   * \\brief Configure the Af given a configInfo\n>   * \\param[in] context The shared IPA context\n> @@ -197,6 +181,22 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>  \treturn 0;\n>  }\n>  \n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Af::prepare(IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t ipu3_uapi_params *params)\n> +{\n> +\tconst struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> +\tparams->acc_param.af.grid_cfg = grid;\n> +\tparams->acc_param.af.filter_config = afFilterConfigDefault;\n> +\n> +\t/* Enable AF processing block */\n> +\tparams->use.acc_af = 1;\n> +}\n> +\n>  /**\n>   * \\brief AF coarse scan\n>   * \\param[in] context The shared IPA context\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index dd7ebc07c534..5abd46215833 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -218,6 +218,89 @@ int Awb::configure(IPAContext &context,\n>  \treturn 0;\n>  }\n>  \n> +constexpr uint16_t Awb::threshold(float value)\n> +{\n> +\t/* AWB thresholds are in the range [0, 8191] */\n> +\treturn value * 8191;\n> +}\n> +\n> +constexpr uint16_t Awb::gainValue(double gain)\n> +{\n> +\t/*\n> +\t * The colour gains applied by the BNR for the four channels (Gr, R, B\n> +\t * and Gb) are expressed in the parameters structure as 16-bit integers\n> +\t * that store a fixed-point U3.13 value in the range [0, 8[.\n> +\t *\n> +\t * The real gain value is equal to the gain parameter plus one, i.e.\n> +\t *\n> +\t * Pout = Pin * (1 + gain / 8192)\n> +\t *\n> +\t * where 'Pin' is the input pixel value, 'Pout' the output pixel value,\n> +\t * and 'gain' the gain in the parameters structure as a 16-bit integer.\n> +\t */\n> +\treturn std::clamp((gain - 1.0) * 8192, 0.0, 65535.0);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Awb::prepare(IPAContext &context,\n> +\t\t  [[maybe_unused]] const uint32_t frame,\n> +\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t  ipu3_uapi_params *params)\n> +{\n> +\t/*\n> +\t * Green saturation thresholds are reduced because we are using the\n> +\t * green channel only in the exposure computation.\n> +\t */\n> +\tparams->acc_param.awb.config.rgbs_thr_r = threshold(1.0);\n> +\tparams->acc_param.awb.config.rgbs_thr_gr = threshold(0.9);\n> +\tparams->acc_param.awb.config.rgbs_thr_gb = threshold(0.9);\n> +\tparams->acc_param.awb.config.rgbs_thr_b = threshold(1.0);\n> +\n> +\t/*\n> +\t * Enable saturation inclusion on thr_b for ImgU to update the\n> +\t * ipu3_uapi_awb_set_item->sat_ratio field.\n> +\t */\n> +\tparams->acc_param.awb.config.rgbs_thr_b |= IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n> +\t\t\t\t\t\t   IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> +\n> +\tconst ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;\n> +\n> +\tparams->acc_param.awb.config.grid = context.configuration.grid.bdsGrid;\n> +\n> +\t/*\n> +\t * Optical center is column start (respectively row start) of the\n> +\t * cell of interest minus its X center (respectively Y center).\n> +\t *\n> +\t * For the moment use BDS as a first approximation, but it should\n> +\t * be calculated based on Shading (SHD) parameters.\n> +\t */\n> +\tparams->acc_param.bnr = imguCssBnrDefaults;\n> +\tSize &bdsOutputSize = context.configuration.grid.bdsOutputSize;\n> +\tparams->acc_param.bnr.column_size = bdsOutputSize.width;\n> +\tparams->acc_param.bnr.opt_center.x_reset = grid.x_start - (bdsOutputSize.width / 2);\n> +\tparams->acc_param.bnr.opt_center.y_reset = grid.y_start - (bdsOutputSize.height / 2);\n> +\tparams->acc_param.bnr.opt_center_sqr.x_sqr_reset = params->acc_param.bnr.opt_center.x_reset\n> +\t\t\t\t\t\t\t* params->acc_param.bnr.opt_center.x_reset;\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> +\n> +\tparams->acc_param.bnr.wb_gains.gr = gainValue(context.activeState.awb.gains.green);\n> +\tparams->acc_param.bnr.wb_gains.r  = gainValue(context.activeState.awb.gains.red);\n> +\tparams->acc_param.bnr.wb_gains.b  = gainValue(context.activeState.awb.gains.blue);\n> +\tparams->acc_param.bnr.wb_gains.gb = gainValue(context.activeState.awb.gains.green);\n> +\n> +\tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n> +\n> +\t/* The CCM matrix may change when color temperature will be used */\n> +\tparams->acc_param.ccm = imguCssCcmDefault;\n> +\n> +\tparams->use.acc_awb = 1;\n> +\tparams->use.acc_bnr = 1;\n> +\tparams->use.acc_ccm = 1;\n> +}\n> +\n>  /**\n>   * The function estimates the correlated color temperature using\n>   * from RGB color space input.\n> @@ -415,89 +498,6 @@ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t\t     context.activeState.awb.temperatureK);\n>  }\n>  \n> -constexpr uint16_t Awb::threshold(float value)\n> -{\n> -\t/* AWB thresholds are in the range [0, 8191] */\n> -\treturn value * 8191;\n> -}\n> -\n> -constexpr uint16_t Awb::gainValue(double gain)\n> -{\n> -\t/*\n> -\t * The colour gains applied by the BNR for the four channels (Gr, R, B\n> -\t * and Gb) are expressed in the parameters structure as 16-bit integers\n> -\t * that store a fixed-point U3.13 value in the range [0, 8[.\n> -\t *\n> -\t * The real gain value is equal to the gain parameter plus one, i.e.\n> -\t *\n> -\t * Pout = Pin * (1 + gain / 8192)\n> -\t *\n> -\t * where 'Pin' is the input pixel value, 'Pout' the output pixel value,\n> -\t * and 'gain' the gain in the parameters structure as a 16-bit integer.\n> -\t */\n> -\treturn std::clamp((gain - 1.0) * 8192, 0.0, 65535.0);\n> -}\n> -\n> -/**\n> - * \\copydoc libcamera::ipa::Algorithm::prepare\n> - */\n> -void Awb::prepare(IPAContext &context,\n> -\t\t  [[maybe_unused]] const uint32_t frame,\n> -\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n> -\t\t  ipu3_uapi_params *params)\n> -{\n> -\t/*\n> -\t * Green saturation thresholds are reduced because we are using the\n> -\t * green channel only in the exposure computation.\n> -\t */\n> -\tparams->acc_param.awb.config.rgbs_thr_r = threshold(1.0);\n> -\tparams->acc_param.awb.config.rgbs_thr_gr = threshold(0.9);\n> -\tparams->acc_param.awb.config.rgbs_thr_gb = threshold(0.9);\n> -\tparams->acc_param.awb.config.rgbs_thr_b = threshold(1.0);\n> -\n> -\t/*\n> -\t * Enable saturation inclusion on thr_b for ImgU to update the\n> -\t * ipu3_uapi_awb_set_item->sat_ratio field.\n> -\t */\n> -\tparams->acc_param.awb.config.rgbs_thr_b |= IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n> -\t\t\t\t\t\t   IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> -\n> -\tconst ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;\n> -\n> -\tparams->acc_param.awb.config.grid = context.configuration.grid.bdsGrid;\n> -\n> -\t/*\n> -\t * Optical center is column start (respectively row start) of the\n> -\t * cell of interest minus its X center (respectively Y center).\n> -\t *\n> -\t * For the moment use BDS as a first approximation, but it should\n> -\t * be calculated based on Shading (SHD) parameters.\n> -\t */\n> -\tparams->acc_param.bnr = imguCssBnrDefaults;\n> -\tSize &bdsOutputSize = context.configuration.grid.bdsOutputSize;\n> -\tparams->acc_param.bnr.column_size = bdsOutputSize.width;\n> -\tparams->acc_param.bnr.opt_center.x_reset = grid.x_start - (bdsOutputSize.width / 2);\n> -\tparams->acc_param.bnr.opt_center.y_reset = grid.y_start - (bdsOutputSize.height / 2);\n> -\tparams->acc_param.bnr.opt_center_sqr.x_sqr_reset = params->acc_param.bnr.opt_center.x_reset\n> -\t\t\t\t\t\t\t* params->acc_param.bnr.opt_center.x_reset;\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> -\n> -\tparams->acc_param.bnr.wb_gains.gr = gainValue(context.activeState.awb.gains.green);\n> -\tparams->acc_param.bnr.wb_gains.r  = gainValue(context.activeState.awb.gains.red);\n> -\tparams->acc_param.bnr.wb_gains.b  = gainValue(context.activeState.awb.gains.blue);\n> -\tparams->acc_param.bnr.wb_gains.gb = gainValue(context.activeState.awb.gains.green);\n> -\n> -\tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n> -\n> -\t/* The CCM matrix may change when color temperature will be used */\n> -\tparams->acc_param.ccm = imguCssCcmDefault;\n> -\n> -\tparams->use.acc_awb = 1;\n> -\tparams->use.acc_bnr = 1;\n> -\tparams->use.acc_ccm = 1;\n> -}\n> -\n>  REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n>  \n>  } /* namespace ipa::ipu3::algorithms */\n> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n> index 71f583840a5a..bc1c29a6dbcc 100644\n> --- a/src/ipa/libipa/algorithm.cpp\n> +++ b/src/ipa/libipa/algorithm.cpp\n> @@ -66,23 +66,6 @@ namespace ipa {\n>   * \\return 0 if successful, an error code otherwise\n>   */\n>  \n> -/**\n> - * \\fn Algorithm::prepare()\n> - * \\brief Fill the \\a params buffer with ISP processing parameters for a frame\n> - * \\param[in] context The shared IPA context\n> - * \\param[in] frame The frame context sequence number\n> - * \\param[in] frameContext The FrameContext for this frame\n> - * \\param[out] params The ISP specific parameters\n> - *\n> - * This function is called for every frame when the camera is running before it\n> - * is processed by the ISP to prepare the ISP processing parameters for that\n> - * frame.\n> - *\n> - * Algorithms shall fill in the parameter structure fields appropriately to\n> - * configure the ISP processing blocks that they are responsible for. This\n> - * includes setting fields and flags that enable those processing blocks.\n> - */\n> -\n>  /**\n>   * \\fn Algorithm::queueRequest()\n>   * \\brief Provide control values to the algorithm\n> @@ -100,6 +83,23 @@ namespace ipa {\n>   * use during frame processing.\n>   */\n>  \n> +/**\n> + * \\fn Algorithm::prepare()\n> + * \\brief Fill the \\a params buffer with ISP processing parameters for a frame\n> + * \\param[in] context The shared IPA context\n> + * \\param[in] frame The frame context sequence number\n> + * \\param[in] frameContext The FrameContext for this frame\n> + * \\param[out] params The ISP specific parameters\n> + *\n> + * This function is called for every frame when the camera is running before it\n> + * is processed by the ISP to prepare the ISP processing parameters for that\n> + * frame.\n> + *\n> + * Algorithms shall fill in the parameter structure fields appropriately to\n> + * configure the ISP processing blocks that they are responsible for. This\n> + * includes setting fields and flags that enable those processing blocks.\n> + */\n> +\n>  /**\n>   * \\fn Algorithm::process()\n>   * \\brief Process ISP statistics, and run algorithm operations\n> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h\n> index 38b3231c5d0e..987e3e4ce777 100644\n> --- a/src/ipa/libipa/algorithm.h\n> +++ b/src/ipa/libipa/algorithm.h\n> @@ -38,13 +38,6 @@ public:\n>  \t\treturn 0;\n>  \t}\n>  \n> -\tvirtual void prepare([[maybe_unused]] typename Module::Context &context,\n> -\t\t\t     [[maybe_unused]] const uint32_t frame,\n> -\t\t\t     [[maybe_unused]] typename Module::FrameContext &frameContext,\n> -\t\t\t     [[maybe_unused]] typename Module::Params *params)\n> -\t{\n> -\t}\n> -\n>  \tvirtual void queueRequest([[maybe_unused]] typename Module::Context &context,\n>  \t\t\t\t  [[maybe_unused]] const uint32_t frame,\n>  \t\t\t\t  [[maybe_unused]] typename Module::FrameContext &frameContext,\n> @@ -52,6 +45,13 @@ public:\n>  \t{\n>  \t}\n>  \n> +\tvirtual void prepare([[maybe_unused]] typename Module::Context &context,\n> +\t\t\t     [[maybe_unused]] const uint32_t frame,\n> +\t\t\t     [[maybe_unused]] typename Module::FrameContext &frameContext,\n> +\t\t\t     [[maybe_unused]] typename Module::Params *params)\n> +\t{\n> +\t}\n> +\n>  \tvirtual void process([[maybe_unused]] typename Module::Context &context,\n>  \t\t\t     [[maybe_unused]] const uint32_t frame,\n>  \t\t\t     [[maybe_unused]] typename Module::FrameContext &frameContext,\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index a06e9f732a8f..3fcbfa608467 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -107,6 +107,46 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>  \treturn 0;\n>  }\n>  \n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Agc::prepare(IPAContext &context, const uint32_t frame,\n> +\t\t  IPAFrameContext &frameContext, rkisp1_params_cfg *params)\n> +{\n> +\tframeContext.agc.exposure = context.activeState.agc.exposure;\n> +\tframeContext.agc.gain = context.activeState.agc.gain;\n> +\n> +\tif (frame > 0)\n> +\t\treturn;\n> +\n> +\t/* Configure the measurement window. */\n> +\tparams->meas.aec_config.meas_window = context.configuration.agc.measureWindow;\n> +\t/* Use a continuous method for measure. */\n> +\tparams->meas.aec_config.autostop = RKISP1_CIF_ISP_EXP_CTRL_AUTOSTOP_0;\n> +\t/* Estimate Y as (R + G + B) x (85/256). */\n> +\tparams->meas.aec_config.mode = RKISP1_CIF_ISP_EXP_MEASURING_MODE_1;\n> +\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AEC;\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> +\n> +\t/* Configure histogram. */\n> +\tparams->meas.hst_config.meas_window = context.configuration.agc.measureWindow;\n> +\t/* Produce the luminance histogram. */\n> +\tparams->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;\n> +\t/* Set an average weighted histogram. */\n> +\tfor (unsigned int histBin = 0; histBin < numHistBins_; histBin++)\n> +\t\tparams->meas.hst_config.hist_weight[histBin] = 1;\n> +\t/* Step size can't be less than 3. */\n> +\tparams->meas.hst_config.histogram_predivider = 4;\n> +\n> +\t/* Update the configuration for histogram. */\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;\n> +\t/* Enable the histogram measure unit. */\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_HST;\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_HST;\n> +}\n> +\n>  /**\n>   * \\brief Apply a filter on the exposure value to limit the speed of changes\n>   * \\param[in] exposureValue The target exposure from the AGC algorithm\n> @@ -348,46 +388,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \tmetadata.set(controls::FrameDuration, frameDuration.get<std::micro>());\n>  }\n>  \n> -/**\n> - * \\copydoc libcamera::ipa::Algorithm::prepare\n> - */\n> -void Agc::prepare(IPAContext &context, const uint32_t frame,\n> -\t\t  IPAFrameContext &frameContext, rkisp1_params_cfg *params)\n> -{\n> -\tframeContext.agc.exposure = context.activeState.agc.exposure;\n> -\tframeContext.agc.gain = context.activeState.agc.gain;\n> -\n> -\tif (frame > 0)\n> -\t\treturn;\n> -\n> -\t/* Configure the measurement window. */\n> -\tparams->meas.aec_config.meas_window = context.configuration.agc.measureWindow;\n> -\t/* Use a continuous method for measure. */\n> -\tparams->meas.aec_config.autostop = RKISP1_CIF_ISP_EXP_CTRL_AUTOSTOP_0;\n> -\t/* Estimate Y as (R + G + B) x (85/256). */\n> -\tparams->meas.aec_config.mode = RKISP1_CIF_ISP_EXP_MEASURING_MODE_1;\n> -\n> -\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> -\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AEC;\n> -\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> -\n> -\t/* Configure histogram. */\n> -\tparams->meas.hst_config.meas_window = context.configuration.agc.measureWindow;\n> -\t/* Produce the luminance histogram. */\n> -\tparams->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;\n> -\t/* Set an average weighted histogram. */\n> -\tfor (unsigned int histBin = 0; histBin < numHistBins_; histBin++)\n> -\t\tparams->meas.hst_config.hist_weight[histBin] = 1;\n> -\t/* Step size can't be less than 3. */\n> -\tparams->meas.hst_config.histogram_predivider = 4;\n> -\n> -\t/* Update the configuration for histogram. */\n> -\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;\n> -\t/* Enable the histogram measure unit. */\n> -\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_HST;\n> -\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_HST;\n> -}\n> -\n>  REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n>  \n>  } /* namespace ipa::rkisp1::algorithms */\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 083c2d982c52..744f4a386c1a 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -67,20 +67,41 @@ int Awb::configure(IPAContext &context,\n>  \treturn 0;\n>  }\n>  \n> -uint32_t Awb::estimateCCT(double red, double green, double blue)\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void Awb::queueRequest(IPAContext &context,\n> +\t\t       [[maybe_unused]] const uint32_t frame,\n> +\t\t       IPAFrameContext &frameContext,\n> +\t\t       const ControlList &controls)\n>  {\n> -\t/* Convert the RGB values to CIE tristimulus values (XYZ) */\n> -\tdouble X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);\n> -\tdouble Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);\n> -\tdouble Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);\n> -\n> -\t/* Calculate the normalized chromaticity values */\n> -\tdouble x = X / (X + Y + Z);\n> -\tdouble y = Y / (X + Y + Z);\n> -\n> -\t/* Calculate CCT */\n> -\tdouble n = (x - 0.3320) / (0.1858 - y);\n> -\treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n> +\tauto &awb = context.activeState.awb;\n> +\n> +\tconst auto &awbEnable = controls.get(controls::AwbEnable);\n> +\tif (awbEnable && *awbEnable != awb.autoEnabled) {\n> +\t\tawb.autoEnabled = *awbEnable;\n> +\n> +\t\tLOG(RkISP1Awb, Debug)\n> +\t\t\t<< (*awbEnable ? \"Enabling\" : \"Disabling\") << \" AWB\";\n> +\t}\n> +\n> +\tconst auto &colourGains = controls.get(controls::ColourGains);\n> +\tif (colourGains && !awb.autoEnabled) {\n> +\t\tawb.gains.manual.red = (*colourGains)[0];\n> +\t\tawb.gains.manual.blue = (*colourGains)[1];\n> +\n> +\t\tLOG(RkISP1Awb, Debug)\n> +\t\t\t<< \"Set colour gains to red: \" << awb.gains.manual.red\n> +\t\t\t<< \", blue: \" << awb.gains.manual.blue;\n> +\t}\n> +\n> +\tframeContext.awb.autoEnabled = awb.autoEnabled;\n> +\n> +\tif (!awb.autoEnabled) {\n> +\t\tframeContext.awb.gains.red = awb.gains.manual.red;\n> +\t\tframeContext.awb.gains.green = 1.0;\n> +\t\tframeContext.awb.gains.blue = awb.gains.manual.blue;\n> +\t}\n>  }\n>  \n>  /**\n> @@ -165,41 +186,20 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n>  \tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB;\n>  }\n>  \n> -/**\n> - * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> - */\n> -void Awb::queueRequest(IPAContext &context,\n> -\t\t       [[maybe_unused]] const uint32_t frame,\n> -\t\t       IPAFrameContext &frameContext,\n> -\t\t       const ControlList &controls)\n> +uint32_t Awb::estimateCCT(double red, double green, double blue)\n>  {\n> -\tauto &awb = context.activeState.awb;\n> -\n> -\tconst auto &awbEnable = controls.get(controls::AwbEnable);\n> -\tif (awbEnable && *awbEnable != awb.autoEnabled) {\n> -\t\tawb.autoEnabled = *awbEnable;\n> -\n> -\t\tLOG(RkISP1Awb, Debug)\n> -\t\t\t<< (*awbEnable ? \"Enabling\" : \"Disabling\") << \" AWB\";\n> -\t}\n> -\n> -\tconst auto &colourGains = controls.get(controls::ColourGains);\n> -\tif (colourGains && !awb.autoEnabled) {\n> -\t\tawb.gains.manual.red = (*colourGains)[0];\n> -\t\tawb.gains.manual.blue = (*colourGains)[1];\n> -\n> -\t\tLOG(RkISP1Awb, Debug)\n> -\t\t\t<< \"Set colour gains to red: \" << awb.gains.manual.red\n> -\t\t\t<< \", blue: \" << awb.gains.manual.blue;\n> -\t}\n> -\n> -\tframeContext.awb.autoEnabled = awb.autoEnabled;\n> -\n> -\tif (!awb.autoEnabled) {\n> -\t\tframeContext.awb.gains.red = awb.gains.manual.red;\n> -\t\tframeContext.awb.gains.green = 1.0;\n> -\t\tframeContext.awb.gains.blue = awb.gains.manual.blue;\n> -\t}\n> +\t/* Convert the RGB values to CIE tristimulus values (XYZ) */\n> +\tdouble X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);\n> +\tdouble Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);\n> +\tdouble Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);\n> +\n> +\t/* Calculate the normalized chromaticity values */\n> +\tdouble x = X / (X + Y + Z);\n> +\tdouble y = Y / (X + Y + Z);\n> +\n> +\t/* Calculate CCT */\n> +\tdouble n = (x - 0.3320) / (0.1858 - y);\n> +\treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n>  }\n>  \n>  /**\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index f5633b1171a0..9d45a442339c 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -20,12 +20,12 @@ public:\n>  \t~Awb() = default;\n>  \n>  \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> -\tvoid prepare(IPAContext &context, const uint32_t frame,\n> -\t\t     IPAFrameContext &frameContext,\n> -\t\t     rkisp1_params_cfg *params) override;\n>  \tvoid queueRequest(IPAContext &context, const uint32_t frame,\n>  \t\t\t  IPAFrameContext &frameContext,\n>  \t\t\t  const ControlList &controls) override;\n> +\tvoid prepare(IPAContext &context, const uint32_t frame,\n> +\t\t     IPAFrameContext &frameContext,\n> +\t\t     rkisp1_params_cfg *params) override;\n>  \tvoid process(IPAContext &context, const uint32_t frame,\n>  \t\t     IPAFrameContext &frameContext,\n>  \t\t     const rkisp1_stat_buffer *stats,\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 23CEEBDB16\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Oct 2022 01:29:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 55B3D62ED9;\n\tMon, 24 Oct 2022 03:29:16 +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 726A262EA6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Oct 2022 03:29:15 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 61476471;\n\tMon, 24 Oct 2022 03:29:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666574956;\n\tbh=g9gLuAMkCDZ7OFMkmj5kRzcMN+voTey0Ua3nHlOnG5o=;\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=18MCMYuMJ0wNy//DdqdBDAc46ztyyIAPP9IDWXyGXsA5eqcDbb3c330DI9xz0D2D1\n\taGBrRLeX08cETa4ALjZ3vlS1UhiV7DdNFIobaUvJcWuJnL4O/FrKCPQkYEB6yDtOpm\n\tvxA6yKsf1VN3UrRr2atJz1RsO4F1iJffCmW8yBX/pgLB6yoHWXkdsZfzyh2kgtkjjj\n\tC2iKoq7onvMWwS1zFFlsQphziV4o2KbkFFqexMhtkH5daNrDo+xn14GqlNhkJyoXlw\n\t572RAKgF0ftuTAN9riRxqDV703Qa22KpJ6C2yevZCAhzbhe/Tjre7ZlqiD40wVCs+B\n\tm3w2Ir720EauA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666574954;\n\tbh=g9gLuAMkCDZ7OFMkmj5kRzcMN+voTey0Ua3nHlOnG5o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qCvMXTgQ8/XPCz0BnnJDcVU3WRCItZAGOM6AzmD/hUWM+Y/URDFVcEYwr1t8uL427\n\t68G9Jg7ifW7nRf0F7a32GKK8VFqR7niNVO8Qh5Uf49j4U6ro4h8aWUWqJTW9viFi+L\n\tVE52djMV69iO+5XsB9a/WTTqNhpRVVsgyE5Q2bvw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"qCvMXTgQ\"; dkim-atps=neutral","Date":"Mon, 24 Oct 2022 10:29:07 +0900","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20221024012907.GS3874866@pyrite.rasen.tech>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20221024000356.29521-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 01/13] ipa: Sort algorithm\n\toperations based on calling order","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":"Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"paul.elder@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":25596,"web_url":"https://patchwork.libcamera.org/comment/25596/","msgid":"<20221026134050.esbzfwqjlxenoytg@uno.localdomain>","date":"2022-10-26T13:40:50","subject":"Re: [libcamera-devel] [PATCH v3 01/13] ipa: Sort algorithm\n\toperations based on calling order","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Mon, Oct 24, 2022 at 03:03:44AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Reorder functions in the base ipa::Algorithm and its derived classes to\n> match the calling order: queueRequest(), prepare() and process(). This\n> makes the code flow easier to read. No functional change intended.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nThe md5sum of the file changes, so I guess it's just moving code\naround.. Anyway, at the naked eye I haven't spotted anything wrong\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n> ---\n>  src/ipa/ipu3/algorithms/af.cpp    |  32 +++---\n>  src/ipa/ipu3/algorithms/awb.cpp   | 166 +++++++++++++++---------------\n>  src/ipa/libipa/algorithm.cpp      |  34 +++---\n>  src/ipa/libipa/algorithm.h        |  14 +--\n>  src/ipa/rkisp1/algorithms/agc.cpp |  80 +++++++-------\n>  src/ipa/rkisp1/algorithms/awb.cpp |  94 ++++++++---------\n>  src/ipa/rkisp1/algorithms/awb.h   |   6 +-\n>  7 files changed, 213 insertions(+), 213 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index 5a0452a5719b..12927eecf613 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -113,22 +113,6 @@ Af::Af()\n>  {\n>  }\n>\n> -/**\n> - * \\copydoc libcamera::ipa::Algorithm::prepare\n> - */\n> -void Af::prepare(IPAContext &context,\n> -\t\t [[maybe_unused]] const uint32_t frame,\n> -\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> -\t\t ipu3_uapi_params *params)\n> -{\n> -\tconst struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> -\tparams->acc_param.af.grid_cfg = grid;\n> -\tparams->acc_param.af.filter_config = afFilterConfigDefault;\n> -\n> -\t/* Enable AF processing block */\n> -\tparams->use.acc_af = 1;\n> -}\n> -\n>  /**\n>   * \\brief Configure the Af given a configInfo\n>   * \\param[in] context The shared IPA context\n> @@ -197,6 +181,22 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>  \treturn 0;\n>  }\n>\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Af::prepare(IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t ipu3_uapi_params *params)\n> +{\n> +\tconst struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> +\tparams->acc_param.af.grid_cfg = grid;\n> +\tparams->acc_param.af.filter_config = afFilterConfigDefault;\n> +\n> +\t/* Enable AF processing block */\n> +\tparams->use.acc_af = 1;\n> +}\n> +\n>  /**\n>   * \\brief AF coarse scan\n>   * \\param[in] context The shared IPA context\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index dd7ebc07c534..5abd46215833 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -218,6 +218,89 @@ int Awb::configure(IPAContext &context,\n>  \treturn 0;\n>  }\n>\n> +constexpr uint16_t Awb::threshold(float value)\n> +{\n> +\t/* AWB thresholds are in the range [0, 8191] */\n> +\treturn value * 8191;\n> +}\n> +\n> +constexpr uint16_t Awb::gainValue(double gain)\n> +{\n> +\t/*\n> +\t * The colour gains applied by the BNR for the four channels (Gr, R, B\n> +\t * and Gb) are expressed in the parameters structure as 16-bit integers\n> +\t * that store a fixed-point U3.13 value in the range [0, 8[.\n> +\t *\n> +\t * The real gain value is equal to the gain parameter plus one, i.e.\n> +\t *\n> +\t * Pout = Pin * (1 + gain / 8192)\n> +\t *\n> +\t * where 'Pin' is the input pixel value, 'Pout' the output pixel value,\n> +\t * and 'gain' the gain in the parameters structure as a 16-bit integer.\n> +\t */\n> +\treturn std::clamp((gain - 1.0) * 8192, 0.0, 65535.0);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Awb::prepare(IPAContext &context,\n> +\t\t  [[maybe_unused]] const uint32_t frame,\n> +\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t  ipu3_uapi_params *params)\n> +{\n> +\t/*\n> +\t * Green saturation thresholds are reduced because we are using the\n> +\t * green channel only in the exposure computation.\n> +\t */\n> +\tparams->acc_param.awb.config.rgbs_thr_r = threshold(1.0);\n> +\tparams->acc_param.awb.config.rgbs_thr_gr = threshold(0.9);\n> +\tparams->acc_param.awb.config.rgbs_thr_gb = threshold(0.9);\n> +\tparams->acc_param.awb.config.rgbs_thr_b = threshold(1.0);\n> +\n> +\t/*\n> +\t * Enable saturation inclusion on thr_b for ImgU to update the\n> +\t * ipu3_uapi_awb_set_item->sat_ratio field.\n> +\t */\n> +\tparams->acc_param.awb.config.rgbs_thr_b |= IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n> +\t\t\t\t\t\t   IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> +\n> +\tconst ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;\n> +\n> +\tparams->acc_param.awb.config.grid = context.configuration.grid.bdsGrid;\n> +\n> +\t/*\n> +\t * Optical center is column start (respectively row start) of the\n> +\t * cell of interest minus its X center (respectively Y center).\n> +\t *\n> +\t * For the moment use BDS as a first approximation, but it should\n> +\t * be calculated based on Shading (SHD) parameters.\n> +\t */\n> +\tparams->acc_param.bnr = imguCssBnrDefaults;\n> +\tSize &bdsOutputSize = context.configuration.grid.bdsOutputSize;\n> +\tparams->acc_param.bnr.column_size = bdsOutputSize.width;\n> +\tparams->acc_param.bnr.opt_center.x_reset = grid.x_start - (bdsOutputSize.width / 2);\n> +\tparams->acc_param.bnr.opt_center.y_reset = grid.y_start - (bdsOutputSize.height / 2);\n> +\tparams->acc_param.bnr.opt_center_sqr.x_sqr_reset = params->acc_param.bnr.opt_center.x_reset\n> +\t\t\t\t\t\t\t* params->acc_param.bnr.opt_center.x_reset;\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> +\n> +\tparams->acc_param.bnr.wb_gains.gr = gainValue(context.activeState.awb.gains.green);\n> +\tparams->acc_param.bnr.wb_gains.r  = gainValue(context.activeState.awb.gains.red);\n> +\tparams->acc_param.bnr.wb_gains.b  = gainValue(context.activeState.awb.gains.blue);\n> +\tparams->acc_param.bnr.wb_gains.gb = gainValue(context.activeState.awb.gains.green);\n> +\n> +\tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n> +\n> +\t/* The CCM matrix may change when color temperature will be used */\n> +\tparams->acc_param.ccm = imguCssCcmDefault;\n> +\n> +\tparams->use.acc_awb = 1;\n> +\tparams->use.acc_bnr = 1;\n> +\tparams->use.acc_ccm = 1;\n> +}\n> +\n>  /**\n>   * The function estimates the correlated color temperature using\n>   * from RGB color space input.\n> @@ -415,89 +498,6 @@ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t\t     context.activeState.awb.temperatureK);\n>  }\n>\n> -constexpr uint16_t Awb::threshold(float value)\n> -{\n> -\t/* AWB thresholds are in the range [0, 8191] */\n> -\treturn value * 8191;\n> -}\n> -\n> -constexpr uint16_t Awb::gainValue(double gain)\n> -{\n> -\t/*\n> -\t * The colour gains applied by the BNR for the four channels (Gr, R, B\n> -\t * and Gb) are expressed in the parameters structure as 16-bit integers\n> -\t * that store a fixed-point U3.13 value in the range [0, 8[.\n> -\t *\n> -\t * The real gain value is equal to the gain parameter plus one, i.e.\n> -\t *\n> -\t * Pout = Pin * (1 + gain / 8192)\n> -\t *\n> -\t * where 'Pin' is the input pixel value, 'Pout' the output pixel value,\n> -\t * and 'gain' the gain in the parameters structure as a 16-bit integer.\n> -\t */\n> -\treturn std::clamp((gain - 1.0) * 8192, 0.0, 65535.0);\n> -}\n> -\n> -/**\n> - * \\copydoc libcamera::ipa::Algorithm::prepare\n> - */\n> -void Awb::prepare(IPAContext &context,\n> -\t\t  [[maybe_unused]] const uint32_t frame,\n> -\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n> -\t\t  ipu3_uapi_params *params)\n> -{\n> -\t/*\n> -\t * Green saturation thresholds are reduced because we are using the\n> -\t * green channel only in the exposure computation.\n> -\t */\n> -\tparams->acc_param.awb.config.rgbs_thr_r = threshold(1.0);\n> -\tparams->acc_param.awb.config.rgbs_thr_gr = threshold(0.9);\n> -\tparams->acc_param.awb.config.rgbs_thr_gb = threshold(0.9);\n> -\tparams->acc_param.awb.config.rgbs_thr_b = threshold(1.0);\n> -\n> -\t/*\n> -\t * Enable saturation inclusion on thr_b for ImgU to update the\n> -\t * ipu3_uapi_awb_set_item->sat_ratio field.\n> -\t */\n> -\tparams->acc_param.awb.config.rgbs_thr_b |= IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT |\n> -\t\t\t\t\t\t   IPU3_UAPI_AWB_RGBS_THR_B_EN;\n> -\n> -\tconst ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;\n> -\n> -\tparams->acc_param.awb.config.grid = context.configuration.grid.bdsGrid;\n> -\n> -\t/*\n> -\t * Optical center is column start (respectively row start) of the\n> -\t * cell of interest minus its X center (respectively Y center).\n> -\t *\n> -\t * For the moment use BDS as a first approximation, but it should\n> -\t * be calculated based on Shading (SHD) parameters.\n> -\t */\n> -\tparams->acc_param.bnr = imguCssBnrDefaults;\n> -\tSize &bdsOutputSize = context.configuration.grid.bdsOutputSize;\n> -\tparams->acc_param.bnr.column_size = bdsOutputSize.width;\n> -\tparams->acc_param.bnr.opt_center.x_reset = grid.x_start - (bdsOutputSize.width / 2);\n> -\tparams->acc_param.bnr.opt_center.y_reset = grid.y_start - (bdsOutputSize.height / 2);\n> -\tparams->acc_param.bnr.opt_center_sqr.x_sqr_reset = params->acc_param.bnr.opt_center.x_reset\n> -\t\t\t\t\t\t\t* params->acc_param.bnr.opt_center.x_reset;\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> -\n> -\tparams->acc_param.bnr.wb_gains.gr = gainValue(context.activeState.awb.gains.green);\n> -\tparams->acc_param.bnr.wb_gains.r  = gainValue(context.activeState.awb.gains.red);\n> -\tparams->acc_param.bnr.wb_gains.b  = gainValue(context.activeState.awb.gains.blue);\n> -\tparams->acc_param.bnr.wb_gains.gb = gainValue(context.activeState.awb.gains.green);\n> -\n> -\tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n> -\n> -\t/* The CCM matrix may change when color temperature will be used */\n> -\tparams->acc_param.ccm = imguCssCcmDefault;\n> -\n> -\tparams->use.acc_awb = 1;\n> -\tparams->use.acc_bnr = 1;\n> -\tparams->use.acc_ccm = 1;\n> -}\n> -\n>  REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n>\n>  } /* namespace ipa::ipu3::algorithms */\n> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n> index 71f583840a5a..bc1c29a6dbcc 100644\n> --- a/src/ipa/libipa/algorithm.cpp\n> +++ b/src/ipa/libipa/algorithm.cpp\n> @@ -66,23 +66,6 @@ namespace ipa {\n>   * \\return 0 if successful, an error code otherwise\n>   */\n>\n> -/**\n> - * \\fn Algorithm::prepare()\n> - * \\brief Fill the \\a params buffer with ISP processing parameters for a frame\n> - * \\param[in] context The shared IPA context\n> - * \\param[in] frame The frame context sequence number\n> - * \\param[in] frameContext The FrameContext for this frame\n> - * \\param[out] params The ISP specific parameters\n> - *\n> - * This function is called for every frame when the camera is running before it\n> - * is processed by the ISP to prepare the ISP processing parameters for that\n> - * frame.\n> - *\n> - * Algorithms shall fill in the parameter structure fields appropriately to\n> - * configure the ISP processing blocks that they are responsible for. This\n> - * includes setting fields and flags that enable those processing blocks.\n> - */\n> -\n>  /**\n>   * \\fn Algorithm::queueRequest()\n>   * \\brief Provide control values to the algorithm\n> @@ -100,6 +83,23 @@ namespace ipa {\n>   * use during frame processing.\n>   */\n>\n> +/**\n> + * \\fn Algorithm::prepare()\n> + * \\brief Fill the \\a params buffer with ISP processing parameters for a frame\n> + * \\param[in] context The shared IPA context\n> + * \\param[in] frame The frame context sequence number\n> + * \\param[in] frameContext The FrameContext for this frame\n> + * \\param[out] params The ISP specific parameters\n> + *\n> + * This function is called for every frame when the camera is running before it\n> + * is processed by the ISP to prepare the ISP processing parameters for that\n> + * frame.\n> + *\n> + * Algorithms shall fill in the parameter structure fields appropriately to\n> + * configure the ISP processing blocks that they are responsible for. This\n> + * includes setting fields and flags that enable those processing blocks.\n> + */\n> +\n>  /**\n>   * \\fn Algorithm::process()\n>   * \\brief Process ISP statistics, and run algorithm operations\n> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h\n> index 38b3231c5d0e..987e3e4ce777 100644\n> --- a/src/ipa/libipa/algorithm.h\n> +++ b/src/ipa/libipa/algorithm.h\n> @@ -38,13 +38,6 @@ public:\n>  \t\treturn 0;\n>  \t}\n>\n> -\tvirtual void prepare([[maybe_unused]] typename Module::Context &context,\n> -\t\t\t     [[maybe_unused]] const uint32_t frame,\n> -\t\t\t     [[maybe_unused]] typename Module::FrameContext &frameContext,\n> -\t\t\t     [[maybe_unused]] typename Module::Params *params)\n> -\t{\n> -\t}\n> -\n>  \tvirtual void queueRequest([[maybe_unused]] typename Module::Context &context,\n>  \t\t\t\t  [[maybe_unused]] const uint32_t frame,\n>  \t\t\t\t  [[maybe_unused]] typename Module::FrameContext &frameContext,\n> @@ -52,6 +45,13 @@ public:\n>  \t{\n>  \t}\n>\n> +\tvirtual void prepare([[maybe_unused]] typename Module::Context &context,\n> +\t\t\t     [[maybe_unused]] const uint32_t frame,\n> +\t\t\t     [[maybe_unused]] typename Module::FrameContext &frameContext,\n> +\t\t\t     [[maybe_unused]] typename Module::Params *params)\n> +\t{\n> +\t}\n> +\n>  \tvirtual void process([[maybe_unused]] typename Module::Context &context,\n>  \t\t\t     [[maybe_unused]] const uint32_t frame,\n>  \t\t\t     [[maybe_unused]] typename Module::FrameContext &frameContext,\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index a06e9f732a8f..3fcbfa608467 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -107,6 +107,46 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>  \treturn 0;\n>  }\n>\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Agc::prepare(IPAContext &context, const uint32_t frame,\n> +\t\t  IPAFrameContext &frameContext, rkisp1_params_cfg *params)\n> +{\n> +\tframeContext.agc.exposure = context.activeState.agc.exposure;\n> +\tframeContext.agc.gain = context.activeState.agc.gain;\n> +\n> +\tif (frame > 0)\n> +\t\treturn;\n> +\n> +\t/* Configure the measurement window. */\n> +\tparams->meas.aec_config.meas_window = context.configuration.agc.measureWindow;\n> +\t/* Use a continuous method for measure. */\n> +\tparams->meas.aec_config.autostop = RKISP1_CIF_ISP_EXP_CTRL_AUTOSTOP_0;\n> +\t/* Estimate Y as (R + G + B) x (85/256). */\n> +\tparams->meas.aec_config.mode = RKISP1_CIF_ISP_EXP_MEASURING_MODE_1;\n> +\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AEC;\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> +\n> +\t/* Configure histogram. */\n> +\tparams->meas.hst_config.meas_window = context.configuration.agc.measureWindow;\n> +\t/* Produce the luminance histogram. */\n> +\tparams->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;\n> +\t/* Set an average weighted histogram. */\n> +\tfor (unsigned int histBin = 0; histBin < numHistBins_; histBin++)\n> +\t\tparams->meas.hst_config.hist_weight[histBin] = 1;\n> +\t/* Step size can't be less than 3. */\n> +\tparams->meas.hst_config.histogram_predivider = 4;\n> +\n> +\t/* Update the configuration for histogram. */\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;\n> +\t/* Enable the histogram measure unit. */\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_HST;\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_HST;\n> +}\n> +\n>  /**\n>   * \\brief Apply a filter on the exposure value to limit the speed of changes\n>   * \\param[in] exposureValue The target exposure from the AGC algorithm\n> @@ -348,46 +388,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \tmetadata.set(controls::FrameDuration, frameDuration.get<std::micro>());\n>  }\n>\n> -/**\n> - * \\copydoc libcamera::ipa::Algorithm::prepare\n> - */\n> -void Agc::prepare(IPAContext &context, const uint32_t frame,\n> -\t\t  IPAFrameContext &frameContext, rkisp1_params_cfg *params)\n> -{\n> -\tframeContext.agc.exposure = context.activeState.agc.exposure;\n> -\tframeContext.agc.gain = context.activeState.agc.gain;\n> -\n> -\tif (frame > 0)\n> -\t\treturn;\n> -\n> -\t/* Configure the measurement window. */\n> -\tparams->meas.aec_config.meas_window = context.configuration.agc.measureWindow;\n> -\t/* Use a continuous method for measure. */\n> -\tparams->meas.aec_config.autostop = RKISP1_CIF_ISP_EXP_CTRL_AUTOSTOP_0;\n> -\t/* Estimate Y as (R + G + B) x (85/256). */\n> -\tparams->meas.aec_config.mode = RKISP1_CIF_ISP_EXP_MEASURING_MODE_1;\n> -\n> -\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> -\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AEC;\n> -\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> -\n> -\t/* Configure histogram. */\n> -\tparams->meas.hst_config.meas_window = context.configuration.agc.measureWindow;\n> -\t/* Produce the luminance histogram. */\n> -\tparams->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;\n> -\t/* Set an average weighted histogram. */\n> -\tfor (unsigned int histBin = 0; histBin < numHistBins_; histBin++)\n> -\t\tparams->meas.hst_config.hist_weight[histBin] = 1;\n> -\t/* Step size can't be less than 3. */\n> -\tparams->meas.hst_config.histogram_predivider = 4;\n> -\n> -\t/* Update the configuration for histogram. */\n> -\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;\n> -\t/* Enable the histogram measure unit. */\n> -\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_HST;\n> -\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_HST;\n> -}\n> -\n>  REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n>\n>  } /* namespace ipa::rkisp1::algorithms */\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 083c2d982c52..744f4a386c1a 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -67,20 +67,41 @@ int Awb::configure(IPAContext &context,\n>  \treturn 0;\n>  }\n>\n> -uint32_t Awb::estimateCCT(double red, double green, double blue)\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void Awb::queueRequest(IPAContext &context,\n> +\t\t       [[maybe_unused]] const uint32_t frame,\n> +\t\t       IPAFrameContext &frameContext,\n> +\t\t       const ControlList &controls)\n>  {\n> -\t/* Convert the RGB values to CIE tristimulus values (XYZ) */\n> -\tdouble X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);\n> -\tdouble Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);\n> -\tdouble Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);\n> -\n> -\t/* Calculate the normalized chromaticity values */\n> -\tdouble x = X / (X + Y + Z);\n> -\tdouble y = Y / (X + Y + Z);\n> -\n> -\t/* Calculate CCT */\n> -\tdouble n = (x - 0.3320) / (0.1858 - y);\n> -\treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n> +\tauto &awb = context.activeState.awb;\n> +\n> +\tconst auto &awbEnable = controls.get(controls::AwbEnable);\n> +\tif (awbEnable && *awbEnable != awb.autoEnabled) {\n> +\t\tawb.autoEnabled = *awbEnable;\n> +\n> +\t\tLOG(RkISP1Awb, Debug)\n> +\t\t\t<< (*awbEnable ? \"Enabling\" : \"Disabling\") << \" AWB\";\n> +\t}\n> +\n> +\tconst auto &colourGains = controls.get(controls::ColourGains);\n> +\tif (colourGains && !awb.autoEnabled) {\n> +\t\tawb.gains.manual.red = (*colourGains)[0];\n> +\t\tawb.gains.manual.blue = (*colourGains)[1];\n> +\n> +\t\tLOG(RkISP1Awb, Debug)\n> +\t\t\t<< \"Set colour gains to red: \" << awb.gains.manual.red\n> +\t\t\t<< \", blue: \" << awb.gains.manual.blue;\n> +\t}\n> +\n> +\tframeContext.awb.autoEnabled = awb.autoEnabled;\n> +\n> +\tif (!awb.autoEnabled) {\n> +\t\tframeContext.awb.gains.red = awb.gains.manual.red;\n> +\t\tframeContext.awb.gains.green = 1.0;\n> +\t\tframeContext.awb.gains.blue = awb.gains.manual.blue;\n> +\t}\n>  }\n>\n>  /**\n> @@ -165,41 +186,20 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n>  \tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AWB;\n>  }\n>\n> -/**\n> - * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> - */\n> -void Awb::queueRequest(IPAContext &context,\n> -\t\t       [[maybe_unused]] const uint32_t frame,\n> -\t\t       IPAFrameContext &frameContext,\n> -\t\t       const ControlList &controls)\n> +uint32_t Awb::estimateCCT(double red, double green, double blue)\n>  {\n> -\tauto &awb = context.activeState.awb;\n> -\n> -\tconst auto &awbEnable = controls.get(controls::AwbEnable);\n> -\tif (awbEnable && *awbEnable != awb.autoEnabled) {\n> -\t\tawb.autoEnabled = *awbEnable;\n> -\n> -\t\tLOG(RkISP1Awb, Debug)\n> -\t\t\t<< (*awbEnable ? \"Enabling\" : \"Disabling\") << \" AWB\";\n> -\t}\n> -\n> -\tconst auto &colourGains = controls.get(controls::ColourGains);\n> -\tif (colourGains && !awb.autoEnabled) {\n> -\t\tawb.gains.manual.red = (*colourGains)[0];\n> -\t\tawb.gains.manual.blue = (*colourGains)[1];\n> -\n> -\t\tLOG(RkISP1Awb, Debug)\n> -\t\t\t<< \"Set colour gains to red: \" << awb.gains.manual.red\n> -\t\t\t<< \", blue: \" << awb.gains.manual.blue;\n> -\t}\n> -\n> -\tframeContext.awb.autoEnabled = awb.autoEnabled;\n> -\n> -\tif (!awb.autoEnabled) {\n> -\t\tframeContext.awb.gains.red = awb.gains.manual.red;\n> -\t\tframeContext.awb.gains.green = 1.0;\n> -\t\tframeContext.awb.gains.blue = awb.gains.manual.blue;\n> -\t}\n> +\t/* Convert the RGB values to CIE tristimulus values (XYZ) */\n> +\tdouble X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);\n> +\tdouble Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);\n> +\tdouble Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);\n> +\n> +\t/* Calculate the normalized chromaticity values */\n> +\tdouble x = X / (X + Y + Z);\n> +\tdouble y = Y / (X + Y + Z);\n> +\n> +\t/* Calculate CCT */\n> +\tdouble n = (x - 0.3320) / (0.1858 - y);\n> +\treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n>  }\n>\n>  /**\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index f5633b1171a0..9d45a442339c 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -20,12 +20,12 @@ public:\n>  \t~Awb() = default;\n>\n>  \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> -\tvoid prepare(IPAContext &context, const uint32_t frame,\n> -\t\t     IPAFrameContext &frameContext,\n> -\t\t     rkisp1_params_cfg *params) override;\n>  \tvoid queueRequest(IPAContext &context, const uint32_t frame,\n>  \t\t\t  IPAFrameContext &frameContext,\n>  \t\t\t  const ControlList &controls) override;\n> +\tvoid prepare(IPAContext &context, const uint32_t frame,\n> +\t\t     IPAFrameContext &frameContext,\n> +\t\t     rkisp1_params_cfg *params) override;\n>  \tvoid process(IPAContext &context, const uint32_t frame,\n>  \t\t     IPAFrameContext &frameContext,\n>  \t\t     const rkisp1_stat_buffer *stats,\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 62AD1BD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 26 Oct 2022 13:40:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA9B062F53;\n\tWed, 26 Oct 2022 15:40:55 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA17561F4B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 26 Oct 2022 15:40:53 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id E6DD824000A;\n\tWed, 26 Oct 2022 13:40:52 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666791655;\n\tbh=Sv5L1O2Q/EbBuZHXzQqIf3tnAK2naeJGhn1KbLPWGpY=;\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=C0a/P5TPqJlFn/5wr2ZODLtbNiOUB97C1y7wxWxk5SeW5h05mBbwoa+nRdAqS7Dlm\n\tnZkdKzVxGGO7LXs3wDeEAElRyTglIuQdNuNLYafNLmQTLgC0AIwyG2Bid5BQcHbYei\n\tWc9AhZtGjEXxlhVpu36WhIYUsdvONr8yBlbaNYITN/MXYgTPru3qsEUSBLKuB5GmWd\n\t/wY+HYcBdmhEbJf1kHNVimuY94SyGr6SsdGQhlWe6Om9FlZirGfxfRdxp/5YNhkC17\n\ta43Td5DYU6JO9ixYgTlp6wqhGtLPmxM0VyL9uLz3MDBSzFL3X+foZZYDp+1CzCW1XZ\n\tbAs7YPdVnFh8Q==","Date":"Wed, 26 Oct 2022 15:40:50 +0200","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20221026134050.esbzfwqjlxenoytg@uno.localdomain>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221024000356.29521-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 01/13] ipa: Sort algorithm\n\toperations based on calling order","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>"}}]