[libcamera-devel] [RFC v5] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Kate Hsuan
hpa at redhat.com
Sat Feb 5 07:11:37 CET 2022
Hi Jean-Michel,
== The second part of the mail :)
On Fri, Jan 21, 2022 at 5:52 PM Jean-Michel Hautbois
<jeanmichel.hautbois at ideasonboard.com> wrote:
>
> Sorry, this is the second part of my review, bad key stroke :-(
>
> On 21/01/2022 10:26, Jean-Michel Hautbois wrote:
> > Hi Kate,
> >
> > Thanks for the patch !
> >
> > On 20/01/2022 12:41, 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.
> > s/approah/approach
> >
> >>
> >> The grid configuration is implemented as a context. Also,
> >> the grid parameter- AF_MIN_BLOCK_WIDTH is set to 4 (default
> >> is 3) since if the default value is used, x_start
> >> (x_start > 640) will be at an incorrect location of the
> >> image (rightmost of the sensor).
> >
> > We should investigate this, as it should not be a blocker ?
> >
> >>
> >> Signed-off-by: Kate Hsuan <hpa at redhat.com>
> >> ---
> >> src/ipa/ipu3/algorithms/af.cpp | 367 ++++++++++++++++++++++++++++
> >> src/ipa/ipu3/algorithms/af.h | 79 ++++++
> >> src/ipa/ipu3/algorithms/meson.build | 3 +-
> >> src/ipa/ipu3/ipa_context.cpp | 24 ++
> >> src/ipa/ipu3/ipa_context.h | 10 +
> >> src/ipa/ipu3/ipu3.cpp | 32 +++
> >> 6 files changed, 514 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..26c98868
> >> --- /dev/null
> >> +++ b/src/ipa/ipu3/algorithms/af.cpp
> >> @@ -0,0 +1,367 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2021, Red Hat
> >> + *
> >> + * af.cpp - IPU3 auto focus control
> >> + */
> >> +
> >> +#include "af.h"
> >> +
> >> +#include <algorithm>
> >> +#include <chrono>
> >> +#include <cmath>
> >> +#include <fcntl.h>
> >> +#include <numeric>
> >> +#include <sys/ioctl.h>
> >> +#include <sys/stat.h>
> >> +#include <sys/types.h>
> >> +#include <unistd.h>
> >> +
> >> +#include <linux/videodev2.h>
> >> +
> >> +#include <libcamera/base/log.h>
> >> +
> >> +#include <libcamera/ipa/core_ipa_interface.h>
> >> +
> >> +#include "libipa/histogram.h"
> >> +
> >> +/**
> >> + * \file af.h
> >> + */
> >> +
> >> +/**
> >> + * \def AF_MIN_GRID_WIDTH
> >> + * \brief the minimum width of AF grid.
> >> + * The minimum grid horizontal dimensions, in number of grid
> >> blocks(cells).
> >> +*/
> >
> > This is true for this one and all the defines, please replace those by
> > static constexpr instead ?
> >
> >> +
> >> +/**
> >> + * \def AF_MIN_GRID_HEIGHT
> >> + * \brief the minimum height of AF grid.
> >> + * The minimum grid horizontal dimensions, in number of grid
> >> blocks(cells).
> >> +*/
> >> +
> >> +/**
> >> + * \def AF_MIN_BLOCK_WIDTH
> >> + * \brief the minimum block size of the width.
> >> + */
> >> +
> >> +/**
> >> + * \def AF_MAX_GRID_HEIGHT
> >> + * \brief the minimum height of AF grid.
> >> + * The minimum grid horizontal dimensions, in number of grid
> >> blocks(cells).
> >> +*/
> >> +
> >> +/**
> >> + * \def AF_MAX_BLOCK_WIDTH
> >> + * \brief the minimum block size of the width.
> >> + */
> >> +
> >> +/**
> >> + * \def AF_MAX_BLOCK_WIDTH
> >> + * \brief the maximum block size of the width.
> >> + */
> >> +
> >> +/**
> >> + * \def AF_MIN_BLOCK_HEIGHT
> >> + * \brief the minimum block size of the height.
> >> + */
> >> +
> >> +/**
> >> + * \def AF_MAX_BLOCK_HEIGHT
> >> + * \brief the maximum block size of the height.
> >> + */
> >> +
> >> +/**
> >> + * \def AF_DEFAULT_HEIGHT_PER_SLICE
> >> + * \brief The default number of blocks in vertical axis per slice.
> >> + */
> >> +
> >> +namespace libcamera {
> >> +
> >> +using namespace std::literals::chrono_literals;
> >> +
> >> +namespace ipa::ipu3::algorithms {
> >> +
> >> +/**
> >> + * \class Af
> >> + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
> >
> > 'An auto-focus algorithm based on IPU3 statistics' maybe ?
> > This proposal should be taken with a grain of salt, as I am not a native
> > speaker ;-).
> >
> >> + *
> >> + * This algorithm is used to determine the position of the lens and
> >> get a
> >> + * focused image. The IPU3 AF accelerator computes the statistics,
> >> composed
> > s/accelerator/processing block/
> >
> >> + * by high pass and low pass filtered value and stores in a AF buffer.
> >> + * Typically, for a focused image, it has relative high contrast than a
> >> + * blurred image, i.e. an out of focus image. Therefore, if an image
> >> with the
> >> + * highest contrast can be found from the AF scan, the lens' position
> >> is the
> >> + * best step of the focus.
> >> + *
> >> + */
> >> +
> >> +LOG_DEFINE_CATEGORY(IPU3Af)
> >> +
> >> +/**
> >> + * Maximum focus value of the VCM control
> >> + * \todo should be obtained from the VCM driver
> >> + */
> >> +static constexpr uint32_t MaxFocusSteps_ = 1023;
> >
> > Remove the trailing underscore (same below):
> > s/MaxFocusSteps_/maxFocusSteps/
> >
> >> +
> >> +/* minimum focus step for searching appropriate focus */
> >> +static constexpr uint32_t coarseSearchStep_ = 10;
> >> +static constexpr uint32_t fineSearchStep_ = 1;
> >
> > Correct me if I am wrong:
> > - the coarse step is used to determine a first approximation of the
> > focused image
> > - The fine step is then used to find the optimal lens position
> >
> > The VCM used on SGo2 has 1024 steps, so, you may search for ~100 frames
> > (ie ~3 seconds at 30fps) before going into the fine search step ?
> >
> >> +
> >> +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0 */
> >> +static constexpr double MaxChange_ = 0.2;
> >> +
> >> +/* the numbers of frame to be ignored, before performing focus scan. */
> >> +static constexpr uint32_t ignoreFrame_ = 10;
> >> +
> >> +/* fine scan range 0 < findRange_ < 1 */
> >> +static constexpr double findRange_ = 0.05;
> >> +
> >> +/* settings for Auto Focus from the kernel */
> > Those filters are not the ones defined in the kernel ;-).
> >
> >> +static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {
> >> + { 0, 1, 3, 7 },
> >> + { 11, 13, 1, 2 },
> >> + { 8, 19, 34, 242 },
> >> + 0x7fdffbfe,
> >> + { 0, 1, 6, 6 },
> >> + { 13, 25, 3, 0 },
> >> + { 25, 3, 177, 254 },
> >> + 0x4e53ca72,
> >> + .y_calc = { 8, 8, 8, 8 },
> >> + .nf = { 0, 9, 0, 9, 0 },
> >> +};
> >
> > Use the fields names for all the structure:
> > static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {
> > .y1_coeff_0 = { 0, 1, 3, 7 },
> > .y1_coeff_1 = { 11, 13, 1, 2 },
> > etc.
> > };
> >
> >> +
> >> +Af::Af()
> >> + : focus_(0), goodFocus_(0), currentVariance_(0.0),
> >> previousVariance_(0.0),
> >> + coarseComplete_(false), fineComplete_(false)
> >> +{
> >> + maxStep_ = MaxFocusSteps_;
> >> +}
> >> +
> >> +Af::~Af()
> >> +{
> >> +}
> >> +
> >> +/**
> >> + * \copydoc libcamera::ipa::Algorithm::prepare
> >> + */
> >> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
> >> +{
> >> + const struct ipu3_uapi_grid_config &grid =
> >> context.configuration.af.afGrid;
> >> + params->acc_param.af.grid_cfg = grid;
> >> + params->acc_param.af.filter_config = afFilterConfigDefault;
> >> +
> >> + /* enable AF acc */
> > s/acc/processing block/
> >
> >> + params->use.acc_af = 1;
> >> +}
> >> +
> >> +/**
> >> + * \brief Configure the Af given a configInfo
> >> + * \param[in] context The shared IPA context
> >> + * \param[in] configInfo The IPA configuration data
> >> + *
> >> + * \return 0
> >> + */
> >> +int Af::configure(IPAContext &context,
> >> + [[maybe_unused]] const IPAConfigInfo &configInfo)
> >
> > Are you using the utils/hooks/ in your git repo ?
> > If so, I think you should have a misaligned warning here ?
> >
> >> +{
> >> + /* determined focus value i.e. current focus value */
> >> + context.frameContext.af.focus = 0;
> >> + /* maximum variance of the AF statistics */
> >> + context.frameContext.af.maxVariance = 0;
> >> + /* the stable AF value flag. if it is true, the AF should be in a
> >> stable state. */
> >> + context.frameContext.af.stable = false;
> >> + /* AF buffer length */
> >> + afRawBufferLen_ = context.configuration.af.afGrid.width *
> >> + context.configuration.af.afGrid.height;
> >
> > AFAIK, this is only used in the process() function, maybe could it be a
> > local variable ?
> >
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * \brief AF coarse scan
> >> + * \param[in] context The shared IPA context
> >> + *
> >> + */
> >> +void Af::afCoarseScan(IPAContext &context)
> >> +{
> >> + if (coarseComplete_ == true)
> >> + return;
> >> +
> >> + if (afScan(context, coarseSearchStep_)) {
> >> + coarseComplete_ = true;
> >> + context.frameContext.af.maxVariance = 0;
> >> + focus_ = context.frameContext.af.focus -
> >> (context.frameContext.af.focus * findRange_);
> >> + context.frameContext.af.focus = focus_;
> >> + previousVariance_ = 0;
> >> + maxStep_ = std::clamp(static_cast<uint32_t>(focus_ + (focus_
> >> * findRange_)), 0U, MaxFocusSteps_);
> >> + }
> >> +}
> >> +
> >> +/**
> >> + * \brief AF fine scan
> >> + *
> >> + * Finetune the lens position with moving 1 step for each variance
> >> computation.
> >> + *
> >> + * \param[in] context The shared IPA context
> >> + *
> >> + */
> >> +void Af::afFineScan(IPAContext &context)
> >> +{
> >> + if (coarseComplete_ != true)
> >> + return;
> >> +
> >> + if (afScan(context, fineSearchStep_)) {
> >> + context.frameContext.af.stable = true;
> >> + fineComplete_ = true;
> >> + }
> >> +}
> >> +
> >> +/**
> >> + * \brief AF reset
> >> + *
> >> + * Reset all the parameter to start over the AF process.
> >> + *
> >> + * \param[in] context The shared IPA context
> >> + *
> >> + */
> >> +void Af::afReset(IPAContext &context)
> >> +{
> >> + context.frameContext.af.maxVariance = 0;
> >> + context.frameContext.af.focus = 0;
> >> + focus_ = 0;
> >> + context.frameContext.af.stable = false;
> >> + ignoreCounter_ = ignoreFrame_;
> >> + previousVariance_ = 0.0;
> >> + coarseComplete_ = false;
> >> + fineComplete_ = false;
> >> + maxStep_ = MaxFocusSteps_;
> >> +}
> >> +
> >> +/**
> >> + * \brief AF scan
> >> + * \param[in] context The shared IPA context
> >> + *
> >> + * \return True, if it finds a AF value.
> >> + */
> >> +bool Af::afScan(IPAContext &context, int min_step)
> >> +{
> >> + /* find the maximum variance during the AF scan using a greedy
> >> strategy */
>
> I am interested here: why greedy :-) ?
>
> >> + if (currentVariance_ > context.frameContext.af.maxVariance) {
> >> + context.frameContext.af.maxVariance = currentVariance_;
> >> + goodFocus_ = focus_;
> >> + }
> >> +
> >> + if (focus_ > maxStep_) {
> >> + /* if reach the max step, move lens to the position and set
> >> "focus stable". */
> >> + context.frameContext.af.focus = goodFocus_;
> >> + return true;
> >> + } else {
> >> + /* check negative gradient */
>
> You are mixing gradient and variance terms. You are checking the
> direction of the variance derivate, right ?
>
> >> + if ((currentVariance_ - context.frameContext.af.maxVariance)
> >> > -(context.frameContext.af.maxVariance * 0.15)) {
> >> + focus_ += min_step;
> >> + context.frameContext.af.focus = focus_;
> >> + } else {
> >> + context.frameContext.af.focus = goodFocus_;
> >> + previousVariance_ = currentVariance_;
> >> + return true;
> >> + }
> >> + }
> >> + LOG(IPU3Af, Debug) << "Variance previous: "
> >> + << previousVariance_
> >> + << " current: "
> >> + << currentVariance_
> >> + << " Diff: "
> >> + << (currentVariance_ -
> >> context.frameContext.af.maxVariance);
> >> + previousVariance_ = currentVariance_;
> >> + LOG(IPU3Af, Debug) << "Focus searching max variance is: "
> >> + << context.frameContext.af.maxVariance
> >> + << " Focus step is "
> >> + << goodFocus_
> >> + << " Current scan is "
> >> + << focus_;
> >> + return false;
> >> +}
> >> +
> >> +/**
> >> + * \brief Determine the max contrast image and lens position. y_table
> >> is the
> >> + * statictic data from IPU3 and is composed of low pass and high pass
> >> filtered
> s/statictic/statistics
>
> >> + * value. High pass filtered value also represents the sharpness of
> >> the image.
> >> + * Based on this, if the image with highest variance of the high pass
> >> filtered
> >> + * value (contrast) during the AF scan, the position of the lens
> >> should be the
> >> + * best focus.
> >> + * \param[in] context The shared IPA context.
> >> + * \param[in] stats The statistic buffer of 3A from the IPU3.
> >> + */
> >> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> >> +{
> >> + uint32_t total = 0;
> >> + double mean;
> >> + uint64_t var_sum = 0;
> >> + y_table_item_t *y_item;
> >> + uint32_t z = 0;
> >> +
> >> + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
>
> static_cast<> ?
OK, I'll try to use static_cast.
>
> >> +
> >> + /**
> >> + * Calculate the mean and the varience AF statistics, since IPU3
> >> only determine the AF value
> s/varience/variance
>
> >> + * for a given grid.
> >> + * For coarse: low pass results are used.
> >> + * For fine: high pass results are used.
> >> + */
> >> + if (coarseComplete_) {
> >> + for (z = 0; z < afRawBufferLen_; z++) {
> >> + total = total + y_item[z].y2_avg;
> >> + }
> >> + mean = total / afRawBufferLen_;
> >> +
> >> + for (z = 0; z < afRawBufferLen_; z++) {
> >> + var_sum = var_sum + ((y_item[z].y2_avg - mean) *
> >> (y_item[z].y2_avg - mean));
> >> + }
> >> + } else {
> >> + for (z = 0; z < afRawBufferLen_; z++) {
> >> + total = total + y_item[z].y1_avg;
> >> + }
> >> + mean = total / afRawBufferLen_;
> >> +
> >> + for (z = 0; z < afRawBufferLen_; z++) {
> >> + var_sum = var_sum + ((y_item[z].y1_avg - mean) *
> >> (y_item[z].y1_avg - mean));
> >> + }
> >> + }
> >> +
> >> + /* Determine the average variance of the frame. */
> >> + currentVariance_ = static_cast<double>(var_sum) /
> >> static_cast<double>(afRawBufferLen_);
> >> + LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
> >> +
> >> + if (context.frameContext.af.stable == true) {
> >> + const uint32_t diff_var = std::abs(currentVariance_ -
> >> context.frameContext.af.maxVariance);
> >> + const double var_ratio = diff_var /
> >> context.frameContext.af.maxVariance;
> >> + LOG(IPU3Af, Debug) << "Rate of variance change: "
> >> + << var_ratio
> >> + << " current focus step: "
> >> + << context.frameContext.af.focus;
> >> + /**
> >> + * If the change ratio of contrast is over Maxchange_ (out of
> >> focus),
> >> + * trigger AF again.
> >> + */
> >> + if (var_ratio > MaxChange_) {
> >> + if (ignoreCounter_ == 0) {
> >> + afReset(context);
> >> + } else
> >> + ignoreCounter_--;
> >> + } else
> >> + ignoreCounter_ = ignoreFrame_;
> >> + } else {
> >> + if (ignoreCounter_ != 0)
> >> + ignoreCounter_--;
> >> + else {
> >> + afCoarseScan(context);
> >> + afFineScan(context);
> >> + }
> >> + }
> >> +}
> >> +
> >> +} /* namespace ipa::ipu3::algorithms */
> >> +
> >> +} /* namespace libcamera */
> >> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> >> new file mode 100644
> >> index 00000000..5654a964
> >> --- /dev/null
> >> +++ b/src/ipa/ipu3/algorithms/af.h
> >> @@ -0,0 +1,79 @@
> >> +/* 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"
> >> +
> >> +/* Definitions from repo of chromium */
> >> +#define AF_MIN_GRID_WIDTH 16
> >> +#define AF_MIN_GRID_HEIGHT 16
> >> +#define AF_MAX_GRID_WIDTH 32
> >> +#define AF_MAX_GRID_HEIGHT 24
> >> +#define AF_MIN_BLOCK_WIDTH 4
> >> +#define AF_MIN_BLOCK_HEIGHT 3
> >> +#define AF_MAX_BLOCK_WIDTH 6
> >> +#define AF_MAX_BLOCK_HEIGHT 6
> >> +#define AF_DEFAULT_HEIGHT_PER_SLICE 2
>
> Those would better be static constexpr ?
OK
>
> >> +
> >> +namespace libcamera {
> >> +
> >> +namespace ipa::ipu3::algorithms {
> >> +
> >> +class Af : public Algorithm
> >> +{
> >> + /* The format of y_table. From ipu3-ipa repo */
> >> + typedef struct __attribute__((packed)) y_table_item {
> >> + uint16_t y1_avg;
> >> + uint16_t y2_avg;
> >> + } y_table_item_t;
> >> +
> >> +public:
> >> + Af();
> >> + ~Af();
> >> +
> >> + void prepare(IPAContext &context, ipu3_uapi_params *params)
> >> override;
> >> + int configure(IPAContext &context, const IPAConfigInfo
> >> &configInfo) override;
> >> + void process(IPAContext &context, const ipu3_uapi_stats_3a
> >> *stats) override;
> >> +
> >> +private:
> >> + void afCoarseScan(IPAContext &context);
> >> + void afFineScan(IPAContext &context);
> >> + bool afScan(IPAContext &context, int min_step);
> >> + void afReset(IPAContext &context);
> >> +
> >> + /* Used for focus scan. */
> >> + uint32_t focus_;
> >> + /* Focus good */
> >> + uint32_t goodFocus_;
> >> + /* Recent AF statistic variance. */
> >> + double currentVariance_;
> >> + /* The frames to be ignore before starting measuring. */
> >> + uint32_t ignoreCounter_;
> >> + /* previous variance. it is used to determine the gradient */
> >> + double previousVariance_;
> >> + /* Max scan steps of each pass of AF scaning */
> >> + uint32_t maxStep_;
> >> + /* coarse scan stable. Complete low pass search (coarse) scan) */
> >> + bool coarseComplete_;
> >> + /* fine scan stable. Complete high pass scan (fine scan) */
> >> + bool fineComplete_;
> >> + /* Raw buffer length */
> >> + uint32_t afRawBufferLen_;
> >> +};
> >> +
> >> +} /* 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 4db6ae1d..e1099169 100644
> >> --- a/src/ipa/ipu3/algorithms/meson.build
> >> +++ b/src/ipa/ipu3/algorithms/meson.build
> >> @@ -1,8 +1,9 @@
> >> # SPDX-License-Identifier: CC0-1.0
> >> ipu3_ipa_algorithms = files([
> >> + 'af.cpp',
> >> 'agc.cpp',
> >> 'awb.cpp',
> >> 'blc.cpp',
> >> - 'tone_mapping.cpp',
> >> + 'tone_mapping.cpp'
> >> ])
> >> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> >> index 86794ac1..ecb2b90a 100644
> >> --- a/src/ipa/ipu3/ipa_context.cpp
> >> +++ b/src/ipa/ipu3/ipa_context.cpp
> >> @@ -67,6 +67,30 @@ namespace libcamera::ipa::ipu3 {
> >> *
> >> * \var IPASessionConfiguration::grid.stride
> >> * \brief Number of cells on one line including the ImgU padding
> >> + *
> >> + */
> >> +
> >> +/**
> >> + * \var IPASessionConfiguration::af
> >> + * \brief AF grid configuration of the IPA
> >> + *
> >> + * \var IPASessionConfiguration::af.afGrid
> >> + *
> >> + */
> >> +
> >> +/**
> >> + * \var IPAFrameContext::af
> >> + * \brief Context for the Automatic Focus algorithm
> >> + *
> >> + * \struct IPAFrameContext::af
> >> + * \var IPAFrameContext::af.focus
> >> + * \brief Current position of the lens
> >> + *
> >> + * \var IPAFrameContext::af.maxVariance
> >> + * \brief The maximum variance of the current image.
> >> + *
> >> + * \var IPAFrameContext::af.stable
> >> + * \brief is the image focused?
> >> */
> >> /**
> >> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> >> index c6dc0814..e643d38f 100644
> >> --- a/src/ipa/ipu3/ipa_context.h
> >> +++ b/src/ipa/ipu3/ipa_context.h
> >> @@ -31,6 +31,10 @@ struct IPASessionConfiguration {
> >> double minAnalogueGain;
> >> double maxAnalogueGain;
> >> } agc;
> >> +
> >> + struct {
> >> + ipu3_uapi_grid_config afGrid;
> >> + } af;
>
> Alphabetical order (af should be before agc).
>
> >> };
> >> struct IPAFrameContext {
> >> @@ -49,6 +53,12 @@ struct IPAFrameContext {
> >> double temperatureK;
> >> } awb;
>
> Add a blank line.
>
> >> + struct {
> >> + uint32_t focus;
> >> + double maxVariance;
> >> + bool stable;
> >> + } af;
>
> Alphabetical order.
>
> >> +
> >> struct {
> >> uint32_t exposure;
> >> double gain;
> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> >> index 3d307708..b5149bcd 100644
> >> --- a/src/ipa/ipu3/ipu3.cpp
> >> +++ b/src/ipa/ipu3/ipu3.cpp
> >> @@ -30,6 +30,7 @@
> >> #include "libcamera/internal/mapped_framebuffer.h"
> >> +#include "algorithms/af.h"
> >> #include "algorithms/agc.h"
> >> #include "algorithms/algorithm.h"
> >> #include "algorithms/awb.h"
> >> @@ -157,6 +158,7 @@ private:
> >> void setControls(unsigned int frame);
> >> void calculateBdsGrid(const Size &bdsOutputSize);
> >> + void initAfGrid(const Size &bdsOutputSize);
> >> std::map<unsigned int, MappedFrameBuffer> buffers_;
> >> @@ -294,6 +296,7 @@ int IPAIPU3::init(const IPASettings &settings,
> >> }
> >> /* Construct our Algorithms */
> >> + algorithms_.push_back(std::make_unique<algorithms::Af>());H
> >> algorithms_.push_back(std::make_unique<algorithms::Agc>());
> >> algorithms_.push_back(std::make_unique<algorithms::Awb>());
> >>
> >> algorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());
> >>
> >> @@ -395,6 +398,33 @@ void IPAIPU3::calculateBdsGrid(const Size
> >> &bdsOutputSize)
> >> << (int)bdsGrid.height << " << " <<
> >> (int)bdsGrid.block_height_log2 << ")";
> >> }
> >> +/**
> >> + * \brief Configure the IPU3 AF grid
> >> + *
> >> + * This function gives the default values for the AF grid configuration.
> >> + * All the parameters are set to the minimum acceptable values.
> >> + *
> >> + * \param bdsOutputSize The bsd output size
> s/bsd/Bayer Down Scaler/
>
> >> + */
> >> +void IPAIPU3::initAfGrid(const Size &bdsOutputSize)
> >> +{
> >> + struct ipu3_uapi_grid_config &grid =
> >> context_.configuration.af.afGrid;
> >> + grid.width = AF_MIN_GRID_WIDTH;
> >> + grid.height = AF_MIN_GRID_HEIGHT;
> >> + grid.block_width_log2 = AF_MIN_BLOCK_WIDTH;
> >> + grid.block_height_log2 = AF_MIN_BLOCK_HEIGHT;
> >> + grid.height_per_slice = AF_DEFAULT_HEIGHT_PER_SLICE;
> >> + /* x_start and y start are default to BDS center */
> >> + grid.x_start = (bdsOutputSize.width / 2) -
> >> + (((grid.width << grid.block_width_log2) / 2));
> >> + grid.y_start = (bdsOutputSize.height / 2) -
> >> + (((grid.height << grid.block_height_log2) / 2));
> >> + /* make sure x_start is multiplied by 10 and y_start is
> >> multiplied by 2 */
> >> + grid.x_start = (grid.x_start / 10) * 10;
> >> + grid.y_start = (grid.y_start / 2) * 2;
>
> I am not sure to understand why you are doing this ?
> We nned to be sure x_start is at least 10 pixels from the left of the
> frame, and y_start needs to be 2 pixels from the top of the frame. I am
> not sure we need to have them as multiples of 10 (and 2) ?
Here is the chrome OS mentioned.
https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h#84
unsigned short x_start; /**< default value 10, X top left corner of
the grid x_start is even */ (should be even)
unsigned short y_start; /**< default value 2, Y top left corner of the
grid y_start is even */ (should be even)
I actually set both x_start and y_start to odd numbers and the AF
could not work properly.
I would double-check that behaviour and try to make sure both of them
are even numbers.
Also, I'll keep x_start and y_start are multiplied by 2 in the next v6 patch.
>
> >> + grid.y_start = grid.y_start | IPU3_UAPI_GRID_Y_START_EN;
> >> +}
> >> +
> >> /**
> >> * \brief Configure the IPU3 IPA
> >> * \param[in] configInfo The IPA configuration data, received from
> >> the pipeline
> >> @@ -461,6 +491,8 @@ int IPAIPU3::configure(const IPAConfigInfo
> >> &configInfo,
> >> lineDuration_ = sensorInfo_.lineLength * 1.0s /
> >> sensorInfo_.pixelRate;
> >> + initAfGrid(configInfo.bdsOutputSize);
> >> +
> >> /* Update the camera controls using the new sensor settings. */
> >> updateControls(sensorInfo_, ctrls_, ipaControls);
>
--
BR,
Kate
More information about the libcamera-devel
mailing list