[libcamera-devel] [RFC v4 1/1] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Mon Dec 27 13:54:59 CET 2021
Hi Kate !
On 27/12/2021 10:54, Kate Hsuan wrote:
> Hi Jean-Michel,
>
> On Sat, Dec 18, 2021 at 12:18 AM Jean-Michel Hautbois
> <jeanmichel.hautbois at ideasonboard.com> wrote:
>>
>> Hi Kate,
>>
>> On 17/12/2021 09:57, Kate Hsuan wrote:
>>> "Hi Jean-Michel,
>>>
>>> Thank you for your review.
>>>
>>> On Thu, Dec 16, 2021 at 11:32 PM Jean-Michel Hautbois
>>> <jeanmichel.hautbois at ideasonboard.com> wrote:
>>>>
>>>> Hi Kate,
>>>>
>>>> Thanks for the patch :-) !
>>>>
>>>> I have a few comments below, and I have to tell you I couldn't make it
>>>> work here... I don't know how you are testing it ? I can see the lens
>>>> position changing but it passes over the correct focus and never locks
>>>> on it.
>>>>
>>>> On 13/12/2021 13:28, Kate Hsuan wrote:
>>>>> Since VCM for surface Go 2 (dw9719) had been successfully
>>>>> driven, this Af module can be used to control the VCM and
>>>>> determine the focus value based on the IPU3 AF state.
>>>>>
>>>>> The variance of each focus step is determined and a greedy
>>>>> approah is used to find the maximum variance of the AF
>>>>> state and a appropriate focus value.
>>>>>
>>>>> Changes in v2:
>>>>> 1. Add grid configuration interface.
>>>>> 2. Add AF accelerator configuration.
>>>>> 3. Move default focus area to the center of the image.
>>>>>
>>>>> Changes in v3:
>>>>> 1. Squash Daniel's commit- Remove v4l2 interaction from AF
>>>>> algorithm.
>>>>> 2. Fix greedy AF algorithm since V4l2 interface had been
>>>>> removed.
>>>>> 3. Simplify default grid config for AF.
>>>>> 4. AF start area is determined by bdsOutputSize.
>>>>>
>>>>> Changes in v4:
>>>>> In v4, it significant improves the AF scan time using a two pass
>>>>> scaning method and the AF scan will be terminated when it finds
>>>>> a negative gradient.
>>>>> 1. Introduce 2 pass AF scan (coarse and fine scan) to increase
>>>>> the AF success rate.
>>>>> 2. The low pass filter convolution results are used to determine
>>>>> a coarse AF result.
>>>>> 3. The high pass convolution result is used to find a good lens
>>>>> step in a given AF range determined in pass 1.
>>>>>
>>>>
>>>> The lines above describe the changes between the versions, which is
>>>> great ! But those should go after the "---" as we don't want those in
>>>> the final commit message.
>>>
>>> Ok. Thank you I'll move the changes after "---".
>>>
>>>>
>>>>> Signed-off-by: Kate Hsuan <hpa at redhat.com>
>>>>> ---
>>>>
>>>> Changes in ...
>>>>
>>>> ---
>>>>> src/ipa/ipu3/algorithms/af.cpp | 334 ++++++++++++++++++++++++++++
>>>>> src/ipa/ipu3/algorithms/af.h | 66 ++++++
>>>>> src/ipa/ipu3/algorithms/meson.build | 3 +-
>>>>> src/ipa/ipu3/ipa_context.cpp | 27 +++
>>>>> src/ipa/ipu3/ipa_context.h | 11 +
>>>>> src/ipa/ipu3/ipu3.cpp | 2 +
>>>>> 6 files changed, 442 insertions(+), 1 deletion(-)
>>>>> create mode 100644 src/ipa/ipu3/algorithms/af.cpp
>>>>> create mode 100644 src/ipa/ipu3/algorithms/af.h
>>>>>
>>>>
>>>> I applied your patch, and I have compilation issues:
>>>> ../src/ipa/ipu3/algorithms/af.cpp:89:1: error: missing initializer for
>>>> member ‘ipu3_uapi_af_config_s::padding’ [-Werror=missing-field-initializers]
>>>> 89 | };
>>>> | ^
>>>> ../src/ipa/ipu3/algorithms/af.cpp:89:1: error: missing initializer for
>>>> member ‘ipu3_uapi_grid_config::height_per_slice’
>>>> [-Werror=missing-field-initializers]
>>>> ../src/ipa/ipu3/algorithms/af.cpp:89:1: error: missing initializer for
>>>> member ‘ipu3_uapi_grid_config::x_end’ [-Werror=missing-field-initializers]
>>>> ../src/ipa/ipu3/algorithms/af.cpp:89:1: error: missing initializer for
>>>> member ‘ipu3_uapi_grid_config::y_end’ [-Werror=missing-field-initializers]
>>>> ../src/ipa/ipu3/algorithms/af.cpp: In member function ‘virtual void
>>>> libcamera::ipa::ipu3::algorithms::Af::process(libcamera::ipa::ipu3::IPAContext&,
>>>> const ipu3_uapi_stats_3a*)’:
>>>> ../src/ipa/ipu3/algorithms/af.cpp:265:50: error: taking address of
>>>> packed member of ‘ipu3_uapi_stats_3a’ may result in an unaligned pointer
>>>> value [-Werror=address-of-packed-member]
>>>> 265 | y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
>>>>
>>>> I solved them locally (with a quick and dirty patch) how did you not
>>>> have those ?
>>>
>>> It was my fault. I can build them successfully before. But, after
>>> re-cloning the git repo and patching my v4 patch, it happened. I'll
>>> fix those issues.
>>> Sorry about the inconvenience. T_T
>>
>> No problem, but it was surprising :-).
>>
>>>
>>>>
>>>>> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
>>>>> new file mode 100644
>>>>> index 00000000..620be9fa
>>>>> --- /dev/null
>>>>> +++ b/src/ipa/ipu3/algorithms/af.cpp
>>>>> @@ -0,0 +1,334 @@
>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>>>> +/*
>>>>> + * Copyright (C) 2021, Red Hat
>>>>> + *
>>>>> + * af.cpp - IPU3 auto focus control
>>>>> + */
>>>>> +
>>>>> +#include "af.h"
>>>>> +
>>>>> +#include <algorithm>
>>>>> +#include <chrono>
>>>>> +#include <cmath>
>>>>> +#include <fcntl.h>
>>>>> +#include <numeric>
>>>>> +#include <sys/ioctl.h>
>>>>> +#include <sys/stat.h>
>>>>> +#include <sys/types.h>
>>>>> +#include <unistd.h>
>>>>> +
>>>>> +#include <linux/videodev2.h>
>>>>> +
>>>>> +#include <libcamera/base/log.h>
>>>>> +
>>>>> +#include <libcamera/ipa/core_ipa_interface.h>
>>>>> +
>>>>> +#include "libipa/histogram.h"
>>>>> +
>>>>> +/**
>>>>> + * \file af.h
>>>>> + */
>>>>> +
>>>>> +namespace libcamera {
>>>>> +
>>>>> +using namespace std::literals::chrono_literals;
>>>>> +
>>>>> +namespace ipa::ipu3::algorithms {
>>>>> +
>>>>> +/**
>>>>> + * \class Af
>>>>> + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
>>>>> + *
>>>>> + * This algorithm is used to determine the position of the lens and get a
>>>>> + * focused image. The IPU3 AF accelerator computes the statistics, composed
>>>>> + * by high pass and low pass filtered value and stores in a AF buffer.
>>>>> + * Typically, for a focused image, it has relative high contrast than a
>>>>> + * blurred image, i.e. an out of focus image. Therefore, if an image with the
>>>>> + * highest contrast can be found from the AF scan, the lens' position is the
>>>>> + * best step of the focus.
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +LOG_DEFINE_CATEGORY(IPU3Af)
>>>>> +
>>>>> +/**
>>>>> + * Maximum focus value of the VCM control
>>>>> + * \todo should be obtained from the VCM driver
>>>>> + */
>>>>> +static constexpr uint32_t MaxFocusSteps_ = 1023;
>>>>> +
>>>>> +/* minimum focus step for searching appropriate focus*/
>>>>> +static constexpr uint32_t coarseSearchStep_ = 10;
>>>>> +static constexpr uint32_t fineSearchStep_ = 1;
>>>>> +
>>>>> +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
>>>>> +static constexpr double MaxChange_ = 0.8;
>>>>
>>>> Why 0.8 ? Any explanation on how to set it ?
>>>
>>> OK. I'll put my explanation here. Basically, it is used to represent
>>> the boundary of a focused and blurred image.
>>>
>>>
>>>>
>>>>> +
>>>>> +/* settings for Auto Focus from the kernel */
>>>>> +static struct ipu3_uapi_af_config_s imgu_css_af_defaults = {
>>>>> + .filter_config = {
>>>>> + { 0, 0, 0, 0 },
>>>>> + { 0, 0, 0, 0 },
>>>>> + { 0, 0, 0, 128 },
>>>>> + 0,
>>>>> + { 0, 0, 0, 0 },
>>>>> + { 0, 0, 0, 0 },
>>>>> + { 0, 0, 0, 128 },
>>>>> + 0,
>>>>> + .y_calc = { 8, 8, 8, 8 },
>>>>> + .nf = { 0, 7, 0, 7, 0 },
>>>>> + },
>>
>> I would like your thinking here (and perhaps someone else). It looks
>> like the ImgU implements a Gabor filter, but I can't see all the
>> parameters I would expect from it. Putting it down, I can't see with
>> this default filter how y1 and y2 could represent low-pass and high
>> pass, as the filters are the same. But, it could be a horizontal and a
>> vertical filter instead, with y1 the horizontal and y2 the vertical
>> resulting filtered values.
>>
>> It would be very interesting to display the y1_avg and y2_avg values as
>> an image, I just don't have lots of time for it. We have 16 bits values,
>> so we could dump those into a binary file and use python to display the
>> values as a raw gray image or something similar.
>
> You could check out the URL below to see the grey scale image of the AF buffer.
> https://drive.google.com/file/d/15rrfeRKA1hgGngzRRk-a9Uzhf8tYEU1T/view?usp=sharing
>
> Since the result is a tiny 16x16 image, all the detail of the original
> image was compressed into a black block.
>
Nice :-).
As expected, as y1 and y2 are the same filter, images are the same.
Could you test those values ?
y1 =>
(0, 1, 3, 7
11, 13, 1, 2
8, 19, 34, 242)
vector: 7fdffbfe - normalization factor: 9
y2 =>
(0, 1, 6, 6
13, 25, 3, 0
25, 3, 117, 254)
vector: 4e53ca72 - normalization factor: 9
Channels coefficients: (gr: 8, r: 8, gb: 8, b: 8)
>>
>> The other things which I find annoying is the non-reliable values I get
>> right now for the variance. Same scene, exposure and gain fixed to a
>> value, controled luminosity, and between two executions I might not have
>> the same result...
>>
>>>>> + .grid_cfg = {
>>>>> + .width = 16,
>>>>> + .height = 16,
>>>>> + .block_width_log2 = 3,
>>>>> + .block_height_log2 = 3,
>>>>> + .x_start = 10,
>>>>> + .y_start = 2 | IPU3_UAPI_GRID_Y_START_EN,
>>>>> + },
>>>>> +};
>>>>
>>>> As mentionned, you missed some fields:
>>>
>>> I'll fix this. Thank you.
>>>
>>>>
>>>> '''
>>>> @@ -78,13 +78,17 @@ static struct ipu3_uapi_af_config_s
>>>> imgu_css_af_defaults = {
>>>> .y_calc = { 8, 8, 8, 8 },
>>>> .nf = { 0, 7, 0, 7, 0 },
>>>> },
>>>> + .padding = { 0, 0, 0, 0 },
>>>> .grid_cfg = {
>>>> .width = 16,
>>>> .height = 16,
>>>> .block_width_log2 = 3,
>>>> .block_height_log2 = 3,
>>>> + .height_per_slice = 1,
>>>> .x_start = 10,
>>>> .y_start = 2 | IPU3_UAPI_GRID_Y_START_EN,
>>>> + .x_end = 138,
>>>> + .y_end = 130,
>>>> },
>>>> };
>>>> '''
>>>>
>>>>> +
>>>>> +Af::Af()
>>>>> + : focus_(0), goodFocus_(0), currentVariance_(0.0), previousVariance_(0.0),
>>>>> + pass1Done_(false), pass2Done_(false)
>>>>> +{
>>>>> + maxStep_ = MaxFocusSteps_;
>>>>> +}
>>>>> +
>>>>> +Af::~Af()
>>>>> +{
>>>>> +}
>>>>> +
>>>>> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
>>>>
>>>> Missing documentation for prepare() => you should build the doc ;-)
>>>
>>> Ok. I'll add the prepare() document here.
>>>
>>>>
>>>>> +{
>>>>> + params->use.acc_af = 1;
>>>>> + params->acc_param.af = imgu_css_af_defaults;
>>>>> + params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
>>>>> + params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;
>>>>
>>>> You are never changing the grid size, is it intended ?
>>>
>>> Do you mean "ipu3_uapi_grid_config"? Could you please explain this
>>> more detail? I can modify this according to your opinions.
>>
>> Well, yes, you are always using the default 128x128 grid size. But you
>> can go from 16*2^3 to 32*2^6 in width, and from 16*2^3 to 24*2^6 in
>> height. Thus, it means we could evaluate the scene on a small part or
>> almost all of it (copying the bds grid is not doable as it might have
>> larger block width, up to 80x60).
>>
>> See:
>> https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h#32
>>
>> Note:
>> https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h#74
>>
>> => It makes me wonder if the filtering is fast, or if it may take some
>> time, meaning that focus evaluation in one frame could be done only at
>> frame+2 or something like that.
>>
>> Could you verify that the focus estimated is in the center of the scene
>> right now, as it is what you have configured ?
>>
>>>
>>>>
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * \brief Configure the Af given a configInfo
>>>>> + * \param[in] context The shared IPA context
>>>>> + * \param[in] configInfo The IPA configuration data
>>>>> + *
>>>>> + * \return 0
>>>>> + */
>>>>> +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>>>>> +{
>>>>> + /* determined focus value i.e. current focus value */
>>>>> + context.frameContext.af.focus = 0;
>>>>> + /* maximum variance of the AF statistics */
>>>>> + context.frameContext.af.maxVariance = 0;
>>>> double type, maybe s/= 0/= 0.0/ ?
>>>
>>> 0.0 is correct. I'll correct this thank you.
>>>
>>>>
>>>>> + /* is focused? if it is true, the AF should be in a stable state. */
>>>>> + context.frameContext.af.stable = false;
>>>>> + /* frame to be ignored before start to estimate AF variance. */
>>>>> + ignoreFrame_ = 10;
>>>>
>>>> Could be a constexpr ?
>>>
>>> It is a counter to ignore frames since we have to ignore some frames
>>> to wait for the value from the accelerator to be stable.
>>> If this should be a constant variable, I can modify this. :)
>>>
>>>
>>>>
>>>>> +
>>>>> + /*
>>>>> + * AF default area configuration
>>>>> + * Move AF area to the center of the image.
>>>>> + */
>>>>> + /* Default AF width is 16x8 = 128 */
>>>>> + context.configuration.af.start_x = (configInfo.bdsOutputSize.width / 2) - 64;
>>>>
>>>> Why are you using a fixed value for the grid ? You could use the value
>>>> stored in imgu_css_af_defaults directly instead of applying 128/2 manualy ?
>>>>
>>>>> + context.configuration.af.start_y = (configInfo.bdsOutputSize.height / 2) - 64;
>>>>> +
>>>>> + LOG(IPU3Af, Debug) << "BDS X: "
>>>>> + << configInfo.bdsOutputSize.width
>>>>> + << " Y: "
>>>>> + << configInfo.bdsOutputSize.height;
>>>>> + LOG(IPU3Af, Debug) << "AF start from X: "
>>>>> + << context.configuration.af.start_x
>>>>> + << " Y: "
>>>>> + << context.configuration.af.start_y;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * \brief AF coarse scan
>>>>> + * \param[in] context The shared IPA context
>>>>> + *
>>>>> + */
>>>>> +void Af::af_coarse_scan(IPAContext &context)
>>>>
>>>> Bad naming (camelCase only). Same below.
>>>
>>> Ops, sure, I'll modify this.
>>>
>>>>
>>>>> +{
>>>>> + if (pass1Done_ == true)
>>>>> + return;
>>>>> +
>>>>> + if (af_scan(context, coarseSearchStep_)) {
>>>>> + pass1Done_ = true;
>>>>> + context.frameContext.af.maxVariance = 0;
>>>>> + focus_ = context.frameContext.af.focus - (context.frameContext.af.focus * 0.1);
>>>>> + context.frameContext.af.focus = focus_;
>>>>> + previousVariance_ = 0;
>>>>> + maxStep_ = focus_ + (focus_ * 0.2);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * \brief AF fine scan
>>>>> + * \param[in] context The shared IPA context
>>>>> + *
>>>>> + */
>>>>> +void Af::af_fine_scan(IPAContext &context)
>>>>> +{
>>>>> + if (pass1Done_ != true)
>>>>> + return;
>>>>> +
>>>>> + if (af_scan(context, fineSearchStep_)) {
>>>>> + context.frameContext.af.stable = true;
>>>>> + pass2Done_ = true;
>>>>> + }
>>>>> +}
>>>> I am not a big fan of these pass1Done_ and pass2Done_ variables, let's
>>>> seee where it is used...
>>>
>>> Since we used two stage AF searching strategy I simply say this is a
>>> two pass algorithm :)
>>> I'll renaming this.
>>>
>>>>
>>>>> +
>>>>> +/**
>>>>> + * \brief AF reset
>>>>> + * \param[in] context The shared IPA context
>>>>> + *
>>>>> + */
>>>>> +void Af::af_reset(IPAContext &context)
>>>>> +{
>>>>> + context.frameContext.af.maxVariance = 0;
>>>>> + context.frameContext.af.focus = 0;
>>>>> + focus_ = 0;
>>>>> + context.frameContext.af.stable = false;
>>>>> + ignoreFrame_ = 60;
>>>>
>>>> Why 60 after reset and 10 at configure ? It seems to be a lot ?
>>>
>>> We can tweak this value.
>>>
>>>>
>>>>> + previousVariance_ = 0.0;
>>>>> + pass1Done_ = false;
>>>>> + pass2Done_ = false;
>>>>> + maxStep_ = MaxFocusSteps_;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * \brief AF scan
>>>>> + * \param[in] context The shared IPA context
>>>>> + *
>>>>> + * \return True, if it finds a AF value.
>>>>> + */
>>>>> +bool Af::af_scan(IPAContext &context, int min_step)
>>>>> +{
>>>>> + /* find the maximum variance during the AF scan using a greedy strategy */
>>>>> + if (currentVariance_ > context.frameContext.af.maxVariance) {
>>>>> + context.frameContext.af.maxVariance = currentVariance_;
>>>>> + goodFocus_ = focus_;
>>>>> + }
>>>>> +
>>>>> + if (focus_ > maxStep_) {
>>>>> + /* if reach the max step, move lens to the position and set "focus stable". */
>>>>> + context.frameContext.af.focus = goodFocus_;
>>>>> + return true;
>>>>> + } else {
>>>>> + /* check negative gradient */
>>>>> + if ((currentVariance_ - context.frameContext.af.maxVariance) > -(context.frameContext.af.maxVariance * 0.15)) {
>>>>
>>>> Where is the 0.15 coming from ?
>>>
>>> After some testing, I think that If the negative gradient ismore than
>>> 15% maxVaraiance, the focused image and lens position we get!
>>> I could put this as a constant variable in my v5 patch.
>>>
>>>>
>>>>> + focus_ += min_step;
>>>>> + context.frameContext.af.focus = focus_;
>>>>> + } else {
>>>>> + context.frameContext.af.focus = goodFocus_;
>>>>> + previousVariance_ = currentVariance_;
>>>>> + return true;
>>>>> + }
>>>>> + }
>>>>> + LOG(IPU3Af, Debug) << "Variance prevrious: "
>>>> s/prevrious/previous/
>>>
>>> Ops. sorry about the typo. T_T
>>>
>>>>
>>>>> + << previousVariance_
>>>>> + << " current: "
>>>>> + << currentVariance_
>>>>> + << " Diff: "
>>>>> + << (currentVariance_ - context.frameContext.af.maxVariance);
>>>>
>>>> Interestingly, it seems there is an issue here, according to the log I get ?
>>>>
>>>> [0:58:23.867136346] [6811] DEBUG IPU3Af af.cpp:238 Variance prevrious: 0
>>>> current: 20012.4 Diff: 0
>>>>
>>>> Shouldn't diff be 20012.4 ?
>>>>
>>>> I am not sure the algorithm is very strong though... You may never
>>>> converge and stay in a local minimum without knowing ?
>>>>
>>>>> + previousVariance_ = currentVariance_;
>>>>> + LOG(IPU3Af, Debug) << "Focus searching max variance is: "
>>>>> + << context.frameContext.af.maxVariance
>>>>> + << " Focus step is "
>>>>> + << goodFocus_
>>>>> + << " Current scan is "
>>>>> + << focus_;
>>>>> + return false;
>>>>> +}
>>>>
>>>> I am not sure to understand how this function should behave... Is it a
>>>> minimum searching ? Are you looking for argmin(currentVariance -
>>>> maxVariance) ?
>>>
>>> Actually. it is a maximum search. I try to search for the maximum
>>> variance of the image.
>>> If the difference is positive, it means the variance still increases.
>>> We can continue search for the maximum value and update maxVariance_.
>>> Until, we found a negative difference which means the variance starts
>>> to go down and image will become blurred.
>>>
>>>
>>>>
>>>>> +
>>>>> +/**
>>>>> + * \brief Determine the max contrast image and lens position. y_table is the
>>>>> + * statictic data from IPU3 and is composed of low pass and high pass filtered
>>>>> + * value. High pass filtered value also represents the sharpness of the image.
>>>>> + * Based on this, if the image with highest variance of the high pass filtered
>>>>> + * value (contrast) during the AF scan, the position of the len should be the
>>>>> + * best focus.
>>>>> + * \param[in] context The shared IPA context.
>>>>> + * \param[in] stats The statistic buffer of 3A from the IPU3.
>>>>> + */
>>>>> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>>>>> +{
>>>>> + uint32_t total = 0;
>>>>> + double mean;
>>>>> + uint64_t var_sum = 0;
>>>>> + y_table_item_t *y_item;
>>>>> + int z = 0;
>>>>> +
>>>>> + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
>>>>> +
>>>>> + /**
>>>>> + * Calculate the mean and the varience of each non-zero AF statistics, since IPU3 only determine the AF value
>>>>> + * for a given grid.
>>>>> + * For pass1: low pass results are used.
>>>>> + * For pass2: high pass results are used.
>>>>> + */
>>>>> + if (pass1Done_) {
>>>>> + for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
>>>>> + total = total + y_item[z].y2_avg;
>>>>> + if (y_item[z].y2_avg == 0)
>>>>> + break;
>>>>> + }
>>>>> + mean = total / z;
>>>>> +
>>>>> + for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4 && y_item[z].y2_avg != 0; z++) {
>>>>> + var_sum = var_sum + ((y_item[z].y2_avg - mean) * (y_item[z].y2_avg - mean));
>>>>> + if (y_item[z].y2_avg == 0)
>>>>> + break;
>>>>> + }
>>>>> + } else {
>>>>
>>>> /* Calculate the mean of the low pass filter values. */
>>>>
>>>>> + for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
>>>>> + total = total + y_item[z].y1_avg;
>>>>> + if (y_item[z].y1_avg == 0)
>>>>> + break;
>>>> Are you sure of that ? Shouldn't you use the grid you set to parse the
>>>> right number of data ? If you have a local dark area you may stop even
>>>> if it is not yet the end of the grid ?
>>>
>>> I'll find another way to determine the range of the results. May be it
>>> can based on the grids setting.
>>>
>>>>
>>>>> + }
>>>>> + mean = total / z;
>>>>> +
>>>>
>>>> /* Calulate the deviation from the mean. */
>>>>
>>>>> + for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4 && y_item[z].y1_avg != 0; z++) {
>>>>> + var_sum = var_sum + ((y_item[z].y1_avg - mean) * (y_item[z].y1_avg - mean));
>>>>> + if (y_item[z].y1_avg == 0)
>>>>> + break;
>>>>> + }
>>>>> + }
>>>>
>>>> The two if() blocks are very similar, the only exception beeing the item
>>>> of the table used (y_item[z].y1_avg in the first pass, y_item[z].y2_avg
>>>> in the second one). This is not easy to read (according to me).
>>>
>>> They are high pass (y2) and low pass (y1) filtered convolution
>>> results. This structure referees from ipu-ipa repo and I would like to
>>> keep their original naming.
>>>
>>>
>>>>
>>>>> + /* Determine the average variance of the frame. */
>>>>> + currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
>>>>
>>>> Side note:
>>>> If my stats are still correct, I think it should be divided by (z-1) and
>>>> not z, as we have a sample and not a full population ?
>>>
>>> it is z-1 and my fault. I'll correct this. :)
>>>
>>>>
>>>>> + LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
>>>>> +
>>>>> + if (context.frameContext.af.stable == true) {
>>>>> + const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
>>>>> + const double var_ratio = diff_var / context.frameContext.af.maxVariance;
>>>>> + LOG(IPU3Af, Debug) << "Change ratio: "
>>>>> + << var_ratio
>>>>> + << " current focus: "
>>>>> + << context.frameContext.af.focus;
>>>>> + /**
>>>>> + * If the change ratio of contrast is over Maxchange_ (out of focus),
>>>>> + * trigger AF again.
>>>>> + */
>>>>> + if (var_ratio > MaxChange_) {
>>>>> + if (ignoreFrame_ == 0) {
>>>>> + af_reset(context);
>>>>> + } else
>>>>> + ignoreFrame_--;
>>>>> + } else
>>>>> + ignoreFrame_ = 10;
>>>>> + } else {
>>>>> + if (ignoreFrame_ != 0)
>>>>> + ignoreFrame_--;
>>>>> + else {
>>>>> + af_coarse_scan(context);
>>>>> + af_fine_scan(context);
>>>>> + }
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +} /* namespace ipa::ipu3::algorithms */
>>>>> +
>>>>> +} /* namespace libcamera */
>>>>> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
>>>>> new file mode 100644
>>>>> index 00000000..b9295b19
>>>>> --- /dev/null
>>>>> +++ b/src/ipa/ipu3/algorithms/af.h
>>>>> @@ -0,0 +1,66 @@
>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>>>> +/*
>>>>> + * Copyright (C) 2021, Red Hat
>>>>> + *
>>>>> + * af.h - IPU3 Af control
>>>>> + */
>>>>> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AF_H__
>>>>> +#define __LIBCAMERA_IPU3_ALGORITHMS_AF_H__
>>>>> +
>>>>> +#include <linux/intel-ipu3.h>
>>>>> +
>>>>> +#include <libcamera/base/utils.h>
>>>>> +
>>>>> +#include <libcamera/geometry.h>
>>>>> +
>>>>> +#include "algorithm.h"
>>>>> +
>>>>> +namespace libcamera {
>>>>> +
>>>>> +namespace ipa::ipu3::algorithms {
>>>>> +
>>>>> +class Af : public Algorithm
>>>>> +{
>>>>> + /* The format of y_table. From ipu3-ipa repo */
>>>>> + typedef struct y_table_item {
>>>>> + uint16_t y1_avg;
>>>>> + uint16_t y2_avg;
>>>>> + } y_table_item_t;
>>>>> +
>>>>> +public:
>>>>> + Af();
>>>>> + ~Af();
>>>>> +
>>>>> + void prepare(IPAContext &context, ipu3_uapi_params *params) override;
>>>>> + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>>>>> + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
>>>>> +
>>>>> +private:
>>>>> + void af_coarse_scan(IPAContext &context);
>>>>> + void af_fine_scan(IPAContext &context);
>>>>> + bool af_scan(IPAContext &context, int min_step);
>>>>> + void af_reset(IPAContext &context);
>>>>> +
>>>>> + /* Used for focus scan. */
>>>>> + uint32_t focus_;
>>>>> + /* Focus good */
>>>>> + uint32_t goodFocus_;
>>>>> + /* Recent AF statistic variance. */
>>>>> + double currentVariance_;
>>>>> + /* The frames to be ignore before starting measuring. */
>>>>> + uint32_t ignoreFrame_;
>>>>> + /* previous variance. it is used to determine the gradient */
>>>>> + double previousVariance_;
>>>>> + /* Max scan steps of each pass of AF scaning */
>>>>> + uint32_t maxStep_;
>>>>> + /* Pass 1 stable. Complete low pass search (coarse) scan) */
>>>>> + bool pass1Done_;
>>>>> + /* Pass 2 stable. Complete high pass scan (fine scan) */
>>>>> + bool pass2Done_;
>>>>> +};
>>>>> +
>>>>> +} /* namespace ipa::ipu3::algorithms */
>>>>> +
>>>>> +} /* namespace libcamera */
>>>>> +
>>>>> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
>>>>> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
>>>>> index 4db6ae1d..e1099169 100644
>>>>> --- a/src/ipa/ipu3/algorithms/meson.build
>>>>> +++ b/src/ipa/ipu3/algorithms/meson.build
>>>>> @@ -1,8 +1,9 @@
>>>>> # SPDX-License-Identifier: CC0-1.0
>>>>>
>>>>> ipu3_ipa_algorithms = files([
>>>>> + 'af.cpp',
>>>>> 'agc.cpp',
>>>>> 'awb.cpp',
>>>>> 'blc.cpp',
>>>>> - 'tone_mapping.cpp',
>>>>> + 'tone_mapping.cpp'
>>>>> ])
>>>>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
>>>>> index 86794ac1..ee644d3c 100644
>>>>> --- a/src/ipa/ipu3/ipa_context.cpp
>>>>> +++ b/src/ipa/ipu3/ipa_context.cpp
>>>>> @@ -67,6 +67,33 @@ namespace libcamera::ipa::ipu3 {
>>>>> *
>>>>> * \var IPASessionConfiguration::grid.stride
>>>>> * \brief Number of cells on one line including the ImgU padding
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +/**
>>>>> + * \var IPASessionConfiguration::af
>>>>> + * \brief AF parameters configuration of the IPA
>>>>> + *
>>>>> + * \var IPASessionConfiguration::af.start_x
>>>>> + * \brief The start X position of the AF area
>>>>> + *
>>>>> + * \var IPASessionConfiguration::af.start_y
>>>>> + * \brief The start Y position of the AF area
>>>>> + */
>>>>> +
>>>>> +/**
>>>>> + * \var IPAFrameContext::af
>>>>> + * \brief Context for the Automatic Focus algorithm
>>>>> + *
>>>>> + * \struct IPAFrameContext::af
>>>>> + * \var IPAFrameContext::af.focus
>>>>> + * \brief Current position of the lens
>>>>> + *
>>>>> + * \var IPAFrameContext::af.maxVariance
>>>>> + * \brief The maximum variance of the current image.
>>>>> + *
>>>>> + * \var IPAFrameContext::af.stable
>>>>> + * \brief is the image focused?
>>>>> */
>>>>>
>>>>> /**
>>>>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>>>>> index c6dc0814..aa5bf97f 100644
>>>>> --- a/src/ipa/ipu3/ipa_context.h
>>>>> +++ b/src/ipa/ipu3/ipa_context.h
>>>>> @@ -31,6 +31,11 @@ struct IPASessionConfiguration {
>>>>> double minAnalogueGain;
>>>>> double maxAnalogueGain;
>>>>> } agc;
>>>>> +
>>>>> + struct {
>>>>> + uint16_t start_x;
>>>>> + uint16_t start_y;
>>>>> + } af;
>>>>> };
>>>>>
>>>>> struct IPAFrameContext {
>>>>> @@ -49,6 +54,12 @@ struct IPAFrameContext {
>>>>> double temperatureK;
>>>>> } awb;
>>>>>
>>>>> + struct {
>>>>> + uint32_t focus;
>>>>> + double maxVariance;
>>>>> + bool stable;
>>>>> + } af;
>>>>> +
>>>>> struct {
>>>>> uint32_t exposure;
>>>>> double gain;
>>>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>>>>> index 3d307708..93966c6f 100644
>>>>> --- a/src/ipa/ipu3/ipu3.cpp
>>>>> +++ b/src/ipa/ipu3/ipu3.cpp
>>>>> @@ -30,6 +30,7 @@
>>>>>
>>>>> #include "libcamera/internal/mapped_framebuffer.h"
>>>>>
>>>>> +#include "algorithms/af.h"
>>>>> #include "algorithms/agc.h"
>>>>> #include "algorithms/algorithm.h"
>>>>> #include "algorithms/awb.h"
>>>>> @@ -294,6 +295,7 @@ int IPAIPU3::init(const IPASettings &settings,
>>>>> }
>>>>>
>>>>> /* Construct our Algorithms */
>>>>> + algorithms_.push_back(std::make_unique<algorithms::Af>());
>>>>> algorithms_.push_back(std::make_unique<algorithms::Agc>());
>>>>> algorithms_.push_back(std::make_unique<algorithms::Awb>());
>>>>> algorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());
>>>>>
>>>>
>>>
>>> Thank you :)
>>>
>>
>
>
More information about the libcamera-devel
mailing list