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