[libcamera-devel] [PATCH] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Mon Nov 15 07:49:38 CET 2021
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 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
> @@ -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>());
>
More information about the libcamera-devel
mailing list