[libcamera-devel] [PATCH] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Nov 18 13:50:13 CET 2021
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 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>
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>());
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list