| Message ID | 20211115054400.17797-1-hpa@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Kate, Thanks for the patch ! On 15/11/2021 06:44, 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 s/approah/approach > state and a appropriate focus value. > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ > src/ipa/ipu3/algorithms/af.h | 48 ++++++++ > src/ipa/ipu3/algorithms/meson.build | 3 +- > src/ipa/ipu3/ipa_context.h | 6 + > src/ipa/ipu3/ipu3.cpp | 2 + > 5 files changed, 223 insertions(+), 1 deletion(-) > create mode 100644 src/ipa/ipu3/algorithms/af.cpp > create mode 100644 src/ipa/ipu3/algorithms/af.h > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > new file mode 100644 > index 00000000..c276b539 > --- /dev/null > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -0,0 +1,165 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Red Hat > + * > + * af.cpp - IPU3 Af control > + */ > + > +#include "af.h" > + > +#include <algorithm> > +#include <chrono> > +#include <cmath> > +#include <numeric> > + > +#include <linux/videodev2.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <fcntl.h> > +#include <sys/ioctl.h> > +#include <unistd.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 { > + > +LOG_DEFINE_CATEGORY(IPU3Af) > + > +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) > +{ > + /* For surface Go 2 back camera VCM */ > + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); > +} IPA algorithms should not know anything about V4L2 controls, subdev, etc. I know some discussion is running about this in "Fwd: Surface Go VCM type (was: Need to pass acpi_enforce_resources=lax on the Surface Go (version1))" on the list, and it is not yet solved afaik. I would say this should be done through the pipeline handler by the CameraSensor class somehow. And some documentation would then follow to specify it in Documentation/sensor_driver_requirements.rst > + > +Af::~Af() > +{ > + if(vcmFd_ != -1) > + close(vcmFd_); > +} > + > +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > +{ > + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; > + context.frameContext.af.focus = 0; > + context.frameContext.af.maxVar = 0; > + context.frameContext.af.stable = false; > + > + maxVariance_ = 0; > + ignoreFrame_ = 100; Those magic values should be at least commented or set as constexpr. > + > + return 0; > +} > + > +void Af::vcmSet(int value) > +{ > + int ret; > + struct v4l2_control ctrl; > + if(vcmFd_ == -1) > + return; > + memset(&ctrl, 0, sizeof(struct v4l2_control)); > + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; > + ctrl.value = value; > + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); Same comment, this is not something which should be done in the algorithm. > + > +} > + > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) > +{ > + typedef struct y_table_item { > + uint16_t y1_avg; > + uint16_t y2_avg; > + }y_table_item_t; Not inside the function. This is the table defined in: https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_raw_buffer How do you know it is splitted in y1_avg and y2_avg ? Is it based in CrOs ? https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h > + > + uint32_t total = 0; > + double mean; > + uint64_t var_sum = 0; > + y_table_item_t *y_item; > + > + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; > + int z = 0; > + > + 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; Some comments to follow the algorithm could help :-). Is the algorithm described in some book or something ? Is it a known one ? If so, may you reference it ? > + > + 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; > + } > + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); > + LOG(IPU3Af, Debug) << "variance: " << currentVar_; > + > + if( context.frameContext.af.stable == true ) > + { > + const uint32_t diff_var = std::abs(currentVar_ - context.frameContext.af.maxVar); > + const double var_ratio = diff_var / context.frameContext.af.maxVar; > + LOG(IPU3Af, Debug) << "Change ratio: " > + << var_ratio > + << " current focus: " > + << context.frameContext.af.focus; > + if(var_ratio > 0.8) > + { > + if(ignoreFrame_ == 0) > + { > + context.frameContext.af.maxVar = 0; > + context.frameContext.af.focus = 0; > + focus_ = 0; > + context.frameContext.af.stable = false; > + ignoreFrame_ = 60; > + } > + else > + ignoreFrame_--; > + }else > + ignoreFrame_ = 60; > + }else > + { > + if(ignoreFrame_ != 0) > + ignoreFrame_--; > + else{ > + if(currentVar_ > context.frameContext.af.maxVar) > + { > + context.frameContext.af.maxVar = currentVar_; > + context.frameContext.af.focus = focus_; > + } > + > + if(focus_ > 1023) 1023, because... ? > + { > + context.frameContext.af.stable = true; > + vcmSet(context.frameContext.af.focus); > + LOG(IPU3Af, Debug) << "Finall Focus " s/Finall/Final > + << context.frameContext.af.focus; > + }else > + { > + focus_ += 5; > + vcmSet(focus_); > + } > + LOG(IPU3Af, Debug) << "Focus searching max var is: " > + << context.frameContext.af.maxVar > + << " Focus step is " > + << context.frameContext.af.focus; > + } > + } > +} > + > + > +} /* namespace ipa::ipu3::algorithms */ > + > +} /* namespace libcamera */ > \ No newline at end of file > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > new file mode 100644 > index 00000000..64c704b1 > --- /dev/null > +++ b/src/ipa/ipu3/algorithms/af.h > @@ -0,0 +1,48 @@ > +/* 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 > +{ > +public: > + Af(); > + ~Af(); > + > + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; Isn't there a need to configure the AF parameters in a prepare() function ? There is a lot of tables associated, and at the very least, I would have expected a grid: https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_config_s > + > +private: > + void filterVariance(double new_var); > + > + void vcmSet(int value); > + > + int vcmFd_; > + int focus_; > + unsigned int ignoreFrame_; > + double maxVariance_; > + double currentVar_; > + > +}; > + > +} /* 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 3ec42f72..d32c61d2 100644 > --- a/src/ipa/ipu3/algorithms/meson.build > +++ b/src/ipa/ipu3/algorithms/meson.build > @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ > 'algorithm.cpp', > 'awb.cpp', > 'blc.cpp', > - 'tone_mapping.cpp', > + 'af.cpp', Alphabetical sorted > + 'tone_mapping.cpp' > ]) > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 1e46c61a..5d92f63c 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -47,6 +47,12 @@ struct IPAFrameContext { > } gains; > } awb; > > + struct { > + uint32_t focus; > + double maxVar; > + bool stable; > + } af; Those fields are not documented in ipa_context.cpp, you probably have a Doxygen warning ? If not, I suggest you activate the documentation generation ;-). > + > struct { > double gamma; > struct ipu3_uapi_gamma_corr_lut gammaCorrection; > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 5c51607d..980815ee 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -31,6 +31,7 @@ > #include "libcamera/internal/mapped_framebuffer.h" > > #include "algorithms/agc.h" > +#include "algorithms/af.h" > #include "algorithms/algorithm.h" > #include "algorithms/awb.h" > #include "algorithms/blc.h" > @@ -298,6 +299,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>()); >
Hi Jean-Michel, On Mon, Nov 15, 2021 at 2:49 PM Jean-Michel Hautbois < jeanmichel.hautbois@ideasonboard.com> wrote: > > Hi Kate, > > Thanks for the patch ! > > On 15/11/2021 06:44, 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 > s/approah/approach I'll correct my typo. T_T > > > state and a appropriate focus value. > > > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > > --- > > src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ > > src/ipa/ipu3/algorithms/af.h | 48 ++++++++ > > src/ipa/ipu3/algorithms/meson.build | 3 +- > > src/ipa/ipu3/ipa_context.h | 6 + > > src/ipa/ipu3/ipu3.cpp | 2 + > > 5 files changed, 223 insertions(+), 1 deletion(-) > > create mode 100644 src/ipa/ipu3/algorithms/af.cpp > > create mode 100644 src/ipa/ipu3/algorithms/af.h > > > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > > new file mode 100644 > > index 00000000..c276b539 > > --- /dev/null > > +++ b/src/ipa/ipu3/algorithms/af.cpp > > @@ -0,0 +1,165 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2021, Red Hat > > + * > > + * af.cpp - IPU3 Af control > > + */ > > + > > +#include "af.h" > > + > > +#include <algorithm> > > +#include <chrono> > > +#include <cmath> > > +#include <numeric> > > + > > +#include <linux/videodev2.h> > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <fcntl.h> > > +#include <sys/ioctl.h> > > +#include <unistd.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 { > > + > > +LOG_DEFINE_CATEGORY(IPU3Af) > > + > > +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) > > +{ > > + /* For surface Go 2 back camera VCM */ > > + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); > > +} > > IPA algorithms should not know anything about V4L2 controls, subdev, > etc. I know some discussion is running about this in "Fwd: Surface Go > VCM type (was: Need to pass acpi_enforce_resources=lax on the Surface Go > (version1))" on the list, and it is not yet solved afaik. > > I would say this should be done through the pipeline handler by the > CameraSensor class somehow. > And some documentation would then follow to specify it in > Documentation/sensor_driver_requirements.rst This is my first version of VCM control and to test the Raw AF data from IPU3 so I open the device in the class directly. Also, I have traced the codes and found the control class of v4l2-subdev. I'll try to update them in my v2 patch. > > > + > > +Af::~Af() > > +{ > > + if(vcmFd_ != -1) > > + close(vcmFd_); > > +} > > + > > +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > +{ > > + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; > > + context.frameContext.af.focus = 0; > > + context.frameContext.af.maxVar = 0; > > + context.frameContext.af.stable = false; > > + > > + maxVariance_ = 0; > > + ignoreFrame_ = 100; > > Those magic values should be at least commented or set as constexpr. I'll add the comments to the code. > > > + > > + return 0; > > +} > > + > > +void Af::vcmSet(int value) > > +{ > > + int ret; > > + struct v4l2_control ctrl; > > + if(vcmFd_ == -1) > > + return; > > + memset(&ctrl, 0, sizeof(struct v4l2_control)); > > + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; > > + ctrl.value = value; > > + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); > > Same comment, this is not something which should be done in the algorithm. OK, I'll try to do this in my v2 patch. > > > + > > +} > > + > > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) > > +{ > > + typedef struct y_table_item { > > + uint16_t y1_avg; > > + uint16_t y2_avg; > > + }y_table_item_t; > > Not inside the function. This is the table defined in: > https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_raw_buffer > > How do you know it is splitted in y1_avg and y2_avg ? Is it based in CrOs ? > https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h I found the y_table format from ipu3-ipa repository ( https://git.libcamera.org/libcamera/ipu3-ipa.git). Also, combined some keywords from the kernel, such as high and low frequency and convolution, the AF raw buffer may contain the result of low and high pass filter convolution (I guess). https://lxr.missinglinkelectronics.com/linux/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h#L256 > > > > + > > + uint32_t total = 0; > > + double mean; > > + uint64_t var_sum = 0; > > + y_table_item_t *y_item; > > + > > + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; > > + int z = 0; > > + > > + 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; > > Some comments to follow the algorithm could help :-). Is the algorithm > described in some book or something ? Is it a known one ? If so, may you > reference it ? Oh. I just calculate the variance for each non-zero y2_avg (high pass) value. For a clear image, the variance should be the largest value from each focus step. By increasing the focus step and searching for the maximum variance, an appropriate focus value can be found. It is something like the mountain climbing algorithm. > > > + > > + 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; > > + } > > + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); > > + LOG(IPU3Af, Debug) << "variance: " << currentVar_; > > + > > + if( context.frameContext.af.stable == true ) > > + { > > + const uint32_t diff_var = std::abs(currentVar_ - context.frameContext.af.maxVar); > > + const double var_ratio = diff_var / context.frameContext.af.maxVar; > > + LOG(IPU3Af, Debug) << "Change ratio: " > > + << var_ratio > > + << " current focus: " > > + << context.frameContext.af.focus; > > + if(var_ratio > 0.8) > > + { > > + if(ignoreFrame_ == 0) > > + { > > + context.frameContext.af.maxVar = 0; > > + context.frameContext.af.focus = 0; > > + focus_ = 0; > > + context.frameContext.af.stable = false; > > + ignoreFrame_ = 60; > > + } > > + else > > + ignoreFrame_--; > > + }else > > + ignoreFrame_ = 60; > > + }else > > + { > > + if(ignoreFrame_ != 0) > > + ignoreFrame_--; > > + else{ > > + if(currentVar_ > context.frameContext.af.maxVar) > > + { > > + context.frameContext.af.maxVar = currentVar_; > > + context.frameContext.af.focus = focus_; > > + } > > + > > + if(focus_ > 1023) > 1023, because... ? It's the maximum focus step of VCM. > > > + { > > + context.frameContext.af.stable = true; > > + vcmSet(context.frameContext.af.focus); > > + LOG(IPU3Af, Debug) << "Finall Focus " > s/Finall/Final Sorry for my typo 😅 > > > + << context.frameContext.af.focus; > > + }else > > + { > > + focus_ += 5; > > + vcmSet(focus_); > > + } > > + LOG(IPU3Af, Debug) << "Focus searching max var is: " > > + << context.frameContext.af.maxVar > > + << " Focus step is " > > + << context.frameContext.af.focus; > > + } > > + } > > +} > > + > > + > > +} /* namespace ipa::ipu3::algorithms */ > > + > > +} /* namespace libcamera */ > > \ No newline at end of file > > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > > new file mode 100644 > > index 00000000..64c704b1 > > --- /dev/null > > +++ b/src/ipa/ipu3/algorithms/af.h > > @@ -0,0 +1,48 @@ > > +/* 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 > > +{ > > +public: > > + Af(); > > + ~Af(); > > + > > + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > > + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > > Isn't there a need to configure the AF parameters in a prepare() > function ? There is a lot of tables associated, and at the very least, I > would have expected a grid: > > https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_config_s In this version, I try to keep all the configurations in default to prove the algorithm and buffer format works. I could add grid configuration in my v2 patch. > > > + > > +private: > > + void filterVariance(double new_var); > > + > > + void vcmSet(int value); > > + > > + int vcmFd_; > > + int focus_; > > + unsigned int ignoreFrame_; > > + double maxVariance_; > > + double currentVar_; > > + > > +}; > > + > > +} /* 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 3ec42f72..d32c61d2 100644 > > --- a/src/ipa/ipu3/algorithms/meson.build > > +++ b/src/ipa/ipu3/algorithms/meson.build > > @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ > > 'algorithm.cpp', > > 'awb.cpp', > > 'blc.cpp', > > - 'tone_mapping.cpp', > > + 'af.cpp', > > Alphabetical sorted I got it. > > > + 'tone_mapping.cpp' > > ]) > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > > index 1e46c61a..5d92f63c 100644 > > --- a/src/ipa/ipu3/ipa_context.h > > +++ b/src/ipa/ipu3/ipa_context.h > > @@ -47,6 +47,12 @@ struct IPAFrameContext { > > } gains; > > } awb; > > > > + struct { > > + uint32_t focus; > > + double maxVar; > > + bool stable; > > + } af; > > Those fields are not documented in ipa_context.cpp, you probably have a > Doxygen warning ? If not, I suggest you activate the documentation > generation ;-). I don't have Doxygen warning. I guess I don't activate the documentation generation. I'll activate it, thank you :) > > > + > > struct { > > double gamma; > > struct ipu3_uapi_gamma_corr_lut gammaCorrection; > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > > index 5c51607d..980815ee 100644 > > --- a/src/ipa/ipu3/ipu3.cpp > > +++ b/src/ipa/ipu3/ipu3.cpp > > @@ -31,6 +31,7 @@ > > #include "libcamera/internal/mapped_framebuffer.h" > > > > #include "algorithms/agc.h" > > +#include "algorithms/af.h" > > #include "algorithms/algorithm.h" > > #include "algorithms/awb.h" > > #include "algorithms/blc.h" > > @@ -298,6 +299,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>()); > > >
Kate, On 15/11/2021 09:39, Kate Hsuan wrote: > Hi Jean-Michel, > > On Mon, Nov 15, 2021 at 2:49 PM Jean-Michel Hautbois > <jeanmichel.hautbois@ideasonboard.com > <mailto:jeanmichel.hautbois@ideasonboard.com>> wrote: > > > > Hi Kate, > > > > Thanks for the patch ! > > > > On 15/11/2021 06:44, 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 > > s/approah/approach > > I'll correct my typo. T_T > > > > > > state and a appropriate focus value. > > > > > > Signed-off-by: Kate Hsuan <hpa@redhat.com <mailto:hpa@redhat.com>> > > > --- > > > src/ipa/ipu3/algorithms/af.cpp | 165 > ++++++++++++++++++++++++++++ > > > src/ipa/ipu3/algorithms/af.h | 48 ++++++++ > > > src/ipa/ipu3/algorithms/meson.build | 3 +- > > > src/ipa/ipu3/ipa_context.h | 6 + > > > src/ipa/ipu3/ipu3.cpp | 2 + > > > 5 files changed, 223 insertions(+), 1 deletion(-) > > > create mode 100644 src/ipa/ipu3/algorithms/af.cpp > > > create mode 100644 src/ipa/ipu3/algorithms/af.h > > > > > > diff --git a/src/ipa/ipu3/algorithms/af.cpp > b/src/ipa/ipu3/algorithms/af.cpp > > > new file mode 100644 > > > index 00000000..c276b539 > > > --- /dev/null > > > +++ b/src/ipa/ipu3/algorithms/af.cpp > > > @@ -0,0 +1,165 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2021, Red Hat > > > + * > > > + * af.cpp - IPU3 Af control > > > + */ > > > + > > > +#include "af.h" > > > + > > > +#include <algorithm> > > > +#include <chrono> > > > +#include <cmath> > > > +#include <numeric> > > > + > > > +#include <linux/videodev2.h> > > > +#include <sys/types.h> > > > +#include <sys/stat.h> > > > +#include <fcntl.h> > > > +#include <sys/ioctl.h> > > > +#include <unistd.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 { > > > + > > > +LOG_DEFINE_CATEGORY(IPU3Af) > > > + > > > +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) > > > +{ > > > + /* For surface Go 2 back camera VCM */ > > > + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); > > > +} > > > > IPA algorithms should not know anything about V4L2 controls, subdev, > > etc. I know some discussion is running about this in "Fwd: Surface Go > > VCM type (was: Need to pass acpi_enforce_resources=lax on the Surface Go > > (version1))" on the list, and it is not yet solved afaik. > > > > I would say this should be done through the pipeline handler by the > > CameraSensor class somehow. > > And some documentation would then follow to specify it in > > Documentation/sensor_driver_requirements.rst > > This is my first version of VCM control and to test the Raw AF data from > IPU3 so I open the device in the class directly. > Also, I have traced the codes and found the control class of > v4l2-subdev. I'll try to update them in my v2 patch. Maybe could you be interested by the thread "ipa: ipu3: Extend ipu3 ipa interface for lens controls" in its v3, maybe would you like to review it, as it sounds like the same beast here :-). > > > > > > + > > > +Af::~Af() > > > +{ > > > + if(vcmFd_ != -1) > > > + close(vcmFd_); > > > +} > > > + > > > +int Af::configure(IPAContext &context, const IPAConfigInfo > &configInfo) > > > +{ > > > + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; > > > + context.frameContext.af.focus = 0; > > > + context.frameContext.af.maxVar = 0; > > > + context.frameContext.af.stable = false; > > > + > > > + maxVariance_ = 0; > > > + ignoreFrame_ = 100; > > > > Those magic values should be at least commented or set as constexpr. > > I'll add the comments to the code. > > > > > > + > > > + return 0; > > > +} > > > + > > > +void Af::vcmSet(int value) > > > +{ > > > + int ret; > > > + struct v4l2_control ctrl; > > > + if(vcmFd_ == -1) > > > + return; > > > + memset(&ctrl, 0, sizeof(struct v4l2_control)); > > > + ctrl.id <http://ctrl.id> = V4L2_CID_FOCUS_ABSOLUTE; > > > + ctrl.value = value; > > > + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); > > > > Same comment, this is not something which should be done in the > algorithm. > > OK, I'll try to do this in my v2 patch. > > > > > > + > > > +} > > > + > > > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a > *stats ) > > > +{ > > > + typedef struct y_table_item { > > > + uint16_t y1_avg; > > > + uint16_t y2_avg; > > > + }y_table_item_t; > > > > Not inside the function. This is the table defined in: > > > https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_raw_buffer > <https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_raw_buffer> > > > > How do you know it is splitted in y1_avg and y2_avg ? Is it based in > CrOs ? > > > https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h > <https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h> > > I found the y_table format from ipu3-ipa repository > (https://git.libcamera.org/libcamera/ipu3-ipa.git > <https://git.libcamera.org/libcamera/ipu3-ipa.git>). Also, combined some > keywords from the kernel, such as high and low frequency and > convolution, the AF raw buffer may contain the result of low and high > pass filter convolution (I guess). > https://lxr.missinglinkelectronics.com/linux/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h#L256 > <https://lxr.missinglinkelectronics.com/linux/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h#L256> > > > > > > > > > + > > > + uint32_t total = 0; > > > + double mean; > > > + uint64_t var_sum = 0; > > > + y_table_item_t *y_item; > > > + > > > + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; > > > + int z = 0; > > > + > > > + 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; > > > > Some comments to follow the algorithm could help :-). Is the algorithm > > described in some book or something ? Is it a known one ? If so, may you > > reference it ? > > Oh. I just calculate the variance for each non-zero y2_avg (high pass) > value. For a clear image, the variance should be the largest value from > each focus step. By increasing the focus step and searching for the > maximum variance, an appropriate focus value can be found. It is > something like the mountain climbing algorithm. I mean, not everybody will be an image processing expert as you are, so adding a documentation in the code could help following how AF works in this case (looks like something like a contrast detection algorithm). > > > > > > > + > > > + 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; > > > + } > > > + currentVar_ = static_cast<double>(var_sum) > /static_cast<double>(z); > > > + LOG(IPU3Af, Debug) << "variance: " << currentVar_; > > > + > > > + if( context.frameContext.af.stable == true ) > > > + { > > > + const uint32_t diff_var = std::abs(currentVar_ - > context.frameContext.af.maxVar); > > > + const double var_ratio = diff_var / > context.frameContext.af.maxVar; > > > + LOG(IPU3Af, Debug) << "Change ratio: " > > > + << var_ratio > > > + << " current focus: " > > > + << context.frameContext.af.focus; > > > + if(var_ratio > 0.8) > > > + { > > > + if(ignoreFrame_ == 0) > > > + { > > > + context.frameContext.af.maxVar = 0; > > > + context.frameContext.af.focus = 0; > > > + focus_ = 0; > > > + context.frameContext.af.stable = false; > > > + ignoreFrame_ = 60; > > > + } > > > + else > > > + ignoreFrame_--; > > > + }else > > > + ignoreFrame_ = 60; > > > + }else > > > + { > > > + if(ignoreFrame_ != 0) > > > + ignoreFrame_--; > > > + else{ > > > + if(currentVar_ > context.frameContext.af.maxVar) > > > + { > > > + context.frameContext.af.maxVar = > currentVar_; > > > + context.frameContext.af.focus = focus_; > > > + } > > > + > > > + if(focus_ > 1023) > > 1023, because... ? > > It's the maximum focus step of VCM. > > > > > > + { > > > + context.frameContext.af.stable = true; > > > + vcmSet(context.frameContext.af.focus); > > > + LOG(IPU3Af, Debug) << "Finall Focus " > > s/Finall/Final > > Sorry for my typo 😅 > > > > > > + << > context.frameContext.af.focus; > > > + }else > > > + { > > > + focus_ += 5; > > > + vcmSet(focus_); > > > + } > > > + LOG(IPU3Af, Debug) << "Focus searching max > var is: " > > > + << > context.frameContext.af.maxVar > > > + << " Focus step is " > > > + << > context.frameContext.af.focus; > > > + } > > > + } > > > +} > > > + > > > + > > > +} /* namespace ipa::ipu3::algorithms */ > > > + > > > +} /* namespace libcamera */ > > > \ No newline at end of file > > > diff --git a/src/ipa/ipu3/algorithms/af.h > b/src/ipa/ipu3/algorithms/af.h > > > new file mode 100644 > > > index 00000000..64c704b1 > > > --- /dev/null > > > +++ b/src/ipa/ipu3/algorithms/af.h > > > @@ -0,0 +1,48 @@ > > > +/* 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 > > > +{ > > > +public: > > > + Af(); > > > + ~Af(); > > > + > > > + int configure(IPAContext &context, const IPAConfigInfo > &configInfo) override; > > > + void process(IPAContext &context, const ipu3_uapi_stats_3a > *stats) override; > > > > Isn't there a need to configure the AF parameters in a prepare() > > function ? There is a lot of tables associated, and at the very least, I > > would have expected a grid: > > > > > https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_config_s > <https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_config_s> > > In this version, I try to keep all the configurations in default to > prove the algorithm and buffer format works. > I could add grid configuration in my v2 patch. > > > > > > + > > > +private: > > > + void filterVariance(double new_var); > > > + > > > + void vcmSet(int value); > > > + > > > + int vcmFd_; > > > + int focus_; > > > + unsigned int ignoreFrame_; > > > + double maxVariance_; > > > + double currentVar_; > > > + > > > +}; > > > + > > > +} /* 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 3ec42f72..d32c61d2 100644 > > > --- a/src/ipa/ipu3/algorithms/meson.build > > > +++ b/src/ipa/ipu3/algorithms/meson.build > > > @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ > > > 'algorithm.cpp', > > > 'awb.cpp', > > > 'blc.cpp', > > > - 'tone_mapping.cpp', > > > + 'af.cpp', > > > > Alphabetical sorted > > I got it. > > > > > > + 'tone_mapping.cpp' > > > ]) > > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > > > index 1e46c61a..5d92f63c 100644 > > > --- a/src/ipa/ipu3/ipa_context.h > > > +++ b/src/ipa/ipu3/ipa_context.h > > > @@ -47,6 +47,12 @@ struct IPAFrameContext { > > > } gains; > > > } awb; > > > > > > + struct { > > > + uint32_t focus; > > > + double maxVar; > > > + bool stable; > > > + } af; > > > > Those fields are not documented in ipa_context.cpp, you probably have a > > Doxygen warning ? If not, I suggest you activate the documentation > > generation ;-). > > I don't have Doxygen warning. I guess I don't activate the documentation > generation. I'll activate it, thank you :) > > > > > > + > > > struct { > > > double gamma; > > > struct ipu3_uapi_gamma_corr_lut gammaCorrection; > > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > > > index 5c51607d..980815ee 100644 > > > --- a/src/ipa/ipu3/ipu3.cpp > > > +++ b/src/ipa/ipu3/ipu3.cpp > > > @@ -31,6 +31,7 @@ > > > #include "libcamera/internal/mapped_framebuffer.h" > > > > > > #include "algorithms/agc.h" > > > +#include "algorithms/af.h" > > > #include "algorithms/algorithm.h" > > > #include "algorithms/awb.h" > > > #include "algorithms/blc.h" > > > @@ -298,6 +299,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>()); > > > > > > > > -- > BR, > Kate
Hi Kate, Thank you for you work on this. While I understand this is an MVP (as posted by you in one of the other autofocus threads) here are some of my high-level queries and understanding. The interface to handle autofocus (and focus controls) in general is missing from libcamera, so I understand the current limitations as well, to move this forward. On 11/15/21 11:14 AM, 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. Can you please suggest a high level reading of finding the focus on the image? I don't understand the implementation / algorithm for the focus-sweep implemented in process()? > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ > src/ipa/ipu3/algorithms/af.h | 48 ++++++++ > src/ipa/ipu3/algorithms/meson.build | 3 +- > src/ipa/ipu3/ipa_context.h | 6 + > src/ipa/ipu3/ipu3.cpp | 2 + > 5 files changed, 223 insertions(+), 1 deletion(-) > create mode 100644 src/ipa/ipu3/algorithms/af.cpp > create mode 100644 src/ipa/ipu3/algorithms/af.h > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > new file mode 100644 > index 00000000..c276b539 > --- /dev/null > +++ b/src/ipa/ipu3/algorithms/af.cpp I won't get into style issues here (as don't seem relevant for now), but for your information we do have a style checker at: $libcamera/utils/checkstyle.py <commit-id-of-the-patch> > @@ -0,0 +1,165 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Red Hat > + * > + * af.cpp - IPU3 Af control would be better to expand on the acronym "IPU3 Auto Focus control" maybe > + */ > + > +#include "af.h" > + > +#include <algorithm> > +#include <chrono> > +#include <cmath> > +#include <numeric> > + > +#include <linux/videodev2.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <fcntl.h> > +#include <sys/ioctl.h> > +#include <unistd.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 { > + > +LOG_DEFINE_CATEGORY(IPU3Af) > + > +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) > +{ > + /* For surface Go 2 back camera VCM */ > + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); So I have nautilus (Samsung Chromebook v2) which as a UVC camera and a IPU3 one (with IMX258 sensor). The VCM there is dw9807. I tried this patch on nautilus by mapping the v4l-subdevX to dw9807 and it did work. I can literally hear the VCM move and tries to focus on the scene. However, there are a few niggles I have noticed: The autofocus seems to lock up quite frequently leaving an un-focused streaming. However if I put up a object close to lens it does tries to focus it again (then locks up again but works sometimes). I am deliberately leaving the details out as it's too broad to say anything specific from nautilus' side. I am interesting in getting better understanding of the values reported, so once I have a overall reading on the topic, I can start debugging the process(). > +} > + > +Af::~Af() > +{ > + if(vcmFd_ != -1) > + close(vcmFd_); > +} > + > +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) I think [[maybe_unused]] on configInfo as a replacement for the below line > +{ > + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; > + context.frameContext.af.focus = 0; > + context.frameContext.af.maxVar = 0; > + context.frameContext.af.stable = false; > + > + maxVariance_ = 0; > + ignoreFrame_ = 100; > + > + return 0; > +} > + > +void Af::vcmSet(int value) > +{ > + int ret; > + struct v4l2_control ctrl; > + if(vcmFd_ == -1) > + return; Maybe we should return -EINVAL? > + memset(&ctrl, 0, sizeof(struct v4l2_control)); > + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; > + ctrl.value = value; > + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); and return ret instead? To check if we really succeeded on setting the control > + > +} > + > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) > +{ > + typedef struct y_table_item { > + uint16_t y1_avg; > + uint16_t y2_avg; > + }y_table_item_t; > + > + uint32_t total = 0; > + double mean; > + uint64_t var_sum = 0; > + y_table_item_t *y_item; > + > + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; > + int z = 0; > + > + 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; > + } > + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); > + LOG(IPU3Af, Debug) << "variance: " << currentVar_; > + > + if( context.frameContext.af.stable == true ) > + { > + const uint32_t diff_var = std::abs(currentVar_ - context.frameContext.af.maxVar); > + const double var_ratio = diff_var / context.frameContext.af.maxVar; > + LOG(IPU3Af, Debug) << "Change ratio: " > + << var_ratio > + << " current focus: " > + << context.frameContext.af.focus; > + if(var_ratio > 0.8) hmm, I think we should opt for "variance_" instead "var_" as var in general has variable meaning, nevermind, details... > + { > + if(ignoreFrame_ == 0) > + { > + context.frameContext.af.maxVar = 0; > + context.frameContext.af.focus = 0; > + focus_ = 0; > + context.frameContext.af.stable = false; > + ignoreFrame_ = 60; > + } > + else > + ignoreFrame_--; > + }else > + ignoreFrame_ = 60; > + }else > + { > + if(ignoreFrame_ != 0) > + ignoreFrame_--; > + else{ > + if(currentVar_ > context.frameContext.af.maxVar) > + { > + context.frameContext.af.maxVar = currentVar_; > + context.frameContext.af.focus = focus_; > + } > + > + if(focus_ > 1023) > + { > + context.frameContext.af.stable = true; > + vcmSet(context.frameContext.af.focus); > + LOG(IPU3Af, Debug) << "Finall Focus " > + << context.frameContext.af.focus; > + }else > + { > + focus_ += 5; > + vcmSet(focus_); > + } > + LOG(IPU3Af, Debug) << "Focus searching max var is: " > + << context.frameContext.af.maxVar > + << " Focus step is " > + << context.frameContext.af.focus; > + } > + } > +} > + > + > +} /* namespace ipa::ipu3::algorithms */ > + > +} /* namespace libcamera */ > \ No newline at end of file > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > new file mode 100644 > index 00000000..64c704b1 > --- /dev/null > +++ b/src/ipa/ipu3/algorithms/af.h > @@ -0,0 +1,48 @@ > +/* 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 > +{ > +public: > + Af(); > + ~Af(); > + > + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + > +private: > + void filterVariance(double new_var); > + > + void vcmSet(int value); > + > + int vcmFd_; > + int focus_; > + unsigned int ignoreFrame_; What's this stand for? Is it ignoring a number of frames? > + double maxVariance_; > + double currentVar_; > + > +}; > + > +} /* 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 3ec42f72..d32c61d2 100644 > --- a/src/ipa/ipu3/algorithms/meson.build > +++ b/src/ipa/ipu3/algorithms/meson.build > @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ > 'algorithm.cpp', > 'awb.cpp', > 'blc.cpp', > - 'tone_mapping.cpp', > + 'af.cpp', > + 'tone_mapping.cpp' alphabetical order please, I think checkstyle.py will report this as well > ]) > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 1e46c61a..5d92f63c 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -47,6 +47,12 @@ struct IPAFrameContext { > } gains; > } awb; > > + struct { > + uint32_t focus; > + double maxVar; > + bool stable; > + } af; > + > struct { > double gamma; > struct ipu3_uapi_gamma_corr_lut gammaCorrection; > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 5c51607d..980815ee 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -31,6 +31,7 @@ > #include "libcamera/internal/mapped_framebuffer.h" > > #include "algorithms/agc.h" > +#include "algorithms/af.h" Ditto. > #include "algorithms/algorithm.h" > #include "algorithms/awb.h" > #include "algorithms/blc.h" > @@ -298,6 +299,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>());
Hi Umang, On 18/11/2021 13:31, Umang Jain wrote: > Hi Kate, > > Thank you for you work on this. > > While I understand this is an MVP (as posted by you in one of the other > autofocus threads) here are some of my high-level queries and > understanding. The interface to handle autofocus (and focus controls) in > general is missing from libcamera, so I understand the current > limitations as well, to move this forward. > > On 11/15/21 11:14 AM, 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. > > > Can you please suggest a high level reading of finding the focus on the > image? I don't understand the implementation / algorithm for the > focus-sweep implemented in process()? > >> >> Signed-off-by: Kate Hsuan <hpa@redhat.com> >> --- >> src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ >> src/ipa/ipu3/algorithms/af.h | 48 ++++++++ >> src/ipa/ipu3/algorithms/meson.build | 3 +- >> src/ipa/ipu3/ipa_context.h | 6 + >> src/ipa/ipu3/ipu3.cpp | 2 + >> 5 files changed, 223 insertions(+), 1 deletion(-) >> create mode 100644 src/ipa/ipu3/algorithms/af.cpp >> create mode 100644 src/ipa/ipu3/algorithms/af.h >> >> diff --git a/src/ipa/ipu3/algorithms/af.cpp >> b/src/ipa/ipu3/algorithms/af.cpp >> new file mode 100644 >> index 00000000..c276b539 >> --- /dev/null >> +++ b/src/ipa/ipu3/algorithms/af.cpp > > > I won't get into style issues here (as don't seem relevant for now), but > for your information we do have a style checker at: > > $libcamera/utils/checkstyle.py <commit-id-of-the-patch> > >> @@ -0,0 +1,165 @@ >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >> +/* >> + * Copyright (C) 2021, Red Hat >> + * >> + * af.cpp - IPU3 Af control > > > would be better to expand on the acronym "IPU3 Auto Focus control" maybe > >> + */ >> + >> +#include "af.h" >> + >> +#include <algorithm> >> +#include <chrono> >> +#include <cmath> >> +#include <numeric> >> + >> +#include <linux/videodev2.h> >> +#include <sys/types.h> >> +#include <sys/stat.h> >> +#include <fcntl.h> >> +#include <sys/ioctl.h> >> +#include <unistd.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 { >> + >> +LOG_DEFINE_CATEGORY(IPU3Af) >> + >> +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) >> +{ >> + /* For surface Go 2 back camera VCM */ >> + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); > > > So I have nautilus (Samsung Chromebook v2) which as a UVC camera and a > IPU3 one (with IMX258 sensor). The VCM there is dw9807. > > I tried this patch on nautilus by mapping the v4l-subdevX to dw9807 and > it did work. I can literally hear the VCM move and tries to focus on the > scene. However, there are a few niggles I have noticed: > > The autofocus seems to lock up quite frequently leaving an un-focused > streaming. However if I put up a object close to lens it does tries to > focus it again (then locks up again but works sometimes). I am > deliberately leaving the details out as it's too broad to say anything > specific from nautilus' side. I will let Kate answer in depth obviously. From what I know, the AF needs a grid configuration in IPU3, and the default grid is set to [16*8,16*8] pixels, starting at x=10 and y=2: https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9587 It means that if your BDS output size is 1280x720 for instance, you will focus on the 10% left part and 17% top part of your scene. Depending on the scene, you might very well be out-of-focus :-). That's why I suggested adding a configure() call to set a grid based on the BDS grid, to make it focus in the center as a default. > > I am interesting in getting better understanding of the values reported, > so once I have a overall reading on the topic, I can start debugging the > process(). > >> +} >> + >> +Af::~Af() >> +{ >> + if(vcmFd_ != -1) >> + close(vcmFd_); >> +} >> + >> +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > > I think [[maybe_unused]] on configInfo as a replacement for the below line > >> +{ >> + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; >> + context.frameContext.af.focus = 0; >> + context.frameContext.af.maxVar = 0; >> + context.frameContext.af.stable = false; >> + >> + maxVariance_ = 0; >> + ignoreFrame_ = 100; >> + >> + return 0; >> +} >> + >> +void Af::vcmSet(int value) >> +{ >> + int ret; >> + struct v4l2_control ctrl; >> + if(vcmFd_ == -1) >> + return; > > > Maybe we should return -EINVAL? > >> + memset(&ctrl, 0, sizeof(struct v4l2_control)); >> + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; >> + ctrl.value = value; >> + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); > > > and return ret instead? To check if we really succeeded on setting the > control > >> + >> +} >> + >> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) >> +{ >> + typedef struct y_table_item { >> + uint16_t y1_avg; >> + uint16_t y2_avg; >> + }y_table_item_t; >> + >> + uint32_t total = 0; >> + double mean; >> + uint64_t var_sum = 0; >> + y_table_item_t *y_item; >> + >> + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; >> + int z = 0; >> + >> + 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; >> + } >> + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); >> + LOG(IPU3Af, Debug) << "variance: " << currentVar_; >> + >> + if( context.frameContext.af.stable == true ) >> + { >> + const uint32_t diff_var = std::abs(currentVar_ - >> context.frameContext.af.maxVar); >> + const double var_ratio = diff_var / >> context.frameContext.af.maxVar; >> + LOG(IPU3Af, Debug) << "Change ratio: " >> + << var_ratio >> + << " current focus: " >> + << context.frameContext.af.focus; >> + if(var_ratio > 0.8) > > > hmm, I think we should opt for "variance_" instead "var_" as var in > general has variable meaning, nevermind, details... > >> + { >> + if(ignoreFrame_ == 0) >> + { >> + context.frameContext.af.maxVar = 0; >> + context.frameContext.af.focus = 0; >> + focus_ = 0; >> + context.frameContext.af.stable = false; >> + ignoreFrame_ = 60; >> + } >> + else >> + ignoreFrame_--; >> + }else >> + ignoreFrame_ = 60; >> + }else >> + { >> + if(ignoreFrame_ != 0) >> + ignoreFrame_--; >> + else{ >> + if(currentVar_ > context.frameContext.af.maxVar) >> + { >> + context.frameContext.af.maxVar = currentVar_; >> + context.frameContext.af.focus = focus_; >> + } >> + >> + if(focus_ > 1023) >> + { >> + context.frameContext.af.stable = true; >> + vcmSet(context.frameContext.af.focus); >> + LOG(IPU3Af, Debug) << "Finall Focus " >> + << context.frameContext.af.focus; >> + }else >> + { >> + focus_ += 5; >> + vcmSet(focus_); >> + } >> + LOG(IPU3Af, Debug) << "Focus searching max var is: " >> + << context.frameContext.af.maxVar >> + << " Focus step is " >> + << context.frameContext.af.focus; >> + } >> + } >> +} >> + >> + >> +} /* namespace ipa::ipu3::algorithms */ >> + >> +} /* namespace libcamera */ >> \ No newline at end of file >> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h >> new file mode 100644 >> index 00000000..64c704b1 >> --- /dev/null >> +++ b/src/ipa/ipu3/algorithms/af.h >> @@ -0,0 +1,48 @@ >> +/* 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 >> +{ >> +public: >> + Af(); >> + ~Af(); >> + >> + int configure(IPAContext &context, const IPAConfigInfo >> &configInfo) override; >> + void process(IPAContext &context, const ipu3_uapi_stats_3a >> *stats) override; >> + >> +private: >> + void filterVariance(double new_var); >> + >> + void vcmSet(int value); >> + >> + int vcmFd_; >> + int focus_; >> + unsigned int ignoreFrame_; > > > What's this stand for? Is it ignoring a number of frames? > >> + double maxVariance_; >> + double currentVar_; >> + >> +}; >> + >> +} /* 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 3ec42f72..d32c61d2 100644 >> --- a/src/ipa/ipu3/algorithms/meson.build >> +++ b/src/ipa/ipu3/algorithms/meson.build >> @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ >> 'algorithm.cpp', >> 'awb.cpp', >> 'blc.cpp', >> - 'tone_mapping.cpp', >> + 'af.cpp', >> + 'tone_mapping.cpp' > > > alphabetical order please, I think checkstyle.py will report this as well > >> ]) >> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h >> index 1e46c61a..5d92f63c 100644 >> --- a/src/ipa/ipu3/ipa_context.h >> +++ b/src/ipa/ipu3/ipa_context.h >> @@ -47,6 +47,12 @@ struct IPAFrameContext { >> } gains; >> } awb; >> + struct { >> + uint32_t focus; >> + double maxVar; >> + bool stable; >> + } af; >> + >> struct { >> double gamma; >> struct ipu3_uapi_gamma_corr_lut gammaCorrection; >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp >> index 5c51607d..980815ee 100644 >> --- a/src/ipa/ipu3/ipu3.cpp >> +++ b/src/ipa/ipu3/ipu3.cpp >> @@ -31,6 +31,7 @@ >> #include "libcamera/internal/mapped_framebuffer.h" >> #include "algorithms/agc.h" >> +#include "algorithms/af.h" > > > Ditto. > >> #include "algorithms/algorithm.h" >> #include "algorithms/awb.h" >> #include "algorithms/blc.h" >> @@ -298,6 +299,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>()); >>
On Thu, Nov 18, 2021 at 06:01:35PM +0530, Umang Jain wrote: > Hi Kate, > > Thank you for you work on this. > > While I understand this is an MVP (as posted by you in one of the other > autofocus threads) here are some of my high-level queries and > understanding. The interface to handle autofocus (and focus controls) in > general is missing from libcamera, so I understand the current > limitations as well, to move this forward. > > On 11/15/21 11:14 AM, 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. > > Can you please suggest a high level reading of finding the focus on the > image? I don't understand the implementation / algorithm for the > focus-sweep implemented in process()? > > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > > --- > > src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ > > src/ipa/ipu3/algorithms/af.h | 48 ++++++++ > > src/ipa/ipu3/algorithms/meson.build | 3 +- > > src/ipa/ipu3/ipa_context.h | 6 + > > src/ipa/ipu3/ipu3.cpp | 2 + > > 5 files changed, 223 insertions(+), 1 deletion(-) > > create mode 100644 src/ipa/ipu3/algorithms/af.cpp > > create mode 100644 src/ipa/ipu3/algorithms/af.h > > > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > > new file mode 100644 > > index 00000000..c276b539 > > --- /dev/null > > +++ b/src/ipa/ipu3/algorithms/af.cpp > > I won't get into style issues here (as don't seem relevant for now), but > for your information we do have a style checker at: > > $libcamera/utils/checkstyle.py <commit-id-of-the-patch> Which can be automated with a git commit hook: cp utils/hooks/post-commit .git/hooks/ > > @@ -0,0 +1,165 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2021, Red Hat > > + * > > + * af.cpp - IPU3 Af control > > would be better to expand on the acronym "IPU3 Auto Focus control" maybe > > > + */ > > + > > +#include "af.h" > > + > > +#include <algorithm> > > +#include <chrono> > > +#include <cmath> > > +#include <numeric> > > + > > +#include <linux/videodev2.h> > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <fcntl.h> > > +#include <sys/ioctl.h> > > +#include <unistd.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 { > > + > > +LOG_DEFINE_CATEGORY(IPU3Af) > > + > > +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) > > +{ > > + /* For surface Go 2 back camera VCM */ > > + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); > > So I have nautilus (Samsung Chromebook v2) which as a UVC camera and a > IPU3 one (with IMX258 sensor). The VCM there is dw9807. > > I tried this patch on nautilus by mapping the v4l-subdevX to dw9807 and > it did work. I can literally hear the VCM move and tries to focus on the > scene. However, there are a few niggles I have noticed: > > The autofocus seems to lock up quite frequently leaving an un-focused > streaming. However if I put up a object close to lens it does tries to > focus it again (then locks up again but works sometimes). I am > deliberately leaving the details out as it's too broad to say anything > specific from nautilus' side. > > I am interesting in getting better understanding of the values reported, > so once I have a overall reading on the topic, I can start debugging the > process(). > > > +} > > + > > +Af::~Af() > > +{ > > + if(vcmFd_ != -1) > > + close(vcmFd_); > > +} > > + > > +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > I think [[maybe_unused]] on configInfo as a replacement for the below line > > > +{ > > + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; > > + context.frameContext.af.focus = 0; > > + context.frameContext.af.maxVar = 0; > > + context.frameContext.af.stable = false; > > + > > + maxVariance_ = 0; > > + ignoreFrame_ = 100; > > + > > + return 0; > > +} > > + > > +void Af::vcmSet(int value) > > +{ > > + int ret; > > + struct v4l2_control ctrl; > > + if(vcmFd_ == -1) > > + return; > > Maybe we should return -EINVAL? > > > + memset(&ctrl, 0, sizeof(struct v4l2_control)); > > + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; > > + ctrl.value = value; > > + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); > > > and return ret instead? To check if we really succeeded on setting the > control > > > + > > +} > > + > > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) > > +{ > > + typedef struct y_table_item { > > + uint16_t y1_avg; > > + uint16_t y2_avg; > > + }y_table_item_t; > > + > > + uint32_t total = 0; > > + double mean; > > + uint64_t var_sum = 0; > > + y_table_item_t *y_item; > > + > > + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; > > + int z = 0; > > + > > + 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; > > + } > > + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); > > + LOG(IPU3Af, Debug) << "variance: " << currentVar_; > > + > > + if( context.frameContext.af.stable == true ) > > + { > > + const uint32_t diff_var = std::abs(currentVar_ - context.frameContext.af.maxVar); > > + const double var_ratio = diff_var / context.frameContext.af.maxVar; > > + LOG(IPU3Af, Debug) << "Change ratio: " > > + << var_ratio > > + << " current focus: " > > + << context.frameContext.af.focus; > > + if(var_ratio > 0.8) > > > hmm, I think we should opt for "variance_" instead "var_" as var in > general has variable meaning, nevermind, details... > > > + { > > + if(ignoreFrame_ == 0) > > + { > > + context.frameContext.af.maxVar = 0; > > + context.frameContext.af.focus = 0; > > + focus_ = 0; > > + context.frameContext.af.stable = false; > > + ignoreFrame_ = 60; > > + } > > + else > > + ignoreFrame_--; > > + }else > > + ignoreFrame_ = 60; > > + }else > > + { > > + if(ignoreFrame_ != 0) > > + ignoreFrame_--; > > + else{ > > + if(currentVar_ > context.frameContext.af.maxVar) > > + { > > + context.frameContext.af.maxVar = currentVar_; > > + context.frameContext.af.focus = focus_; > > + } > > + > > + if(focus_ > 1023) > > + { > > + context.frameContext.af.stable = true; > > + vcmSet(context.frameContext.af.focus); > > + LOG(IPU3Af, Debug) << "Finall Focus " > > + << context.frameContext.af.focus; > > + }else > > + { > > + focus_ += 5; > > + vcmSet(focus_); > > + } > > + LOG(IPU3Af, Debug) << "Focus searching max var is: " > > + << context.frameContext.af.maxVar > > + << " Focus step is " > > + << context.frameContext.af.focus; > > + } > > + } > > +} > > + > > + > > +} /* namespace ipa::ipu3::algorithms */ > > + > > +} /* namespace libcamera */ > > \ No newline at end of file > > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > > new file mode 100644 > > index 00000000..64c704b1 > > --- /dev/null > > +++ b/src/ipa/ipu3/algorithms/af.h > > @@ -0,0 +1,48 @@ > > +/* 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 > > +{ > > +public: > > + Af(); > > + ~Af(); > > + > > + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > > + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > > + > > +private: > > + void filterVariance(double new_var); > > + > > + void vcmSet(int value); > > + > > + int vcmFd_; > > + int focus_; > > + unsigned int ignoreFrame_; > > > What's this stand for? Is it ignoring a number of frames? > > > + double maxVariance_; > > + double currentVar_; > > + > > +}; > > + > > +} /* 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 3ec42f72..d32c61d2 100644 > > --- a/src/ipa/ipu3/algorithms/meson.build > > +++ b/src/ipa/ipu3/algorithms/meson.build > > @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ > > 'algorithm.cpp', > > 'awb.cpp', > > 'blc.cpp', > > - 'tone_mapping.cpp', > > + 'af.cpp', > > + 'tone_mapping.cpp' > > > alphabetical order please, I think checkstyle.py will report this as well > > > ]) > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > > index 1e46c61a..5d92f63c 100644 > > --- a/src/ipa/ipu3/ipa_context.h > > +++ b/src/ipa/ipu3/ipa_context.h > > @@ -47,6 +47,12 @@ struct IPAFrameContext { > > } gains; > > } awb; > > > > + struct { > > + uint32_t focus; > > + double maxVar; > > + bool stable; > > + } af; > > + > > struct { > > double gamma; > > struct ipu3_uapi_gamma_corr_lut gammaCorrection; > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > > index 5c51607d..980815ee 100644 > > --- a/src/ipa/ipu3/ipu3.cpp > > +++ b/src/ipa/ipu3/ipu3.cpp > > @@ -31,6 +31,7 @@ > > #include "libcamera/internal/mapped_framebuffer.h" > > > > #include "algorithms/agc.h" > > +#include "algorithms/af.h" > > > Ditto. > > > #include "algorithms/algorithm.h" > > #include "algorithms/awb.h" > > #include "algorithms/blc.h" > > @@ -298,6 +299,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>());
On 18/11/2021 13:42, Jean-Michel Hautbois wrote: > Hi Umang, > > On 18/11/2021 13:31, Umang Jain wrote: >> Hi Kate, >> >> Thank you for you work on this. >> >> While I understand this is an MVP (as posted by you in one of the >> other autofocus threads) here are some of my high-level queries and >> understanding. The interface to handle autofocus (and focus controls) >> in general is missing from libcamera, so I understand the current >> limitations as well, to move this forward. >> >> On 11/15/21 11:14 AM, 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. >> >> >> Can you please suggest a high level reading of finding the focus on >> the image? I don't understand the implementation / algorithm for the >> focus-sweep implemented in process()? >> >>> >>> Signed-off-by: Kate Hsuan <hpa@redhat.com> >>> --- >>> src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ >>> src/ipa/ipu3/algorithms/af.h | 48 ++++++++ >>> src/ipa/ipu3/algorithms/meson.build | 3 +- >>> src/ipa/ipu3/ipa_context.h | 6 + >>> src/ipa/ipu3/ipu3.cpp | 2 + >>> 5 files changed, 223 insertions(+), 1 deletion(-) >>> create mode 100644 src/ipa/ipu3/algorithms/af.cpp >>> create mode 100644 src/ipa/ipu3/algorithms/af.h >>> >>> diff --git a/src/ipa/ipu3/algorithms/af.cpp >>> b/src/ipa/ipu3/algorithms/af.cpp >>> new file mode 100644 >>> index 00000000..c276b539 >>> --- /dev/null >>> +++ b/src/ipa/ipu3/algorithms/af.cpp >> >> >> I won't get into style issues here (as don't seem relevant for now), >> but for your information we do have a style checker at: >> >> $libcamera/utils/checkstyle.py <commit-id-of-the-patch> >> >>> @@ -0,0 +1,165 @@ >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>> +/* >>> + * Copyright (C) 2021, Red Hat >>> + * >>> + * af.cpp - IPU3 Af control >> >> >> would be better to expand on the acronym "IPU3 Auto Focus control" maybe >> >>> + */ >>> + >>> +#include "af.h" >>> + >>> +#include <algorithm> >>> +#include <chrono> >>> +#include <cmath> >>> +#include <numeric> >>> + >>> +#include <linux/videodev2.h> >>> +#include <sys/types.h> >>> +#include <sys/stat.h> >>> +#include <fcntl.h> >>> +#include <sys/ioctl.h> >>> +#include <unistd.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 { >>> + >>> +LOG_DEFINE_CATEGORY(IPU3Af) >>> + >>> +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) >>> +{ >>> + /* For surface Go 2 back camera VCM */ >>> + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); >> >> >> So I have nautilus (Samsung Chromebook v2) which as a UVC camera and a >> IPU3 one (with IMX258 sensor). The VCM there is dw9807. >> >> I tried this patch on nautilus by mapping the v4l-subdevX to dw9807 >> and it did work. I can literally hear the VCM move and tries to focus >> on the scene. However, there are a few niggles I have noticed: >> >> The autofocus seems to lock up quite frequently leaving an un-focused >> streaming. However if I put up a object close to lens it does tries to >> focus it again (then locks up again but works sometimes). I am >> deliberately leaving the details out as it's too broad to say anything >> specific from nautilus' side. > > I will let Kate answer in depth obviously. From what I know, the AF > needs a grid configuration in IPU3, and the default grid is set to > [16*8,16*8] pixels, starting at x=10 and y=2: > https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9587 > > > It means that if your BDS output size is 1280x720 for instance, you will > focus on the 10% left part and 17% top part of your scene. Depending on > the scene, you might very well be out-of-focus :-). > > That's why I suggested adding a configure() call to set a grid based on > the BDS grid, to make it focus in the center as a default. > Be aware that AF grid can't always be the same as AWB grid, as the limits are not the same: https://lore.kernel.org/linux-media/1634525295-1410-1-git-send-email-bingbu.cao@intel.com/ And I can see in CrOS that the log2 may not be 7 but 6: https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h#32 >> >> I am interesting in getting better understanding of the values >> reported, so once I have a overall reading on the topic, I can start >> debugging the process(). >> >>> +} >>> + >>> +Af::~Af() >>> +{ >>> + if(vcmFd_ != -1) >>> + close(vcmFd_); >>> +} >>> + >>> +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) >> >> >> I think [[maybe_unused]] on configInfo as a replacement for the below >> line >> >>> +{ >>> + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; >>> + context.frameContext.af.focus = 0; >>> + context.frameContext.af.maxVar = 0; >>> + context.frameContext.af.stable = false; >>> + >>> + maxVariance_ = 0; >>> + ignoreFrame_ = 100; >>> + >>> + return 0; >>> +} >>> + >>> +void Af::vcmSet(int value) >>> +{ >>> + int ret; >>> + struct v4l2_control ctrl; >>> + if(vcmFd_ == -1) >>> + return; >> >> >> Maybe we should return -EINVAL? >> >>> + memset(&ctrl, 0, sizeof(struct v4l2_control)); >>> + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; >>> + ctrl.value = value; >>> + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); >> >> >> and return ret instead? To check if we really succeeded on setting the >> control >> >>> + >>> +} >>> + >>> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) >>> +{ >>> + typedef struct y_table_item { >>> + uint16_t y1_avg; >>> + uint16_t y2_avg; >>> + }y_table_item_t; >>> + >>> + uint32_t total = 0; >>> + double mean; >>> + uint64_t var_sum = 0; >>> + y_table_item_t *y_item; >>> + >>> + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; >>> + int z = 0; >>> + >>> + 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; >>> + } >>> + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); >>> + LOG(IPU3Af, Debug) << "variance: " << currentVar_; >>> + >>> + if( context.frameContext.af.stable == true ) >>> + { >>> + const uint32_t diff_var = std::abs(currentVar_ - >>> context.frameContext.af.maxVar); >>> + const double var_ratio = diff_var / >>> context.frameContext.af.maxVar; >>> + LOG(IPU3Af, Debug) << "Change ratio: " >>> + << var_ratio >>> + << " current focus: " >>> + << context.frameContext.af.focus; >>> + if(var_ratio > 0.8) >> >> >> hmm, I think we should opt for "variance_" instead "var_" as var in >> general has variable meaning, nevermind, details... >> >>> + { >>> + if(ignoreFrame_ == 0) >>> + { >>> + context.frameContext.af.maxVar = 0; >>> + context.frameContext.af.focus = 0; >>> + focus_ = 0; >>> + context.frameContext.af.stable = false; >>> + ignoreFrame_ = 60; >>> + } >>> + else >>> + ignoreFrame_--; >>> + }else >>> + ignoreFrame_ = 60; >>> + }else >>> + { >>> + if(ignoreFrame_ != 0) >>> + ignoreFrame_--; >>> + else{ >>> + if(currentVar_ > context.frameContext.af.maxVar) >>> + { >>> + context.frameContext.af.maxVar = currentVar_; >>> + context.frameContext.af.focus = focus_; >>> + } >>> + >>> + if(focus_ > 1023) >>> + { >>> + context.frameContext.af.stable = true; >>> + vcmSet(context.frameContext.af.focus); >>> + LOG(IPU3Af, Debug) << "Finall Focus " >>> + << context.frameContext.af.focus; >>> + }else >>> + { >>> + focus_ += 5; >>> + vcmSet(focus_); >>> + } >>> + LOG(IPU3Af, Debug) << "Focus searching max var is: " >>> + << context.frameContext.af.maxVar >>> + << " Focus step is " >>> + << context.frameContext.af.focus; >>> + } >>> + } >>> +} >>> + >>> + >>> +} /* namespace ipa::ipu3::algorithms */ >>> + >>> +} /* namespace libcamera */ >>> \ No newline at end of file >>> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h >>> new file mode 100644 >>> index 00000000..64c704b1 >>> --- /dev/null >>> +++ b/src/ipa/ipu3/algorithms/af.h >>> @@ -0,0 +1,48 @@ >>> +/* 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 >>> +{ >>> +public: >>> + Af(); >>> + ~Af(); >>> + >>> + int configure(IPAContext &context, const IPAConfigInfo >>> &configInfo) override; >>> + void process(IPAContext &context, const ipu3_uapi_stats_3a >>> *stats) override; >>> + >>> +private: >>> + void filterVariance(double new_var); >>> + >>> + void vcmSet(int value); >>> + >>> + int vcmFd_; >>> + int focus_; >>> + unsigned int ignoreFrame_; >> >> >> What's this stand for? Is it ignoring a number of frames? >> >>> + double maxVariance_; >>> + double currentVar_; >>> + >>> +}; >>> + >>> +} /* 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 3ec42f72..d32c61d2 100644 >>> --- a/src/ipa/ipu3/algorithms/meson.build >>> +++ b/src/ipa/ipu3/algorithms/meson.build >>> @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ >>> 'algorithm.cpp', >>> 'awb.cpp', >>> 'blc.cpp', >>> - 'tone_mapping.cpp', >>> + 'af.cpp', >>> + 'tone_mapping.cpp' >> >> >> alphabetical order please, I think checkstyle.py will report this as well >> >>> ]) >>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h >>> index 1e46c61a..5d92f63c 100644 >>> --- a/src/ipa/ipu3/ipa_context.h >>> +++ b/src/ipa/ipu3/ipa_context.h >>> @@ -47,6 +47,12 @@ struct IPAFrameContext { >>> } gains; >>> } awb; >>> + struct { >>> + uint32_t focus; >>> + double maxVar; >>> + bool stable; >>> + } af; >>> + >>> struct { >>> double gamma; >>> struct ipu3_uapi_gamma_corr_lut gammaCorrection; >>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp >>> index 5c51607d..980815ee 100644 >>> --- a/src/ipa/ipu3/ipu3.cpp >>> +++ b/src/ipa/ipu3/ipu3.cpp >>> @@ -31,6 +31,7 @@ >>> #include "libcamera/internal/mapped_framebuffer.h" >>> #include "algorithms/agc.h" >>> +#include "algorithms/af.h" >> >> >> Ditto. >> >>> #include "algorithms/algorithm.h" >>> #include "algorithms/awb.h" >>> #include "algorithms/blc.h" >>> @@ -298,6 +299,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>()); >>>
Hi Umang, On Thu, Nov 18, 2021 at 8:40 PM Umang Jain <umang.jain@ideasonboard.com> wrote: > > Hi Kate, > Thank you for you work on this. > > While I understand this is an MVP (as posted by you in one of the other > autofocus threads) here are some of my high-level queries and > understanding. The interface to handle autofocus (and focus controls) in > general is missing from libcamera, so I understand the current > limitations as well, to move this forward. Yes, I agree with you. Since VCM was driven and works, I quickly made this POC version of the autofocus then write the missing interface piece by piece. > > On 11/15/21 11:14 AM, 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. > > > Can you please suggest a high level reading of finding the focus on the > image? I don't understand the implementation / algorithm for the > focus-sweep implemented in process()? > The idea of autofocus is to find the edge of the image then determine the contrast of the image of every focus step. The blurred image will get a lower value of the contrast and the clearer image will get a higher contrast. Based on this, the max contrast image can be found in a greedy manner by comparing all the contrast values of each image from 0 to max step of the lens position. Laplacian of Gaussian and Sodel can be used to find the edge of the image but they require more CPU resources to find the edge and compute the contrast. I traced the IPU3 kernel code and ipu3-ipa repo a little bit and found it provides an AF raw buffer and contains the low pass and high pass filter statistic values. Typically, the high pass can be used to determine the sharpness (contrast) of the image. (I'm not an expert on image processing. if it is wrong please correct me :) ). So, the problem is simplified to calculate the variance of the AF buffer and search the image with the maximum variance. The algorithm is simple, move the lens from 0 - 1023 (max step of dw9719), estimate the variance of each step, and find the lens position with max variance. Finally, move the lens to the position. > > > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > > --- > > src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ > > src/ipa/ipu3/algorithms/af.h | 48 ++++++++ > > src/ipa/ipu3/algorithms/meson.build | 3 +- > > src/ipa/ipu3/ipa_context.h | 6 + > > src/ipa/ipu3/ipu3.cpp | 2 + > > 5 files changed, 223 insertions(+), 1 deletion(-) > > create mode 100644 src/ipa/ipu3/algorithms/af.cpp > > create mode 100644 src/ipa/ipu3/algorithms/af.h > > > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > > new file mode 100644 > > index 00000000..c276b539 > > --- /dev/null > > +++ b/src/ipa/ipu3/algorithms/af.cpp > > > I won't get into style issues here (as don't seem relevant for now), but > for your information we do have a style checker at: > > $libcamera/utils/checkstyle.py <commit-id-of-the-patch> Thanks for noticing me this :) > > > @@ -0,0 +1,165 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2021, Red Hat > > + * > > + * af.cpp - IPU3 Af control > > > would be better to expand on the acronym "IPU3 Auto Focus control" maybe Ok thanks. > > > + */ > > + > > +#include "af.h" > > + > > +#include <algorithm> > > +#include <chrono> > > +#include <cmath> > > +#include <numeric> > > + > > +#include <linux/videodev2.h> > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <fcntl.h> > > +#include <sys/ioctl.h> > > +#include <unistd.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 { > > + > > +LOG_DEFINE_CATEGORY(IPU3Af) > > + > > +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) > > +{ > > + /* For surface Go 2 back camera VCM */ > > + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); > > > So I have nautilus (Samsung Chromebook v2) which as a UVC camera and a > IPU3 one (with IMX258 sensor). The VCM there is dw9807. > > I tried this patch on nautilus by mapping the v4l-subdevX to dw9807 and > it did work. I can literally hear the VCM move and tries to focus on the > scene. However, there are a few niggles I have noticed: > > The autofocus seems to lock up quite frequently leaving an un-focused > streaming. However if I put up a object close to lens it does tries to The AF area should be at the top left corner of the sensor (default). I don't configure the AF grid in this patch. > focus it again (then locks up again but works sometimes). I am > deliberately leaving the details out as it's too broad to say anything > specific from nautilus' side. Since the AE can not work correctly on my Surface Go 2, I manually set the exposure and gain to get a stable image. You could try it. > > I am interesting in getting better understanding of the values reported, > so once I have a overall reading on the topic, I can start debugging the > process(). The value of debug messages are all around variance and focus step. :) > > > +} > > + > > +Af::~Af() > > +{ > > + if(vcmFd_ != -1) > > + close(vcmFd_); > > +} > > + > > +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > > I think [[maybe_unused]] on configInfo as a replacement for the below line OK thanks. > > > +{ > > + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; > > + context.frameContext.af.focus = 0; > > + context.frameContext.af.maxVar = 0; > > + context.frameContext.af.stable = false; > > + > > + maxVariance_ = 0; > > + ignoreFrame_ = 100; > > + > > + return 0; > > +} > > + > > +void Af::vcmSet(int value) > > +{ > > + int ret; > > + struct v4l2_control ctrl; > > + if(vcmFd_ == -1) > > + return; > > > Maybe we should return -EINVAL? Sure. > > > + memset(&ctrl, 0, sizeof(struct v4l2_control)); > > + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; > > + ctrl.value = value; > > + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); > > > and return ret instead? To check if we really succeeded on setting the > control Here I may improve the error checking but we don't actually put ioctl here. It may move to someplace of libcamera. > > > + > > +} > > + > > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) > > +{ > > + typedef struct y_table_item { > > + uint16_t y1_avg; > > + uint16_t y2_avg; > > + }y_table_item_t; > > + > > + uint32_t total = 0; > > + double mean; > > + uint64_t var_sum = 0; > > + y_table_item_t *y_item; > > + > > + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; > > + int z = 0; > > + > > + 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; > > + } > > + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); > > + LOG(IPU3Af, Debug) << "variance: " << currentVar_; > > + > > + if( context.frameContext.af.stable == true ) > > + { > > + const uint32_t diff_var = std::abs(currentVar_ - context.frameContext.af.maxVar); > > + const double var_ratio = diff_var / context.frameContext.af.maxVar; > > + LOG(IPU3Af, Debug) << "Change ratio: " > > + << var_ratio > > + << " current focus: " > > + << context.frameContext.af.focus; > > + if(var_ratio > 0.8) > > > hmm, I think we should opt for "variance_" instead "var_" as var in > general has variable meaning, nevermind, details... OK, sure. thank you. > > > + { > > + if(ignoreFrame_ == 0) > > + { > > + context.frameContext.af.maxVar = 0; > > + context.frameContext.af.focus = 0; > > + focus_ = 0; > > + context.frameContext.af.stable = false; > > + ignoreFrame_ = 60; > > + } > > + else > > + ignoreFrame_--; > > + }else > > + ignoreFrame_ = 60; > > + }else > > + { > > + if(ignoreFrame_ != 0) > > + ignoreFrame_--; > > + else{ > > + if(currentVar_ > context.frameContext.af.maxVar) > > + { > > + context.frameContext.af.maxVar = currentVar_; > > + context.frameContext.af.focus = focus_; > > + } > > + > > + if(focus_ > 1023) > > + { > > + context.frameContext.af.stable = true; > > + vcmSet(context.frameContext.af.focus); > > + LOG(IPU3Af, Debug) << "Finall Focus " > > + << context.frameContext.af.focus; > > + }else > > + { > > + focus_ += 5; > > + vcmSet(focus_); > > + } > > + LOG(IPU3Af, Debug) << "Focus searching max var is: " > > + << context.frameContext.af.maxVar > > + << " Focus step is " > > + << context.frameContext.af.focus; > > + } > > + } > > +} > > + > > + > > +} /* namespace ipa::ipu3::algorithms */ > > + > > +} /* namespace libcamera */ > > \ No newline at end of file > > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > > new file mode 100644 > > index 00000000..64c704b1 > > --- /dev/null > > +++ b/src/ipa/ipu3/algorithms/af.h > > @@ -0,0 +1,48 @@ > > +/* 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 > > +{ > > +public: > > + Af(); > > + ~Af(); > > + > > + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > > + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > > + > > +private: > > + void filterVariance(double new_var); > > + > > + void vcmSet(int value); > > + > > + int vcmFd_; > > + int focus_; > > + unsigned int ignoreFrame_; > > > What's this stand for? Is it ignoring a number of frames? If we suddenly move the lens position, the variance will be out of the range of change and trigger another AF scan. So, after ignoring some frames and waiting for the AF raw buffer stable, it starts to calculate variance again. > > > + double maxVariance_; > > + double currentVar_; > > + > > +}; > > + > > +} /* 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 3ec42f72..d32c61d2 100644 > > --- a/src/ipa/ipu3/algorithms/meson.build > > +++ b/src/ipa/ipu3/algorithms/meson.build > > @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ > > 'algorithm.cpp', > > 'awb.cpp', > > 'blc.cpp', > > - 'tone_mapping.cpp', > > + 'af.cpp', > > + 'tone_mapping.cpp' > > > alphabetical order please, I think checkstyle.py will report this as well Ok thanks. > > > ]) > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > > index 1e46c61a..5d92f63c 100644 > > --- a/src/ipa/ipu3/ipa_context.h > > +++ b/src/ipa/ipu3/ipa_context.h > > @@ -47,6 +47,12 @@ struct IPAFrameContext { > > } gains; > > } awb; > > > > + struct { > > + uint32_t focus; > > + double maxVar; > > + bool stable; > > + } af; > > + > > struct { > > double gamma; > > struct ipu3_uapi_gamma_corr_lut gammaCorrection; > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > > index 5c51607d..980815ee 100644 > > --- a/src/ipa/ipu3/ipu3.cpp > > +++ b/src/ipa/ipu3/ipu3.cpp > > @@ -31,6 +31,7 @@ > > #include "libcamera/internal/mapped_framebuffer.h" > > > > #include "algorithms/agc.h" > > +#include "algorithms/af.h" > > > Ditto. > > > #include "algorithms/algorithm.h" > > #include "algorithms/awb.h" > > #include "algorithms/blc.h" > > @@ -298,6 +299,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.
Hi Jean-Michel, On Thu, Nov 18, 2021 at 8:43 PM Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> wrote: > > Hi Umang, > > On 18/11/2021 13:31, Umang Jain wrote: > > Hi Kate, > > > > Thank you for you work on this. > > > > While I understand this is an MVP (as posted by you in one of the other > > autofocus threads) here are some of my high-level queries and > > understanding. The interface to handle autofocus (and focus controls) in > > general is missing from libcamera, so I understand the current > > limitations as well, to move this forward. > > > > On 11/15/21 11:14 AM, 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. > > > > > > Can you please suggest a high level reading of finding the focus on the > > image? I don't understand the implementation / algorithm for the > > focus-sweep implemented in process()? > > > >> > >> Signed-off-by: Kate Hsuan <hpa@redhat.com> > >> --- > >> src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ > >> src/ipa/ipu3/algorithms/af.h | 48 ++++++++ > >> src/ipa/ipu3/algorithms/meson.build | 3 +- > >> src/ipa/ipu3/ipa_context.h | 6 + > >> src/ipa/ipu3/ipu3.cpp | 2 + > >> 5 files changed, 223 insertions(+), 1 deletion(-) > >> create mode 100644 src/ipa/ipu3/algorithms/af.cpp > >> create mode 100644 src/ipa/ipu3/algorithms/af.h > >> > >> diff --git a/src/ipa/ipu3/algorithms/af.cpp > >> b/src/ipa/ipu3/algorithms/af.cpp > >> new file mode 100644 > >> index 00000000..c276b539 > >> --- /dev/null > >> +++ b/src/ipa/ipu3/algorithms/af.cpp > > > > > > I won't get into style issues here (as don't seem relevant for now), but > > for your information we do have a style checker at: > > > > $libcamera/utils/checkstyle.py <commit-id-of-the-patch> > > > >> @@ -0,0 +1,165 @@ > >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > >> +/* > >> + * Copyright (C) 2021, Red Hat > >> + * > >> + * af.cpp - IPU3 Af control > > > > > > would be better to expand on the acronym "IPU3 Auto Focus control" maybe > > > >> + */ > >> + > >> +#include "af.h" > >> + > >> +#include <algorithm> > >> +#include <chrono> > >> +#include <cmath> > >> +#include <numeric> > >> + > >> +#include <linux/videodev2.h> > >> +#include <sys/types.h> > >> +#include <sys/stat.h> > >> +#include <fcntl.h> > >> +#include <sys/ioctl.h> > >> +#include <unistd.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 { > >> + > >> +LOG_DEFINE_CATEGORY(IPU3Af) > >> + > >> +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) > >> +{ > >> + /* For surface Go 2 back camera VCM */ > >> + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); > > > > > > So I have nautilus (Samsung Chromebook v2) which as a UVC camera and a > > IPU3 one (with IMX258 sensor). The VCM there is dw9807. > > > > I tried this patch on nautilus by mapping the v4l-subdevX to dw9807 and > > it did work. I can literally hear the VCM move and tries to focus on the > > scene. However, there are a few niggles I have noticed: > > > > The autofocus seems to lock up quite frequently leaving an un-focused > > streaming. However if I put up a object close to lens it does tries to > > focus it again (then locks up again but works sometimes). I am > > deliberately leaving the details out as it's too broad to say anything > > specific from nautilus' side. > > I will let Kate answer in depth obviously. From what I know, the AF > needs a grid configuration in IPU3, and the default grid is set to > [16*8,16*8] pixels, starting at x=10 and y=2: > https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9587 Thank you for providing the grid information. I am trying to set this in my v2 patch. > > It means that if your BDS output size is 1280x720 for instance, you will > focus on the 10% left part and 17% top part of your scene. Depending on > the scene, you might very well be out-of-focus :-). Yes, the top left of the sensor. > > That's why I suggested adding a configure() call to set a grid based on > the BDS grid, to make it focus in the center as a default. > > > > > I am interesting in getting better understanding of the values reported, > > so once I have a overall reading on the topic, I can start debugging the > > process(). > > > >> +} > >> + > >> +Af::~Af() > >> +{ > >> + if(vcmFd_ != -1) > >> + close(vcmFd_); > >> +} > >> + > >> +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > > > > > I think [[maybe_unused]] on configInfo as a replacement for the below line > > > >> +{ > >> + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; > >> + context.frameContext.af.focus = 0; > >> + context.frameContext.af.maxVar = 0; > >> + context.frameContext.af.stable = false; > >> + > >> + maxVariance_ = 0; > >> + ignoreFrame_ = 100; > >> + > >> + return 0; > >> +} > >> + > >> +void Af::vcmSet(int value) > >> +{ > >> + int ret; > >> + struct v4l2_control ctrl; > >> + if(vcmFd_ == -1) > >> + return; > > > > > > Maybe we should return -EINVAL? > > > >> + memset(&ctrl, 0, sizeof(struct v4l2_control)); > >> + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; > >> + ctrl.value = value; > >> + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); > > > > > > and return ret instead? To check if we really succeeded on setting the > > control > > > >> + > >> +} > >> + > >> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) > >> +{ > >> + typedef struct y_table_item { > >> + uint16_t y1_avg; > >> + uint16_t y2_avg; > >> + }y_table_item_t; > >> + > >> + uint32_t total = 0; > >> + double mean; > >> + uint64_t var_sum = 0; > >> + y_table_item_t *y_item; > >> + > >> + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; > >> + int z = 0; > >> + > >> + 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; > >> + } > >> + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); > >> + LOG(IPU3Af, Debug) << "variance: " << currentVar_; > >> + > >> + if( context.frameContext.af.stable == true ) > >> + { > >> + const uint32_t diff_var = std::abs(currentVar_ - > >> context.frameContext.af.maxVar); > >> + const double var_ratio = diff_var / > >> context.frameContext.af.maxVar; > >> + LOG(IPU3Af, Debug) << "Change ratio: " > >> + << var_ratio > >> + << " current focus: " > >> + << context.frameContext.af.focus; > >> + if(var_ratio > 0.8) > > > > > > hmm, I think we should opt for "variance_" instead "var_" as var in > > general has variable meaning, nevermind, details... > > > >> + { > >> + if(ignoreFrame_ == 0) > >> + { > >> + context.frameContext.af.maxVar = 0; > >> + context.frameContext.af.focus = 0; > >> + focus_ = 0; > >> + context.frameContext.af.stable = false; > >> + ignoreFrame_ = 60; > >> + } > >> + else > >> + ignoreFrame_--; > >> + }else > >> + ignoreFrame_ = 60; > >> + }else > >> + { > >> + if(ignoreFrame_ != 0) > >> + ignoreFrame_--; > >> + else{ > >> + if(currentVar_ > context.frameContext.af.maxVar) > >> + { > >> + context.frameContext.af.maxVar = currentVar_; > >> + context.frameContext.af.focus = focus_; > >> + } > >> + > >> + if(focus_ > 1023) > >> + { > >> + context.frameContext.af.stable = true; > >> + vcmSet(context.frameContext.af.focus); > >> + LOG(IPU3Af, Debug) << "Finall Focus " > >> + << context.frameContext.af.focus; > >> + }else > >> + { > >> + focus_ += 5; > >> + vcmSet(focus_); > >> + } > >> + LOG(IPU3Af, Debug) << "Focus searching max var is: " > >> + << context.frameContext.af.maxVar > >> + << " Focus step is " > >> + << context.frameContext.af.focus; > >> + } > >> + } > >> +} > >> + > >> + > >> +} /* namespace ipa::ipu3::algorithms */ > >> + > >> +} /* namespace libcamera */ > >> \ No newline at end of file > >> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > >> new file mode 100644 > >> index 00000000..64c704b1 > >> --- /dev/null > >> +++ b/src/ipa/ipu3/algorithms/af.h > >> @@ -0,0 +1,48 @@ > >> +/* 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 > >> +{ > >> +public: > >> + Af(); > >> + ~Af(); > >> + > >> + int configure(IPAContext &context, const IPAConfigInfo > >> &configInfo) override; > >> + void process(IPAContext &context, const ipu3_uapi_stats_3a > >> *stats) override; > >> + > >> +private: > >> + void filterVariance(double new_var); > >> + > >> + void vcmSet(int value); > >> + > >> + int vcmFd_; > >> + int focus_; > >> + unsigned int ignoreFrame_; > > > > > > What's this stand for? Is it ignoring a number of frames? > > > >> + double maxVariance_; > >> + double currentVar_; > >> + > >> +}; > >> + > >> +} /* 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 3ec42f72..d32c61d2 100644 > >> --- a/src/ipa/ipu3/algorithms/meson.build > >> +++ b/src/ipa/ipu3/algorithms/meson.build > >> @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ > >> 'algorithm.cpp', > >> 'awb.cpp', > >> 'blc.cpp', > >> - 'tone_mapping.cpp', > >> + 'af.cpp', > >> + 'tone_mapping.cpp' > > > > > > alphabetical order please, I think checkstyle.py will report this as well > > > >> ]) > >> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > >> index 1e46c61a..5d92f63c 100644 > >> --- a/src/ipa/ipu3/ipa_context.h > >> +++ b/src/ipa/ipu3/ipa_context.h > >> @@ -47,6 +47,12 @@ struct IPAFrameContext { > >> } gains; > >> } awb; > >> + struct { > >> + uint32_t focus; > >> + double maxVar; > >> + bool stable; > >> + } af; > >> + > >> struct { > >> double gamma; > >> struct ipu3_uapi_gamma_corr_lut gammaCorrection; > >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > >> index 5c51607d..980815ee 100644 > >> --- a/src/ipa/ipu3/ipu3.cpp > >> +++ b/src/ipa/ipu3/ipu3.cpp > >> @@ -31,6 +31,7 @@ > >> #include "libcamera/internal/mapped_framebuffer.h" > >> #include "algorithms/agc.h" > >> +#include "algorithms/af.h" > > > > > > Ditto. > > > >> #include "algorithms/algorithm.h" > >> #include "algorithms/awb.h" > >> #include "algorithms/blc.h" > >> @@ -298,6 +299,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>()); > >> >
diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp new file mode 100644 index 00000000..c276b539 --- /dev/null +++ b/src/ipa/ipu3/algorithms/af.cpp @@ -0,0 +1,165 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Red Hat + * + * af.cpp - IPU3 Af control + */ + +#include "af.h" + +#include <algorithm> +#include <chrono> +#include <cmath> +#include <numeric> + +#include <linux/videodev2.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <sys/ioctl.h> +#include <unistd.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 { + +LOG_DEFINE_CATEGORY(IPU3Af) + +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0) +{ + /* For surface Go 2 back camera VCM */ + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR); +} + +Af::~Af() +{ + if(vcmFd_ != -1) + close(vcmFd_); +} + +int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) +{ + const IPAConfigInfo tmp __attribute__((unused)) = configInfo; + context.frameContext.af.focus = 0; + context.frameContext.af.maxVar = 0; + context.frameContext.af.stable = false; + + maxVariance_ = 0; + ignoreFrame_ = 100; + + return 0; +} + +void Af::vcmSet(int value) +{ + int ret; + struct v4l2_control ctrl; + if(vcmFd_ == -1) + return; + memset(&ctrl, 0, sizeof(struct v4l2_control)); + ctrl.id = V4L2_CID_FOCUS_ABSOLUTE; + ctrl.value = value; + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl); + +} + +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats ) +{ + typedef struct y_table_item { + uint16_t y1_avg; + uint16_t y2_avg; + }y_table_item_t; + + uint32_t total = 0; + double mean; + uint64_t var_sum = 0; + y_table_item_t *y_item; + + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table; + int z = 0; + + 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; + } + currentVar_ = static_cast<double>(var_sum) /static_cast<double>(z); + LOG(IPU3Af, Debug) << "variance: " << currentVar_; + + if( context.frameContext.af.stable == true ) + { + const uint32_t diff_var = std::abs(currentVar_ - context.frameContext.af.maxVar); + const double var_ratio = diff_var / context.frameContext.af.maxVar; + LOG(IPU3Af, Debug) << "Change ratio: " + << var_ratio + << " current focus: " + << context.frameContext.af.focus; + if(var_ratio > 0.8) + { + if(ignoreFrame_ == 0) + { + context.frameContext.af.maxVar = 0; + context.frameContext.af.focus = 0; + focus_ = 0; + context.frameContext.af.stable = false; + ignoreFrame_ = 60; + } + else + ignoreFrame_--; + }else + ignoreFrame_ = 60; + }else + { + if(ignoreFrame_ != 0) + ignoreFrame_--; + else{ + if(currentVar_ > context.frameContext.af.maxVar) + { + context.frameContext.af.maxVar = currentVar_; + context.frameContext.af.focus = focus_; + } + + if(focus_ > 1023) + { + context.frameContext.af.stable = true; + vcmSet(context.frameContext.af.focus); + LOG(IPU3Af, Debug) << "Finall Focus " + << context.frameContext.af.focus; + }else + { + focus_ += 5; + vcmSet(focus_); + } + LOG(IPU3Af, Debug) << "Focus searching max var is: " + << context.frameContext.af.maxVar + << " Focus step is " + << context.frameContext.af.focus; + } + } +} + + +} /* namespace ipa::ipu3::algorithms */ + +} /* namespace libcamera */ \ No newline at end of file diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h new file mode 100644 index 00000000..64c704b1 --- /dev/null +++ b/src/ipa/ipu3/algorithms/af.h @@ -0,0 +1,48 @@ +/* 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 +{ +public: + Af(); + ~Af(); + + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; + void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; + +private: + void filterVariance(double new_var); + + void vcmSet(int value); + + int vcmFd_; + int focus_; + unsigned int ignoreFrame_; + double maxVariance_; + double currentVar_; + +}; + +} /* 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 3ec42f72..d32c61d2 100644 --- a/src/ipa/ipu3/algorithms/meson.build +++ b/src/ipa/ipu3/algorithms/meson.build @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([ 'algorithm.cpp', 'awb.cpp', 'blc.cpp', - 'tone_mapping.cpp', + 'af.cpp', + 'tone_mapping.cpp' ]) diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index 1e46c61a..5d92f63c 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -47,6 +47,12 @@ struct IPAFrameContext { } gains; } awb; + struct { + uint32_t focus; + double maxVar; + bool stable; + } af; + struct { double gamma; struct ipu3_uapi_gamma_corr_lut gammaCorrection; diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 5c51607d..980815ee 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -31,6 +31,7 @@ #include "libcamera/internal/mapped_framebuffer.h" #include "algorithms/agc.h" +#include "algorithms/af.h" #include "algorithms/algorithm.h" #include "algorithms/awb.h" #include "algorithms/blc.h" @@ -298,6 +299,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>());
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. Signed-off-by: Kate Hsuan <hpa@redhat.com> --- src/ipa/ipu3/algorithms/af.cpp | 165 ++++++++++++++++++++++++++++ src/ipa/ipu3/algorithms/af.h | 48 ++++++++ src/ipa/ipu3/algorithms/meson.build | 3 +- src/ipa/ipu3/ipa_context.h | 6 + src/ipa/ipu3/ipu3.cpp | 2 + 5 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 src/ipa/ipu3/algorithms/af.cpp create mode 100644 src/ipa/ipu3/algorithms/af.h