[libcamera-devel] [PATCH v3 4/5] ipa: raspberrypi: Use the generic statistics structure in the algorithms
Naushir Patuck
naush at raspberrypi.com
Tue Jan 31 18:08:23 CET 2023
Hi Kieran,
On Tue, 31 Jan 2023, 7:01 pm Kieran Bingham, <
kieran.bingham at ideasonboard.com> wrote:
> Hi Naush,
>
> Quoting Naushir Patuck via libcamera-devel (2022-12-13 11:48:35)
> > Repurpose the StatisticsPtr type from being a
> shared_ptr<bcm2835_isp_stats> to
> > shared_ptr<RPiController::Statistics>. This removes any hardware
> specific header
> > files and structures from the algorithms source code.
> >
> > Add a new function in the Raspberry Pi IPA to populate the generic
> statistics
> > structure from the values provided by the hardware in the
> bcm2835_isp_stats
> > structure.
> >
> > Update the Lux, AWB, AGC, ALSC, Contrast, and Focus algorithms to use the
> > generic statistics structure appropriately in their calculations.
> Additionally,
> > remove references to any hardware specific headers and defines in these
> source
> > files.
>
> I'm afraid this one fails to build as I've merged IMX708 support with
> the AF algorithm additions.
>
> Could you rebase this series please?
>
Ouch!
This is because we've removed the bcm2835-isp.h header from the controller
in this series.
Once this series has been merged, we do have the task of converting the
focus stats to a generic region struct. For now I'll probably post an
updated patch with the header included back.
Regards,
Naush
>
>
> [5/119] Compiling C++ object
> src/ipa/raspberrypi/ipa_rpi.so.p/controller_rpi_af.cpp.o
> FAILED: src/ipa/raspberrypi/ipa_rpi.so.p/controller_rpi_af.cpp.o
> c++ -Isrc/ipa/raspberrypi/ipa_rpi.so.p -Isrc/ipa/raspberrypi
> -I../../src/ipa/raspberrypi -Iinclude -I../../include -Isrc/ipa
> -I../../src/ipa -I../../src/ipa/raspberrypi/controller
> -Iinclude/libcamera/ipa -Iinclude/libcamera -fdiagnostics-color=always
> -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra
> -Werror -std=c++17 -g -Wshadow -include
> /home/kbingham/iob/libcamera/libcamera/build/gcc/config.h -fPIC
> -DLIBCAMERA_BASE_PRIVATE -MD -MQ
> src/ipa/raspberrypi/ipa_rpi.so.p/controller_rpi_af.cpp.o -MF
> src/ipa/raspberrypi/ipa_rpi.so.p/controller_rpi_af.cpp.o.d -o
> src/ipa/raspberrypi/ipa_rpi.so.p/controller_rpi_af.cpp.o -c
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp
> In file included from ../../src/ipa/raspberrypi/controller/rpi/af.cpp:8:
> ../../src/ipa/raspberrypi/controller/rpi/af.h:120:77: error:
> ‘FOCUS_REGIONS’ was not declared in this scope
> 120 | double getContrast(struct bcm2835_isp_stats_focus const
> focus_stats[FOCUS_REGIONS]) const;
> |
> ^~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/controller/rpi/af.h:141:35: error:
> ‘FOCUS_REGIONS’ was not declared in this scope
> 141 | uint16_t contrastWeights_[FOCUS_REGIONS];
> | ^~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp: In constructor
> ‘RPiController::Af::Af(RPiController::Controller*)’:
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:178:11: error: class
> ‘RPiController::Af’ does not have any field named ‘contrastWeights_’
> 178 | contrastWeights_{},
> | ^~~~~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp: In member function ‘void
> RPiController::Af::computeWeights()’:
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:300:23: error:
> ‘FOCUS_REGIONS’ was not declared in this scope
> 300 | static_assert(FOCUS_REGIONS == FocusStatsRows *
> FocusStatsCols);
> | ^~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:313:25: error:
> ‘contrastWeights_’ was not declared in this scope; did you mean
> ‘phaseWeights_’?
> 313 | contrastWeights_[FocusStatsCols * i + j] =
> w;
> | ^~~~~~~~~~~~~~~~
> | phaseWeights_
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:316:38: error:
> ‘contrastWeights_’ was not declared in this scope; did you mean
> ‘phaseWeights_’?
> 316 | <<
> contrastWeights_[FocusStatsCols * i + 0] << " "
> | ^~~~~~~~~~~~~~~~
> | phaseWeights_
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp: At global scope:
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:355:73: error:
> ‘FOCUS_REGIONS’ was not declared in this scope
> 355 | double Af::getContrast(struct bcm2835_isp_stats_focus const
> focus_stats[FOCUS_REGIONS]) const
> |
> ^~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp: In member function
> ‘double RPiController::Af::getContrast(...) const’:
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:359:34: error:
> ‘FOCUS_REGIONS’ was not declared in this scope
> 359 | for (unsigned i = 0; i < FOCUS_REGIONS; ++i) {
> | ^~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:360:30: error:
> ‘contrastWeights_’ was not declared in this scope; did you mean
> ‘phaseWeights_’?
> 360 | unsigned w = contrastWeights_[i];
> | ^~~~~~~~~~~~~~~~
> | phaseWeights_
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:361:31: error:
> ‘focus_stats’ was not declared in this scope
> 361 | sumWc += w * (focus_stats[i].contrast_val[1][1] >>
> 10);
> | ^~~~~~~~~~~
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp: In member function
> ‘virtual void RPiController::Af::process(RPiController::StatisticsPtr&,
> RPiController::Metadata*)’:
> ../../src/ipa/raspberrypi/controller/rpi/af.cpp:669:44: error: ‘using
> element_type = struct RPiController::Statistics’ {aka ‘struct
> RPiController::Statistics’} has no member named ‘focus_stats’
> 669 | prevContrast_ = getContrast(stats->focus_stats);
> | ^~~~~~~~~~~
> [12/119] Compiling C++ object
> src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o
> FAILED: src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o
> c++ -Isrc/ipa/raspberrypi/ipa_rpi.so.p -Isrc/ipa/raspberrypi
> -I../../src/ipa/raspberrypi -Iinclude -I../../include -Isrc/ipa
> -I../../src/ipa -I../../src/ipa/raspberrypi/controller
> -Iinclude/libcamera/ipa -Iinclude/libcamera -fdiagnostics-color=always
> -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra
> -Werror -std=c++17 -g -Wshadow -include
> /home/kbingham/iob/libcamera/libcamera/build/gcc/config.h -fPIC
> -DLIBCAMERA_BASE_PRIVATE -MD -MQ
> src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o -MF
> src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o.d -o
> src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o -c
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp: In member function ‘void
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)’:
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:332:23: error: ‘using
> element_type = struct RPiController::Statistics’ {aka ‘struct
> RPiController::Statistics’} has no member named ‘hist’
> 332 | memcpy(stats->hist[0].g_hist, aeHistLinear_,
> sizeof(stats->hist[0].g_hist));
> | ^~~~
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:332:68: error: ‘using
> element_type = struct RPiController::Statistics’ {aka ‘struct
> RPiController::Statistics’} has no member named ‘hist’
> 332 | memcpy(stats->hist[0].g_hist, aeHistLinear_,
> sizeof(stats->hist[0].g_hist));
> |
> ^~~~
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:336:29: error:
> ‘AGC_REGIONS’ was not declared in this scope
> 336 | for (int i = 0; i < AGC_REGIONS; i++) {
> | ^~~~~~~~~~~
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:337:61: error: ‘using
> element_type = struct RPiController::Statistics’ {aka ‘struct
> RPiController::Statistics’} has no member named ‘agc_stats’
> 337 | struct bcm2835_isp_stats_region &r =
> stats->agc_stats[i];
> |
> ^~~~~~~~~
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:338:17: error: invalid use
> of incomplete type ‘struct
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)::bcm2835_isp_stats_region’
> 338 | r.r_sum = r.b_sum = r.g_sum = r.counted * v;
> | ^
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:337:24: note: forward
> declaration of ‘struct
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)::bcm2835_isp_stats_region’
> 337 | struct bcm2835_isp_stats_region &r =
> stats->agc_stats[i];
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:338:27: error: invalid use
> of incomplete type ‘struct
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)::bcm2835_isp_stats_region’
> 338 | r.r_sum = r.b_sum = r.g_sum = r.counted * v;
> | ^
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:337:24: note: forward
> declaration of ‘struct
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)::bcm2835_isp_stats_region’
> 337 | struct bcm2835_isp_stats_region &r =
> stats->agc_stats[i];
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:338:37: error: invalid use
> of incomplete type ‘struct
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)::bcm2835_isp_stats_region’
> 338 | r.r_sum = r.b_sum = r.g_sum = r.counted * v;
> | ^
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:337:24: note: forward
> declaration of ‘struct
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)::bcm2835_isp_stats_region’
> 337 | struct bcm2835_isp_stats_region &r =
> stats->agc_stats[i];
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:338:47: error: invalid use
> of incomplete type ‘struct
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)::bcm2835_isp_stats_region’
> 338 | r.r_sum = r.b_sum = r.g_sum = r.counted * v;
> | ^
> ../../src/ipa/raspberrypi/cam_helper_imx708.cpp:337:24: note: forward
> declaration of ‘struct
> CamHelperImx708::putAGCStatistics(RPiController::StatisticsPtr)::bcm2835_isp_stats_region’
> 337 | struct bcm2835_isp_stats_region &r =
> stats->agc_stats[i];
> | ^~~~~~~~~~~~~~~~~~~~~~~~
>
> --
> Kieran
>
>
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> > src/ipa/raspberrypi/controller/controller.h | 4 +-
> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 27 +++++-------
> > src/ipa/raspberrypi/controller/rpi/agc.h | 2 +-
> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 32 +++++++-------
> > src/ipa/raspberrypi/controller/rpi/alsc.h | 3 +-
> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 20 ++++-----
> > src/ipa/raspberrypi/controller/rpi/awb.h | 1 +
> > .../raspberrypi/controller/rpi/contrast.cpp | 8 ++--
> > src/ipa/raspberrypi/controller/rpi/focus.cpp | 7 ++-
> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 14 +-----
> > src/ipa/raspberrypi/raspberrypi.cpp | 44 ++++++++++++++++++-
> > src/ipa/raspberrypi/statistics.h | 2 +
> > 12 files changed, 96 insertions(+), 68 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/controller.h
> b/src/ipa/raspberrypi/controller/controller.h
> > index 3e1e051703b3..e6c950c3a509 100644
> > --- a/src/ipa/raspberrypi/controller/controller.h
> > +++ b/src/ipa/raspberrypi/controller/controller.h
> > @@ -15,19 +15,17 @@
> > #include <vector>
> > #include <string>
> >
> > -#include <linux/bcm2835-isp.h>
> > -
> > #include "libcamera/internal/yaml_parser.h"
> >
> > #include "camera_mode.h"
> > #include "device_status.h"
> > #include "metadata.h"
> > +#include "statistics.h"
> >
> > namespace RPiController {
> >
> > class Algorithm;
> > typedef std::unique_ptr<Algorithm> AlgorithmPtr;
> > -typedef std::shared_ptr<bcm2835_isp_stats> StatisticsPtr;
> >
> > /*
> > * The Controller holds a pointer to some global_metadata, which is how
> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > index 46dcc81ae14c..868c30f03d66 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > @@ -9,8 +9,6 @@
> > #include <map>
> > #include <tuple>
> >
> > -#include <linux/bcm2835-isp.h>
> > -
> > #include <libcamera/base/log.h>
> >
> > #include "../awb_status.h"
> > @@ -451,7 +449,7 @@ void Agc::process(StatisticsPtr &stats, Metadata
> *imageMetadata)
> > fetchCurrentExposure(imageMetadata);
> > /* Compute the total gain we require relative to the current
> exposure. */
> > double gain, targetY;
> > - computeGain(stats.get(), imageMetadata, gain, targetY);
> > + computeGain(stats, imageMetadata, gain, targetY);
> > /* Now compute the target (final) exposure which we think we
> want. */
> > computeTargetExposure(gain);
> > /*
> > @@ -585,24 +583,23 @@ void Agc::fetchAwbStatus(Metadata *imageMetadata)
> > LOG(RPiAgc, Debug) << "No AWB status found";
> > }
> >
> > -static double computeInitialY(bcm2835_isp_stats *stats, AwbStatus const
> &awb,
> > +static double computeInitialY(StatisticsPtr &stats, AwbStatus const
> &awb,
> > double weights[], double gain)
> > {
> > - bcm2835_isp_stats_region *regions = stats->agc_stats;
> > /*
> > * Note how the calculation below means that equal weights give
> you
> > * "average" metering (i.e. all pixels equally important).
> > */
> > double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;
> > - for (unsigned int i = 0; i < AgcStatsSize; i++) {
> > - double counted = regions[i].counted;
> > - double rAcc = std::min(regions[i].r_sum * gain, ((1 <<
> PipelineBits) - 1) * counted);
> > - double gAcc = std::min(regions[i].g_sum * gain, ((1 <<
> PipelineBits) - 1) * counted);
> > - double bAcc = std::min(regions[i].b_sum * gain, ((1 <<
> PipelineBits) - 1) * counted);
> > + for (unsigned int i = 0; i < stats->agcRegions.numRegions();
> i++) {
> > + auto ®ion = stats->agcRegions.get(i);
> > + double rAcc = std::min<double>(region.val.rSum * gain,
> ((1 << PipelineBits) - 1) * region.counted);
> > + double gAcc = std::min<double>(region.val.gSum * gain,
> ((1 << PipelineBits) - 1) * region.counted);
> > + double bAcc = std::min<double>(region.val.bSum * gain,
> ((1 << PipelineBits) - 1) * region.counted);
> > rSum += rAcc * weights[i];
> > gSum += gAcc * weights[i];
> > bSum += bAcc * weights[i];
> > - pixelSum += counted * weights[i];
> > + pixelSum += region.counted * weights[i];
> > }
> > if (pixelSum == 0.0) {
> > LOG(RPiAgc, Warning) << "computeInitialY: pixelSum is
> zero";
> > @@ -624,23 +621,23 @@ static double computeInitialY(bcm2835_isp_stats
> *stats, AwbStatus const &awb,
> >
> > static constexpr double EvGainYTargetLimit = 0.9;
> >
> > -static double constraintComputeGain(AgcConstraint &c, Histogram &h,
> double lux,
> > +static double constraintComputeGain(AgcConstraint &c, const Histogram
> &h, double lux,
> > double evGain, double &targetY)
> > {
> > targetY = c.yTarget.eval(c.yTarget.domain().clip(lux));
> > targetY = std::min(EvGainYTargetLimit, targetY * evGain);
> > double iqm = h.interQuantileMean(c.qLo, c.qHi);
> > - return (targetY * NUM_HISTOGRAM_BINS) / iqm;
> > + return (targetY * h.bins()) / iqm;
> > }
> >
> > -void Agc::computeGain(bcm2835_isp_stats *statistics, Metadata
> *imageMetadata,
> > +void Agc::computeGain(StatisticsPtr &statistics, Metadata
> *imageMetadata,
> > double &gain, double &targetY)
> > {
> > struct LuxStatus lux = {};
> > lux.lux = 400; /* default lux level to 400 in case no metadata
> found */
> > if (imageMetadata->get("lux.status", lux) != 0)
> > LOG(RPiAgc, Warning) << "No lux level found";
> > - Histogram h(statistics->hist[0].g_hist, NUM_HISTOGRAM_BINS);
> > + const Histogram &h = statistics->yHist;
> > double evGain = status_.ev * config_.baseEv;
> > /*
> > * The initial gain and target_Y come from some of the regions.
> After
> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h
> b/src/ipa/raspberrypi/controller/rpi/agc.h
> > index cf04da1973f1..f04896ca25ad 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/agc.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.h
> > @@ -96,7 +96,7 @@ private:
> > void housekeepConfig();
> > void fetchCurrentExposure(Metadata *imageMetadata);
> > void fetchAwbStatus(Metadata *imageMetadata);
> > - void computeGain(bcm2835_isp_stats *statistics, Metadata
> *imageMetadata,
> > + void computeGain(StatisticsPtr &statistics, Metadata
> *imageMetadata,
> > double &gain, double &targetY);
> > void computeTargetExposure(double gain);
> > bool applyDigitalGain(double gain, double targetY);
> > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> > index a4afaf841c41..eb4e2f9496e1 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> > @@ -310,19 +310,21 @@ double getCt(Metadata *metadata, double defaultCt)
> > return awbStatus.temperatureK;
> > }
> >
> > -static void copyStats(bcm2835_isp_stats_region regions[XY],
> StatisticsPtr &stats,
> > +static void copyStats(RgbyRegions ®ions, StatisticsPtr &stats,
> > AlscStatus const &status)
> > {
> > - bcm2835_isp_stats_region *inputRegions = stats->awb_stats;
> > + if (!regions.numRegions())
> > + regions.init(stats->awbRegions.size());
> > +
> > double *rTable = (double *)status.r;
> > double *gTable = (double *)status.g;
> > double *bTable = (double *)status.b;
> > - for (int i = 0; i < XY; i++) {
> > - regions[i].r_sum = inputRegions[i].r_sum / rTable[i];
> > - regions[i].g_sum = inputRegions[i].g_sum / gTable[i];
> > - regions[i].b_sum = inputRegions[i].b_sum / bTable[i];
> > - regions[i].counted = inputRegions[i].counted;
> > - /* (don't care about the uncounted value) */
> > + for (unsigned int i = 0; i < stats->awbRegions.numRegions();
> i++) {
> > + auto r = stats->awbRegions.get(i);
> > + r.val.rSum = static_cast<uint64_t>(r.val.rSum /
> rTable[i]);
> > + r.val.gSum = static_cast<uint64_t>(r.val.gSum /
> gTable[i]);
> > + r.val.bSum = static_cast<uint64_t>(r.val.bSum /
> bTable[i]);
> > + regions.set(i, r);
> > }
> > }
> >
> > @@ -512,19 +514,19 @@ void resampleCalTable(double const calTableIn[XY],
> > }
> >
> > /* Calculate chrominance statistics (R/G and B/G) for each region. */
> > -static_assert(XY == AWB_REGIONS, "ALSC/AWB statistics region mismatch");
> > -static void calculateCrCb(bcm2835_isp_stats_region *awbRegion, double
> cr[XY],
> > +static void calculateCrCb(const RgbyRegions &awbRegion, double cr[XY],
> > double cb[XY], uint32_t minCount, uint16_t
> minG)
> > {
> > for (int i = 0; i < XY; i++) {
> > - bcm2835_isp_stats_region &zone = awbRegion[i];
> > - if (zone.counted <= minCount ||
> > - zone.g_sum / zone.counted <= minG) {
> > + auto s = awbRegion.get(i);
> > +
> > + if (s.counted <= minCount || s.val.gSum / s.counted <=
> minG) {
> > cr[i] = cb[i] = InsufficientData;
> > continue;
> > }
> > - cr[i] = zone.r_sum / (double)zone.g_sum;
> > - cb[i] = zone.b_sum / (double)zone.g_sum;
> > +
> > + cr[i] = s.val.rSum / (double)s.val.gSum;
> > + cb[i] = s.val.bSum / (double)s.val.gSum;
> > }
> > }
> >
> > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.h
> b/src/ipa/raspberrypi/controller/rpi/alsc.h
> > index a858ef5a6551..9167c9ffa2e3 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/alsc.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.h
> > @@ -12,6 +12,7 @@
> >
> > #include "../algorithm.h"
> > #include "../alsc_status.h"
> > +#include "../statistics.h"
> >
> > namespace RPiController {
> >
> > @@ -98,7 +99,7 @@ private:
> > /* copy out the results from the async thread so that it can be
> restarted */
> > void fetchAsyncResults();
> > double ct_;
> > - bcm2835_isp_stats_region statistics_[AlscCellsY * AlscCellsX];
> > + RgbyRegions statistics_;
> > double asyncResults_[3][AlscCellsY][AlscCellsX];
> > double asyncLambdaR_[AlscCellsX * AlscCellsY];
> > double asyncLambdaB_[AlscCellsX * AlscCellsY];
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > index 04d1c8783654..ef3435d66106 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > @@ -21,9 +21,6 @@ LOG_DEFINE_CATEGORY(RPiAwb)
> >
> > #define NAME "rpi.awb"
> >
> > -static constexpr unsigned int AwbStatsSizeX = DEFAULT_AWB_REGIONS_X;
> > -static constexpr unsigned int AwbStatsSizeY = DEFAULT_AWB_REGIONS_Y;
> > -
> > /*
> > * todo - the locking in this algorithm needs some tidying up as has
> been done
> > * elsewhere (ALSC and AGC).
> > @@ -401,17 +398,16 @@ void Awb::asyncFunc()
> > }
> >
> > static void generateStats(std::vector<Awb::RGB> &zones,
> > - bcm2835_isp_stats_region *stats, double
> minPixels,
> > + RgbyRegions &stats, double minPixels,
> > double minG)
> > {
> > - for (unsigned int i = 0; i < AwbStatsSizeX * AwbStatsSizeY; i++)
> {
> > + for (auto const ®ion : stats) {
> > Awb::RGB zone;
> > - double counted = stats[i].counted;
> > - if (counted >= minPixels) {
> > - zone.G = stats[i].g_sum / counted;
> > + if (region.counted >= minPixels) {
> > + zone.G = region.val.gSum / region.counted;
> > if (zone.G >= minG) {
> > - zone.R = stats[i].r_sum / counted;
> > - zone.B = stats[i].b_sum / counted;
> > + zone.R = region.val.rSum /
> region.counted;
> > + zone.B = region.val.bSum /
> region.counted;
> > zones.push_back(zone);
> > }
> > }
> > @@ -425,7 +421,7 @@ void Awb::prepareStats()
> > * LSC has already been applied to the stats in this pipeline,
> so stop
> > * any LSC compensation. We also ignore config_.fast in this
> version.
> > */
> > - generateStats(zones_, statistics_->awb_stats, config_.minPixels,
> > + generateStats(zones_, statistics_->awbRegions, config_.minPixels,
> > config_.minG);
> > /*
> > * apply sensitivities, so values appear to come from our
> "canonical"
> > @@ -641,7 +637,7 @@ void Awb::awbBayes()
> > * valid... not entirely sure about this.
> > */
> > Pwl prior = interpolatePrior();
> > - prior *= zones_.size() / (double)(AwbStatsSizeX * AwbStatsSizeY);
> > + prior *= zones_.size() /
> (double)(statistics_->awbRegions.numRegions());
> > prior.map([](double x, double y) {
> > LOG(RPiAwb, Debug) << "(" << x << "," << y << ")";
> > });
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h
> b/src/ipa/raspberrypi/controller/rpi/awb.h
> > index 2254c3ed2cc4..e7d49cd8036b 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.h
> > @@ -13,6 +13,7 @@
> > #include "../awb_algorithm.h"
> > #include "../pwl.h"
> > #include "../awb_status.h"
> > +#include "../statistics.h"
> >
> > namespace RPiController {
> >
> > diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> b/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> > index 5b37edcbd46a..a4f8c4f04fc4 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> > @@ -106,7 +106,7 @@ Pwl computeStretchCurve(Histogram const &histogram,
> > * bit.
> > */
> > double histLo = histogram.quantile(config.loHistogram) *
> > - (65536 / NUM_HISTOGRAM_BINS);
> > + (65536 / histogram.bins());
> > double levelLo = config.loLevel * 65536;
> > LOG(RPiContrast, Debug)
> > << "Move histogram point " << histLo << " to " <<
> levelLo;
> > @@ -119,7 +119,7 @@ Pwl computeStretchCurve(Histogram const &histogram,
> > * Keep the mid-point (median) in the same place, though, to
> limit the
> > * apparent amount of global brightness shift.
> > */
> > - double mid = histogram.quantile(0.5) * (65536 /
> NUM_HISTOGRAM_BINS);
> > + double mid = histogram.quantile(0.5) * (65536 /
> histogram.bins());
> > enhance.append(mid, mid);
> >
> > /*
> > @@ -127,7 +127,7 @@ Pwl computeStretchCurve(Histogram const &histogram,
> > * there up.
> > */
> > double histHi = histogram.quantile(config.hiHistogram) *
> > - (65536 / NUM_HISTOGRAM_BINS);
> > + (65536 / histogram.bins());
> > double levelHi = config.hiLevel * 65536;
> > LOG(RPiContrast, Debug)
> > << "Move histogram point " << histHi << " to " <<
> levelHi;
> > @@ -158,7 +158,7 @@ Pwl applyManualContrast(Pwl const &gammaCurve,
> double brightness,
> > void Contrast::process(StatisticsPtr &stats,
> > [[maybe_unused]] Metadata *imageMetadata)
> > {
> > - Histogram histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS);
> > + Histogram &histogram = stats->yHist;
> > /*
> > * We look at the histogram and adjust the gamma curve in the
> following
> > * ways: 1. Adjust the gamma curve so as to pull the start of the
> > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp
> b/src/ipa/raspberrypi/controller/rpi/focus.cpp
> > index 8c5029bd0e95..ea3cc00e42c3 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp
> > @@ -31,10 +31,9 @@ char const *Focus::name() const
> > void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)
> > {
> > FocusStatus status;
> > - unsigned int i;
> > - for (i = 0; i < FOCUS_REGIONS; i++)
> > - status.focusMeasures[i] =
> stats->focus_stats[i].contrast_val[1][1] / 1000;
> > - status.num = i;
> > + for (unsigned int i = 0; i < stats->focusRegions.numRegions();
> i++)
> > + status.focusMeasures[i] = stats->focusRegions.get(i).val;
> > + status.num = stats->focusRegions.numRegions();
> > imageMetadata->set("focus.status", status);
> >
> > LOG(RPiFocus, Debug)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > index 9759186afacf..06625f3a5ea3 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > @@ -6,8 +6,6 @@
> > */
> > #include <math.h>
> >
> > -#include <linux/bcm2835-isp.h>
> > -
> > #include <libcamera/base/log.h>
> >
> > #include "../device_status.h"
> > @@ -83,20 +81,12 @@ void Lux::process(StatisticsPtr &stats, Metadata
> *imageMetadata)
> > if (imageMetadata->get("device.status", deviceStatus) == 0) {
> > double currentGain = deviceStatus.analogueGain;
> > double currentAperture =
> deviceStatus.aperture.value_or(currentAperture_);
> > - uint64_t sum = 0;
> > - uint32_t num = 0;
> > - uint32_t *bin = stats->hist[0].g_hist;
> > - const int numBins = sizeof(stats->hist[0].g_hist) /
> > - sizeof(stats->hist[0].g_hist[0]);
> > - for (int i = 0; i < numBins; i++)
> > - sum += bin[i] * (uint64_t)i, num += bin[i];
> > - /* add .5 to reflect the mid-points of bins */
> > - double currentY = sum / (double)num + .5;
> > + double currentY = stats->yHist.interQuantileMean(0, 1);
> > double gainRatio = referenceGain_ / currentGain;
> > double shutterSpeedRatio =
> > referenceShutterSpeed_ /
> deviceStatus.shutterSpeed;
> > double apertureRatio = referenceAperture_ /
> currentAperture;
> > - double yRatio = currentY * (65536 / numBins) /
> referenceY_;
> > + double yRatio = currentY * (65536 / stats->yHist.bins())
> / referenceY_;
> > double estimatedLux = shutterSpeedRatio * gainRatio *
> > apertureRatio * apertureRatio *
> > yRatio * referenceLux_;
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> > index bead436def3c..cff079bbafb3 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -51,6 +51,7 @@
> > #include "metadata.h"
> > #include "sharpen_algorithm.h"
> > #include "sharpen_status.h"
> > +#include "statistics.h"
> >
> > namespace libcamera {
> >
> > @@ -139,6 +140,7 @@ private:
> > void prepareISP(const ISPConfig &data);
> > void reportMetadata(unsigned int ipaContext);
> > void fillDeviceStatus(const ControlList &sensorControls,
> unsigned int ipaContext);
> > + RPiController::StatisticsPtr fillStatistics(bcm2835_isp_stats
> *stats) const;
> > void processStats(unsigned int bufferId, unsigned int
> ipaContext);
> > void applyFrameDurations(Duration minFrameDuration, Duration
> maxFrameDuration);
> > void applyAGC(const struct AgcStatus *agcStatus, ControlList
> &ctrls);
> > @@ -1141,6 +1143,46 @@ void IPARPi::fillDeviceStatus(const ControlList
> &sensorControls, unsigned int ip
> > rpiMetadata_[ipaContext].set("device.status", deviceStatus);
> > }
> >
> > +RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats
> *stats) const
> > +{
> > + using namespace RPiController;
> > +
> > + unsigned int i;
> > + StatisticsPtr statistics =
> > +
> std::make_unique<Statistics>(Statistics::AgcStatsPos::PreWb,
> Statistics::ColourStatsPos::PostLsc);
> > +
> > + /* RGB histograms are not used, so do not populate them. */
> > + statistics->yHist =
> std::move(RPiController::Histogram(stats->hist[0].g_hist,
> NUM_HISTOGRAM_BINS));
> > +
> > + statistics->awbRegions.init({ DEFAULT_AWB_REGIONS_X,
> DEFAULT_AWB_REGIONS_Y });
> > + for (i = 0; i < statistics->awbRegions.numRegions(); i++)
> > + statistics->awbRegions.set(i, { {
> stats->awb_stats[i].r_sum,
> > +
> stats->awb_stats[i].g_sum,
> > +
> stats->awb_stats[i].b_sum },
> > +
> stats->awb_stats[i].counted,
> > +
> stats->awb_stats[i].notcounted });
> > +
> > + /*
> > + * There are only ever 15 regions computed by the firmware due
> to zoning,
> > + * but the HW defines AGC_REGIONS == 16!
> > + */
> > + statistics->agcRegions.init(15);
> > + for (i = 0; i < statistics->agcRegions.numRegions(); i++)
> > + statistics->agcRegions.set(i, { {
> stats->agc_stats[i].r_sum,
> > +
> stats->agc_stats[i].g_sum,
> > +
> stats->agc_stats[i].b_sum },
> > +
> stats->agc_stats[i].counted,
> > +
> stats->awb_stats[i].notcounted });
> > +
> > + statistics->focusRegions.init({ 4, 3 });
> > + for (i = 0; i < statistics->focusRegions.numRegions(); i++)
> > + statistics->focusRegions.set(i, {
> stats->focus_stats[i].contrast_val[1][1] / 1000,
> > +
> stats->focus_stats[i].contrast_val_num[1][1],
> > +
> stats->focus_stats[i].contrast_val_num[1][0] });
> > +
> > + return statistics;
> > +}
> > +
> > void IPARPi::processStats(unsigned int bufferId, unsigned int
> ipaContext)
> > {
> > RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext];
> > @@ -1153,7 +1195,7 @@ void IPARPi::processStats(unsigned int bufferId,
> unsigned int ipaContext)
> >
> > Span<uint8_t> mem = it->second.planes()[0];
> > bcm2835_isp_stats *stats = reinterpret_cast<bcm2835_isp_stats
> *>(mem.data());
> > - RPiController::StatisticsPtr statistics =
> std::make_shared<bcm2835_isp_stats>(*stats);
> > + RPiController::StatisticsPtr statistics = fillStatistics(stats);
> > helper_->process(statistics, rpiMetadata);
> > controller_.process(statistics, &rpiMetadata);
> >
> > diff --git a/src/ipa/raspberrypi/statistics.h
> b/src/ipa/raspberrypi/statistics.h
> > index a762bf3d41aa..affb7272c963 100644
> > --- a/src/ipa/raspberrypi/statistics.h
> > +++ b/src/ipa/raspberrypi/statistics.h
> > @@ -67,4 +67,6 @@ struct Statistics {
> > FocusRegions focusRegions;
> > };
> >
> > +using StatisticsPtr = std::shared_ptr<Statistics>;
> > +
> > } /* namespace RPiController */
> > --
> > 2.25.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20230131/398f9232/attachment.htm>
More information about the libcamera-devel
mailing list