[{"id":22160,"web_url":"https://patchwork.libcamera.org/comment/22160/","msgid":"<164448777681.3354066.18018320193243523810@Monstersaurus>","date":"2022-02-10T10:09:36","subject":"Re: [libcamera-devel] [PATCH v7] ipa: ipu3: af: Auto focus for\n\tdw9719 Surface Go2 VCM","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Kate,\n\nA few minor style comments in here, but seeing this is at v7 those could\npossibly be fixed while applying anyway.\n\nBut I'll leave the algorithm review itself to JM.\n\nQuoting Kate Hsuan (2022-02-10 08:57:59)\n> Since VCM for surface Go 2 (dw9719) had been successfully\n> driven, this Af module can be used to control the VCM and\n> determine the focus value based on the IPU3 AF statistics.\n> \n> Based on the values from the IPU3 AF buffer, the variance\n> of each focus step is determined and a greedy approach is\n> used to find the maximum variance of the AF state and an\n> appropriate focus value.\n> \n> The grid configuration is implemented as a context. Also,\n> the grid parameter- AF_MIN_BLOCK_WIDTH is set to 4 (default\n> is 3) since if the default value is used, x_start\n> (x_start > 640) will be at an incorrect location of the\n> image (rightmost of the sensor).\n> \n> Signed-off-by: Kate Hsuan \n> Tested-by: Jean-Michel Hautbois \n\nI can't see which values are to be passed to the pipeline handler / lens\nyet, but I think that's expected as that plumbing isn't merged. But I\nthink it's good to progress this, and it could even be merged before the\nplumbing.\n\nOverall though, I like this, as I can see how it's acting to sweep\nthrough and when it makes a decision to go back if it passes a peak.\n\n> ---\n> Changes in v7:\n> 1. Improved comments and fixed typo.\n> 2. Simplified AF algorithm. \"ignore frame\" had been moved into a\n> function and remove a unnecessary \"if\" expression.\n> 3. afRawBufferLen_ keeped to be a local variable not a class member.\n> 4. Initial configurations are moved to configure().\n> ---\n> src/ipa/ipu3/algorithms/af.cpp | 426 ++++++++++++++++++++++++++++\n> src/ipa/ipu3/algorithms/af.h | 78 +++++\n> src/ipa/ipu3/algorithms/meson.build | 3 +-\n> src/ipa/ipu3/ipa_context.cpp | 24 ++\n> src/ipa/ipu3/ipa_context.h | 10 +\n> src/ipa/ipu3/ipu3.cpp | 2 +\n> 6 files changed, 542 insertions(+), 1 deletion(-)\n> create mode 100644 src/ipa/ipu3/algorithms/af.cpp\n> create mode 100644 src/ipa/ipu3/algorithms/af.h\n> \n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> new file mode 100644\n> index 00000000..b26aa018\n> --- /dev/null\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -0,0 +1,426 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Red Hat\n> + *\n> + * af.cpp - IPU3 auto focus algorithm\n> + */\n> +\n> +#include \"af.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libipa/histogram.h\"\n> +\n> +/**\n> + * \\file af.h\n> + */\n> +\n> +/**\n> + * \\var kAfMinGridWidth\n> + * \\brief the minimum width of AF grid.\n> + * The minimum grid horizontal dimensions, in number of grid blocks(cells).\n> +*/\n> +\n> +/**\n> + * \\var kAfMinGridHeight\n> + * \\brief the minimum height of AF grid.\n> + * The minimum grid vertical dimensions, in number of grid blocks(cells).\n> +*/\n> +\n> +/**\n> + * \\var kAfMaxGridWidth\n> + * \\brief the maximum width of AF grid.\n> + * The maximum grid horizontal dimensions, in number of grid blocks(cells).\n> +*/\n> +\n> +/**\n> + * \\var kAfMaxGridHeight\n> + * \\brief the maximum height of AF grid.\n> + * The maximum grid vertical dimensions, in number of grid blocks(cells).\n> +*/\n> +\n> +/**\n> + * \\var kAfMinGridBlockWidth\n> + * \\brief the minimum block size of the width.\n> + */\n> +\n> +/**\n> + * \\var kAfMinGridBlockHeight\n> + * \\brief the minimum block size of the height.\n> + */\n> +\n> +/**\n> + * \\def kAfMaxGridBlockWidth\n> + * \\brief the maximum block size of the width.\n> + */\n> +\n> +/**\n> + * \\var kAfMaxGridBlockHeight\n> + * \\brief the maximum block size of the height.\n> + */\n> +\n> +/**\n> + * \\var kAfDefaultHeightPerSlice\n> + * \\brief The default number of blocks in vertical axis per slice.\n> + */\n> +\n> +namespace libcamera {\n> +\n> +using namespace std::literals::chrono_literals;\n> +\n> +namespace ipa::ipu3::algorithms {\n> +\n> +/**\n> + * \\class Af\n> + * \\brief An auto-focus algorithm based on IPU3 statistics\n> + *\n> + * This algorithm is used to determine the position of the lens to make a\n> + * focused image. The IPU3 AF processing block computes the statistics that\n> + * are composed by two types of filtered value and stores in a AF buffer.\n> + * Typically, for a clear image, it has a relatively higher contrast than a\n> + * blurred one. Therefore, if an image with the highest contrast can be\n> + * found through the scan, the position of the len indicates to a clearest\n> + * image.\n> + *\n\nNo need for the blank line (well, it has a star but that can be removed)\n\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(IPU3Af)\n> +\n> +/**\n> + * Maximum focus steps of the VCM control\n> + * \\todo should be obtained from the VCM driver\n> + */\n> +static constexpr uint32_t kMaxFocusSteps = 1023;\n> +\n> +/* minimum focus step for searching appropriate focus */\n> +static constexpr uint32_t kCoarseSearchStep = 30;\n> +static constexpr uint32_t kFineSearchStep = 1;\n> +\n> +/* max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n> +static constexpr double kMaxChange = 0.5;\n> +\n> +/* the numbers of frame to be ignored, before performing focus scan. */\n> +static constexpr uint32_t kIgnoreFrame = 10;\n> +\n> +/* fine scan range 0 < kFineRange < 1 */\n> +static constexpr double kFineRange = 0.05;\n> +\n> +/* settings for IPU3 AF filter */\n> +static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {\n> + .y1_coeff_0 = { 0, 1, 3, 7 },\n> + .y1_coeff_1 = { 11, 13, 1, 2 },\n> + .y1_coeff_2 = { 8, 19, 34, 242 },\n> + .y1_sign_vec = 0x7fdffbfe,\n> + .y2_coeff_0 = { 0, 1, 6, 6 },\n> + .y2_coeff_1 = { 13, 25, 3, 0 },\n> + .y2_coeff_2 = { 25, 3, 177, 254 },\n> + .y2_sign_vec = 0x4e53ca72,\n> + .y_calc = { 8, 8, 8, 8 },\n> + .nf = { 0, 9, 0, 9, 0 },\n> +};\n\nAre these existing values from the kernel, or is there anything we can\ndo to reference what they represent or any existing documentation for\nhow to configure them?\n\n> +\n> +Af::Af()\n> + : focus_(0), goodFocus_(0), currentVariance_(0.0), previousVariance_(0.0),\n> + coarseCompleted_(false), fineCompleted_(false)\n> +{\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)\n> +{\n> + const struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> + params->acc_param.af.grid_cfg = grid;\n> + params->acc_param.af.filter_config = afFilterConfigDefault;\n> +\n> + /* enable AF processing block */\n> + params->use.acc_af = 1;\n> +}\n> +\n> +/**\n> + * \\brief Configure the Af given a configInfo\n> + *\n> + * \\param[in] context The shared IPA context\n> + * \\param[in] configInfo The IPA configuration data\n> + * \\return 0\n> + */\n> +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> +{\n> + struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> + grid.width = kAfMinGridWidth;\n> + grid.height = kAfMinGridHeight;\n> + grid.block_width_log2 = kAfMinGridBlockWidth;\n> + grid.block_height_log2 = kAfMinGridBlockHeight;\n> + grid.height_per_slice = kAfDefaultHeightPerSlice;\n> +\n> + /* x_start and y start are default to BDS center */\n> + grid.x_start = (configInfo.bdsOutputSize.width / 2) -\n> + (((grid.width << grid.block_width_log2) / 2));\n> + grid.y_start = (configInfo.bdsOutputSize.height / 2) -\n> + (((grid.height << grid.block_height_log2) / 2));\n> +\n> + /* x_start and y_start should be even */\n> + grid.x_start = (grid.x_start / 2) * 2;\n> + grid.y_start = (grid.y_start / 2) * 2;\n> + grid.y_start = grid.y_start | IPU3_UAPI_GRID_Y_START_EN;\n> +\n> + /* inital max focus step */\n> + maxStep_ = kMaxFocusSteps;\n> +\n> + /* determined focus value i.e. current focus value */\n> + context.frameContext.af.focus = 0;\n> + /* maximum variance of the AF statistics */\n> + context.frameContext.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> +\n> + return 0;\n> +}\n> +\n> +/**\n> + * \\brief AF coarse scan\n> + *\n> + * Find a near focused image using a coarse step. The step is determined by coarseSearchStep.\n> + *\n> + * \\param[in] context The shared IPA context\n> + *\n\nWe can remove this extra line too. Looks like that's throughout.\n\n> + */\n> +void Af::afCoarseScan(IPAContext &context)\n> +{\n> + if (coarseCompleted_ == true)\n> + return;\n> +\n> + if (afScan(context, kCoarseSearchStep)) {\n> + coarseCompleted_ = true;\n> + context.frameContext.af.maxVariance = 0;\n> + focus_ = context.frameContext.af.focus - (context.frameContext.af.focus * kFineRange);\n> + context.frameContext.af.focus = focus_;\n> + previousVariance_ = 0;\n> + maxStep_ = std::clamp(static_cast(focus_ + (focus_ * kFineRange)), 0U, kMaxFocusSteps);\n> + }\n> +}\n> +\n> +/**\n> + * \\brief AF fine scan\n> + *\n> + * Find an optimum lens position with moving 1 step for each search.\n> + *\n> + * \\param[in] context The shared IPA context\n> + *\n> + */\n> +void Af::afFineScan(IPAContext &context)\n> +{\n> + if (coarseCompleted_ != true)\n> + return;\n> +\n> + if (afScan(context, kFineSearchStep)) {\n> + context.frameContext.af.stable = true;\n> + fineCompleted_ = true;\n> + }\n> +}\n> +\n> +/**\n> + * \\brief AF reset\n> + *\n> + * Reset all the parameters to start over the AF process.\n> + *\n> + * \\param[in] context The shared IPA context\n> + *\n> + */\n> +void Af::afReset(IPAContext &context)\n> +{\n> + context.frameContext.af.maxVariance = 0;\n> + context.frameContext.af.focus = 0;\n> + focus_ = 0;\n> + context.frameContext.af.stable = false;\n> + ignoreCounter_ = kIgnoreFrame;\n> + previousVariance_ = 0.0;\n> + coarseCompleted_ = false;\n> + fineCompleted_ = false;\n> + maxStep_ = kMaxFocusSteps;\n> +}\n> +\n> +/**\n> + * \\brief AF variance comparison.\n> + *\n> + * This fuction compares the previous and current variance. It always picks\n\ns/fuction/function/ but I don't think we need to say 'this function'\nwhen describing a function, but it doesn't hurt.\n\n> + * the largest variance to replace the previous one. The image with a larger\n> + * variance also indicates it is a clearer image than previous one. If it\n> + * finds the negative sign of derivative, it returns immediately.\n> + *\n> + * \\param[in] context The shared IPA context\n> + * \\return True, if it finds a AF value.\n> + */\n> +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 = goodFocus_;\n\nI'd be tempted to call goodFocus_ bestFocus_ as it's the 'best' focus\npoint we've seen? But that's not a requirement. 'good' can also indicate\nit was the known 'good' focus that we have stored.\n\n\n> + return true;\n> + } else {\n> + /*\n> + * find the maximum of the variance by estimating its\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> + /*\n> + * positive and zero derivative:\n> + * The variance is still increasing. The focus could be\n> + * increased for the next comparison. Also, the max variance\n> + * and previous focus value are updated.\n> + */\n> + goodFocus_ = focus_;\n> + focus_ += min_step;\n> + context.frameContext.af.focus = focus_;\n> + context.frameContext.af.maxVariance = currentVariance_;\n> + } else {\n> + /*\n> + * negative derivative:\n> + * The variance starts to decrease which means the maximum\n> + * variance is found. Set focus step to previous good one\n> + * then returnimmediately.\n\ns/returnimmediately./return immediately./\n\n> + */\n> + context.frameContext.af.focus = goodFocus_;\n> + return true;\n> + }\n> + }\n> +\n> + previousVariance_ = currentVariance_;\n> + LOG(IPU3Af, Debug) << \" Previous step is \"\n> + << goodFocus_\n> + << \" Current step is \"\n> + << focus_;\n> + return false;\n> +}\n> +\n> +/**\n> + * \\brief Determine the frame to be ignored.\n> + *\n> + * \\return Return true the frame is ignored.\n> + * \\return Return false the frame should be processed.\n> + */\n> +\n\nThis blank line can be removed so the comment hugs the function.\n\n> +bool Af::afNeedIgnoreFrame()\n> +{\n> + if (ignoreCounter_ == 0)\n> + return false;\n> + else\n> + ignoreCounter_--;\n> + return true;\n> +}\n> +\n> +/**\n> + * @brief Reset frame ignore counter.\n\ns/@brief/\\brief/ to match doxygen style.\n\n> + *\n\nAnd no extra line.\n\n> + */\n> +void Af::afIgnoreFrameReset()\n> +{\n> + ignoreCounter_ = kIgnoreFrame;\n> +}\n> +\n> +/**\n> + * \\brief Determine the max contrast image and lens position.\n> + *\n> + * y_table is the AF statistic of IPU3 and is composed of two kinds of filtered\n> + * values. Based on this, the variance of a image could be used to determine\n\nDo we know what the values represent? Could we say perhaps:\n\n\"\"\"\ny_table is the AF statistic of IPU3 and is composed of two kinds of\nfiltered values representing the contrast levels of the image.\n\"\"\"\n\nHowever, don't take that as verbatim, I'm guessing based on context ;-)\n\n\n> + * the clearness of the given image. Ideally, a clear image also has a\n> + * raletively higher contrast. So, the VCM is moved step by step and variance\n\ns/raletively/relatively/\n\n> + * of each frame are calculated to find a maximum variance which corresponds\n> + * with a specific focus step. If it is found, that is the best focus step of\n> + * current scene.\n> + *\n> + * \\param[in] context The shared IPA context.\n> + * \\param[in] stats The statistic buffer of 3A of IPU3.\n> + */\n> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n> +{\n> + uint32_t total = 0;\n> + double mean;\n> + uint64_t var_sum = 0;\n> + y_table_item_t y_item[IPU3_UAPI_AF_Y_TABLE_MAX_SIZE / sizeof(y_table_item_t)];\n> + uint32_t z = 0;\n> + uint32_t afRawBufferLen_;\n> +\n> + /* evaluate the AF buffer length */\n> + afRawBufferLen_ = context.configuration.af.afGrid.width *\n> + context.configuration.af.afGrid.height;\n> +\n> + memcpy(y_item, stats->af_raw_buffer.y_table, afRawBufferLen_ * sizeof(y_table_item_t));\n> +\n> + /*\n> + * calculate the mean and the variance of AF statistics for a given grid.\n> + *\n> + * For coarse: y1 are used.\n> + * For fine: y2 results are used.\n> + */\n> + if (coarseCompleted_) {\n> + for (z = 0; z < afRawBufferLen_; z++) {\n> + total = total + y_item[z].y2_avg;\n> + }\n> + mean = total / afRawBufferLen_;\n> +\n> + for (z = 0; z < afRawBufferLen_; z++) {\n> + var_sum = var_sum + ((y_item[z].y2_avg - mean) *\n> + (y_item[z].y2_avg - mean));\n> + }\n> + } else {\n> + for (z = 0; z < afRawBufferLen_; z++) {\n> + total = total + y_item[z].y1_avg;\n> + }\n> + mean = total / afRawBufferLen_;\n> +\n> + for (z = 0; z < afRawBufferLen_; z++) {\n> + var_sum = var_sum + ((y_item[z].y1_avg - mean) *\n> + (y_item[z].y1_avg - mean));\n> + }\n> + }\n> +\n> + /* determine the average variance of the AF statistic. */\n> + currentVariance_ = static_cast(var_sum) / static_cast(afRawBufferLen_);\n> +\n> + if (context.frameContext.af.stable == true) {\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> + LOG(IPU3Af, Debug) << \"Variance change rate: \"\n> + << var_ratio\n> + << \" Current VCM step: \"\n> + << context.frameContext.af.focus;\n> +\n> + /*\n> + * If the change rate is greater than kMaxChange (out of focus),\n> + * trigger AF again.\n> + */\n> + if (var_ratio > kMaxChange) {\n> + if (!afNeedIgnoreFrame())\n> + afReset(context);\n> + } else\n> + afIgnoreFrameReset();\n> + } else {\n> + if (!afNeedIgnoreFrame()) {\n> + afCoarseScan(context);\n> + afFineScan(context);\n> + }\n> + }\n> +}\n> +\n> +} /* namespace ipa::ipu3::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h\n> new file mode 100644\n> index 00000000..bc5d2064\n> --- /dev/null\n> +++ b/src/ipa/ipu3/algorithms/af.h\n> @@ -0,0 +1,78 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Red Hat\n> + *\n> + * af.h - IPU3 Af algorithm\n> + */\n> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AF_H__\n> +#define __LIBCAMERA_IPU3_ALGORITHMS_AF_H__\n\nWe can use #pragma once here now. (The whole code base was converted).\n\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"algorithm.h\"\n> +\n> +/* static variables from repo of chromium */\n> +static constexpr uint8_t kAfMinGridWidth = 16;\n> +static constexpr uint8_t kAfMinGridHeight = 16;\n> +static constexpr uint8_t kAfMaxGridWidth = 32;\n> +static constexpr uint8_t kAfMaxGridHeight = 24;\n> +static constexpr uint16_t kAfMinGridBlockWidth = 4;\n> +static constexpr uint16_t kAfMinGridBlockHeight = 3;\n> +static constexpr uint16_t kAfMaxGridBlockWidth = 6;\n> +static constexpr uint16_t kAfMaxGridBlockHeight = 6;\n> +static constexpr uint16_t kAfDefaultHeightPerSlice = 2;\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::ipu3::algorithms {\n> +\n> +class Af : public Algorithm\n> +{\n> + /* The format of y_table. From ipu3-ipa repo */\n> + typedef struct __attribute__((packed)) y_table_item {\n> + uint16_t y1_avg;\n> + uint16_t y2_avg;\n> + } y_table_item_t;\n> +public:\n> + Af();\n> + ~Af() = default;\n> +\n> + void prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;\n> +\n> +private:\n> + void afCoarseScan(IPAContext &context);\n> + void afFineScan(IPAContext &context);\n> + bool afScan(IPAContext &context, int min_step);\n> + void afReset(IPAContext &context);\n> + bool afNeedIgnoreFrame();\n> + void afIgnoreFrameReset();\n> +\n> + /* Used for focus scan. */\n\nIf we're describing it, what is it? Is it the current focus position\nwhen scanning?\n\n> + uint32_t focus_;\n> + /* Focus good */\n\nFocus Good doesn't really describe a variable called good Focus. What\ngets put in it? Is it the last known good focus value?\n\n> + uint32_t goodFocus_;\n> + /* Recent AF statistic variance. */\n> + double currentVariance_;\n> + /* The frames to be ignore before starting measuring. */\n> + uint32_t ignoreCounter_;\n> + /* previous variance. it is used to determine the gradient */\n> + double previousVariance_;\n> + /* Max scan steps of each pass of AF scaning */\n> + uint32_t maxStep_;\n> + /* coarse scan stable. Complete low pass search (coarse) scan) */\n> + bool coarseCompleted_;\n> + /* fine scan stable. Complete high pass scan (fine scan) */\n> + bool fineCompleted_;\n> +};\n> +\n> +} /* namespace ipa::ipu3::algorithms */\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */\n> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n> index 4db6ae1d..e1099169 100644\n> --- a/src/ipa/ipu3/algorithms/meson.build\n> +++ b/src/ipa/ipu3/algorithms/meson.build\n> @@ -1,8 +1,9 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> ipu3_ipa_algorithms = files([\n> + 'af.cpp',\n> 'agc.cpp',\n> 'awb.cpp',\n> 'blc.cpp',\n> - 'tone_mapping.cpp',\n> + 'tone_mapping.cpp'\n\nWe should keep the ',' at the end of tone_mapping.cpp so that we don't have to\nmodify extra lines when appending to the list.\n\n> ])\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 86794ac1..ecb2b90a 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -67,6 +67,30 @@ namespace libcamera::ipa::ipu3 {\n> *\n> * \\var IPASessionConfiguration::grid.stride\n> * \\brief Number of cells on one line including the ImgU padding\n> + *\n\nNo need for extra blank line\n\n> + */\n> +\n> +/**\n> + * \\var IPASessionConfiguration::af\n> + * \\brief AF grid configuration of the IPA\n> + *\n> + * \\var IPASessionConfiguration::af.afGrid\n> + *\n\nNo need for extra blank line\n\n> + */\n> +\n> +/**\n> + * \\var IPAFrameContext::af\n> + * \\brief Context for the Automatic Focus algorithm\n> + *\n> + * \\struct IPAFrameContext::af\n> + * \\var IPAFrameContext::af.focus\n> + * \\brief Current position of the lens\n> + *\n> + * \\var IPAFrameContext::af.maxVariance\n> + * \\brief The maximum variance of the current image.\n> + *\n> + * \\var IPAFrameContext::af.stable\n> + * \\brief is the image focused?\n> */\n> \n> /**\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index c6dc0814..60ad3194 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -25,6 +25,10 @@ struct IPASessionConfiguration {\n> uint32_t stride;\n> } grid;\n> \n> + struct {\n> + ipu3_uapi_grid_config afGrid;\n> + } af;\n> +\n> struct {\n> utils::Duration minShutterSpeed;\n> utils::Duration maxShutterSpeed;\n> @@ -34,6 +38,12 @@ struct IPASessionConfiguration {\n> };\n> \n> struct IPAFrameContext {\n> + struct {\n> + uint32_t focus;\n\nIs this the focus point/value of the current frame, or the desired value to\nset on the next frame?\n\n> + double maxVariance;\n> + bool stable;\n> + } af;\n> +\n> struct {\n> uint32_t exposure;\n> double gain;\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index e44a31bb..417e0562 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -30,6 +30,7 @@\n> \n> #include \"libcamera/internal/mapped_framebuffer.h\"\n> \n> +#include \"algorithms/af.h\"\n> #include \"algorithms/agc.h\"\n> #include \"algorithms/algorithm.h\"\n> #include \"algorithms/awb.h\"\n> @@ -295,6 +296,7 @@ int IPAIPU3::init(const IPASettings &settings,\n> }\n> \n> /* Construct our Algorithms */\n> + algorithms_.push_back(std::make_unique());\n> algorithms_.push_back(std::make_unique());\n> algorithms_.push_back(std::make_unique());\n> algorithms_.push_back(std::make_unique());\n> -- \n> 2.33.1\n>","headers":{"Return-Path":"","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 BD425BDCBF\n\tfor ;\n\tThu, 10 Feb 2022 10:09:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36234610AD;\n\tThu, 10 Feb 2022 11:09:42 +0100 (CET)","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 00F6C61098\n\tfor ;\n\tThu, 10 Feb 2022 11:09:39 +0100 (CET)","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 7F58293;\n\tThu, 10 Feb 2022 11:09:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"n/rQ5VZv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1644487779;\n\tbh=IDI2Dok3U3s0rGfV9suy5tXkIm9AP++9kXoGH3tLlHQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=n/rQ5VZvUcT4tnhgKH3uTmSx6Fkl7oiAq0f9kDK9xU9OEBnx4PEWFR5vdhXAJmCqQ\n\tucObanqFOL/0PNwmZ9Fkpozl35D1EIiFexhePcsaIXwionELZMBt86mN0reS3ZQiiu\n\tnUg7LLDs/0E5uu10nRz8STTkgNqmIl5NCx0+ELGQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220210085759.71419-1-hpa@redhat.com>","References":"<20220210085759.71419-1-hpa@redhat.com>","From":"Kieran Bingham ","To":"Kate Hsuan ,\n\tlibcamera devel ","Date":"Thu, 10 Feb 2022 10:09:36 +0000","Message-ID":"<164448777681.3354066.18018320193243523810@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v7] ipa: ipu3: af: Auto focus for\n\tdw9719 Surface Go2 VCM","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22165,"web_url":"https://patchwork.libcamera.org/comment/22165/","msgid":"<53cd384f-6af6-c550-436e-4358154a73ac@ideasonboard.com>","date":"2022-02-10T15:41:40","subject":"Re: [libcamera-devel] [PATCH v7] ipa: ipu3: af: Auto focus for\n\tdw9719 Surface Go2 VCM","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Kate,\n\nThanks for the patch !\n\nOn 10/02/2022 11:09, Kieran Bingham wrote:\n> Hi Kate,\n> \n> A few minor style comments in here, but seeing this is at v7 those could\n> possibly be fixed while applying anyway.\n> \n> But I'll leave the algorithm review itself to JM.\n\nSure. Thanks (?) ;-).\n\n> \n> Quoting Kate Hsuan (2022-02-10 08:57:59)\n>> Since VCM for surface Go 2 (dw9719) had been successfully\n>> driven, this Af module can be used to control the VCM and\n>> determine the focus value based on the IPU3 AF statistics.\n>>\n>> Based on the values from the IPU3 AF buffer, the variance\n>> of each focus step is determined and a greedy approach is\n>> used to find the maximum variance of the AF state and an\n>> appropriate focus value.\n>>\n>> The grid configuration is implemented as a context. Also,\n>> the grid parameter- AF_MIN_BLOCK_WIDTH is set to 4 (default\n>> is 3) since if the default value is used, x_start\n>> (x_start > 640) will be at an incorrect location of the\n>> image (rightmost of the sensor).\n>>\n>> Signed-off-by: Kate Hsuan \n>> Tested-by: Jean-Michel Hautbois \n> \n> I can't see which values are to be passed to the pipeline handler / lens\n> yet, but I think that's expected as that plumbing isn't merged. But I\n> think it's good to progress this, and it could even be merged before the\n> plumbing.\n\nIt could indeed be interesting to at least refer to the ancillary links \npatch series from Dan [1] and [2]. Those are needed for this algorithm \nto have an effect :-).\n\n[1]: https://lore.kernel.org/all/20220130235821.48076-1-djrscally@gmail.com/\n[2]: https://patchwork.libcamera.org/project/libcamera/list/?series=2906\n\n> \n> Overall though, I like this, as I can see how it's acting to sweep\n> through and when it makes a decision to go back if it passes a peak.\n> \n>> ---\n>> Changes in v7:\n>> 1. Improved comments and fixed typo.\n>> 2. Simplified AF algorithm. \"ignore frame\" had been moved into a\n>> function and remove a unnecessary \"if\" expression.\n>> 3. afRawBufferLen_ keeped to be a local variable not a class member.\n>> 4. Initial configurations are moved to configure().\n>> ---\n>> src/ipa/ipu3/algorithms/af.cpp | 426 ++++++++++++++++++++++++++++\n>> src/ipa/ipu3/algorithms/af.h | 78 +++++\n>> src/ipa/ipu3/algorithms/meson.build | 3 +-\n>> src/ipa/ipu3/ipa_context.cpp | 24 ++\n>> src/ipa/ipu3/ipa_context.h | 10 +\n>> src/ipa/ipu3/ipu3.cpp | 2 +\n>> 6 files changed, 542 insertions(+), 1 deletion(-)\n>> create mode 100644 src/ipa/ipu3/algorithms/af.cpp\n>> create mode 100644 src/ipa/ipu3/algorithms/af.h\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n>> new file mode 100644\n>> index 00000000..b26aa018\n>> --- /dev/null\n>> +++ b/src/ipa/ipu3/algorithms/af.cpp\n>> @@ -0,0 +1,426 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Red Hat\n>> + *\n>> + * af.cpp - IPU3 auto focus algorithm\n>> + */\n>> +\n>> +#include \"af.h\"\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"libipa/histogram.h\"\n>> +\n>> +/**\n>> + * \\file af.h\n>> + */\n>> +\n>> +/**\n>> + * \\var kAfMinGridWidth\n>> + * \\brief the minimum width of AF grid.\n>> + * The minimum grid horizontal dimensions, in number of grid blocks(cells).\n>> +*/\n>> +\n>> +/**\n>> + * \\var kAfMinGridHeight\n>> + * \\brief the minimum height of AF grid.\n>> + * The minimum grid vertical dimensions, in number of grid blocks(cells).\n>> +*/\n>> +\n>> +/**\n>> + * \\var kAfMaxGridWidth\n>> + * \\brief the maximum width of AF grid.\n>> + * The maximum grid horizontal dimensions, in number of grid blocks(cells).\n>> +*/\n>> +\n>> +/**\n>> + * \\var kAfMaxGridHeight\n>> + * \\brief the maximum height of AF grid.\n>> + * The maximum grid vertical dimensions, in number of grid blocks(cells).\n>> +*/\n>> +\n>> +/**\n>> + * \\var kAfMinGridBlockWidth\n>> + * \\brief the minimum block size of the width.\n>> + */\n>> +\n>> +/**\n>> + * \\var kAfMinGridBlockHeight\n>> + * \\brief the minimum block size of the height.\n>> + */\n>> +\n>> +/**\n>> + * \\def kAfMaxGridBlockWidth\n>> + * \\brief the maximum block size of the width.\n>> + */\n>> +\n>> +/**\n>> + * \\var kAfMaxGridBlockHeight\n>> + * \\brief the maximum block size of the height.\n>> + */\n>> +\n>> +/**\n>> + * \\var kAfDefaultHeightPerSlice\n>> + * \\brief The default number of blocks in vertical axis per slice.\n>> + */\n>> +\n>> +namespace libcamera {\n>> +\n>> +using namespace std::literals::chrono_literals;\n>> +\n>> +namespace ipa::ipu3::algorithms {\n>> +\n>> +/**\n>> + * \\class Af\n>> + * \\brief An auto-focus algorithm based on IPU3 statistics\n>> + *\n>> + * This algorithm is used to determine the position of the lens to make a\n>> + * focused image. The IPU3 AF processing block computes the statistics that\n>> + * are composed by two types of filtered value and stores in a AF buffer.\n>> + * Typically, for a clear image, it has a relatively higher contrast than a\n>> + * blurred one. Therefore, if an image with the highest contrast can be\n>> + * found through the scan, the position of the len indicates to a clearest\n>> + * image.\n>> + *\n> \n> No need for the blank line (well, it has a star but that can be removed)\n> \n>> + */\n>> +\n>> +LOG_DEFINE_CATEGORY(IPU3Af)\n>> +\n>> +/**\n>> + * Maximum focus steps of the VCM control\n>> + * \\todo should be obtained from the VCM driver\n>> + */\n>> +static constexpr uint32_t kMaxFocusSteps = 1023;\n>> +\n>> +/* minimum focus step for searching appropriate focus */\n>> +static constexpr uint32_t kCoarseSearchStep = 30;\n>> +static constexpr uint32_t kFineSearchStep = 1;\n>> +\n>> +/* max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n>> +static constexpr double kMaxChange = 0.5;\n>> +\n>> +/* the numbers of frame to be ignored, before performing focus scan. */\n>> +static constexpr uint32_t kIgnoreFrame = 10;\n>> +\n>> +/* fine scan range 0 < kFineRange < 1 */\n>> +static constexpr double kFineRange = 0.05;\n>> +\n>> +/* settings for IPU3 AF filter */\n>> +static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {\n>> + .y1_coeff_0 = { 0, 1, 3, 7 },\n>> + .y1_coeff_1 = { 11, 13, 1, 2 },\n>> + .y1_coeff_2 = { 8, 19, 34, 242 },\n>> + .y1_sign_vec = 0x7fdffbfe,\n>> + .y2_coeff_0 = { 0, 1, 6, 6 },\n>> + .y2_coeff_1 = { 13, 25, 3, 0 },\n>> + .y2_coeff_2 = { 25, 3, 177, 254 },\n>> + .y2_sign_vec = 0x4e53ca72,\n>> + .y_calc = { 8, 8, 8, 8 },\n>> + .nf = { 0, 9, 0, 9, 0 },\n>> +};\n> \n> Are these existing values from the kernel, or is there anything we can\n> do to reference what they represent or any existing documentation for\n> how to configure them?\n> \n\nThose are the values used by the intel closed source library, not the \ndefault kernel ones (which are identical for both y1 and y2). I don't \nknow if Kate has a complete description of those ?\n\n>> +\n>> +Af::Af()\n>> + : focus_(0), goodFocus_(0), currentVariance_(0.0), previousVariance_(0.0),\n>> + coarseCompleted_(false), fineCompleted_(false)\n>> +{\n>> +}\n>> +\n>> +/**\n>> + * \\copydoc libcamera::ipa::Algorithm::prepare\n>> + */\n>> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)\n>> +{\n>> + const struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n>> + params->acc_param.af.grid_cfg = grid;\n>> + params->acc_param.af.filter_config = afFilterConfigDefault;\n>> +\n>> + /* enable AF processing block */\n>> + params->use.acc_af = 1;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Configure the Af given a configInfo\n>> + *\n>> + * \\param[in] context The shared IPA context\n>> + * \\param[in] configInfo The IPA configuration data\n>> + * \\return 0\n>> + */\n>> +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>> +{\n>> + struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n>> + grid.width = kAfMinGridWidth;\n>> + grid.height = kAfMinGridHeight;\n>> + grid.block_width_log2 = kAfMinGridBlockWidth;\n>> + grid.block_height_log2 = kAfMinGridBlockHeight;\n>> + grid.height_per_slice = kAfDefaultHeightPerSlice;\n>> +\n>> + /* x_start and y start are default to BDS center */\n>> + grid.x_start = (configInfo.bdsOutputSize.width / 2) -\n>> + (((grid.width << grid.block_width_log2) / 2));\n>> + grid.y_start = (configInfo.bdsOutputSize.height / 2) -\n>> + (((grid.height << grid.block_height_log2) / 2));\n>> +\n>> + /* x_start and y_start should be even */\n>> + grid.x_start = (grid.x_start / 2) * 2;\n>> + grid.y_start = (grid.y_start / 2) * 2;\n>> + grid.y_start = grid.y_start | IPU3_UAPI_GRID_Y_START_EN;\n>> +\n>> + /* inital max focus step */\n>> + maxStep_ = kMaxFocusSteps;\n>> +\n>> + /* determined focus value i.e. current focus value */\n>> + context.frameContext.af.focus = 0;\n>> + /* maximum variance of the AF statistics */\n>> + context.frameContext.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>> +\n>> + return 0;\n>> +}\n>> +\n>> +/**\n>> + * \\brief AF coarse scan\n>> + *\n>> + * Find a near focused image using a coarse step. The step is determined by coarseSearchStep.\n>> + *\n>> + * \\param[in] context The shared IPA context\n>> + *\n> \n> We can remove this extra line too. Looks like that's throughout.\n> \n>> + */\n>> +void Af::afCoarseScan(IPAContext &context)\n>> +{\n>> + if (coarseCompleted_ == true)\n>> + return;\n>> +\n>> + if (afScan(context, kCoarseSearchStep)) {\n>> + coarseCompleted_ = true;\n>> + context.frameContext.af.maxVariance = 0;\n>> + focus_ = context.frameContext.af.focus - (context.frameContext.af.focus * kFineRange);\n>> + context.frameContext.af.focus = focus_;\n>> + previousVariance_ = 0;\n>> + maxStep_ = std::clamp(static_cast(focus_ + (focus_ * kFineRange)), 0U, kMaxFocusSteps);\n>> + }\n>> +}\n>> +\n>> +/**\n>> + * \\brief AF fine scan\n>> + *\n>> + * Find an optimum lens position with moving 1 step for each search.\n>> + *\n>> + * \\param[in] context The shared IPA context\n>> + *\n>> + */\n>> +void Af::afFineScan(IPAContext &context)\n>> +{\n>> + if (coarseCompleted_ != true)\n>> + return;\n>> +\n>> + if (afScan(context, kFineSearchStep)) {\n>> + context.frameContext.af.stable = true;\n>> + fineCompleted_ = true;\n>> + }\n>> +}\n>> +\n>> +/**\n>> + * \\brief AF reset\n>> + *\n>> + * Reset all the parameters to start over the AF process.\n>> + *\n>> + * \\param[in] context The shared IPA context\n>> + *\n>> + */\n>> +void Af::afReset(IPAContext &context)\n>> +{\n>> + context.frameContext.af.maxVariance = 0;\n>> + context.frameContext.af.focus = 0;\n>> + focus_ = 0;\n>> + context.frameContext.af.stable = false;\n>> + ignoreCounter_ = kIgnoreFrame;\n>> + previousVariance_ = 0.0;\n>> + coarseCompleted_ = false;\n>> + fineCompleted_ = false;\n>> + maxStep_ = kMaxFocusSteps;\n>> +}\n>> +\n>> +/**\n>> + * \\brief AF variance comparison.\n>> + *\n>> + * This fuction compares the previous and current variance. It always picks\n> \n> s/fuction/function/ but I don't think we need to say 'this function'\n> when describing a function, but it doesn't hurt.\n> \n>> + * the largest variance to replace the previous one. The image with a larger\n>> + * variance also indicates it is a clearer image than previous one. If it\n>> + * finds the negative sign of derivative, it returns immediately.\n>> + *\n>> + * \\param[in] context The shared IPA context\n>> + * \\return True, if it finds a AF value.\n>> + */\n>> +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 = goodFocus_;\n> \n> I'd be tempted to call goodFocus_ bestFocus_ as it's the 'best' focus\n> point we've seen? But that's not a requirement. 'good' can also indicate\n> it was the known 'good' focus that we have stored.\n> \n> \n>> + return true;\n>> + } else {\n>> + /*\n>> + * find the maximum of the variance by estimating its\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>> + /*\n>> + * positive and zero derivative:\n>> + * The variance is still increasing. The focus could be\n>> + * increased for the next comparison. Also, the max variance\n>> + * and previous focus value are updated.\n>> + */\n>> + goodFocus_ = focus_;\n>> + focus_ += min_step;\n>> + context.frameContext.af.focus = focus_;\n>> + context.frameContext.af.maxVariance = currentVariance_;\n>> + } else {\n>> + /*\n>> + * negative derivative:\n>> + * The variance starts to decrease which means the maximum\n>> + * variance is found. Set focus step to previous good one\n>> + * then returnimmediately.\n> \n> s/returnimmediately./return immediately./\n> \n>> + */\n>> + context.frameContext.af.focus = goodFocus_;\n>> + return true;\n>> + }\n>> + }\n>> +\n>> + previousVariance_ = currentVariance_;\n>> + LOG(IPU3Af, Debug) << \" Previous step is \"\n>> + << goodFocus_\n>> + << \" Current step is \"\n>> + << focus_;\n>> + return false;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Determine the frame to be ignored.\n>> + *\n>> + * \\return Return true the frame is ignored.\n>> + * \\return Return false the frame should be processed.\n>> + */\n>> +\n> \n> This blank line can be removed so the comment hugs the function.\n> \n>> +bool Af::afNeedIgnoreFrame()\n>> +{\n>> + if (ignoreCounter_ == 0)\n>> + return false;\n>> + else\n>> + ignoreCounter_--;\n>> + return true;\n>> +}\n>> +\n>> +/**\n>> + * @brief Reset frame ignore counter.\n> \n> s/@brief/\\brief/ to match doxygen style.\n> \n>> + *\n> \n> And no extra line.\n> \n>> + */\n>> +void Af::afIgnoreFrameReset()\n>> +{\n>> + ignoreCounter_ = kIgnoreFrame;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Determine the max contrast image and lens position.\n>> + *\n>> + * y_table is the AF statistic of IPU3 and is composed of two kinds of filtered\n>> + * values. Based on this, the variance of a image could be used to determine\n> \n> Do we know what the values represent? Could we say perhaps:\n> \n> \"\"\"\n> y_table is the AF statistic of IPU3 and is composed of two kinds of\n> filtered values representing the contrast levels of the image.\n> \"\"\"\n> \n> However, don't take that as verbatim, I'm guessing based on context ;-)\n> \n> \n>> + * the clearness of the given image. Ideally, a clear image also has a\n>> + * raletively higher contrast. So, the VCM is moved step by step and variance\n> \n> s/raletively/relatively/\n> \n>> + * of each frame are calculated to find a maximum variance which corresponds\n>> + * with a specific focus step. If it is found, that is the best focus step of\n>> + * current scene.\n>> + *\n>> + * \\param[in] context The shared IPA context.\n>> + * \\param[in] stats The statistic buffer of 3A of IPU3.\n>> + */\n>> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>> +{\n>> + uint32_t total = 0;\n>> + double mean;\n>> + uint64_t var_sum = 0;\n>> + y_table_item_t y_item[IPU3_UAPI_AF_Y_TABLE_MAX_SIZE / sizeof(y_table_item_t)];\n>> + uint32_t z = 0;\n>> + uint32_t afRawBufferLen_;\n>> +\n>> + /* evaluate the AF buffer length */\n>> + afRawBufferLen_ = context.configuration.af.afGrid.width *\n>> + context.configuration.af.afGrid.height;\n>> +\n>> + memcpy(y_item, stats->af_raw_buffer.y_table, afRawBufferLen_ * sizeof(y_table_item_t));\n>> +\n>> + /*\n>> + * calculate the mean and the variance of AF statistics for a given grid.\n>> + *\n>> + * For coarse: y1 are used.\n>> + * For fine: y2 results are used.\n>> + */\n>> + if (coarseCompleted_) {\n>> + for (z = 0; z < afRawBufferLen_; z++) {\n>> + total = total + y_item[z].y2_avg;\n>> + }\n>> + mean = total / afRawBufferLen_;\n>> +\n>> + for (z = 0; z < afRawBufferLen_; z++) {\n>> + var_sum = var_sum + ((y_item[z].y2_avg - mean) *\n>> + (y_item[z].y2_avg - mean));\n\nvar_sum += ((y_item[z].y2_avg - mean) *\n\t (y_item[z].y2_avg - mean));\n\nsame below.\n\nI would have loved seeing a function like estimateVariance(y_item) which \nwould contain all the calculation and return the variance as a double ;-).\n\n>> + }\n>> + } else {\n>> + for (z = 0; z < afRawBufferLen_; z++) {\n>> + total = total + y_item[z].y1_avg;\n>> + }\n>> + mean = total / afRawBufferLen_;\n>> +\n>> + for (z = 0; z < afRawBufferLen_; z++) {\n>> + var_sum = var_sum + ((y_item[z].y1_avg - mean) *\n>> + (y_item[z].y1_avg - mean));\n>> + }\n>> + }\n>> +\n>> + /* determine the average variance of the AF statistic. */\n>> + currentVariance_ = static_cast(var_sum) / static_cast(afRawBufferLen_);\n>> +\n>> + if (context.frameContext.af.stable == true) {\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>> + LOG(IPU3Af, Debug) << \"Variance change rate: \"\n>> + << var_ratio\n>> + << \" Current VCM step: \"\n>> + << context.frameContext.af.focus;\n>> +\n>> + /*\n>> + * If the change rate is greater than kMaxChange (out of focus),\n>> + * trigger AF again.\n>> + */\n>> + if (var_ratio > kMaxChange) {\n>> + if (!afNeedIgnoreFrame())\n>> + afReset(context);\n>> + } else\n>> + afIgnoreFrameReset();\n>> + } else {\n>> + if (!afNeedIgnoreFrame()) {\n>> + afCoarseScan(context);\n>> + afFineScan(context);\n>> + }\n>> + }\n\nThis is a matter of taste. As if we are stable we are not just returning \nearly, I would have inverted the conditional, to have:\n\nif (!context.frameContext.af.stable) {\n\tscan();\n} else {\n\tdouble varRatio = estimateRatio();\n\t/*\n\t * If the change rate is greater than kMaxRatio, we are out\n\t * of focus and need to trigger AF again.\n\t */\n\tif (varRatio > kMaxRatio)\n\t\tafReset(context);\n\telse\n\t\tignoreCounter_ = kIgnoreFrame;\n}\n\nYou could change afReset and have:\nvoid Af::afReset(IPAContext &context)\n{\n\tif (afNeedIgnoreFrame())\n\t\treturn;\n\n\tcontext.frameContext.af.maxVariance = 0;\n\tcontext.frameContext.af.focus = 0;\n\tfocus_ = 0;\n\tcontext.frameContext.af.stable = false;\n\tignoreCounter_ = kIgnoreFrame;\n\tpreviousVariance_ = 0.0;\n\tcoarseCompleted_ = false;\n\tfineCompleted_ = false;\n\tmaxStep_ = kMaxFocusSteps;\n}\n\nNot sure if it is better, but as you call afReset only in the case \n!afNeedIgnoreFrame() it could simplify the process() reading.\n\n>> +}\n>> +\n>> +} /* namespace ipa::ipu3::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h\n>> new file mode 100644\n>> index 00000000..bc5d2064\n>> --- /dev/null\n>> +++ b/src/ipa/ipu3/algorithms/af.h\n>> @@ -0,0 +1,78 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Red Hat\n>> + *\n>> + * af.h - IPU3 Af algorithm\n>> + */\n>> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AF_H__\n>> +#define __LIBCAMERA_IPU3_ALGORITHMS_AF_H__\n> \n> We can use #pragma once here now. (The whole code base was converted).\n> \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +/* static variables from repo of chromium */\n>> +static constexpr uint8_t kAfMinGridWidth = 16;\n>> +static constexpr uint8_t kAfMinGridHeight = 16;\n>> +static constexpr uint8_t kAfMaxGridWidth = 32;\n>> +static constexpr uint8_t kAfMaxGridHeight = 24;\n>> +static constexpr uint16_t kAfMinGridBlockWidth = 4;\n>> +static constexpr uint16_t kAfMinGridBlockHeight = 3;\n>> +static constexpr uint16_t kAfMaxGridBlockWidth = 6;\n>> +static constexpr uint16_t kAfMaxGridBlockHeight = 6;\n>> +static constexpr uint16_t kAfDefaultHeightPerSlice = 2;\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::ipu3::algorithms {\n>> +\n>> +class Af : public Algorithm\n>> +{\n>> + /* The format of y_table. From ipu3-ipa repo */\n>> + typedef struct __attribute__((packed)) y_table_item {\n>> + uint16_t y1_avg;\n>> + uint16_t y2_avg;\n>> + } y_table_item_t;\n>> +public:\n>> + Af();\n>> + ~Af() = default;\n>> +\n>> + void prepare(IPAContext &context, ipu3_uapi_params *params) override;\n>> + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n>> + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;\n>> +\n>> +private:\n>> + void afCoarseScan(IPAContext &context);\n>> + void afFineScan(IPAContext &context);\n>> + bool afScan(IPAContext &context, int min_step);\n>> + void afReset(IPAContext &context);\n>> + bool afNeedIgnoreFrame();\n>> + void afIgnoreFrameReset();\n>> +\n>> + /* Used for focus scan. */\n> \n> If we're describing it, what is it? Is it the current focus position\n> when scanning?\n> \n>> + uint32_t focus_;\n>> + /* Focus good */\n> \n> Focus Good doesn't really describe a variable called good Focus. What\n> gets put in it? Is it the last known good focus value?\n> \n>> + uint32_t goodFocus_;\n>> + /* Recent AF statistic variance. */\n>> + double currentVariance_;\n>> + /* The frames to be ignore before starting measuring. */\n>> + uint32_t ignoreCounter_;\n>> + /* previous variance. it is used to determine the gradient */\n>> + double previousVariance_;\n>> + /* Max scan steps of each pass of AF scaning */\n>> + uint32_t maxStep_;\n>> + /* coarse scan stable. Complete low pass search (coarse) scan) */\n>> + bool coarseCompleted_;\n>> + /* fine scan stable. Complete high pass scan (fine scan) */\n>> + bool fineCompleted_;\n>> +};\n>> +\n>> +} /* namespace ipa::ipu3::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> +\n>> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */\n>> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n>> index 4db6ae1d..e1099169 100644\n>> --- a/src/ipa/ipu3/algorithms/meson.build\n>> +++ b/src/ipa/ipu3/algorithms/meson.build\n>> @@ -1,8 +1,9 @@\n>> # SPDX-License-Identifier: CC0-1.0\n>> \n>> ipu3_ipa_algorithms = files([\n>> + 'af.cpp',\n>> 'agc.cpp',\n>> 'awb.cpp',\n>> 'blc.cpp',\n>> - 'tone_mapping.cpp',\n>> + 'tone_mapping.cpp'\n> \n> We should keep the ',' at the end of tone_mapping.cpp so that we don't have to\n> modify extra lines when appending to the list.\n> \n>> ])\n>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n>> index 86794ac1..ecb2b90a 100644\n>> --- a/src/ipa/ipu3/ipa_context.cpp\n>> +++ b/src/ipa/ipu3/ipa_context.cpp\n>> @@ -67,6 +67,30 @@ namespace libcamera::ipa::ipu3 {\n>> *\n>> * \\var IPASessionConfiguration::grid.stride\n>> * \\brief Number of cells on one line including the ImgU padding\n>> + *\n> \n> No need for extra blank line\n> \n>> + */\n>> +\n>> +/**\n>> + * \\var IPASessionConfiguration::af\n>> + * \\brief AF grid configuration of the IPA\n>> + *\n>> + * \\var IPASessionConfiguration::af.afGrid\n>> + *\n> \n> No need for extra blank line\n> \n>> + */\n>> +\n>> +/**\n>> + * \\var IPAFrameContext::af\n>> + * \\brief Context for the Automatic Focus algorithm\n>> + *\n>> + * \\struct IPAFrameContext::af\n>> + * \\var IPAFrameContext::af.focus\n>> + * \\brief Current position of the lens\n>> + *\n>> + * \\var IPAFrameContext::af.maxVariance\n>> + * \\brief The maximum variance of the current image.\n>> + *\n>> + * \\var IPAFrameContext::af.stable\n>> + * \\brief is the image focused?\n>> */\n>> \n>> /**\n>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n>> index c6dc0814..60ad3194 100644\n>> --- a/src/ipa/ipu3/ipa_context.h\n>> +++ b/src/ipa/ipu3/ipa_context.h\n>> @@ -25,6 +25,10 @@ struct IPASessionConfiguration {\n>> uint32_t stride;\n>> } grid;\n>> \n>> + struct {\n>> + ipu3_uapi_grid_config afGrid;\n>> + } af;\n>> +\n>> struct {\n>> utils::Duration minShutterSpeed;\n>> utils::Duration maxShutterSpeed;\n>> @@ -34,6 +38,12 @@ struct IPASessionConfiguration {\n>> };\n>> \n>> struct IPAFrameContext {\n>> + struct {\n>> + uint32_t focus;\n> \n> Is this the focus point/value of the current frame, or the desired value to\n> set on the next frame?\n> \n>> + double maxVariance;\n>> + bool stable;\n>> + } af;\n>> +\n>> struct {\n>> uint32_t exposure;\n>> double gain;\n>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>> index e44a31bb..417e0562 100644\n>> --- a/src/ipa/ipu3/ipu3.cpp\n>> +++ b/src/ipa/ipu3/ipu3.cpp\n>> @@ -30,6 +30,7 @@\n>> \n>> #include \"libcamera/internal/mapped_framebuffer.h\"\n>> \n>> +#include \"algorithms/af.h\"\n>> #include \"algorithms/agc.h\"\n>> #include \"algorithms/algorithm.h\"\n>> #include \"algorithms/awb.h\"\n>> @@ -295,6 +296,7 @@ int IPAIPU3::init(const IPASettings &settings,\n>> }\n>> \n>> /* Construct our Algorithms */\n>> + algorithms_.push_back(std::make_unique());\n>> algorithms_.push_back(std::make_unique());\n>> algorithms_.push_back(std::make_unique());\n>> algorithms_.push_back(std::make_unique());\n>> -- \n>> 2.33.1\n>>","headers":{"Return-Path":"","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 0FDA9BDCBF\n\tfor ;\n\tThu, 10 Feb 2022 15:41:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 297C1610E2;\n\tThu, 10 Feb 2022 16:41:45 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5294C60E70\n\tfor ;\n\tThu, 10 Feb 2022 16:41:43 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:c44d:925d:d63:5b07] (unknown\n\t[IPv6:2a01:e0a:169:7140:c44d:925d:d63:5b07])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DE42B93;\n\tThu, 10 Feb 2022 16:41:42 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"u34IN8a5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1644507703;\n\tbh=oCJoY6XOHzbnuBDLoSdWdvC8vszDjkaBlHdmARa3JlM=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=u34IN8a505++SGgFe5ytvjl3UnDAH4SiOcMSe87PlUGSXvCV2BAn7wFF4AkiM9Yll\n\tv0w708O/8Brdx3PFhXaL5tn1lsdcdU2Brv+2biCMrvvlyZJXVhhfWM9E1icEBAJ0BT\n\tkMv2331FfASETqf8igz8RtOalox+XRsSMe8Zh8z4=","Message-ID":"<53cd384f-6af6-c550-436e-4358154a73ac@ideasonboard.com>","Date":"Thu, 10 Feb 2022 16:41:40 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.5.0","Content-Language":"en-US","To":"Kieran Bingham ,\n\tKate Hsuan ,\n\tlibcamera devel ","References":"<20220210085759.71419-1-hpa@redhat.com>\n\t<164448777681.3354066.18018320193243523810@Monstersaurus>","From":"Jean-Michel Hautbois ","In-Reply-To":"<164448777681.3354066.18018320193243523810@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v7] ipa: ipu3: af: Auto focus for\n\tdw9719 Surface Go2 VCM","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22167,"web_url":"https://patchwork.libcamera.org/comment/22167/","msgid":"","date":"2022-02-11T06:42:00","subject":"Re: [libcamera-devel] [PATCH v7] ipa: ipu3: af: Auto focus for\n\tdw9719 Surface Go2 VCM","submitter":{"id":105,"url":"https://patchwork.libcamera.org/api/people/105/","name":"Kate Hsuan","email":"hpa@redhat.com"},"content":")Hi Jean-Michel and Kieran,\n\nThanks for your feedback.\n\nOn Thu, Feb 10, 2022 at 11:43 PM Jean-Michel Hautbois\n wrote:\n>\n> Hi Kate,\n>\n> Thanks for the patch !\n>\n> On 10/02/2022 11:09, Kieran Bingham wrote:\n> > Hi Kate,\n> >\n> > A few minor style comments in here, but seeing this is at v7 those could\n> > possibly be fixed while applying anyway.\n> >\n> > But I'll leave the algorithm review itself to JM.\n>\n> Sure. Thanks (?) ;-).\n\nOnce again, thank you, folks :)\n\n>\n> >\n> > Quoting Kate Hsuan (2022-02-10 08:57:59)\n> >> Since VCM for surface Go 2 (dw9719) had been successfully\n> >> driven, this Af module can be used to control the VCM and\n> >> determine the focus value based on the IPU3 AF statistics.\n> >>\n> >> Based on the values from the IPU3 AF buffer, the variance\n> >> of each focus step is determined and a greedy approach is\n> >> used to find the maximum variance of the AF state and an\n> >> appropriate focus value.\n> >>\n> >> The grid configuration is implemented as a context. Also,\n> >> the grid parameter- AF_MIN_BLOCK_WIDTH is set to 4 (default\n> >> is 3) since if the default value is used, x_start\n> >> (x_start > 640) will be at an incorrect location of the\n> >> image (rightmost of the sensor).\n> >>\n> >> Signed-off-by: Kate Hsuan \n> >> Tested-by: Jean-Michel Hautbois \n> >\n> > I can't see which values are to be passed to the pipeline handler / lens\n> > yet, but I think that's expected as that plumbing isn't merged. But I\n> > think it's good to progress this, and it could even be merged before the\n> > plumbing.\n>\n> It could indeed be interesting to at least refer to the ancillary links\n> patch series from Dan [1] and [2]. Those are needed for this algorithm\n> to have an effect :-).\n>\n> [1]: https://lore.kernel.org/all/20220130235821.48076-1-djrscally@gmail.com/\n> [2]: https://patchwork.libcamera.org/project/libcamera/list/?series=2906\n>\n\nThanks to the folks' works :)\n\n> >\n> > Overall though, I like this, as I can see how it's acting to sweep\n> > through and when it makes a decision to go back if it passes a peak.\n> >\n> >> ---\n> >> Changes in v7:\n> >> 1. Improved comments and fixed typo.\n> >> 2. Simplified AF algorithm. \"ignore frame\" had been moved into a\n> >> function and remove a unnecessary \"if\" expression.\n> >> 3. afRawBufferLen_ keeped to be a local variable not a class member.\n> >> 4. Initial configurations are moved to configure().\n> >> ---\n> >> src/ipa/ipu3/algorithms/af.cpp | 426 ++++++++++++++++++++++++++++\n> >> src/ipa/ipu3/algorithms/af.h | 78 +++++\n> >> src/ipa/ipu3/algorithms/meson.build | 3 +-\n> >> src/ipa/ipu3/ipa_context.cpp | 24 ++\n> >> src/ipa/ipu3/ipa_context.h | 10 +\n> >> src/ipa/ipu3/ipu3.cpp | 2 +\n> >> 6 files changed, 542 insertions(+), 1 deletion(-)\n> >> create mode 100644 src/ipa/ipu3/algorithms/af.cpp\n> >> create mode 100644 src/ipa/ipu3/algorithms/af.h\n> >>\n> >> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> >> new file mode 100644\n> >> index 00000000..b26aa018\n> >> --- /dev/null\n> >> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> >> @@ -0,0 +1,426 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Red Hat\n> >> + *\n> >> + * af.cpp - IPU3 auto focus algorithm\n> >> + */\n> >> +\n> >> +#include \"af.h\"\n> >> +\n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \"libipa/histogram.h\"\n> >> +\n> >> +/**\n> >> + * \\file af.h\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var kAfMinGridWidth\n> >> + * \\brief the minimum width of AF grid.\n> >> + * The minimum grid horizontal dimensions, in number of grid blocks(cells).\n> >> +*/\n> >> +\n> >> +/**\n> >> + * \\var kAfMinGridHeight\n> >> + * \\brief the minimum height of AF grid.\n> >> + * The minimum grid vertical dimensions, in number of grid blocks(cells).\n> >> +*/\n> >> +\n> >> +/**\n> >> + * \\var kAfMaxGridWidth\n> >> + * \\brief the maximum width of AF grid.\n> >> + * The maximum grid horizontal dimensions, in number of grid blocks(cells).\n> >> +*/\n> >> +\n> >> +/**\n> >> + * \\var kAfMaxGridHeight\n> >> + * \\brief the maximum height of AF grid.\n> >> + * The maximum grid vertical dimensions, in number of grid blocks(cells).\n> >> +*/\n> >> +\n> >> +/**\n> >> + * \\var kAfMinGridBlockWidth\n> >> + * \\brief the minimum block size of the width.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var kAfMinGridBlockHeight\n> >> + * \\brief the minimum block size of the height.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def kAfMaxGridBlockWidth\n> >> + * \\brief the maximum block size of the width.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var kAfMaxGridBlockHeight\n> >> + * \\brief the maximum block size of the height.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var kAfDefaultHeightPerSlice\n> >> + * \\brief The default number of blocks in vertical axis per slice.\n> >> + */\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +using namespace std::literals::chrono_literals;\n> >> +\n> >> +namespace ipa::ipu3::algorithms {\n> >> +\n> >> +/**\n> >> + * \\class Af\n> >> + * \\brief An auto-focus algorithm based on IPU3 statistics\n> >> + *\n> >> + * This algorithm is used to determine the position of the lens to make a\n> >> + * focused image. The IPU3 AF processing block computes the statistics that\n> >> + * are composed by two types of filtered value and stores in a AF buffer.\n> >> + * Typically, for a clear image, it has a relatively higher contrast than a\n> >> + * blurred one. Therefore, if an image with the highest contrast can be\n> >> + * found through the scan, the position of the len indicates to a clearest\n> >> + * image.\n> >> + *\n> >\n> > No need for the blank line (well, it has a star but that can be removed)\n\n\n> >\n> >> + */\n> >> +\n> >> +LOG_DEFINE_CATEGORY(IPU3Af)\n> >> +\n> >> +/**\n> >> + * Maximum focus steps of the VCM control\n> >> + * \\todo should be obtained from the VCM driver\n> >> + */\n> >> +static constexpr uint32_t kMaxFocusSteps = 1023;\n> >> +\n> >> +/* minimum focus step for searching appropriate focus */\n> >> +static constexpr uint32_t kCoarseSearchStep = 30;\n> >> +static constexpr uint32_t kFineSearchStep = 1;\n> >> +\n> >> +/* max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n> >> +static constexpr double kMaxChange = 0.5;\n> >> +\n> >> +/* the numbers of frame to be ignored, before performing focus scan. */\n> >> +static constexpr uint32_t kIgnoreFrame = 10;\n> >> +\n> >> +/* fine scan range 0 < kFineRange < 1 */\n> >> +static constexpr double kFineRange = 0.05;\n> >> +\n> >> +/* settings for IPU3 AF filter */\n> >> +static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {\n> >> + .y1_coeff_0 = { 0, 1, 3, 7 },\n> >> + .y1_coeff_1 = { 11, 13, 1, 2 },\n> >> + .y1_coeff_2 = { 8, 19, 34, 242 },\n> >> + .y1_sign_vec = 0x7fdffbfe,\n> >> + .y2_coeff_0 = { 0, 1, 6, 6 },\n> >> + .y2_coeff_1 = { 13, 25, 3, 0 },\n> >> + .y2_coeff_2 = { 25, 3, 177, 254 },\n> >> + .y2_sign_vec = 0x4e53ca72,\n> >> + .y_calc = { 8, 8, 8, 8 },\n> >> + .nf = { 0, 9, 0, 9, 0 },\n> >> +};\n> >\n> > Are these existing values from the kernel, or is there anything we can\n> > do to reference what they represent or any existing documentation for\n> > how to configure them?\n> >\n>\n> Those are the values used by the intel closed source library, not the\n> default kernel ones (which are identical for both y1 and y2). I don't\n> know if Kate has a complete description of those ?\n>\n\nActually, I can't. I have to thank JM to provide this solution and\nsecret numbers :).\n\n> >> +\n> >> +Af::Af()\n> >> + : focus_(0), goodFocus_(0), currentVariance_(0.0), previousVariance_(0.0),\n> >> + coarseCompleted_(false), fineCompleted_(false)\n> >> +{\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> >> + */\n> >> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)\n> >> +{\n> >> + const struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> >> + params->acc_param.af.grid_cfg = grid;\n> >> + params->acc_param.af.filter_config = afFilterConfigDefault;\n> >> +\n> >> + /* enable AF processing block */\n> >> + params->use.acc_af = 1;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Configure the Af given a configInfo\n> >> + *\n> >> + * \\param[in] context The shared IPA context\n> >> + * \\param[in] configInfo The IPA configuration data\n> >> + * \\return 0\n> >> + */\n> >> +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> >> +{\n> >> + struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> >> + grid.width = kAfMinGridWidth;\n> >> + grid.height = kAfMinGridHeight;\n> >> + grid.block_width_log2 = kAfMinGridBlockWidth;\n> >> + grid.block_height_log2 = kAfMinGridBlockHeight;\n> >> + grid.height_per_slice = kAfDefaultHeightPerSlice;\n> >> +\n> >> + /* x_start and y start are default to BDS center */\n> >> + grid.x_start = (configInfo.bdsOutputSize.width / 2) -\n> >> + (((grid.width << grid.block_width_log2) / 2));\n> >> + grid.y_start = (configInfo.bdsOutputSize.height / 2) -\n> >> + (((grid.height << grid.block_height_log2) / 2));\n> >> +\n> >> + /* x_start and y_start should be even */\n> >> + grid.x_start = (grid.x_start / 2) * 2;\n> >> + grid.y_start = (grid.y_start / 2) * 2;\n> >> + grid.y_start = grid.y_start | IPU3_UAPI_GRID_Y_START_EN;\n> >> +\n> >> + /* inital max focus step */\n> >> + maxStep_ = kMaxFocusSteps;\n> >> +\n> >> + /* determined focus value i.e. current focus value */\n> >> + context.frameContext.af.focus = 0;\n> >> + /* maximum variance of the AF statistics */\n> >> + context.frameContext.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> >> +\n> >> + return 0;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief AF coarse scan\n> >> + *\n> >> + * Find a near focused image using a coarse step. The step is determined by coarseSearchStep.\n> >> + *\n> >> + * \\param[in] context The shared IPA context\n> >> + *\n> >\n> > We can remove this extra line too. Looks like that's throughout.\n> >\n> >> + */\n> >> +void Af::afCoarseScan(IPAContext &context)\n> >> +{\n> >> + if (coarseCompleted_ == true)\n> >> + return;\n> >> +\n> >> + if (afScan(context, kCoarseSearchStep)) {\n> >> + coarseCompleted_ = true;\n> >> + context.frameContext.af.maxVariance = 0;\n> >> + focus_ = context.frameContext.af.focus - (context.frameContext.af.focus * kFineRange);\n> >> + context.frameContext.af.focus = focus_;\n> >> + previousVariance_ = 0;\n> >> + maxStep_ = std::clamp(static_cast(focus_ + (focus_ * kFineRange)), 0U, kMaxFocusSteps);\n> >> + }\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief AF fine scan\n> >> + *\n> >> + * Find an optimum lens position with moving 1 step for each search.\n> >> + *\n> >> + * \\param[in] context The shared IPA context\n> >> + *\n> >> + */\n> >> +void Af::afFineScan(IPAContext &context)\n> >> +{\n> >> + if (coarseCompleted_ != true)\n> >> + return;\n> >> +\n> >> + if (afScan(context, kFineSearchStep)) {\n> >> + context.frameContext.af.stable = true;\n> >> + fineCompleted_ = true;\n> >> + }\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief AF reset\n> >> + *\n> >> + * Reset all the parameters to start over the AF process.\n> >> + *\n> >> + * \\param[in] context The shared IPA context\n> >> + *\n> >> + */\n> >> +void Af::afReset(IPAContext &context)\n> >> +{\n> >> + context.frameContext.af.maxVariance = 0;\n> >> + context.frameContext.af.focus = 0;\n> >> + focus_ = 0;\n> >> + context.frameContext.af.stable = false;\n> >> + ignoreCounter_ = kIgnoreFrame;\n> >> + previousVariance_ = 0.0;\n> >> + coarseCompleted_ = false;\n> >> + fineCompleted_ = false;\n> >> + maxStep_ = kMaxFocusSteps;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief AF variance comparison.\n> >> + *\n> >> + * This fuction compares the previous and current variance. It always picks\n> >\n> > s/fuction/function/ but I don't think we need to say 'this function'\n> > when describing a function, but it doesn't hurt.\n\nOK\n\n> >\n> >> + * the largest variance to replace the previous one. The image with a larger\n> >> + * variance also indicates it is a clearer image than previous one. If it\n> >> + * finds the negative sign of derivative, it returns immediately.\n> >> + *\n> >> + * \\param[in] context The shared IPA context\n> >> + * \\return True, if it finds a AF value.\n> >> + */\n> >> +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 = goodFocus_;\n> >\n> > I'd be tempted to call goodFocus_ bestFocus_ as it's the 'best' focus\n> > point we've seen? But that's not a requirement. 'good' can also indicate\n> > it was the known 'good' focus that we have stored.\n\nIt holds a local maximum (local best) value and finally, it will\nbecome the best focus value (global best) of the entire AF process.\nI think it can be called \"bestFocus_\". I used the term \"good\" to\nprevent misunderstanding.\n\n> >\n> >\n> >> + return true;\n> >> + } else {\n> >> + /*\n> >> + * find the maximum of the variance by estimating its\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> >> + /*\n> >> + * positive and zero derivative:\n> >> + * The variance is still increasing. The focus could be\n> >> + * increased for the next comparison. Also, the max variance\n> >> + * and previous focus value are updated.\n> >> + */\n> >> + goodFocus_ = focus_;\n> >> + focus_ += min_step;\n> >> + context.frameContext.af.focus = focus_;\n> >> + context.frameContext.af.maxVariance = currentVariance_;\n> >> + } else {\n> >> + /*\n> >> + * negative derivative:\n> >> + * The variance starts to decrease which means the maximum\n> >> + * variance is found. Set focus step to previous good one\n> >> + * then returnimmediately.\n> >\n> > s/returnimmediately./return immediately./\n> >\n> >> + */\n> >> + context.frameContext.af.focus = goodFocus_;\n> >> + return true;\n> >> + }\n> >> + }\n> >> +\n> >> + previousVariance_ = currentVariance_;\n> >> + LOG(IPU3Af, Debug) << \" Previous step is \"\n> >> + << goodFocus_\n> >> + << \" Current step is \"\n> >> + << focus_;\n> >> + return false;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Determine the frame to be ignored.\n> >> + *\n> >> + * \\return Return true the frame is ignored.\n> >> + * \\return Return false the frame should be processed.\n> >> + */\n> >> +\n> >\n> > This blank line can be removed so the comment hugs the function.\n> >\n> >> +bool Af::afNeedIgnoreFrame()\n> >> +{\n> >> + if (ignoreCounter_ == 0)\n> >> + return false;\n> >> + else\n> >> + ignoreCounter_--;\n> >> + return true;\n> >> +}\n> >> +\n> >> +/**\n> >> + * @brief Reset frame ignore counter.\n> >\n> > s/@brief/\\brief/ to match doxygen style.\n> >\n> >> + *\n> >\n> > And no extra line.\n> >\n> >> + */\n> >> +void Af::afIgnoreFrameReset()\n> >> +{\n> >> + ignoreCounter_ = kIgnoreFrame;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Determine the max contrast image and lens position.\n> >> + *\n> >> + * y_table is the AF statistic of IPU3 and is composed of two kinds of filtered\n> >> + * values. Based on this, the variance of a image could be used to determine\n> >\n> > Do we know what the values represent? Could we say perhaps:\n\nThis is my understanding. y_table holds y1 and y2 filtered results\nwhich are controlled by the filter configuration. (you can see from\nthe afFilterConfigDefault)\nSo, those y1 and y2 can be kinds of the result of edge detection of IPU3.\n\n> >\n> > \"\"\"\n> > y_table is the AF statistic of IPU3 and is composed of two kinds of\n> > filtered values representing the contrast levels of the image.\n> > \"\"\"\n> >\n> > However, don't take that as verbatim, I'm guessing based on context ;-)\n\nok, I'll revise this.\n> >\n> >\n> >> + * the clearness of the given image. Ideally, a clear image also has a\n> >> + * raletively higher contrast. So, the VCM is moved step by step and variance\n> >\n> > s/raletively/relatively/\n> >\n> >> + * of each frame are calculated to find a maximum variance which corresponds\n> >> + * with a specific focus step. If it is found, that is the best focus step of\n> >> + * current scene.\n> >> + *\n> >> + * \\param[in] context The shared IPA context.\n> >> + * \\param[in] stats The statistic buffer of 3A of IPU3.\n> >> + */\n> >> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n> >> +{\n> >> + uint32_t total = 0;\n> >> + double mean;\n> >> + uint64_t var_sum = 0;\n> >> + y_table_item_t y_item[IPU3_UAPI_AF_Y_TABLE_MAX_SIZE / sizeof(y_table_item_t)];\n> >> + uint32_t z = 0;\n> >> + uint32_t afRawBufferLen_;\n> >> +\n> >> + /* evaluate the AF buffer length */\n> >> + afRawBufferLen_ = context.configuration.af.afGrid.width *\n> >> + context.configuration.af.afGrid.height;\n> >> +\n> >> + memcpy(y_item, stats->af_raw_buffer.y_table, afRawBufferLen_ * sizeof(y_table_item_t));\n> >> +\n> >> + /*\n> >> + * calculate the mean and the variance of AF statistics for a given grid.\n> >> + *\n> >> + * For coarse: y1 are used.\n> >> + * For fine: y2 results are used.\n> >> + */\n> >> + if (coarseCompleted_) {\n> >> + for (z = 0; z < afRawBufferLen_; z++) {\n> >> + total = total + y_item[z].y2_avg;\n> >> + }\n> >> + mean = total / afRawBufferLen_;\n> >> +\n> >> + for (z = 0; z < afRawBufferLen_; z++) {\n> >> + var_sum = var_sum + ((y_item[z].y2_avg - mean) *\n> >> + (y_item[z].y2_avg - mean));\n>\n> var_sum += ((y_item[z].y2_avg - mean) *\n> (y_item[z].y2_avg - mean));\n>\n> same below.\n>\n> I would have loved seeing a function like estimateVariance(y_item) which\n> would contain all the calculation and return the variance as a double ;-).\n\nOK\n\n>\n> >> + }\n> >> + } else {\n> >> + for (z = 0; z < afRawBufferLen_; z++) {\n> >> + total = total + y_item[z].y1_avg;\n> >> + }\n> >> + mean = total / afRawBufferLen_;\n> >> +\n> >> + for (z = 0; z < afRawBufferLen_; z++) {\n> >> + var_sum = var_sum + ((y_item[z].y1_avg - mean) *\n> >> + (y_item[z].y1_avg - mean));\n> >> + }\n> >> + }\n> >> +\n> >> + /* determine the average variance of the AF statistic. */\n> >> + currentVariance_ = static_cast(var_sum) / static_cast(afRawBufferLen_);\n> >> +\n> >> + if (context.frameContext.af.stable == true) {\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> >> + LOG(IPU3Af, Debug) << \"Variance change rate: \"\n> >> + << var_ratio\n> >> + << \" Current VCM step: \"\n> >> + << context.frameContext.af.focus;\n> >> +\n> >> + /*\n> >> + * If the change rate is greater than kMaxChange (out of focus),\n> >> + * trigger AF again.\n> >> + */\n> >> + if (var_ratio > kMaxChange) {\n> >> + if (!afNeedIgnoreFrame())\n> >> + afReset(context);\n> >> + } else\n> >> + afIgnoreFrameReset();\n> >> + } else {\n> >> + if (!afNeedIgnoreFrame()) {\n> >> + afCoarseScan(context);\n> >> + afFineScan(context);\n> >> + }\n> >> + }\n>\n> This is a matter of taste. As if we are stable we are not just returning\n> early, I would have inverted the conditional, to have:\n>\n> if (!context.frameContext.af.stable) {\n> scan();\n> } else {\n> double varRatio = estimateRatio();\n> /*\n> * If the change rate is greater than kMaxRatio, we are out\n> * of focus and need to trigger AF again.\n> */\n> if (varRatio > kMaxRatio)\n> afReset(context);\n> else\n> ignoreCounter_ = kIgnoreFrame;\n> }\n>\n> You could change afReset and have:\n> void Af::afReset(IPAContext &context)\n> {\n> if (afNeedIgnoreFrame())\n> return;\n>\n> context.frameContext.af.maxVariance = 0;\n> context.frameContext.af.focus = 0;\n> focus_ = 0;\n> context.frameContext.af.stable = false;\n> ignoreCounter_ = kIgnoreFrame;\n> previousVariance_ = 0.0;\n> coarseCompleted_ = false;\n> fineCompleted_ = false;\n> maxStep_ = kMaxFocusSteps;\n> }\n>\n> Not sure if it is better, but as you call afReset only in the case\n> !afNeedIgnoreFrame() it could simplify the process() reading.\n\nGot it. I'll move all the detail to a function and only put the\nalgorithm itself in the process().\n\n>\n> >> +}\n> >> +\n> >> +} /* namespace ipa::ipu3::algorithms */\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h\n> >> new file mode 100644\n> >> index 00000000..bc5d2064\n> >> --- /dev/null\n> >> +++ b/src/ipa/ipu3/algorithms/af.h\n> >> @@ -0,0 +1,78 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Red Hat\n> >> + *\n> >> + * af.h - IPU3 Af algorithm\n> >> + */\n> >> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AF_H__\n> >> +#define __LIBCAMERA_IPU3_ALGORITHMS_AF_H__\n> >\n> > We can use #pragma once here now. (The whole code base was converted).\n> >\n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \"algorithm.h\"\n> >> +\n> >> +/* static variables from repo of chromium */\n> >> +static constexpr uint8_t kAfMinGridWidth = 16;\n> >> +static constexpr uint8_t kAfMinGridHeight = 16;\n> >> +static constexpr uint8_t kAfMaxGridWidth = 32;\n> >> +static constexpr uint8_t kAfMaxGridHeight = 24;\n> >> +static constexpr uint16_t kAfMinGridBlockWidth = 4;\n> >> +static constexpr uint16_t kAfMinGridBlockHeight = 3;\n> >> +static constexpr uint16_t kAfMaxGridBlockWidth = 6;\n> >> +static constexpr uint16_t kAfMaxGridBlockHeight = 6;\n> >> +static constexpr uint16_t kAfDefaultHeightPerSlice = 2;\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +namespace ipa::ipu3::algorithms {\n> >> +\n> >> +class Af : public Algorithm\n> >> +{\n> >> + /* The format of y_table. From ipu3-ipa repo */\n> >> + typedef struct __attribute__((packed)) y_table_item {\n> >> + uint16_t y1_avg;\n> >> + uint16_t y2_avg;\n> >> + } y_table_item_t;\n> >> +public:\n> >> + Af();\n> >> + ~Af() = default;\n> >> +\n> >> + void prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> >> + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> >> + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;\n> >> +\n> >> +private:\n> >> + void afCoarseScan(IPAContext &context);\n> >> + void afFineScan(IPAContext &context);\n> >> + bool afScan(IPAContext &context, int min_step);\n> >> + void afReset(IPAContext &context);\n> >> + bool afNeedIgnoreFrame();\n> >> + void afIgnoreFrameReset();\n> >> +\n> >> + /* Used for focus scan. */\n> >\n> > If we're describing it, what is it? Is it the current focus position\n> > when scanning?\n> >\n> >> + uint32_t focus_;\n> >> + /* Focus good */\n> >\n> > Focus Good doesn't really describe a variable called good Focus. What\n> > gets put in it? Is it the last known good focus value?\n\nI'll revise this part.\n\n> >\n> >> + uint32_t goodFocus_;\n> >> + /* Recent AF statistic variance. */\n> >> + double currentVariance_;\n> >> + /* The frames to be ignore before starting measuring. */\n> >> + uint32_t ignoreCounter_;\n> >> + /* previous variance. it is used to determine the gradient */\n> >> + double previousVariance_;\n> >> + /* Max scan steps of each pass of AF scaning */\n> >> + uint32_t maxStep_;\n> >> + /* coarse scan stable. Complete low pass search (coarse) scan) */\n> >> + bool coarseCompleted_;\n> >> + /* fine scan stable. Complete high pass scan (fine scan) */\n> >> + bool fineCompleted_;\n> >> +};\n> >> +\n> >> +} /* namespace ipa::ipu3::algorithms */\n> >> +\n> >> +} /* namespace libcamera */\n> >> +\n> >> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */\n> >> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n> >> index 4db6ae1d..e1099169 100644\n> >> --- a/src/ipa/ipu3/algorithms/meson.build\n> >> +++ b/src/ipa/ipu3/algorithms/meson.build\n> >> @@ -1,8 +1,9 @@\n> >> # SPDX-License-Identifier: CC0-1.0\n> >>\n> >> ipu3_ipa_algorithms = files([\n> >> + 'af.cpp',\n> >> 'agc.cpp',\n> >> 'awb.cpp',\n> >> 'blc.cpp',\n> >> - 'tone_mapping.cpp',\n> >> + 'tone_mapping.cpp'\n> >\n> > We should keep the ',' at the end of tone_mapping.cpp so that we don't have to\n> > modify extra lines when appending to the list.\n\nOK\n\n> >\n> >> ])\n> >> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> >> index 86794ac1..ecb2b90a 100644\n> >> --- a/src/ipa/ipu3/ipa_context.cpp\n> >> +++ b/src/ipa/ipu3/ipa_context.cpp\n> >> @@ -67,6 +67,30 @@ namespace libcamera::ipa::ipu3 {\n> >> *\n> >> * \\var IPASessionConfiguration::grid.stride\n> >> * \\brief Number of cells on one line including the ImgU padding\n> >> + *\n> >\n> > No need for extra blank line\n> >\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var IPASessionConfiguration::af\n> >> + * \\brief AF grid configuration of the IPA\n> >> + *\n> >> + * \\var IPASessionConfiguration::af.afGrid\n> >> + *\n> >\n> > No need for extra blank line\n> >\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var IPAFrameContext::af\n> >> + * \\brief Context for the Automatic Focus algorithm\n> >> + *\n> >> + * \\struct IPAFrameContext::af\n> >> + * \\var IPAFrameContext::af.focus\n> >> + * \\brief Current position of the lens\n> >> + *\n> >> + * \\var IPAFrameContext::af.maxVariance\n> >> + * \\brief The maximum variance of the current image.\n> >> + *\n> >> + * \\var IPAFrameContext::af.stable\n> >> + * \\brief is the image focused?\n> >> */\n> >>\n> >> /**\n> >> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> >> index c6dc0814..60ad3194 100644\n> >> --- a/src/ipa/ipu3/ipa_context.h\n> >> +++ b/src/ipa/ipu3/ipa_context.h\n> >> @@ -25,6 +25,10 @@ struct IPASessionConfiguration {\n> >> uint32_t stride;\n> >> } grid;\n> >>\n> >> + struct {\n> >> + ipu3_uapi_grid_config afGrid;\n> >> + } af;\n> >> +\n> >> struct {\n> >> utils::Duration minShutterSpeed;\n> >> utils::Duration maxShutterSpeed;\n> >> @@ -34,6 +38,12 @@ struct IPASessionConfiguration {\n> >> };\n> >>\n> >> struct IPAFrameContext {\n> >> + struct {\n> >> + uint32_t focus;\n> >\n> > Is this the focus point/value of the current frame, or the desired value to\n> > set on the next frame?\n> >\n> >> + double maxVariance;\n> >> + bool stable;\n> >> + } af;\n> >> +\n> >> struct {\n> >> uint32_t exposure;\n> >> double gain;\n> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> >> index e44a31bb..417e0562 100644\n> >> --- a/src/ipa/ipu3/ipu3.cpp\n> >> +++ b/src/ipa/ipu3/ipu3.cpp\n> >> @@ -30,6 +30,7 @@\n> >>\n> >> #include \"libcamera/internal/mapped_framebuffer.h\"\n> >>\n> >> +#include \"algorithms/af.h\"\n> >> #include \"algorithms/agc.h\"\n> >> #include \"algorithms/algorithm.h\"\n> >> #include \"algorithms/awb.h\"\n> >> @@ -295,6 +296,7 @@ int IPAIPU3::init(const IPASettings &settings,\n> >> }\n> >>\n> >> /* Construct our Algorithms */\n> >> + algorithms_.push_back(std::make_unique());\n> >> algorithms_.push_back(std::make_unique());\n> >> algorithms_.push_back(std::make_unique());\n> >> algorithms_.push_back(std::make_unique());\n> >> --\n> >> 2.33.1\n> >>\n>","headers":{"Return-Path":"","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 E74C6BDCBF\n\tfor ;\n\tFri, 11 Feb 2022 06:42:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 13538610AD;\n\tFri, 11 Feb 2022 07:42:21 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F01EE60E58\n\tfor ;\n\tFri, 11 Feb 2022 07:42:19 +0100 (CET)","from mail-lj1-f200.google.com (mail-lj1-f200.google.com\n\t[209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id\n\tus-mta-124-jbSd8qYyPE6L62K388a0_w-1; Fri, 11 Feb 2022 01:42:14 -0500","by mail-lj1-f200.google.com with SMTP id\n\tw4-20020a05651c102400b0023d50aba238so3702330ljm.22\n\tfor ;\n\tThu, 10 Feb 2022 22:42:14 -0800 (PST)"],"Authentication-Results":["lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"ZuJIo91a\"; dkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hpa@redhat.com"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1644561738;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=zgBm2QJXgutktLCjZbqHKk2PFYmBeXshqZmdrIU7xC4=;\n\tb=ZuJIo91asmrv71KZfTIboNuuqUbZPFLhPY4frnblqxwqTWrj8CHcHe4dvVd4Puc2LH9Coq\n\tk4xFBV4yt58bDupkQWC3h3ThahfisMN706W556wildjFW1c3GMhXNIKWewYzn6sKY3wTEM\n\tLX94pjlH4nSVuafrVpapyGby9H/E+OY=","X-MC-Unique":"jbSd8qYyPE6L62K388a0_w-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=zgBm2QJXgutktLCjZbqHKk2PFYmBeXshqZmdrIU7xC4=;\n\tb=EgUoyR84nnq0mS9nvd0UIHUs5Stuqjxc3uPQKNze+qDGls4m1YDaA9Hwz9lUNxj5lA\n\t/EJP5PWashFN2p/PGsulVtayS7W5du4QSOejBGisxYjlJzoMY6ZdsP2rIeaqHp8PK6a+\n\tBeBKReof+PHAzCl01mskeNH2YBrEXwKhk4Mawd0CWDKT0v4SbfPkn5Kwz5ZgWXNdXkMv\n\t+D2JKsKU2LYvGdoBqYbfVdz+sueZ/LIfqR5tW38nJA2V57kXmcftsEycg6ZQfleU2INF\n\tXqlZP/z4tCtcVJ26S0M8Sn4xuejkqgHPhWqodL5gzy9JgElsmF8MEV8815yTlEwV9u6z\n\tgYFw==","X-Gm-Message-State":"AOAM531HimegJ6y4O3u3vPLHiXI6S7MIHYopZDeg1u9OWn/TdeSb3XF+\n\tWGUGnNQ/L5lV+VUX8KCR7m+50VL7brCNt/Sp1SUxSqrm9e4XAd0H2M2YxSVqAQI12L8ukk+z3QG\n\tN/p//Vv1jbXk4g2z5ScKCZYEbxHsTNJxPSp95oUg1KNk7YlYpcQ==","X-Received":["by 2002:a05:6512:11f2:: with SMTP id\n\tp18mr193020lfs.665.1644561732828; \n\tThu, 10 Feb 2022 22:42:12 -0800 (PST)","by 2002:a05:6512:11f2:: with SMTP id\n\tp18mr193000lfs.665.1644561732074; \n\tThu, 10 Feb 2022 22:42:12 -0800 (PST)"],"X-Google-Smtp-Source":"ABdhPJx4GqbFUAA9PMglgH+0mCEs9T/uH+cnXDFqheBoUjmKG0mLEjtC4GeK7k9K0nAPDQ25d9PFHkhcABO8LSINHPo=","MIME-Version":"1.0","References":"<20220210085759.71419-1-hpa@redhat.com>\n\t<164448777681.3354066.18018320193243523810@Monstersaurus>\n\t<53cd384f-6af6-c550-436e-4358154a73ac@ideasonboard.com>","In-Reply-To":"<53cd384f-6af6-c550-436e-4358154a73ac@ideasonboard.com>","From":"Kate Hsuan ","Date":"Fri, 11 Feb 2022 14:42:00 +0800","Message-ID":"","To":"Jean-Michel Hautbois ","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v7] ipa: ipu3: af: Auto focus for\n\tdw9719 Surface Go2 VCM","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]