[libcamera-devel] [RFC v6] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Kate Hsuan
hpa at redhat.com
Tue Feb 8 06:48:48 CET 2022
coub Hi Jean-Michel,
Thank you for your testing and review.
On Mon, Feb 7, 2022 at 7:35 PM Jean-Michel Hautbois
<jeanmichel.hautbois at ideasonboard.com> wrote:
>
> Hi Kate,
>
> Thanks for the patch !
>
> On 07/02/2022 08:16, 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.
> >
> > Based on the values from the IPU3 AF buffer, the variance
> > of each focus step is determined and a greedy approach is
> > used to find the maximum variance of the AF state and an
> > appropriate focus value.
> >
> > 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).
> >
> > Signed-off-by: Kate Hsuan <hpa at redhat.com>
>
> It works quite well, on top of master and latest's series from Daniel [1].
I also tried it too, Thanks, Daniel and folks!
>
> I am having an issue, I suppose you have the same, with AGC. It is
> blinking on my SGo2 ov8865 device... ?
Me too, I maintained my environment to not too bright and not too dark
to prevent from blinking.
Does it seem an issue with the sensor's driver or AGC? I'm not very sure.
> I took two captures, with the same conditions, on master [2] and with
> your patch (with a local patch to apply the TCC configuration, which
> explains the really better colors) [3].
>
> The feeling I have is it is quite fast to focus, which is good !
>
> [1]: https://patchwork.libcamera.org/project/libcamera/list/?series=2906
> [2]: https://snipboard.io/oOexGl.jpg
> [3]: https://snipboard.io/kazvGC.jpg
>
Nice piano :). We still could improve it. In some low contrast
backgrounds, it was difficult to determine a focus value.
> > ---
> > src/ipa/ipu3/algorithms/af.cpp | 375 ++++++++++++++++++++++++++++
> > src/ipa/ipu3/algorithms/af.h | 78 ++++++
> > 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 | 36 +++
> > 6 files changed, 525 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..f67ab8a9
> > --- /dev/null
> > +++ b/src/ipa/ipu3/algorithms/af.cpp
> > @@ -0,0 +1,375 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Red Hat
> > + *
> > + * af.cpp - IPU3 auto focus algorithm
> > + */
> > +
> > +#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
> > + */
> > +
> > +/**
> > + * \var AF_MIN_GRID_WIDTH
> > + * \brief the minimum width of AF grid.
> > + * The minimum grid horizontal dimensions, in number of grid blocks(cells).
> > +*/
>
> My bad, I said to make it constexpr, but did not ask for naming changes...
> s/AF_MIN_GRID_WIDTH/kAfMinGridWidth/
>
> Same for all those defines ;-).
ok! no problem.
>
> > +
> > +/**
> > + * \var AF_MIN_GRID_HEIGHT
> > + * \brief the minimum height of AF grid.
> > + * The minimum grid vertical dimensions, in number of grid blocks(cells).
> > +*/
> > +
> > +/**
> > + * \var AF_MAX_GRID_WIDTH
> > + * \brief the maximum width of AF grid.
> > + * The maximum grid horizontal dimensions, in number of grid blocks(cells).
> > +*/
> > +
> > +/**
> > + * \var AF_MAX_GRID_HEIGHT
> > + * \brief the maximum height of AF grid.
> > + * The maximum grid vertical dimensions, in number of grid blocks(cells).
> > +*/
> > +
> > +/**
> > + * \var AF_MIN_BLOCK_WIDTH
> > + * \brief the minimum block size of the width.
> > + */
> > +
> > +/**
> > + * \var AF_MIN_BLOCK_HEIGHT
> > + * \brief the minimum block size of the height.
> > + */
> > +
> > +/**
> > + * \def AF_MAX_BLOCK_WIDTH
> > + * \brief the maximum block size of the width.
> > + */
> > +
> > +/**
> > + * \var AF_MAX_BLOCK_HEIGHT
> > + * \brief the maximum block size of the height.
> > + */
> > +
> > +/**
> > + * \var 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 An auto-focus algorithm based on IPU3 statistics
> > + *
> > + * This algorithm is used to determine the position of the lens and get a
> > + * focused image. The IPU3 AF processing block computes the statistics,
> > + * composed 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
> s/has relative high contrast than/has a relatively higher contrast than/
> > + * 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.
> > + *
ops, I would revise this part. my fault.
> > + */
> > +
> > +LOG_DEFINE_CATEGORY(IPU3Af)
> > +
> > +/**
> > + * Maximum focus steps of the VCM control
> > + * \todo should be obtained from the VCM driver
> > + */
>
> Is something missing to implement it and grab it from context ?
The maximum steps of dw9719 VCM is 1023. The max steps should be
obtained from the VCM driver.
So, a kind of method should be implemented to fetch the VCM capability
from the driver.
>
> > +static constexpr uint32_t maxFocusSteps = 1023;
> > +
> > +/* minimum focus step for searching appropriate focus */
> > +static constexpr uint32_t coarseSearchStep = 30;
> > +static constexpr uint32_t fineSearchStep = 1;
>
> Please document each variable ;-).
> And fix the namings:
> s/coarseSearchStep/kCoarseSearchStep/
> s/fineSearchStep/kFineSearchStep/
>
> Same below.
>
> > +
> > +/* max ratio of variance change, 0.0 < maxChange < 1.0 */
> > +static constexpr double maxChange = 0.5;
> > +
> > +/* the numbers of frame to be ignored, before performing focus scan. */
> s/the numbers of frame/the number of frames/
> > +static constexpr uint32_t ignoreFrame = 10;
> > +
> > +/* fine scan range 0 < findRange < 1 */
> > +static constexpr double findRange = 0.05;
> > +
> > +/* settings for IPU3 AF filter */
> > +static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {
> > + .y1_coeff_0 = { 0, 1, 3, 7 },
> > + .y1_coeff_1 = { 11, 13, 1, 2 },
> > + .y1_coeff_2 = { 8, 19, 34, 242 },
> > + .y1_sign_vec = 0x7fdffbfe,
> > + .y2_coeff_0 = { 0, 1, 6, 6 },
> > + .y2_coeff_1 = { 13, 25, 3, 0 },
> > + .y2_coeff_2 = { 25, 3, 177, 254 },
> > + .y2_sign_vec = 0x4e53ca72,
> > + .y_calc = { 8, 8, 8, 8 },
> > + .nf = { 0, 9, 0, 9, 0 },
> > +};
> > +
> > +Af::Af()
> > + : focus_(0), goodFocus_(0), currentVariance_(0.0), previousVariance_(0.0),
> > + coarseComplete_(false), fineComplete_(false)
>
> What would you think of:
> s/coarseComplete_/coarseCompleted_/
> s/fineComplete_/fineCompleted_/
>
> > +{
> > + maxStep_ = maxFocusSteps;
>
> Once the maximum will be grabbed from the configuration context, this
> variable will not be needed anymore.
>
> > +}
> > +
> > +Af::~Af()
> > +{
> > +}
>
> It could be a ~Af() = default; in af.h ?
>
> > +
> > +/**
> > + * \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 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)
> > +{
> > + /* 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;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * \brief AF coarse scan
> > + *
> > + * Find a near focused image using a coarse step. The step is determined by coarseSearchStep.
> > + *
> > + * \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
> > + *
> > + * Find an optimum lens position with moving 1 step for each search.
> > + *
> > + * \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
> > + *
> > + * This fuction compares the previous and current variance. It always pick the largest variance to
> > + * replace the previous one. If it finds the decending variance values, it returns immediately.
> > + *
> > + * \return True, if it finds a AF value.
> > + */
> > +bool Af::afScan(IPAContext &context, int min_step)
> > +{
> > + /* find the maximum variance during the AF scan through
> > + always picking the maximum variance */
>
> Multiple lines comments need to be in the form:
> /*
> * Find the maximum variance during the AF scan through always picking
> * the maximum variance.
> */
>
Ok thanks.
> > + 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 direction of the variance development. */
>
> Could you maybe elaborate a bit ?
> Something lik (please reword ;-)):
> /*
> * Find the maximum of the variance by estimating its
> * derivative. If the direction changes, it means we have
> * passed a maximum one step before.
> */
>
Thank for revising this. I would improve my comments :).
> > + 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_;
>
> Do we need those logs at each step ?
It reveals the variance, current step, and focused step. It could be removed.
>
> > +}
> > +
> > +/**
> > + * \brief Determine the max contrast image and lens position. y_table is the
> > + * statistic data from IPU3 and is composed of low pass and high pass filtered
> > + * 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[IPU3_UAPI_AF_Y_TABLE_MAX_SIZE / sizeof(y_table_item_t)];
> > + uint32_t z = 0;
> > +
> > + memcpy(y_item, stats->af_raw_buffer.y_table, IPU3_UAPI_AF_Y_TABLE_MAX_SIZE);
> > +
> > + /* Evaluate the AF buffer length */
> > + afRawBufferLen_ = context.configuration.af.afGrid.width *
> > + context.configuration.af.afGrid.height;
>
> You could memcpy only afRawBufferLen_ elements ?
> BTW, this is a local used variable... I would expect it to be set in
> configure() if you want it as a class member ?
Sure, I could make afRawBufferLen_ to be a class member and be set in
configure().
>
> Any reason why you copy the elements ?
Datacasting uses static_cast or reinterpret_cast. I could not make
them possible :(
>
> > +
> > + /**
>
> /* only (not a doxygen comment).
>
> > + * Calculate the mean and the variance AF statistics, since IPU3 only determine the AF value
> > + * for a given grid.
>
> split line.
>
> > + * For coarse: y1 are used.
> > + * For fine: y2 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));
>
> split line.
>
> > + }
> > + } 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));
>
> split line.
>
> > + }
> > + }
> > +
> > + /* 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;
>
> split lines.
>
> > + LOG(IPU3Af, Debug) << "Rate of variance change: "
> > + << var_ratio
> > + << " current focus step: "
> > + << context.frameContext.af.focus;
> > + /**
> > + * If the change ratio of contrast is more than maxChange (out of focus),
> > + * trigger AF again.
>
> /* only (not a doxygen comment) and the first line is too long (cut at 80).
my fault, I'll correct this.
>
> > + */
> > + if (var_ratio > maxChange) {
> > + if (ignoreCounter_ == 0) {
> > + afReset(context);
> > + } else
> > + ignoreCounter_--;
> > + } else
> > + ignoreCounter_ = ignoreFrame;
> > + } else {
> > + if (ignoreCounter_ != 0)
> > + ignoreCounter_--;
> > + else {
> > + afCoarseScan(context);
> > + afFineScan(context);
> > + }
> > + }
>
> I think this could be simplified a bit ?
> We always need to know if we need to ignore the frame or not. If so, we
> can return early. If not, we want to know if we are stable or not.
I found that if the lens is reset suddenly, the AF statistics will
bounce until the AGC become stable.
So we may tweak ignoreCounter_ to find an acceptable range to wait for
AF statistics stable.
I'll perform some experiments about this and try to simplify this.
>
> > +}
> > +
> > +} /* 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..981bf3a2
> > --- /dev/null
> > +++ b/src/ipa/ipu3/algorithms/af.h
> > @@ -0,0 +1,78 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Red Hat
> > + *
> > + * af.h - IPU3 Af algorithm
> > + */
> > +#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"
> > +
> > +/* static variables from repo of chromium */
> > +static constexpr uint8_t AF_MIN_GRID_WIDTH = 16;
> > +static constexpr uint8_t AF_MIN_GRID_HEIGHT = 16;
> > +static constexpr uint8_t AF_MAX_GRID_WIDTH = 32;
> > +static constexpr uint8_t AF_MAX_GRID_HEIGHT = 24;
> > +static constexpr uint16_t AF_MIN_BLOCK_WIDTH = 4;
> > +static constexpr uint16_t AF_MIN_BLOCK_HEIGHT = 3;
> > +static constexpr uint16_t AF_MAX_BLOCK_WIDTH = 6;
> > +static constexpr uint16_t AF_MAX_BLOCK_HEIGHT = 6;
> > +static constexpr uint16_t AF_DEFAULT_HEIGHT_PER_SLICE = 2;
> > +
> > +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..60ad3194 100644
> > --- a/src/ipa/ipu3/ipa_context.h
> > +++ b/src/ipa/ipu3/ipa_context.h
> > @@ -25,6 +25,10 @@ struct IPASessionConfiguration {
> > uint32_t stride;
> > } grid;
> >
> > + struct {
> > + ipu3_uapi_grid_config afGrid;
> > + } af;
> > +
> > struct {
> > utils::Duration minShutterSpeed;
> > utils::Duration maxShutterSpeed;
> > @@ -34,6 +38,12 @@ struct IPASessionConfiguration {
> > };
> >
> > struct IPAFrameContext {
> > + struct {
> > + uint32_t focus;
> > + double maxVariance;
> > + bool stable;
> > + } af;
> > +
> > struct {
> > uint32_t exposure;
> > double gain;
> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > index 3d307708..2d23f8f8 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>());
> > 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,37 @@ 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 BDS output size
> > + */
> > +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;
> > + grid.block_width_log2 = AF_MIN_BLOCK_WIDTH;
> > + grid.block_height_log2 = AF_MIN_BLOCK_HEIGHT;
> > +
> > + /* 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));
> > +
> > + /* x_start and y_start should be even */
> > + grid.x_start = (grid.x_start / 2) * 2;
> > + grid.y_start = (grid.y_start / 2) * 2;
> > + grid.y_start = grid.y_start | IPU3_UAPI_GRID_Y_START_EN;
> > +}
>
> Why are you not doing it in the IPU3Af::configure() call ?
> It would let you remove the [[maybe_unused]] for the IPAConfigInfo
> variable, as you would use it to get the bdsOutputSize ?
The bdsOutputSize could be found in IPAConfigInfo. If you feel good
about this, I'll move them to IPU3Af::configure().
>
> > +
> > /**
> > * \brief Configure the IPU3 IPA
> > * \param[in] configInfo The IPA configuration data, received from the pipeline
> > @@ -461,6 +495,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
> >
> > lineDuration_ = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;
> >
> > + initAfGrid(configInfo.bdsOutputSize);
> > +
>
> And this would also be removed ?
Once they move to configure(), it can be removed.
>
> > /* Update the camera controls using the new sensor settings. */
> > updateControls(sensorInfo_, ctrls_, ipaControls);
> >
>
> I think you could now remove the "RFC" tag.
> You can add my
> Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>
Understand, thanks a lot :)
--
BR,
Kate
More information about the libcamera-devel
mailing list