[libcamera-devel] [RFC v4 1/1] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Fri Dec 31 14:13:29 CET 2021
Hi Kate,
On 31/12/2021 12:02, Kate Hsuan wrote:
> Hi Jean-Michel,
>
> On Mon, Dec 27, 2021 at 8:55 PM Jean-Michel Hautbois
> <jeanmichel.hautbois at ideasonboard.com> wrote:
>>
>> 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)
>>
>
> Please check out the results from the URL below.
>
> https://drive.google.com/file/d/1jePzgsw0zwO0EtDnLBrOaCKucx4QWLIg/view?usp=sharing
>
> Y1 became a black image and was difficult to determine the coarse focus value.
>
Sounds surprising... Could you share the way you are generating the
images of Y1 and Y2 BTW ?
> Happy new year :)
Best wishes, hoping for a far away Covid next year !
>
>>>>
>>>> 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