[libcamera-devel] [PATCH 07/12] ipa: ipu3: awb: Correct the relevant zones proportion
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Sep 29 14:36:19 CEST 2021
Hi Jean-Michel,
On Wed, Sep 29, 2021 at 02:32:00PM +0200, Jean-Michel Hautbois wrote:
> On 29/09/2021 14:15, Laurent Pinchart wrote:
> > On Thu, Sep 23, 2021 at 10:16:20AM +0200, Jean-Michel Hautbois wrote:
> >> The algorithm uses the statistics of a cell only if there is not too
> >> much saturated pixels in it. The grey world algorithm works fine when
> >> there are a limited number of outliers.
> >> Consider a valid zone to be at least 80% of unsaturated cells in it.
> >> This value could very well be configurable, and make the algorithm more
> >> or less tolerant.
> >>
> >> While at it, implement it in a configure() call as it will not change
> >> during execution, and cache the cellsPerZone values.
> >>
> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> >> ---
> >> src/ipa/ipu3/algorithms/awb.cpp | 32 +++++++++++++++++++++++---------
> >> src/ipa/ipu3/algorithms/awb.h | 5 +++++
> >> 2 files changed, 28 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> >> index 51a38d05..800d023c 100644
> >> --- a/src/ipa/ipu3/algorithms/awb.cpp
> >> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> >> @@ -17,7 +17,6 @@ namespace ipa::ipu3::algorithms {
> >>
> >> LOG_DEFINE_CATEGORY(IPU3Awb)
> >>
> >> -static constexpr uint32_t kMinZonesCounted = 16;
> >> static constexpr uint32_t kMinGreenLevelInZone = 32;
> >>
> >> /**
> >> @@ -169,6 +168,24 @@ Awb::Awb()
> >>
> >> Awb::~Awb() = default;
> >>
> >> +int Awb::configure(IPAContext &context,
> >> + [[maybe_unused]] const IPAConfigInfo &configInfo)
> >> +{
> >> + const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
> >> +
> >> + cellsPerZoneX_ = round(grid.width / static_cast<double>(kAwbStatsSizeX));
> >
> > s/round/std::round/
> >
> > as you're using cmath (with a small corresponding comment in the commit
> > message).
>
> OK :-).
>
> >> + cellsPerZoneY_ = round(grid.height / static_cast<double>(kAwbStatsSizeY));
> >> +
> >> + /*
> >> + * Configure the minimum proportion of cells counted within a zone
> >> + * for it to be relevant for the grey world algorithm.
> >> + * \todo This proportion could be configured.
> >> + */
> >> + cellsPerZoneThreshold_ = (cellsPerZoneX_ * cellsPerZoneY_) * 80 / 100;
> >
> > No need for parentheses.
> >
> > The patch looks fine to me, so
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > This however led me to notice that if the grid width isn't a multiple of
> > 16, we'll ignore some of the cells. If the grid width is 79, for
> > instance, 15 cells will be ignored. Would it be worth fixing that ?
>
> We could still do :
> cellsPerZoneX_ = std::round(stride_ / static_cast<double>(kAwbStatsSizeX));
>
> But we haven't included stride yet :-)
I think the best solution for this issue may be a variable number of
cells per zone. The question still holds though, is this an issue that
needs to be addressed, or is it harmless in practice ?
> >> +
> >> + return 0;
> >> +}
> >> +
> >> /**
> >> * The function estimates the correlated color temperature using
> >> * from RGB color space input.
> >> @@ -205,7 +222,7 @@ void Awb::generateZones(std::vector<RGB> &zones)
> >> for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {
> >> RGB zone;
> >> double counted = awbStats_[i].counted;
> >> - if (counted >= kMinZonesCounted) {
> >> + if (counted >= cellsPerZoneThreshold_) {
> >> zone.G = awbStats_[i].sum.green / counted;
> >> if (zone.G >= kMinGreenLevelInZone) {
> >> zone.R = awbStats_[i].sum.red / counted;
> >> @@ -220,19 +237,16 @@ void Awb::generateZones(std::vector<RGB> &zones)
> >> void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats,
> >> const ipu3_uapi_grid_config &grid)
> >> {
> >> - uint32_t cellsPerZoneX = round(grid.width / static_cast<double>(kAwbStatsSizeX));
> >> - uint32_t cellsPerZoneY = round(grid.height / static_cast<double>(kAwbStatsSizeY));
> >> -
> >> /*
> >> * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
> >> * (grid.width x grid.height).
> >> */
> >> - for (unsigned int cellY = 0; cellY < kAwbStatsSizeY * cellsPerZoneY; cellY++) {
> >> - for (unsigned int cellX = 0; cellX < kAwbStatsSizeX * cellsPerZoneX; cellX++) {
> >> + for (unsigned int cellY = 0; cellY < kAwbStatsSizeY * cellsPerZoneY_; cellY++) {
> >> + for (unsigned int cellX = 0; cellX < kAwbStatsSizeX * cellsPerZoneX_; cellX++) {
> >> uint32_t cellPosition = (cellY * grid.width + cellX)
> >> * sizeof(Ipu3AwbCell);
> >> - uint32_t zoneX = cellX / cellsPerZoneX;
> >> - uint32_t zoneY = cellY / cellsPerZoneY;
> >> + uint32_t zoneX = cellX / cellsPerZoneX_;
> >> + uint32_t zoneY = cellY / cellsPerZoneY_;
> >>
> >> uint32_t awbZonePosition = zoneY * kAwbStatsSizeX + zoneX;
> >>
> >> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> >> index 3385ebe7..681d8c2b 100644
> >> --- a/src/ipa/ipu3/algorithms/awb.h
> >> +++ b/src/ipa/ipu3/algorithms/awb.h
> >> @@ -48,6 +48,7 @@ public:
> >> Awb();
> >> ~Awb();
> >>
> >> + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> >> void prepare(IPAContext &context, ipu3_uapi_params *params) override;
> >> void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> >>
> >> @@ -85,6 +86,10 @@ private:
> >> std::vector<RGB> zones_;
> >> Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
> >> AwbStatus asyncResults_;
> >> +
> >> + uint32_t cellsPerZoneX_;
> >> + uint32_t cellsPerZoneY_;
> >> + uint32_t cellsPerZoneThreshold_;
> >> };
> >>
> >> } /* namespace ipa::ipu3::algorithms */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list