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