[libcamera-devel,v2,1/5] ipa: raspberrypi: Generalise statistics

17862 diff mbox series

At present, the controller algorithms access the bcm2835_isp_stats structure,
which is hardware specific. It would be desirable to abstract out the statistics
structure to remove hardware specific headers from the algorithms source files.

Define a new templated RegionStats class that encompasses region based
statistics generated by the ISP. For the VC4 ISP, this can be used to hold
RGB sums and focus FoM values.

Define a new Statistics structure that holds all the VC4 ISP statistics output.
This includes AGC hostograms, AGC/AWB region sums and focus FoM regions.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/region_stats.h | 123 ++++++++++++++++++
 src/ipa/raspberrypi/statistics.h              |  70 ++++++++++
 2 files changed, 193 insertions(+)
 create mode 100644 src/ipa/raspberrypi/controller/region_stats.h
 create mode 100644 src/ipa/raspberrypi/statistics.h

Hi Naush

Thanks for the update!

On Thu, 24 Nov 2022 at 10:38, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> At present, the controller algorithms access the bcm2835_isp_stats structure,
> which is hardware specific. It would be desirable to abstract out the statistics
> structure to remove hardware specific headers from the algorithms source files.
>
> Define a new templated RegionStats class that encompasses region based
> statistics generated by the ISP. For the VC4 ISP, this can be used to hold
> RGB sums and focus FoM values.
>
> Define a new Statistics structure that holds all the VC4 ISP statistics output.
> This includes AGC hostograms, AGC/AWB region sums and focus FoM regions.

s/hostograms/histograms/

>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

I think I was happy with this before, and I'm still happy with the new
version (typo notwithstanding)!

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks
David

David

> ---
>  src/ipa/raspberrypi/controller/region_stats.h | 123 ++++++++++++++++++
>  src/ipa/raspberrypi/statistics.h              |  70 ++++++++++
>  2 files changed, 193 insertions(+)
>  create mode 100644 src/ipa/raspberrypi/controller/region_stats.h
>  create mode 100644 src/ipa/raspberrypi/statistics.h
>
> diff --git a/src/ipa/raspberrypi/controller/region_stats.h b/src/ipa/raspberrypi/controller/region_stats.h
> new file mode 100644
> index 000000000000..9aaf3a58a6f7
> --- /dev/null
> +++ b/src/ipa/raspberrypi/controller/region_stats.h
> @@ -0,0 +1,123 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2022, Raspberry Pi Ltd
> + *
> + * region_stats.h - Raspberry Pi region based statistics container
> + */
> +#pragma once
> +
> +#include <array>
> +#include <stdint.h>
> +#include <vector>
> +
> +#include <libcamera/geometry.h>
> +
> +namespace RPiController {
> +
> +template<typename T>
> +class RegionStats
> +{
> +public:
> +       struct Region {
> +               T val;
> +               uint32_t counted;
> +               uint32_t uncounted;
> +       };
> +
> +       RegionStats()
> +               : size_({}), numFloating_(0), default_({})
> +       {
> +       }
> +
> +       void init(const libcamera::Size &size, unsigned int numFloating = 0)
> +       {
> +               size_ = size;
> +               numFloating_ = numFloating;
> +               regions_.clear();
> +               regions_.resize(size_.width * size_.height + numFloating_);
> +       }
> +
> +       void init(unsigned int num)
> +       {
> +               size_ = libcamera::Size(num, 1);
> +               numFloating_ = 0;
> +               regions_.clear();
> +               regions_.resize(num);
> +       }
> +
> +       unsigned int numRegions() const
> +       {
> +               return size_.width * size_.height;
> +       }
> +
> +       unsigned int numFloatingRegions() const
> +       {
> +               return numFloating_;
> +       }
> +
> +       libcamera::Size size() const
> +       {
> +               return size_;
> +       }
> +
> +       void set(unsigned int index, const Region &region)
> +       {
> +               if (index >= numRegions())
> +                       return;
> +               set_(index, region);
> +       }
> +
> +       void set(const libcamera::Point &pos, const Region &region)
> +       {
> +               set(pos.y * size_.width + pos.x, region);
> +       }
> +
> +       void setFloating(unsigned int index, const Region &region)
> +       {
> +               if (index >= numFloatingRegions())
> +                       return;
> +               set(numRegions() + index, region);
> +       }
> +
> +       const Region &get(unsigned int index) const
> +       {
> +               if (index >= numRegions())
> +                       return default_;
> +               return get_(index);
> +       }
> +
> +       const T &get(const libcamera::Point &pos) const
> +       {
> +               return get(pos.y * size_.width + pos.x);
> +       }
> +
> +       const T &getFloating(unsigned int index) const
> +       {
> +               if (index >= numFloatingRegions())
> +                       return default_;
> +               return get_(numRegions() + index);
> +       }
> +
> +       typename std::vector<Region>::iterator begin() { return regions_.begin(); }
> +       typename std::vector<Region>::iterator end() { return regions_.end(); }
> +       typename std::vector<Region>::const_iterator begin() const { return regions_.begin(); }
> +       typename std::vector<Region>::const_iterator end() const { return regions_.end(); }
> +
> +private:
> +       void set_(unsigned int index, const Region &region)
> +       {
> +               regions_[index] = region;
> +       }
> +
> +       const Region &get_(unsigned int index) const
> +       {
> +               return regions_[index];
> +       }
> +
> +       libcamera::Size size_;
> +       unsigned int numFloating_;
> +       std::vector<Region> regions_;
> +       Region default_;
> +};
> +
> +} /* namespace RPiController */
> diff --git a/src/ipa/raspberrypi/statistics.h b/src/ipa/raspberrypi/statistics.h
> new file mode 100644
> index 000000000000..a762bf3d41aa
> --- /dev/null
> +++ b/src/ipa/raspberrypi/statistics.h
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2022, Raspberry Pi Ltd
> + *
> + * statistics.h - Raspberry Pi generic statistics structure
> + */
> +#pragma once
> +
> +#include <memory>
> +#include <stdint.h>
> +#include <vector>
> +
> +#include "histogram.h"
> +#include "region_stats.h"
> +
> +namespace RPiController {
> +
> +struct RgbySums {
> +       RgbySums(uint64_t _rSum = 0, uint64_t _gSum = 0, uint64_t _bSum = 0, uint64_t _ySum = 0)
> +               : rSum(_rSum), gSum(_gSum), bSum(_bSum), ySum(_ySum)
> +       {
> +       }
> +       uint64_t rSum;
> +       uint64_t gSum;
> +       uint64_t bSum;
> +       uint64_t ySum;
> +};
> +
> +using RgbyRegions = RegionStats<RgbySums>;
> +using FocusRegions = RegionStats<uint64_t>;
> +
> +struct Statistics {
> +       /*
> +        * Positioning of the AGC statistics gathering in the pipeline:
> +        * Pre-WB correction or post-WB correction.
> +        * Assume this is post-LSC.
> +        */
> +       enum class AgcStatsPos { PreWb, PostWb };
> +       const AgcStatsPos agcStatsPos;
> +
> +       /*
> +        * Positioning of the AWB/ALSC statistics gathering in the pipeline:
> +        * Pre-LSC or post-LSC.
> +        */
> +       enum class ColourStatsPos { PreLsc, PostLsc };
> +       const ColourStatsPos colourStatsPos;
> +
> +       Statistics(AgcStatsPos a, ColourStatsPos c)
> +               : agcStatsPos(a), colourStatsPos(c)
> +       {
> +       }
> +
> +       /* Histogram statistics. Not all histograms may be populated! */
> +       Histogram rHist;
> +       Histogram gHist;
> +       Histogram bHist;
> +       Histogram yHist;
> +
> +       /* Row sums for flicker avoidance. */
> +       std::vector<RgbySums> rowSums;
> +
> +       /* Region based colour sums. */
> +       RgbyRegions agcRegions;
> +       RgbyRegions awbRegions;
> +
> +       /* Region based focus FoM. */
> +       FocusRegions focusRegions;
> +};
> +
> +} /* namespace RPiController */
> --
> 2.25.1
>