Message ID | 20221122112224.31691-2-naush@raspberrypi.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Naush Thanks for this patch. On Tue, 22 Nov 2022 at 11:22, 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. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> All looks good to me. Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks! David > --- > src/ipa/raspberrypi/controller/region_stats.h | 109 ++++++++++++++++++ > src/ipa/raspberrypi/statistics.h | 70 +++++++++++ > 2 files changed, 179 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..8a3453617654 > --- /dev/null > +++ b/src/ipa/raspberrypi/controller/region_stats.h > @@ -0,0 +1,109 @@ > +/* 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> > + > +namespace RPiController { > + > +template<typename T> > +class RegionStats > +{ > +public: > + RegionStats() > + : numRows_(0), numCols_(0), numFloating_(0) > + { > + } > + > + void init(unsigned int numRows, unsigned int numCols = 1, unsigned int numFloating = 0) > + { > + numRows_ = numRows; > + numCols_ = numCols; > + numFloating_ = numFloating; > + regions_.resize(numRows_ * numCols_ + numFloating_); > + } > + > + unsigned int numRegions() const > + { > + return numRows_ * numCols_; > + } > + > + unsigned int numFloatingRegions() const > + { > + return numFloating_; > + } > + > + void set(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted) > + { > + if (index >= numRegions()) > + return; > + set_(index, value, counted, uncounted); > + } > + > + void set(unsigned int row, unsigned int col, const T &value, uint32_t counted, uint32_t uncounted) > + { > + set(row * numCols_ + col, value, counted, uncounted); > + } > + > + void setFloating(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted) > + { > + if (index >= numFloatingRegions()) > + return; > + set(numRegions() + index, value, counted, uncounted); > + } > + > + const T &get(unsigned int index, uint32_t &counted, uint32_t &uncounted) const > + { > + counted = uncounted = 0; > + if (index >= numRegions()) > + return default_; > + return get_(index, counted, uncounted); > + } > + > + const T &get(unsigned int row, unsigned int col, uint32_t &counted, uint32_t &uncounted) const > + { > + return get(row * numCols_ + col, counted, uncounted); > + } > + > + const T &getFloating(unsigned int index, uint32_t &counted, uint32_t &uncounted) const > + { > + counted = uncounted = 0; > + if (index >= numFloatingRegions()) > + return default_; > + return get_(numRegions() + index, counted, uncounted); > + } > + > +private: > + void set_(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted) > + { > + regions_[index] = { value, counted, uncounted }; > + } > + > + const T &get_(unsigned int index, uint32_t &counted, uint32_t &uncounted) const > + { > + counted = regions_[index].counted; > + uncounted = regions_[index].uncounted; > + return regions_[index].value; > + } > + > + unsigned int numRows_; > + unsigned int numCols_; > + unsigned int numFloating_; > + > + struct Region { > + T value; > + uint32_t counted; > + uint32_t uncounted; > + }; > + > + std::vector<Region> regions_; > + T 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 >
diff --git a/src/ipa/raspberrypi/controller/region_stats.h b/src/ipa/raspberrypi/controller/region_stats.h new file mode 100644 index 000000000000..8a3453617654 --- /dev/null +++ b/src/ipa/raspberrypi/controller/region_stats.h @@ -0,0 +1,109 @@ +/* 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> + +namespace RPiController { + +template<typename T> +class RegionStats +{ +public: + RegionStats() + : numRows_(0), numCols_(0), numFloating_(0) + { + } + + void init(unsigned int numRows, unsigned int numCols = 1, unsigned int numFloating = 0) + { + numRows_ = numRows; + numCols_ = numCols; + numFloating_ = numFloating; + regions_.resize(numRows_ * numCols_ + numFloating_); + } + + unsigned int numRegions() const + { + return numRows_ * numCols_; + } + + unsigned int numFloatingRegions() const + { + return numFloating_; + } + + void set(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted) + { + if (index >= numRegions()) + return; + set_(index, value, counted, uncounted); + } + + void set(unsigned int row, unsigned int col, const T &value, uint32_t counted, uint32_t uncounted) + { + set(row * numCols_ + col, value, counted, uncounted); + } + + void setFloating(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted) + { + if (index >= numFloatingRegions()) + return; + set(numRegions() + index, value, counted, uncounted); + } + + const T &get(unsigned int index, uint32_t &counted, uint32_t &uncounted) const + { + counted = uncounted = 0; + if (index >= numRegions()) + return default_; + return get_(index, counted, uncounted); + } + + const T &get(unsigned int row, unsigned int col, uint32_t &counted, uint32_t &uncounted) const + { + return get(row * numCols_ + col, counted, uncounted); + } + + const T &getFloating(unsigned int index, uint32_t &counted, uint32_t &uncounted) const + { + counted = uncounted = 0; + if (index >= numFloatingRegions()) + return default_; + return get_(numRegions() + index, counted, uncounted); + } + +private: + void set_(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted) + { + regions_[index] = { value, counted, uncounted }; + } + + const T &get_(unsigned int index, uint32_t &counted, uint32_t &uncounted) const + { + counted = regions_[index].counted; + uncounted = regions_[index].uncounted; + return regions_[index].value; + } + + unsigned int numRows_; + unsigned int numCols_; + unsigned int numFloating_; + + struct Region { + T value; + uint32_t counted; + uint32_t uncounted; + }; + + std::vector<Region> regions_; + T 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 */
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 | 109 ++++++++++++++++++ src/ipa/raspberrypi/statistics.h | 70 +++++++++++ 2 files changed, 179 insertions(+) create mode 100644 src/ipa/raspberrypi/controller/region_stats.h create mode 100644 src/ipa/raspberrypi/statistics.h