[libcamera-devel] [PATCH v1 10/10] ipa: raspberrypi: Generalise the autofocus algorithm

Nick Hollinghurst nick.hollinghurst at raspberrypi.com
Mon Mar 27 12:36:54 CEST 2023


Hi Jacopo,

Another patch is on its way.

On Fri, 24 Mar 2023 at 13:39, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Hi Nick
>
> On Fri, Mar 24, 2023 at 11:35:15AM +0000, Nick Hollinghurst wrote:
> > Hi Jacopo,
> >
> > Thanks - comments inline.
> >
> >  Nick
> >
> > On Fri, 24 Mar 2023 at 10:21, Jacopo Mondi
> > <jacopo.mondi at ideasonboard.com> wrote:
> > >
> > > Hi Nick
> > >
> > > On Wed, Mar 22, 2023 at 01:06:12PM +0000, Naushir Patuck via libcamera-devel wrote:
> > > > From: Nick Hollinghurst <nick.hollinghurst at raspberrypi.com>
> > > >
> > > > Remove any hard-coded assumptions about the target hardware platform
> > > > from the autofocus algorithm. Instead, use the "target" string provided
> > > > by the camera tuning config and generalised statistics structures to
> > > > determing parameters such as grid and region sizes.
> > > >
> > > > Additionally, PDAF statistics are represented by a generalised region
> > > > statistics structure to be device agnostic.
> > > >
> > > > These changes also require the autofocus algorithm to initialise
> > > > region weights on the first frame's prepare()/process() call rather
> > > > than during initialisation.
> > > >
> > > > Signed-off-by: Nick Hollinghurst <nick.hollinghurst at raspberrypi.com>
> > > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > > Tested-by: Naushir Patuck <naush at raspberrypi.com>
> > > > ---
> > > >  src/ipa/raspberrypi/cam_helper_imx708.cpp  |  23 ++-
> > > >  src/ipa/raspberrypi/controller/pdaf_data.h |  21 +--
> > > >  src/ipa/raspberrypi/controller/rpi/af.cpp  | 161 ++++++++++-----------
> > > >  src/ipa/raspberrypi/controller/rpi/af.h    |  28 ++--
> > > >  4 files changed, 119 insertions(+), 114 deletions(-)
> > > >
> > > > diff --git a/src/ipa/raspberrypi/cam_helper_imx708.cpp b/src/ipa/raspberrypi/cam_helper_imx708.cpp
> > > > index 1f213d3c0833..641ba18f4b9d 100644
> > > > --- a/src/ipa/raspberrypi/cam_helper_imx708.cpp
> > > > +++ b/src/ipa/raspberrypi/cam_helper_imx708.cpp
> > > > @@ -69,11 +69,14 @@ private:
> > > >       /* Largest long exposure scale factor given as a left shift on the frame length. */
> > > >       static constexpr int longExposureShiftMax = 7;
> > > >
> > > > +     static constexpr int pdafStatsRows = 12;
> > > > +     static constexpr int pdafStatsCols = 16;
> > > > +
> > > >       void populateMetadata(const MdParser::RegisterMap &registers,
> > > >                             Metadata &metadata) const override;
> > > >
> > > >       static bool parsePdafData(const uint8_t *ptr, size_t len, unsigned bpp,
> > > > -                               PdafData &pdaf);
> > > > +                               PdafRegions &pdaf);
> > > >
> > > >       bool parseAEHist(const uint8_t *ptr, size_t len, unsigned bpp);
> > > >       void putAGCStatistics(StatisticsPtr stats);
> > > > @@ -120,11 +123,11 @@ void CamHelperImx708::prepare(libcamera::Span<const uint8_t> buffer, Metadata &m
> > > >       size_t bytesPerLine = (mode_.width * mode_.bitdepth) >> 3;
> > > >
> > > >       if (buffer.size() > 2 * bytesPerLine) {
> > > > -             PdafData pdaf;
> > > > +             PdafRegions pdaf;
> > > >               if (parsePdafData(&buffer[2 * bytesPerLine],
> > > >                                 buffer.size() - 2 * bytesPerLine,
> > > >                                 mode_.bitdepth, pdaf))
> > > > -                     metadata.set("pdaf.data", pdaf);
> > > > +                     metadata.set("pdaf.regions", pdaf);
> > > >       }
> > > >
> > > >       /* Parse AE-HIST data where present */
> > > > @@ -239,7 +242,7 @@ void CamHelperImx708::populateMetadata(const MdParser::RegisterMap &registers,
> > > >  }
> > > >
> > > >  bool CamHelperImx708::parsePdafData(const uint8_t *ptr, size_t len,
> > > > -                                 unsigned bpp, PdafData &pdaf)
> > > > +                                 unsigned bpp, PdafRegions &pdaf)
> > > >  {
> > > >       size_t step = bpp >> 1; /* bytes per PDAF grid entry */
> > > >
> > > > @@ -248,13 +251,17 @@ bool CamHelperImx708::parsePdafData(const uint8_t *ptr, size_t len,
> > > >               return false;
> > > >       }
> > > >
> > > > +     pdaf.init({ pdafStatsCols, pdafStatsRows });
> > > > +
> > > >       ptr += 2 * step;
> > > > -     for (unsigned i = 0; i < PDAF_DATA_ROWS; ++i) {
> > > > -             for (unsigned j = 0; j < PDAF_DATA_COLS; ++j) {
> > > > +     for (unsigned i = 0; i < pdafStatsRows; ++i) {
> > > > +             for (unsigned j = 0; j < pdafStatsCols; ++j) {
> > > >                       unsigned c = (ptr[0] << 3) | (ptr[1] >> 5);
> > > >                       int p = (((ptr[1] & 0x0F) - (ptr[1] & 0x10)) << 6) | (ptr[2] >> 2);
> > > > -                     pdaf.conf[i][j] = c;
> > > > -                     pdaf.phase[i][j] = c ? p : 0;
> > > > +                     PdafData pdafData;
> > > > +                     pdafData.conf = c;
> > > > +                     pdafData.phase = c ? p : 0;
> > > > +                     pdaf.set(libcamera::Point(j, i), { pdafData, 1, 0 });
> > > >                       ptr += step;
> > > >               }
> > > >       }
> > > > diff --git a/src/ipa/raspberrypi/controller/pdaf_data.h b/src/ipa/raspberrypi/controller/pdaf_data.h
> > > > index 03c00d72c0e8..ae6ab996ded0 100644
> > > > --- a/src/ipa/raspberrypi/controller/pdaf_data.h
> > > > +++ b/src/ipa/raspberrypi/controller/pdaf_data.h
> > > > @@ -2,20 +2,23 @@
> > > >  /*
> > > >   * Copyright (C) 2022, Raspberry Pi Ltd
> > > >   *
> > > > - * pdaf_data.h - PDAF Metadata; for now this is
> > > > - * largely based on IMX708's PDAF "Type 1" output.
> > > > + * pdaf_data.h - PDAF Metadata
> > > >   */
> > > >  #pragma once
> > > >
> > > >  #include <stdint.h>
> > > >
> > > > -#define PDAF_DATA_ROWS 12
> > > > -#define PDAF_DATA_COLS 16
> > > > +#include "region_stats.h"
> > > >
> > > > -struct PdafData {
> > > > -     /* Confidence values, in raster order, in arbitrary units */
> > > > -     uint16_t conf[PDAF_DATA_ROWS][PDAF_DATA_COLS];
> > > > +namespace RPiController {
> > > >
> > > > -     /* Phase error, in raster order, in s11 Q4 format (S.6.4) */
> > > > -     int16_t phase[PDAF_DATA_ROWS][PDAF_DATA_COLS];
> > > > +struct PdafData {
> > > > +     /* Confidence, in arbitrary units */
> > > > +     uint16_t conf;
> > > > +     /* Phase error, in s16 Q4 format (S.11.4) */
> > > > +     int16_t phase;
> > > >  };
> > > > +
> > > > +using PdafRegions = RegionStats<PdafData>;
> > > > +
> > > > +}; // namespace RPiController
> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/af.cpp b/src/ipa/raspberrypi/controller/rpi/af.cpp
> > > > index a623651875f2..3d2dad974d8b 100644
> > > > --- a/src/ipa/raspberrypi/controller/rpi/af.cpp
> > > > +++ b/src/ipa/raspberrypi/controller/rpi/af.cpp
> > > > @@ -174,9 +174,8 @@ Af::Af(Controller *controller)
> > > >         statsRegion_(0, 0, 0, 0),
> > > >         windows_(),
> > > >         useWindows_(false),
> > > > -       phaseWeights_{},
> > > > -       contrastWeights_{},
> > > > -       sumWeights_(0),
> > > > +       phaseWeights_(),
> > > > +       contrastWeights_(),
> > > >         scanState_(ScanState::Idle),
> > > >         initted_(false),
> > > >         ftarget_(-1.0),
> > > > @@ -190,6 +189,8 @@ Af::Af(Controller *controller)
> > > >         scanData_(),
> > > >         reportState_(AfState::Idle)
> > > >  {
> > > > +     phaseWeights_.w.reserve(16 * 12);
> > >
> > > Isn't the point of the patch to get these values from the camHelper ?
> >
> > Ideally it would.. But right now, the helper doesn't tell us the
> > dimensions of the grids in advance, so I had to "guess".
> >
>
> Not strictly necessary, but just for my understanding, can't the
> helper return the sizes so that they can be retrieved with
> Controller::getHardwareConfig() ?
>
> > All these reserve() calls are perhaps an unnecessary affectation, an
> > attempt to reduce memory fragmentation -- should I remove them?
> >
>
> No no, it's fine to reserve, maybe if you can't get sizes record with
> a \todo item those should be made configurable

For PDAF stats (from the sensor) we currently lack the plumbing to
retrieve this. 16 * 12 is the larger of two grid sizes supported by
IMX708, and the largest we anticipate. I've added a comment about it.
For Contrast stats (from ISP) we can indeed use
Controller::getHardwareConfig() -- so we'll do that.


> >
> > > > +     contrastWeights_.w.reserve(4 * 3);
> > > >       scanData_.reserve(24);
> > > >  }
> > > >
> > > > @@ -226,7 +227,7 @@ void Af::switchMode(CameraMode const &cameraMode, [[maybe_unused]] Metadata *met
> > > >                         << statsRegion_.y << ','
> > > >                         << statsRegion_.width << ','
> > > >                         << statsRegion_.height;
> > > > -     computeWeights();
> > > > +     invalidateWeights();
> > > >
> > > >       if (scanState_ >= ScanState::Coarse && scanState_ < ScanState::Settle) {
> > > >               /*
> > > > @@ -239,111 +240,94 @@ void Af::switchMode(CameraMode const &cameraMode, [[maybe_unused]] Metadata *met
> > > >       skipCount_ = cfg_.skipFrames;
> > > >  }
> > > >
> > > > -void Af::computeWeights()
> > > > +void Af::computeWeights(RegionWeights *wgts, unsigned rows, unsigned cols)
> > > >  {
> > > > -     constexpr int MaxCellWeight = 240 / (int)MaxWindows;
> > > > +     wgts->rows = rows;
> > > > +     wgts->cols = cols;
> > > > +     wgts->sum = 0;
> > > > +     wgts->w.resize(rows * cols);
> > > > +     std::fill(wgts->w.begin(), wgts->w.end(), 0);
> > > >
> > > > -     sumWeights_ = 0;
> > > > -     for (int i = 0; i < PDAF_DATA_ROWS; ++i)
> > > > -             std::fill(phaseWeights_[i], phaseWeights_[i] + PDAF_DATA_COLS, 0);
> > > > -
> > > > -     if (useWindows_ &&
> > > > -         statsRegion_.width >= PDAF_DATA_COLS && statsRegion_.height >= PDAF_DATA_ROWS) {
> > > > +     if (rows > 0 && cols > 0 && useWindows_ &&
> > > > +         statsRegion_.height >= rows && statsRegion_.width >= cols) {
> > > >               /*
> > > >                * Here we just merge all of the given windows, weighted by area.
> > > >                * \todo Perhaps a better approach might be to find the phase in each
> > > >                * window and choose either the closest or the highest-confidence one?
> > > > -              *
> > > > -              * Using mostly "int" arithmetic, because Rectangle has signed x, y
> > > >                */
> > > > -             int cellH = (int)(statsRegion_.height / PDAF_DATA_ROWS);
> > > > -             int cellW = (int)(statsRegion_.width / PDAF_DATA_COLS);
> > > > -             int cellA = cellH * cellW;
> > > > +             const unsigned maxCellWeight = 46080u / (MaxWindows * rows * cols);
> > >
> > > Where does 46080u comes from ? I can't find it in the diff
> >
> > It's a "highly divisible" number, close to but not exceeding 65536,
> > chosen to anticipate the grid dimensions having only factors of 2 and
> > 3, and MaxWindows==10. I'm not sure if there's a better way to express
> > that in the code? For the IMX708 PDAF grid it yields maxCellWeight =
> > 24, as before.
> >
> > Actually, a numerator of 65535 or 65536 (for denominator > 1) would
> > work almost as well, but it would have different rounding behaviour in
> > cases where a focus window intersects a fraction of a grid cell.
> >
>
> Thanks for the explanation. Would you mind recording that in a
> comment ? It would also help explaning how the weights are computed,
> but this was already like this
>
> >
> > > > +             const unsigned cellH = statsRegion_.height / rows;
> > > > +             const unsigned cellW = statsRegion_.width / cols;
> > > > +             const unsigned cellA = cellH * cellW;
> > > >
> > > >               for (auto &w : windows_) {
> > > > -                     for (int i = 0; i < PDAF_DATA_ROWS; ++i) {
> > > > -                             int y0 = std::max(statsRegion_.y + cellH * i, w.y);
> > > > -                             int y1 = std::min(statsRegion_.y + cellH * (i + 1), w.y + (int)(w.height));
> > > > +                     for (unsigned r = 0; r < rows; ++r) {
> > > > +                             int y0 = std::max(statsRegion_.y + (int)(cellH * r), w.y);
> > > > +                             int y1 = std::min(statsRegion_.y + (int)(cellH * (r + 1)), w.y + (int)(w.height));
> > > >                               if (y0 >= y1)
> > > >                                       continue;
> > > >                               y1 -= y0;
> > > > -                             for (int j = 0; j < PDAF_DATA_COLS; ++j) {
> > > > -                                     int x0 = std::max(statsRegion_.x + cellW * j, w.x);
> > > > -                                     int x1 = std::min(statsRegion_.x + cellW * (j + 1), w.x + (int)(w.width));
> > > > +                             for (unsigned c = 0; c < cols; ++c) {
> > > > +                                     int x0 = std::max(statsRegion_.x + (int)(cellW * c), w.x);
> > > > +                                     int x1 = std::min(statsRegion_.x + (int)(cellW * (c + 1)), w.x + (int)(w.width));
>
> While at here could you:
>                                             int x1 = std::min(statsRegion_.x + (int)(cellW * (c + 1)),
>                                                               w.x + (int)(w.width));
>
> > > >                                       if (x0 >= x1)
> > > >                                               continue;
> > > > -                                     int a = y1 * (x1 - x0);
> > > > -                                     a = (MaxCellWeight * a + cellA - 1) / cellA;
> > > > -                                     phaseWeights_[i][j] += a;
> > > > -                                     sumWeights_ += a;
> > > > +                                     unsigned a = y1 * (x1 - x0);
> > > > +                                     a = (maxCellWeight * a + cellA - 1) / cellA;
> > > > +                                     wgts->w[r * cols + c] += a;
> > > > +                                     wgts->sum += a;
> > > >                               }
> > > >                       }
> > > >               }
> > > >       }
> > > >
> > > > -     if (sumWeights_ == 0) {
> > > > -             /*
> > > > -              * Default AF window is the middle 1/2 width of the middle 1/3 height
> > > > -              * since this maps nicely to both PDAF (16x12) and Focus (4x3) grids.
> > > > -              */
> > > > -             for (int i = PDAF_DATA_ROWS / 3; i < 2 * PDAF_DATA_ROWS / 3; ++i) {
> > > > -                     for (int j = PDAF_DATA_COLS / 4; j < 3 * PDAF_DATA_COLS / 4; ++j) {
> > > > -                             phaseWeights_[i][j] = MaxCellWeight;
> > > > -                             sumWeights_ += MaxCellWeight;
> > > > +     if (wgts->sum == 0) {
> > > > +             /* Default AF window is the middle 1/2 width of the middle 1/3 height */
> > > > +             for (unsigned r = rows / 3; r < rows - rows / 3; ++r) {
> > > > +                     for (unsigned c = cols / 4; c < cols - cols / 4; ++c) {
> > > > +                             wgts->w[r * cols + c] = 1;
> > > > +                             wgts->sum += 1;
> > >
> > > This seems functionally different, or was MaxCellWeight == 1 ?
> >
> > When used, we divide by the total weight (rounding down). I realised
> > that the original MaxCellWeight factor was unnecessary.
> >
> >
> > > >                       }
> > > >               }
> > > >       }
> > > > +}
> > > >
> > > > -     /* Scale from PDAF to Focus Statistics grid (which has fixed size 4x3) */
> > > > -     constexpr int FocusStatsRows = 3;
> > > > -     constexpr int FocusStatsCols = 4;
> > > > -     static_assert(FOCUS_REGIONS == FocusStatsRows * FocusStatsCols);
> > > > -     static_assert(PDAF_DATA_ROWS % FocusStatsRows == 0);
> > > > -     static_assert(PDAF_DATA_COLS % FocusStatsCols == 0);
> > > > -     constexpr int YFactor = PDAF_DATA_ROWS / FocusStatsRows;
> > > > -     constexpr int XFactor = PDAF_DATA_COLS / FocusStatsCols;
> > > > -
> > > > -     LOG(RPiAf, Debug) << "Recomputed weights:";
> > > > -     for (int i = 0; i < FocusStatsRows; ++i) {
> > > > -             for (int j = 0; j < FocusStatsCols; ++j) {
> > > > -                     unsigned w = 0;
> > > > -                     for (int y = 0; y < YFactor; ++y)
> > > > -                             for (int x = 0; x < XFactor; ++x)
> > > > -                                     w += phaseWeights_[YFactor * i + y][XFactor * j + x];
> > > > -                     contrastWeights_[FocusStatsCols * i + j] = w;
> > > > -             }
> > > > -             LOG(RPiAf, Debug) << "   "
> > > > -                               << contrastWeights_[FocusStatsCols * i + 0] << " "
> > > > -                               << contrastWeights_[FocusStatsCols * i + 1] << " "
> > > > -                               << contrastWeights_[FocusStatsCols * i + 2] << " "
> > > > -                               << contrastWeights_[FocusStatsCols * i + 3];
> > > > -     }
> > >
> > > I might have missed where re-scaling is gone.. as you've tested this I
> > > presum it's just me not fully getting what's happening here
> >
> > It's missing because we now call the function twice: to construct
> > weights for each of the PDAF (from the sensor) and CDAF (from the ISP)
> > grids in turn.
> > The code no longer assumes or asserts that one grid's size is an exact
> > integer fraction of the other.
> >
> >
> > >
> > > > +void Af::invalidateWeights()
> > > > +{
> > > > +     phaseWeights_.sum = 0;
> > > > +     contrastWeights_.sum = 0;
> > > >  }
> > > >
> > > > -bool Af::getPhase(PdafData const &data, double &phase, double &conf) const
> > > > +bool Af::getPhase(PdafRegions const &regions, double &phase, double &conf)
> > > >  {
> > > > +     libcamera::Size size = regions.size();
> > > > +     if (size.height != phaseWeights_.rows || size.width != phaseWeights_.cols ||
> > > > +         phaseWeights_.sum == 0) {
> > > > +             LOG(RPiAf, Debug) << "Recompute Phase weights " << size.width << 'x' << size.height;
> > > > +             computeWeights(&phaseWeights_, size.height, size.width);
> > > > +     }
> > > > +
> > > >       uint32_t sumWc = 0;
> > > >       int64_t sumWcp = 0;
> > > > -
> > > > -     for (unsigned i = 0; i < PDAF_DATA_ROWS; ++i) {
> > > > -             for (unsigned j = 0; j < PDAF_DATA_COLS; ++j) {
> > > > -                     if (phaseWeights_[i][j]) {
> > > > -                             uint32_t c = data.conf[i][j];
> > > > -                             if (c >= cfg_.confThresh) {
> > > > -                                     if (c > cfg_.confClip)
> > > > -                                             c = cfg_.confClip;
> > > > -                                     c -= (cfg_.confThresh >> 2);
> > > > -                                     sumWc += phaseWeights_[i][j] * c;
> > > > -                                     c -= (cfg_.confThresh >> 2);
> > > > -                                     sumWcp += phaseWeights_[i][j] * data.phase[i][j] * (int64_t)c;
> > > > -                             }
> > > > +     for (unsigned i = 0; i < regions.numRegions(); ++i) {
> > > > +             unsigned w = phaseWeights_.w[i];
> > > > +             if (w) {
> > > > +                     const PdafData &data = regions.get(i).val;
> > > > +                     unsigned c = data.conf;
> > > > +                     if (c >= cfg_.confThresh) {
> > > > +                             if (c > cfg_.confClip)
> > > > +                                     c = cfg_.confClip;
> > > > +                             c -= (cfg_.confThresh >> 2);
> > > > +                             sumWc += w * c;
> > > > +                             c -= (cfg_.confThresh >> 2);
> > > > +                             sumWcp += (int64_t)(w * c) * (int64_t)data.phase;
> > > >                       }
> > > >               }
> > > >       }
> > > >
> > > > -     if (0 < sumWeights_ && sumWeights_ <= sumWc) {
> > > > +     if (0 < phaseWeights_.sum && phaseWeights_.sum <= sumWc) {
> > > >               phase = (double)sumWcp / (double)sumWc;
> > > > -             conf = (double)sumWc / (double)sumWeights_;
> > > > +             conf = (double)sumWc / (double)phaseWeights_.sum;
> > > >               return true;
> > > >       } else {
> > > >               phase = 0.0;
> > > > @@ -352,14 +336,19 @@ bool Af::getPhase(PdafData const &data, double &phase, double &conf) const
> > > >       }
> > > >  }
> > > >
> > > > -double Af::getContrast(const FocusRegions &focusStats) const
> > > > +double Af::getContrast(const FocusRegions &focusStats)
> > > >  {
> > > > -     uint32_t sumWc = 0;
> > > > +     libcamera::Size size = focusStats.size();
> > > > +     if (size.height != contrastWeights_.rows || size.width != contrastWeights_.cols || contrastWeights_.sum == 0) {
> > >
> > > Please break this to multiple lines
> > >
> > > > +             LOG(RPiAf, Debug) << "Recompute Contrast weights " << size.width << 'x' << size.height;
> > >
> > > it's easy to do the same here
> >
> > OK
> >
> >
> > > > +             computeWeights(&contrastWeights_, size.height, size.width);
> > > > +     }
> > > >
> > > > +     uint64_t sumWc = 0;
> > > >       for (unsigned i = 0; i < focusStats.numRegions(); ++i)
> > > > -             sumWc += contrastWeights_[i] * focusStats.get(i).val;
> > > > +             sumWc += contrastWeights_.w[i] * focusStats.get(i).val;
> > > >
> > > > -     return (sumWeights_ == 0) ? 0.0 : (double)sumWc / (double)sumWeights_;
> > > > +     return (contrastWeights_.sum > 0) ? ((double)sumWc / (double)contrastWeights_.sum) : 0.0;
> > > >  }
> > > >
> > > >  void Af::doPDAF(double phase, double conf)
> > > > @@ -623,14 +612,14 @@ void Af::prepare(Metadata *imageMetadata)
> > > >
> > > >       if (initted_) {
> > > >               /* Get PDAF from the embedded metadata, and run AF algorithm core */
> > > > -             PdafData data;
> > > > +             PdafRegions regions;
> > > >               double phase = 0.0, conf = 0.0;
> > > >               double oldFt = ftarget_;
> > > >               double oldFs = fsmooth_;
> > > >               ScanState oldSs = scanState_;
> > > >               uint32_t oldSt = stepCount_;
> > > > -             if (imageMetadata->get("pdaf.data", data) == 0)
> > > > -                     getPhase(data, phase, conf);
> > > > +             if (imageMetadata->get("pdaf.regions", regions) == 0)
> > > > +                     getPhase(regions, phase, conf);
> > > >               doAF(prevContrast_, phase, conf);
> > > >               updateLensPosition();
> > > >               LOG(RPiAf, Debug) << std::fixed << std::setprecision(2)
> > > > @@ -691,7 +680,7 @@ void Af::setMetering(bool mode)
> > > >  {
> > > >       if (useWindows_ != mode) {
> > > >               useWindows_ = mode;
> > > > -             computeWeights();
> > > > +             invalidateWeights();
> > > >       }
> > > >  }
> > > >
> > > > @@ -708,7 +697,9 @@ void Af::setWindows(libcamera::Span<libcamera::Rectangle const> const &wins)
> > > >               if (windows_.size() >= MaxWindows)
> > > >                       break;
> > > >       }
> > > > -     computeWeights();
> > > > +
> > > > +     if (useWindows_)
> > > > +             invalidateWeights();
> > > >  }
> > > >
> > > >  bool Af::setLensPosition(double dioptres, int *hwpos)
> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/af.h b/src/ipa/raspberrypi/controller/rpi/af.h
> > > > index 7959371baf64..b479feb88c39 100644
> > > > --- a/src/ipa/raspberrypi/controller/rpi/af.h
> > > > +++ b/src/ipa/raspberrypi/controller/rpi/af.h
> > > > @@ -11,12 +11,6 @@
> > > >  #include "../pdaf_data.h"
> > > >  #include "../pwl.h"
> > > >
> > > > -/*
> > > > - * \todo FOCUS_REGIONS is taken from bcm2835-isp.h, but should be made as a
> > > > - * generic RegionStats structure.
> > > > - */
> > > > -#define FOCUS_REGIONS 12
> > > > -
> > > >  /*
> > > >   * This algorithm implements a hybrid of CDAF and PDAF, favouring PDAF.
> > > >   *
> > > > @@ -121,9 +115,20 @@ private:
> > > >               double conf;
> > > >       };
> > > >
> > > > -     void computeWeights();
> > > > -     bool getPhase(PdafData const &data, double &phase, double &conf) const;
> > > > -     double getContrast(const FocusRegions &focusStats) const;
> > > > +     struct RegionWeights {
> > > > +             unsigned rows;
> > > > +             unsigned cols;
> > > > +             uint32_t sum;
> > > > +             std::vector<uint16_t> w;
> > > > +
> > > > +             RegionWeights()
> > > > +                     : rows(0), cols(0), sum(0), w() {}
> > > > +     };
> > > > +
> > > > +     void computeWeights(RegionWeights *wgts, unsigned rows, unsigned cols);
> > > > +     void invalidateWeights();
> > > > +     bool getPhase(PdafRegions const &regions, double &phase, double &conf);
> > > > +     double getContrast(const FocusRegions &focusStats);
> > > >       void doPDAF(double phase, double conf);
> > > >       bool earlyTerminationByPhase(double phase);
> > > >       double findPeak(unsigned index) const;
> > > > @@ -143,9 +148,8 @@ private:
> > > >       libcamera::Rectangle statsRegion_;
> > > >       std::vector<libcamera::Rectangle> windows_;
> > > >       bool useWindows_;
> > > > -     uint8_t phaseWeights_[PDAF_DATA_ROWS][PDAF_DATA_COLS];
> > > > -     uint16_t contrastWeights_[FOCUS_REGIONS];
> > > > -     uint32_t sumWeights_;
> > > > +     RegionWeights phaseWeights_;
> > > > +     RegionWeights contrastWeights_;
> > > >
> > > >       /* Working state. */
> > > >       ScanState scanState_;
> > > > --
> > > > 2.34.1
> > > >


More information about the libcamera-devel mailing list