[libcamera-devel] [PATCH] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Kate Hsuan
hpa at redhat.com
Fri Nov 19 07:06:05 CET 2021
Hi Umang,
On Thu, Nov 18, 2021 at 8:40 PM Umang Jain <umang.jain at 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 at 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.
--
BR,
Kate
More information about the libcamera-devel
mailing list