[{"id":25894,"web_url":"https://patchwork.libcamera.org/comment/25894/","msgid":"<CAHW6GYKnapRdE-DfOc5SZsVJfAsDNuMQvSW3NpjR2T9DWfksZQ@mail.gmail.com>","date":"2022-11-24T12:54:26","subject":"Re: [libcamera-devel] [PATCH v2 4/5] ipa: raspberrypi: Use the\n\tgeneric statistics structure in the algorithms","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the update.\n\nOn Thu, 24 Nov 2022 at 10:38, Naushir Patuck via libcamera-devel\n<libcamera-devel@lists.libcamera.org> wrote:\n>\n> Repurpose the StatisticsPtr type from being a shared_ptr<bcm2835_isp_stats> to\n> shared_ptr<RPiController::Statistics>. This removes any hardware specific header\n> files and structures from the algorithms source code.\n>\n> Add a new function in the Raspberry Pi IPA to populate the generic statistics\n> structure from the values provided by the hardware in the bcm2835_isp_stats\n> structure.\n>\n> Update the Lux, AWB, AGC, ALSC, Contrast, and Focus algorithms to use the\n> generic statistics structure appropriately in their calculations. Additionally,\n> remove references to any hardware specific headers and defines in these source\n> files.\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n\nDidn't spot anything to complain about this time!\n\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks!\nDavid\n\n> ---\n>  src/ipa/raspberrypi/controller/controller.h   |  4 +-\n>  src/ipa/raspberrypi/controller/rpi/agc.cpp    | 27 +++++-------\n>  src/ipa/raspberrypi/controller/rpi/agc.h      |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/alsc.cpp   | 32 +++++++-------\n>  src/ipa/raspberrypi/controller/rpi/alsc.h     |  3 +-\n>  src/ipa/raspberrypi/controller/rpi/awb.cpp    | 20 ++++-----\n>  src/ipa/raspberrypi/controller/rpi/awb.h      |  1 +\n>  .../raspberrypi/controller/rpi/contrast.cpp   |  8 ++--\n>  src/ipa/raspberrypi/controller/rpi/focus.cpp  |  7 ++-\n>  src/ipa/raspberrypi/controller/rpi/lux.cpp    | 14 +-----\n>  src/ipa/raspberrypi/raspberrypi.cpp           | 44 ++++++++++++++++++-\n>  src/ipa/raspberrypi/statistics.h              |  2 +\n>  12 files changed, 96 insertions(+), 68 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/controller/controller.h b/src/ipa/raspberrypi/controller/controller.h\n> index 3e1e051703b3..e6c950c3a509 100644\n> --- a/src/ipa/raspberrypi/controller/controller.h\n> +++ b/src/ipa/raspberrypi/controller/controller.h\n> @@ -15,19 +15,17 @@\n>  #include <vector>\n>  #include <string>\n>\n> -#include <linux/bcm2835-isp.h>\n> -\n>  #include \"libcamera/internal/yaml_parser.h\"\n>\n>  #include \"camera_mode.h\"\n>  #include \"device_status.h\"\n>  #include \"metadata.h\"\n> +#include \"statistics.h\"\n>\n>  namespace RPiController {\n>\n>  class Algorithm;\n>  typedef std::unique_ptr<Algorithm> AlgorithmPtr;\n> -typedef std::shared_ptr<bcm2835_isp_stats> StatisticsPtr;\n>\n>  /*\n>   * The Controller holds a pointer to some global_metadata, which is how\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> index bd54a639d637..a7f0e954407d 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> @@ -9,8 +9,6 @@\n>  #include <map>\n>  #include <tuple>\n>\n> -#include <linux/bcm2835-isp.h>\n> -\n>  #include <libcamera/base/log.h>\n>\n>  #include \"../awb_status.h\"\n> @@ -450,7 +448,7 @@ void Agc::process(StatisticsPtr &stats, Metadata *imageMetadata)\n>         fetchCurrentExposure(imageMetadata);\n>         /* Compute the total gain we require relative to the current exposure. */\n>         double gain, targetY;\n> -       computeGain(stats.get(), imageMetadata, gain, targetY);\n> +       computeGain(stats, imageMetadata, gain, targetY);\n>         /* Now compute the target (final) exposure which we think we want. */\n>         computeTargetExposure(gain);\n>         /*\n> @@ -584,24 +582,23 @@ void Agc::fetchAwbStatus(Metadata *imageMetadata)\n>                 LOG(RPiAgc, Debug) << \"No AWB status found\";\n>  }\n>\n> -static double computeInitialY(bcm2835_isp_stats *stats, AwbStatus const &awb,\n> +static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n>                               double weights[], double gain)\n>  {\n> -       bcm2835_isp_stats_region *regions = stats->agc_stats;\n>         /*\n>          * Note how the calculation below means that equal weights give you\n>          * \"average\" metering (i.e. all pixels equally important).\n>          */\n>         double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> -       for (unsigned int i = 0; i < AgcStatsSize; i++) {\n> -               double counted = regions[i].counted;\n> -               double rAcc = std::min(regions[i].r_sum * gain, ((1 << PipelineBits) - 1) * counted);\n> -               double gAcc = std::min(regions[i].g_sum * gain, ((1 << PipelineBits) - 1) * counted);\n> -               double bAcc = std::min(regions[i].b_sum * gain, ((1 << PipelineBits) - 1) * counted);\n> +       for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n> +               auto &region = stats->agcRegions.get(i);\n> +               double rAcc = std::min<double>(region.val.rSum * gain, ((1 << PipelineBits) - 1) * region.counted);\n> +               double gAcc = std::min<double>(region.val.gSum * gain, ((1 << PipelineBits) - 1) * region.counted);\n> +               double bAcc = std::min<double>(region.val.bSum * gain, ((1 << PipelineBits) - 1) * region.counted);\n>                 rSum += rAcc * weights[i];\n>                 gSum += gAcc * weights[i];\n>                 bSum += bAcc * weights[i];\n> -               pixelSum += counted * weights[i];\n> +               pixelSum += region.counted * weights[i];\n>         }\n>         if (pixelSum == 0.0) {\n>                 LOG(RPiAgc, Warning) << \"computeInitialY: pixelSum is zero\";\n> @@ -623,23 +620,23 @@ static double computeInitialY(bcm2835_isp_stats *stats, AwbStatus const &awb,\n>\n>  static constexpr double EvGainYTargetLimit = 0.9;\n>\n> -static double constraintComputeGain(AgcConstraint &c, Histogram &h, double lux,\n> +static double constraintComputeGain(AgcConstraint &c, const Histogram &h, double lux,\n>                                     double evGain, double &targetY)\n>  {\n>         targetY = c.yTarget.eval(c.yTarget.domain().clip(lux));\n>         targetY = std::min(EvGainYTargetLimit, targetY * evGain);\n>         double iqm = h.interQuantileMean(c.qLo, c.qHi);\n> -       return (targetY * NUM_HISTOGRAM_BINS) / iqm;\n> +       return (targetY * h.bins()) / iqm;\n>  }\n>\n> -void Agc::computeGain(bcm2835_isp_stats *statistics, Metadata *imageMetadata,\n> +void Agc::computeGain(StatisticsPtr &statistics, Metadata *imageMetadata,\n>                       double &gain, double &targetY)\n>  {\n>         struct LuxStatus lux = {};\n>         lux.lux = 400; /* default lux level to 400 in case no metadata found */\n>         if (imageMetadata->get(\"lux.status\", lux) != 0)\n>                 LOG(RPiAgc, Warning) << \"No lux level found\";\n> -       Histogram h(statistics->hist[0].g_hist, NUM_HISTOGRAM_BINS);\n> +       const Histogram &h = statistics->yHist;\n>         double evGain = status_.ev * config_.baseEv;\n>         /*\n>          * The initial gain and target_Y come from some of the regions. After\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h\n> index 6d6b0e5ad857..0ea71e6d1aa0 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.h\n> @@ -98,7 +98,7 @@ private:\n>         void housekeepConfig();\n>         void fetchCurrentExposure(Metadata *imageMetadata);\n>         void fetchAwbStatus(Metadata *imageMetadata);\n> -       void computeGain(bcm2835_isp_stats *statistics, Metadata *imageMetadata,\n> +       void computeGain(StatisticsPtr &statistics, Metadata *imageMetadata,\n>                          double &gain, double &targetY);\n>         void computeTargetExposure(double gain);\n>         bool applyDigitalGain(double gain, double targetY);\n> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> index a4afaf841c41..eb4e2f9496e1 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> @@ -310,19 +310,21 @@ double getCt(Metadata *metadata, double defaultCt)\n>         return awbStatus.temperatureK;\n>  }\n>\n> -static void copyStats(bcm2835_isp_stats_region regions[XY], StatisticsPtr &stats,\n> +static void copyStats(RgbyRegions &regions, StatisticsPtr &stats,\n>                       AlscStatus const &status)\n>  {\n> -       bcm2835_isp_stats_region *inputRegions = stats->awb_stats;\n> +       if (!regions.numRegions())\n> +               regions.init(stats->awbRegions.size());\n> +\n>         double *rTable = (double *)status.r;\n>         double *gTable = (double *)status.g;\n>         double *bTable = (double *)status.b;\n> -       for (int i = 0; i < XY; i++) {\n> -               regions[i].r_sum = inputRegions[i].r_sum / rTable[i];\n> -               regions[i].g_sum = inputRegions[i].g_sum / gTable[i];\n> -               regions[i].b_sum = inputRegions[i].b_sum / bTable[i];\n> -               regions[i].counted = inputRegions[i].counted;\n> -               /* (don't care about the uncounted value) */\n> +       for (unsigned int i = 0; i < stats->awbRegions.numRegions(); i++) {\n> +               auto r = stats->awbRegions.get(i);\n> +               r.val.rSum = static_cast<uint64_t>(r.val.rSum / rTable[i]);\n> +               r.val.gSum = static_cast<uint64_t>(r.val.gSum / gTable[i]);\n> +               r.val.bSum = static_cast<uint64_t>(r.val.bSum / bTable[i]);\n> +               regions.set(i, r);\n>         }\n>  }\n>\n> @@ -512,19 +514,19 @@ void resampleCalTable(double const calTableIn[XY],\n>  }\n>\n>  /* Calculate chrominance statistics (R/G and B/G) for each region. */\n> -static_assert(XY == AWB_REGIONS, \"ALSC/AWB statistics region mismatch\");\n> -static void calculateCrCb(bcm2835_isp_stats_region *awbRegion, double cr[XY],\n> +static void calculateCrCb(const RgbyRegions &awbRegion, double cr[XY],\n>                           double cb[XY], uint32_t minCount, uint16_t minG)\n>  {\n>         for (int i = 0; i < XY; i++) {\n> -               bcm2835_isp_stats_region &zone = awbRegion[i];\n> -               if (zone.counted <= minCount ||\n> -                   zone.g_sum / zone.counted <= minG) {\n> +               auto s = awbRegion.get(i);\n> +\n> +               if (s.counted <= minCount || s.val.gSum / s.counted <= minG) {\n>                         cr[i] = cb[i] = InsufficientData;\n>                         continue;\n>                 }\n> -               cr[i] = zone.r_sum / (double)zone.g_sum;\n> -               cb[i] = zone.b_sum / (double)zone.g_sum;\n> +\n> +               cr[i] = s.val.rSum / (double)s.val.gSum;\n> +               cb[i] = s.val.bSum / (double)s.val.gSum;\n>         }\n>  }\n>\n> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.h b/src/ipa/raspberrypi/controller/rpi/alsc.h\n> index a858ef5a6551..9167c9ffa2e3 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/alsc.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.h\n> @@ -12,6 +12,7 @@\n>\n>  #include \"../algorithm.h\"\n>  #include \"../alsc_status.h\"\n> +#include \"../statistics.h\"\n>\n>  namespace RPiController {\n>\n> @@ -98,7 +99,7 @@ private:\n>         /* copy out the results from the async thread so that it can be restarted */\n>         void fetchAsyncResults();\n>         double ct_;\n> -       bcm2835_isp_stats_region statistics_[AlscCellsY * AlscCellsX];\n> +       RgbyRegions statistics_;\n>         double asyncResults_[3][AlscCellsY][AlscCellsX];\n>         double asyncLambdaR_[AlscCellsX * AlscCellsY];\n>         double asyncLambdaB_[AlscCellsX * AlscCellsY];\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> index 861022014896..c089aedf020c 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> @@ -21,9 +21,6 @@ LOG_DEFINE_CATEGORY(RPiAwb)\n>\n>  #define NAME \"rpi.awb\"\n>\n> -static constexpr unsigned int AwbStatsSizeX = DEFAULT_AWB_REGIONS_X;\n> -static constexpr unsigned int AwbStatsSizeY = DEFAULT_AWB_REGIONS_Y;\n> -\n>  /*\n>   * todo - the locking in this algorithm needs some tidying up as has been done\n>   * elsewhere (ALSC and AGC).\n> @@ -406,17 +403,16 @@ void Awb::asyncFunc()\n>  }\n>\n>  static void generateStats(std::vector<Awb::RGB> &zones,\n> -                         bcm2835_isp_stats_region *stats, double minPixels,\n> +                         RgbyRegions &stats, double minPixels,\n>                           double minG)\n>  {\n> -       for (unsigned int i = 0; i < AwbStatsSizeX * AwbStatsSizeY; i++) {\n> +       for (auto const &region : stats) {\n>                 Awb::RGB zone;\n> -               double counted = stats[i].counted;\n> -               if (counted >= minPixels) {\n> -                       zone.G = stats[i].g_sum / counted;\n> +               if (region.counted >= minPixels) {\n> +                       zone.G = region.val.gSum / region.counted;\n>                         if (zone.G >= minG) {\n> -                               zone.R = stats[i].r_sum / counted;\n> -                               zone.B = stats[i].b_sum / counted;\n> +                               zone.R = region.val.rSum / region.counted;\n> +                               zone.B = region.val.bSum / region.counted;\n>                                 zones.push_back(zone);\n>                         }\n>                 }\n> @@ -430,7 +426,7 @@ void Awb::prepareStats()\n>          * LSC has already been applied to the stats in this pipeline, so stop\n>          * any LSC compensation.  We also ignore config_.fast in this version.\n>          */\n> -       generateStats(zones_, statistics_->awb_stats, config_.minPixels,\n> +       generateStats(zones_, statistics_->awbRegions, config_.minPixels,\n>                       config_.minG);\n>         /*\n>          * apply sensitivities, so values appear to come from our \"canonical\"\n> @@ -646,7 +642,7 @@ void Awb::awbBayes()\n>          * valid... not entirely sure about this.\n>          */\n>         Pwl prior = interpolatePrior();\n> -       prior *= zones_.size() / (double)(AwbStatsSizeX * AwbStatsSizeY);\n> +       prior *= zones_.size() / (double)(statistics_->awbRegions.numRegions());\n>         prior.map([](double x, double y) {\n>                 LOG(RPiAwb, Debug) << \"(\" << x << \",\" << y << \")\";\n>         });\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h b/src/ipa/raspberrypi/controller/rpi/awb.h\n> index 30acd89d0969..d81779dd51ff 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.h\n> @@ -13,6 +13,7 @@\n>  #include \"../awb_algorithm.h\"\n>  #include \"../pwl.h\"\n>  #include \"../awb_status.h\"\n> +#include \"../statistics.h\"\n>\n>  namespace RPiController {\n>\n> diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.cpp b/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> index 5b37edcbd46a..a4f8c4f04fc4 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> @@ -106,7 +106,7 @@ Pwl computeStretchCurve(Histogram const &histogram,\n>          * bit.\n>          */\n>         double histLo = histogram.quantile(config.loHistogram) *\n> -                       (65536 / NUM_HISTOGRAM_BINS);\n> +                       (65536 / histogram.bins());\n>         double levelLo = config.loLevel * 65536;\n>         LOG(RPiContrast, Debug)\n>                 << \"Move histogram point \" << histLo << \" to \" << levelLo;\n> @@ -119,7 +119,7 @@ Pwl computeStretchCurve(Histogram const &histogram,\n>          * Keep the mid-point (median) in the same place, though, to limit the\n>          * apparent amount of global brightness shift.\n>          */\n> -       double mid = histogram.quantile(0.5) * (65536 / NUM_HISTOGRAM_BINS);\n> +       double mid = histogram.quantile(0.5) * (65536 / histogram.bins());\n>         enhance.append(mid, mid);\n>\n>         /*\n> @@ -127,7 +127,7 @@ Pwl computeStretchCurve(Histogram const &histogram,\n>          * there up.\n>          */\n>         double histHi = histogram.quantile(config.hiHistogram) *\n> -                       (65536 / NUM_HISTOGRAM_BINS);\n> +                       (65536 / histogram.bins());\n>         double levelHi = config.hiLevel * 65536;\n>         LOG(RPiContrast, Debug)\n>                 << \"Move histogram point \" << histHi << \" to \" << levelHi;\n> @@ -158,7 +158,7 @@ Pwl applyManualContrast(Pwl const &gammaCurve, double brightness,\n>  void Contrast::process(StatisticsPtr &stats,\n>                        [[maybe_unused]] Metadata *imageMetadata)\n>  {\n> -       Histogram histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS);\n> +       Histogram &histogram = stats->yHist;\n>         /*\n>          * We look at the histogram and adjust the gamma curve in the following\n>          * ways: 1. Adjust the gamma curve so as to pull the start of the\n> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> index 8c5029bd0e95..ea3cc00e42c3 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> @@ -31,10 +31,9 @@ char const *Focus::name() const\n>  void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)\n>  {\n>         FocusStatus status;\n> -       unsigned int i;\n> -       for (i = 0; i < FOCUS_REGIONS; i++)\n> -               status.focusMeasures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;\n> -       status.num = i;\n> +       for (unsigned int i = 0; i < stats->focusRegions.numRegions(); i++)\n> +               status.focusMeasures[i] = stats->focusRegions.get(i).val;\n> +       status.num = stats->focusRegions.numRegions();\n>         imageMetadata->set(\"focus.status\", status);\n>\n>         LOG(RPiFocus, Debug)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> index 9759186afacf..06625f3a5ea3 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> @@ -6,8 +6,6 @@\n>   */\n>  #include <math.h>\n>\n> -#include <linux/bcm2835-isp.h>\n> -\n>  #include <libcamera/base/log.h>\n>\n>  #include \"../device_status.h\"\n> @@ -83,20 +81,12 @@ void Lux::process(StatisticsPtr &stats, Metadata *imageMetadata)\n>         if (imageMetadata->get(\"device.status\", deviceStatus) == 0) {\n>                 double currentGain = deviceStatus.analogueGain;\n>                 double currentAperture = deviceStatus.aperture.value_or(currentAperture_);\n> -               uint64_t sum = 0;\n> -               uint32_t num = 0;\n> -               uint32_t *bin = stats->hist[0].g_hist;\n> -               const int numBins = sizeof(stats->hist[0].g_hist) /\n> -                                   sizeof(stats->hist[0].g_hist[0]);\n> -               for (int i = 0; i < numBins; i++)\n> -                       sum += bin[i] * (uint64_t)i, num += bin[i];\n> -               /* add .5 to reflect the mid-points of bins */\n> -               double currentY = sum / (double)num + .5;\n> +               double currentY = stats->yHist.interQuantileMean(0, 1);\n>                 double gainRatio = referenceGain_ / currentGain;\n>                 double shutterSpeedRatio =\n>                         referenceShutterSpeed_ / deviceStatus.shutterSpeed;\n>                 double apertureRatio = referenceAperture_ / currentAperture;\n> -               double yRatio = currentY * (65536 / numBins) / referenceY_;\n> +               double yRatio = currentY * (65536 / stats->yHist.bins()) / referenceY_;\n>                 double estimatedLux = shutterSpeedRatio * gainRatio *\n>                                       apertureRatio * apertureRatio *\n>                                       yRatio * referenceLux_;\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index b74f1ecf738f..44527357243d 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -51,6 +51,7 @@\n>  #include \"metadata.h\"\n>  #include \"sharpen_algorithm.h\"\n>  #include \"sharpen_status.h\"\n> +#include \"statistics.h\"\n>\n>  namespace libcamera {\n>\n> @@ -136,6 +137,7 @@ private:\n>         void prepareISP(const ISPConfig &data);\n>         void reportMetadata();\n>         void fillDeviceStatus(const ControlList &sensorControls);\n> +       RPiController::StatisticsPtr fillStatistics(bcm2835_isp_stats *stats) const;\n>         void processStats(unsigned int bufferId);\n>         void applyFrameDurations(Duration minFrameDuration, Duration maxFrameDuration);\n>         void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n> @@ -1119,6 +1121,46 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n>         rpiMetadata_.set(\"device.status\", deviceStatus);\n>  }\n>\n> +RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) const\n> +{\n> +       using namespace RPiController;\n> +\n> +       unsigned int i;\n> +       StatisticsPtr statistics =\n> +               std::make_unique<Statistics>(Statistics::AgcStatsPos::PreWb, Statistics::ColourStatsPos::PostLsc);\n> +\n> +       /* RGB histograms are not used, so do not populate them. */\n> +       statistics->yHist = std::move(RPiController::Histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS));\n> +\n> +       statistics->awbRegions.init({ DEFAULT_AWB_REGIONS_X, DEFAULT_AWB_REGIONS_Y });\n> +       for (i = 0; i < statistics->awbRegions.numRegions(); i++)\n> +               statistics->awbRegions.set(i, { { stats->awb_stats[i].r_sum,\n> +                                                 stats->awb_stats[i].g_sum,\n> +                                                 stats->awb_stats[i].b_sum },\n> +                                               stats->awb_stats[i].counted,\n> +                                               stats->awb_stats[i].notcounted });\n> +\n> +       /*\n> +        * There are only ever 15 regions computed by the firmware due to zoning,\n> +        * but the HW defines AGC_REGIONS == 16!\n> +        */\n> +       statistics->agcRegions.init(15);\n> +       for (i = 0; i < statistics->agcRegions.numRegions(); i++)\n> +               statistics->agcRegions.set(i, { { stats->agc_stats[i].r_sum,\n> +                                                 stats->agc_stats[i].g_sum,\n> +                                                 stats->agc_stats[i].b_sum },\n> +                                               stats->agc_stats[i].counted,\n> +                                               stats->awb_stats[i].notcounted });\n> +\n> +       statistics->focusRegions.init({ 4, 3 });\n> +       for (i = 0; i < statistics->focusRegions.numRegions(); i++)\n> +               statistics->focusRegions.set(i, { stats->focus_stats[i].contrast_val[1][1] / 1000,\n> +                                                 stats->focus_stats[i].contrast_val_num[1][1],\n> +                                                 stats->focus_stats[i].contrast_val_num[1][0] });\n> +\n> +       return statistics;\n> +}\n> +\n>  void IPARPi::processStats(unsigned int bufferId)\n>  {\n>         auto it = buffers_.find(bufferId);\n> @@ -1129,7 +1171,7 @@ void IPARPi::processStats(unsigned int bufferId)\n>\n>         Span<uint8_t> mem = it->second.planes()[0];\n>         bcm2835_isp_stats *stats = reinterpret_cast<bcm2835_isp_stats *>(mem.data());\n> -       RPiController::StatisticsPtr statistics = std::make_shared<bcm2835_isp_stats>(*stats);\n> +       RPiController::StatisticsPtr statistics = fillStatistics(stats);\n>         helper_->process(statistics, rpiMetadata_);\n>         controller_.process(statistics, &rpiMetadata_);\n>\n> diff --git a/src/ipa/raspberrypi/statistics.h b/src/ipa/raspberrypi/statistics.h\n> index a762bf3d41aa..affb7272c963 100644\n> --- a/src/ipa/raspberrypi/statistics.h\n> +++ b/src/ipa/raspberrypi/statistics.h\n> @@ -67,4 +67,6 @@ struct Statistics {\n>         FocusRegions focusRegions;\n>  };\n>\n> +using StatisticsPtr = std::shared_ptr<Statistics>;\n> +\n>  } /* namespace RPiController */\n> --\n> 2.25.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0560ABDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 24 Nov 2022 12:54:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 777176331A;\n\tThu, 24 Nov 2022 13:54:40 +0100 (CET)","from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com\n\t[IPv6:2607:f8b0:4864:20::1030])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE4E2632EA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Nov 2022 13:54:38 +0100 (CET)","by mail-pj1-x1030.google.com with SMTP id\n\t3-20020a17090a098300b00219041dcbe9so401495pjo.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Nov 2022 04:54:38 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669294480;\n\tbh=UxkgME4kuEmN/JvJVKayMMzvcMm6Y1n4+TG4lkyauxg=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=F0deonD0dMWqqZqxc50IzBOfuQThi+GlG8izxJ95Tdko/5THfQU76n6fPHV/olCD5\n\taynsyBETNiboG5nsITdCaMV2WLdEr2kFoklfB1LyADYSTsmmVeVPYcVSk0jwA76J+D\n\tfzUt7WZ3KMYHEFWOO/c6HTgkhLHhVKsQa6kbnvBXKJbpSxux1E/51aKT2SwWuWrVKw\n\tMkwdZd0anqOGY1IeL4bLqSrXmYpIx3yX8EmzAS4hRCOXlv4o1BdjNLlKpNp/mFjNkR\n\tJAYAu7LwxZaNXUw56mwkwrcXoUJkqsCENmc1V6HpKI419ZhkkE6SQoBKtlb0SjcIEW\n\tF5uQWUJ8XZlUA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=P1WPjwfZTlsfiBMdwhJ2FkHx+oNT1xgrArCUys2Uwj0=;\n\tb=ZdFkdGbzYLXAN/LfpcDc0zviTWTgD/IeM54+BImXyx39VwUP3NT10GzrWchx5nCnHP\n\tzW7Y6Jmg8wx/NBGMb/shok4OTVBe56FIl1LiWcE0EmoX98FpX5+jfKzV5tRaFov/m/2U\n\tj95Ky3wvofDjTYDU4O9tYZpdl8DsFALjy5uUi3vsFh4THnKDkoggKtuDnLvosmiaNcsG\n\tkk7BQp5Dy49y1tfndKagYIQD6Sbs4gfzvI8aWDoZu8cZ8t0jrwyubDqBQrm1cgR5LBNy\n\th9ghyQIQIxPH/zUJ6DXYd3uuQKeLL4fYgYjHVSxbl0GCO+ozqjYQ+dgnJe5iJLbO53Hl\n\tYJoA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"ZdFkdGbz\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=P1WPjwfZTlsfiBMdwhJ2FkHx+oNT1xgrArCUys2Uwj0=;\n\tb=hWdgFHWPZXQbJoiHyySWhRw10dR5NzcbC6Nc0dQ3E4pV+2gQu5LXN39hLWPHB681pl\n\tT1GIVRIDZ/4n3EjXhCOuRKNZVz5iA7kDMfRlKzGQwTVInyLS4nRCp3C3c/LUUelFpOhs\n\tb+mMzAUHwgK874YWAWcN8PH3gfb6RUT+WJ5p0jxTpI3FwkCKcr6XOaFp0kHRa9P0F6JI\n\tu+H3kCNfOIoUn2uuQ2BILUmLo6qKs4LKZP3uTfKkQ7oPaTWCnwRQ9xb76Z9twphK52I0\n\tuVPwvtfCvhMWxrig7vHvRROoAQ1I9AaIhopIbdqKrp/MB1TiwjihB/4M5T2fHkWy61H8\n\t2sfw==","X-Gm-Message-State":"ANoB5pkOs37DCRM82r1MjkfCvKImkVcQI1V3H8r2EMLOLKhhnsJsAWGl\n\t+GxYuImXLo/zW4BA02Duj32q+NLBfAupFKARmiSS40FM23s=","X-Google-Smtp-Source":"AA0mqf5qfYwrvp7xHnoig4KV83jsv44HZiNoEgPA4h1OiZaXY121fau9NdNLsVCkKYxqgGF7s5diuPgaSq8IYMTcIrM=","X-Received":"by 2002:a17:902:7d93:b0:186:9cf4:e53b with SMTP id\n\ta19-20020a1709027d9300b001869cf4e53bmr17644739plm.50.1669294477126;\n\tThu, 24 Nov 2022 04:54:37 -0800 (PST)","MIME-Version":"1.0","References":"<20221124103832.6172-1-naush@raspberrypi.com>\n\t<20221124103832.6172-5-naush@raspberrypi.com>","In-Reply-To":"<20221124103832.6172-5-naush@raspberrypi.com>","Date":"Thu, 24 Nov 2022 12:54:26 +0000","Message-ID":"<CAHW6GYKnapRdE-DfOc5SZsVJfAsDNuMQvSW3NpjR2T9DWfksZQ@mail.gmail.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 4/5] ipa: raspberrypi: Use the\n\tgeneric statistics structure in the algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]