[libcamera-devel] [RFC v4 1/1] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Kate Hsuan
hpa at redhat.com
Wed Jan 5 08:13:01 CET 2022
Hi Jean-Michel
On Fri, Dec 31, 2021 at 9:13 PM Jean-Michel Hautbois
<jeanmichel.hautbois at ideasonboard.com> wrote:
>
> 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 !
I hope too :)
After some checks, I made some mistakes to convert the image to png file.
Check out the following URL for the new results.
https://drive.google.com/file/d/1o4EcY9EsYrBYu5YTZ-_tlxP8sevDd3QY/view?usp=sharing
But, for the Y2, the result becomes very bright and sometimes the
image will be all white. Are the results right?
My python code for converting the pixels to tiff is shown below.
=========
import numpy as np
from scipy import ndimage, misc
import imageio
from PIL import Image
import sys
rawfile = np.fromfile(sys.argv[1], "uint16")
rawfile.shape = (16,16)
new_image = Image.fromarray(rawfile, mode='I;16')
new_image.convert('L').save('{}.tiff'.format(sys.argv[1][:-4]))
==========
>
> >
> >>>>
> >>>> 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