Message ID | 20221124103832.6172-2-naush@raspberrypi.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
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 ®ion) > + { > + if (index >= numRegions()) > + return; > + set_(index, region); > + } > + > + void set(const libcamera::Point &pos, const Region ®ion) > + { > + set(pos.y * size_.width + pos.x, region); > + } > + > + void setFloating(unsigned int index, const Region ®ion) > + { > + 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 ®ion) > + { > + 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 >
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 ®ion) + { + if (index >= numRegions()) + return; + set_(index, region); + } + + void set(const libcamera::Point &pos, const Region ®ion) + { + set(pos.y * size_.width + pos.x, region); + } + + void setFloating(unsigned int index, const Region ®ion) + { + 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 ®ion) + { + 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 */
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