[libcamera-devel] [RFC v4 1/1] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Kate Hsuan
hpa at redhat.com
Fri Dec 31 12:02:14 CET 2021
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.
Happy new 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 :)
> >>>
> >>
> >
> >
>
--
BR,
Kate
More information about the libcamera-devel
mailing list