| Message ID | 20230426131057.21550-10-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush On Wed, Apr 26, 2023 at 02:10:53PM +0100, Naushir Patuck via libcamera-devel wrote: > From: David Plowman <david.plowman@raspberrypi.com> > > The region weights for the the AGC zones were previously handled by the > AGC algorithm. Now it's the IPA (vc4.cpp) that applies them directly > to the statistics that we pass to the AGC. > > Signed-off-by: David Plowman <david.plowman@raspberrypi> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > --- > src/ipa/rpi/controller/agc_algorithm.h | 3 +++ > src/ipa/rpi/controller/rpi/agc.cpp | 27 +++++++++++++++++--------- > src/ipa/rpi/controller/rpi/agc.h | 1 + > src/ipa/rpi/vc4/vc4.cpp | 26 ++++++++++++++++++------- > 4 files changed, 41 insertions(+), 16 deletions(-) > > diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h > index 36e6c11058ee..b6949daa7135 100644 > --- a/src/ipa/rpi/controller/agc_algorithm.h > +++ b/src/ipa/rpi/controller/agc_algorithm.h > @@ -6,6 +6,8 @@ > */ > #pragma once > > +#include <vector> > + > #include <libcamera/base/utils.h> > > #include "algorithm.h" > @@ -18,6 +20,7 @@ public: > AgcAlgorithm(Controller *controller) : Algorithm(controller) {} > /* An AGC algorithm must provide the following: */ > virtual unsigned int getConvergenceFrames() const = 0; > + virtual std::vector<double> const &getWeights() const = 0; > virtual void setEv(double ev) = 0; > virtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0; > virtual void setFixedShutter(libcamera::utils::Duration fixedShutter) = 0; > diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp > index e6fb7b8dbeb3..e79c82e2e65b 100644 > --- a/src/ipa/rpi/controller/rpi/agc.cpp > +++ b/src/ipa/rpi/controller/rpi/agc.cpp > @@ -292,6 +292,18 @@ unsigned int Agc::getConvergenceFrames() const > return config_.convergenceFrames; > } > > +std::vector<double> const &Agc::getWeights() const > +{ > + /* > + * In case someone calls setMeteringMode and then this before the > + * algorithm has run and updated the meteringMode_ pointer. > + */ > + auto it = config_.meteringModes.find(meteringModeName_); > + if (it == config_.meteringModes.end()) > + return meteringMode_->weights; > + return it->second.weights; > +} > + > void Agc::setEv(double ev) > { > ev_ = ev; > @@ -595,19 +607,16 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb, > ASSERT(weights.size() == stats->agcRegions.numRegions()); > > /* > - * Note how the calculation below means that equal weights give you > - * "average" metering (i.e. all pixels equally important). > + * Note that the weights are applied by the IPA to the statistics directly, > + * before they are given to us here. > */ > double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0; > for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) { > auto ®ion = stats->agcRegions.get(i); > - double rAcc = std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted); > - double gAcc = std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted); > - double bAcc = std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted); > - rSum += rAcc * weights[i]; > - gSum += gAcc * weights[i]; > - bSum += bAcc * weights[i]; > - pixelSum += region.counted * weights[i]; > + rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted); > + gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted); > + bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted); > + pixelSum += region.counted; > } > if (pixelSum == 0.0) { > LOG(RPiAgc, Warning) << "computeInitialY: pixelSum is zero"; > diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h > index 4e5f272fac78..939f97295a02 100644 > --- a/src/ipa/rpi/controller/rpi/agc.h > +++ b/src/ipa/rpi/controller/rpi/agc.h > @@ -69,6 +69,7 @@ public: > char const *name() const override; > int read(const libcamera::YamlObject ¶ms) override; > unsigned int getConvergenceFrames() const override; > + std::vector<double> const &getWeights() const override; > void setEv(double ev) override; > void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override; > void setMaxShutter(libcamera::utils::Duration maxShutter) override; > diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp > index 0d929cda6c4a..0e022c2aeed3 100644 > --- a/src/ipa/rpi/vc4/vc4.cpp > +++ b/src/ipa/rpi/vc4/vc4.cpp > @@ -211,13 +211,25 @@ RPiController::StatisticsPtr IpaVc4::platformProcessStats(Span<uint8_t> mem) > stats->awb_stats[i].counted, > stats->awb_stats[i].notcounted }); > > - statistics->agcRegions.init(hw.agcRegions); > - for (i = 0; i < statistics->agcRegions.numRegions(); i++) > - statistics->agcRegions.set(i, { { stats->agc_stats[i].r_sum << scale, > - stats->agc_stats[i].g_sum << scale, > - stats->agc_stats[i].b_sum << scale }, > - stats->agc_stats[i].counted, > - stats->awb_stats[i].notcounted }); > + RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>( > + controller_.getAlgorithm("agc")); > + if (!agc) { > + LOG(IPARPI, Debug) << "No AGC algorithm - not copying statistics"; > + statistics->agcRegions.init(0); > + } else { > + statistics->agcRegions.init(hw.agcRegions); > + const std::vector<double> &weights = agc->getWeights(); > + for (i = 0; i < statistics->agcRegions.numRegions(); i++) { > + uint64_t rSum = (stats->agc_stats[i].r_sum << scale) * weights[i]; > + uint64_t gSum = (stats->agc_stats[i].g_sum << scale) * weights[i]; > + uint64_t bSum = (stats->agc_stats[i].b_sum << scale) * weights[i]; > + uint32_t counted = stats->agc_stats[i].counted * weights[i]; > + uint32_t notcounted = stats->agc_stats[i].notcounted * weights[i]; > + statistics->agcRegions.set(i, { { rSum, gSum, bSum }, > + counted, > + notcounted }); > + } > + } > > statistics->focusRegions.init(hw.focusRegions); > for (i = 0; i < statistics->focusRegions.numRegions(); i++) > -- > 2.34.1 >
Hi Naush and David, Thank you for the patch. On Wed, Apr 26, 2023 at 02:10:53PM +0100, Naushir Patuck via libcamera-devel wrote: > From: David Plowman <david.plowman@raspberrypi.com> > > The region weights for the the AGC zones were previously handled by the > AGC algorithm. Now it's the IPA (vc4.cpp) that applies them directly > to the statistics that we pass to the AGC. It doesn't need to be changed in this patch, but for future patches, please use the imperative style, and use the present tense to describe the current situation: ---- The region weights for the the AGC zones are handled by the AGC algorithm. Apply them directly in the IPA (vc4.cpp) to the statistics that we pass to the AGC. ---- If you end up submitting a new version of the series for unrelated reason you may want to update this commit message. > Signed-off-by: David Plowman <david.plowman@raspberrypi> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/controller/agc_algorithm.h | 3 +++ > src/ipa/rpi/controller/rpi/agc.cpp | 27 +++++++++++++++++--------- > src/ipa/rpi/controller/rpi/agc.h | 1 + > src/ipa/rpi/vc4/vc4.cpp | 26 ++++++++++++++++++------- > 4 files changed, 41 insertions(+), 16 deletions(-) > > diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h > index 36e6c11058ee..b6949daa7135 100644 > --- a/src/ipa/rpi/controller/agc_algorithm.h > +++ b/src/ipa/rpi/controller/agc_algorithm.h > @@ -6,6 +6,8 @@ > */ > #pragma once > > +#include <vector> > + > #include <libcamera/base/utils.h> > > #include "algorithm.h" > @@ -18,6 +20,7 @@ public: > AgcAlgorithm(Controller *controller) : Algorithm(controller) {} > /* An AGC algorithm must provide the following: */ > virtual unsigned int getConvergenceFrames() const = 0; > + virtual std::vector<double> const &getWeights() const = 0; > virtual void setEv(double ev) = 0; > virtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0; > virtual void setFixedShutter(libcamera::utils::Duration fixedShutter) = 0; > diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp > index e6fb7b8dbeb3..e79c82e2e65b 100644 > --- a/src/ipa/rpi/controller/rpi/agc.cpp > +++ b/src/ipa/rpi/controller/rpi/agc.cpp > @@ -292,6 +292,18 @@ unsigned int Agc::getConvergenceFrames() const > return config_.convergenceFrames; > } > > +std::vector<double> const &Agc::getWeights() const > +{ > + /* > + * In case someone calls setMeteringMode and then this before the > + * algorithm has run and updated the meteringMode_ pointer. > + */ > + auto it = config_.meteringModes.find(meteringModeName_); > + if (it == config_.meteringModes.end()) > + return meteringMode_->weights; > + return it->second.weights; > +} > + > void Agc::setEv(double ev) > { > ev_ = ev; > @@ -595,19 +607,16 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb, > ASSERT(weights.size() == stats->agcRegions.numRegions()); > > /* > - * Note how the calculation below means that equal weights give you > - * "average" metering (i.e. all pixels equally important). > + * Note that the weights are applied by the IPA to the statistics directly, > + * before they are given to us here. > */ > double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0; > for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) { > auto ®ion = stats->agcRegions.get(i); > - double rAcc = std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted); > - double gAcc = std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted); > - double bAcc = std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted); > - rSum += rAcc * weights[i]; > - gSum += gAcc * weights[i]; > - bSum += bAcc * weights[i]; > - pixelSum += region.counted * weights[i]; > + rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted); > + gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted); > + bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted); > + pixelSum += region.counted; > } > if (pixelSum == 0.0) { > LOG(RPiAgc, Warning) << "computeInitialY: pixelSum is zero"; > diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h > index 4e5f272fac78..939f97295a02 100644 > --- a/src/ipa/rpi/controller/rpi/agc.h > +++ b/src/ipa/rpi/controller/rpi/agc.h > @@ -69,6 +69,7 @@ public: > char const *name() const override; > int read(const libcamera::YamlObject ¶ms) override; > unsigned int getConvergenceFrames() const override; > + std::vector<double> const &getWeights() const override; > void setEv(double ev) override; > void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override; > void setMaxShutter(libcamera::utils::Duration maxShutter) override; > diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp > index 0d929cda6c4a..0e022c2aeed3 100644 > --- a/src/ipa/rpi/vc4/vc4.cpp > +++ b/src/ipa/rpi/vc4/vc4.cpp > @@ -211,13 +211,25 @@ RPiController::StatisticsPtr IpaVc4::platformProcessStats(Span<uint8_t> mem) > stats->awb_stats[i].counted, > stats->awb_stats[i].notcounted }); > > - statistics->agcRegions.init(hw.agcRegions); > - for (i = 0; i < statistics->agcRegions.numRegions(); i++) > - statistics->agcRegions.set(i, { { stats->agc_stats[i].r_sum << scale, > - stats->agc_stats[i].g_sum << scale, > - stats->agc_stats[i].b_sum << scale }, > - stats->agc_stats[i].counted, > - stats->awb_stats[i].notcounted }); > + RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>( > + controller_.getAlgorithm("agc")); > + if (!agc) { > + LOG(IPARPI, Debug) << "No AGC algorithm - not copying statistics"; > + statistics->agcRegions.init(0); > + } else { > + statistics->agcRegions.init(hw.agcRegions); > + const std::vector<double> &weights = agc->getWeights(); > + for (i = 0; i < statistics->agcRegions.numRegions(); i++) { > + uint64_t rSum = (stats->agc_stats[i].r_sum << scale) * weights[i]; > + uint64_t gSum = (stats->agc_stats[i].g_sum << scale) * weights[i]; > + uint64_t bSum = (stats->agc_stats[i].b_sum << scale) * weights[i]; > + uint32_t counted = stats->agc_stats[i].counted * weights[i]; > + uint32_t notcounted = stats->agc_stats[i].notcounted * weights[i]; > + statistics->agcRegions.set(i, { { rSum, gSum, bSum }, > + counted, > + notcounted }); > + } > + } > > statistics->focusRegions.init(hw.focusRegions); > for (i = 0; i < statistics->focusRegions.numRegions(); i++)
diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h index 36e6c11058ee..b6949daa7135 100644 --- a/src/ipa/rpi/controller/agc_algorithm.h +++ b/src/ipa/rpi/controller/agc_algorithm.h @@ -6,6 +6,8 @@ */ #pragma once +#include <vector> + #include <libcamera/base/utils.h> #include "algorithm.h" @@ -18,6 +20,7 @@ public: AgcAlgorithm(Controller *controller) : Algorithm(controller) {} /* An AGC algorithm must provide the following: */ virtual unsigned int getConvergenceFrames() const = 0; + virtual std::vector<double> const &getWeights() const = 0; virtual void setEv(double ev) = 0; virtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0; virtual void setFixedShutter(libcamera::utils::Duration fixedShutter) = 0; diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp index e6fb7b8dbeb3..e79c82e2e65b 100644 --- a/src/ipa/rpi/controller/rpi/agc.cpp +++ b/src/ipa/rpi/controller/rpi/agc.cpp @@ -292,6 +292,18 @@ unsigned int Agc::getConvergenceFrames() const return config_.convergenceFrames; } +std::vector<double> const &Agc::getWeights() const +{ + /* + * In case someone calls setMeteringMode and then this before the + * algorithm has run and updated the meteringMode_ pointer. + */ + auto it = config_.meteringModes.find(meteringModeName_); + if (it == config_.meteringModes.end()) + return meteringMode_->weights; + return it->second.weights; +} + void Agc::setEv(double ev) { ev_ = ev; @@ -595,19 +607,16 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb, ASSERT(weights.size() == stats->agcRegions.numRegions()); /* - * Note how the calculation below means that equal weights give you - * "average" metering (i.e. all pixels equally important). + * Note that the weights are applied by the IPA to the statistics directly, + * before they are given to us here. */ double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0; for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) { auto ®ion = stats->agcRegions.get(i); - double rAcc = std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted); - double gAcc = std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted); - double bAcc = std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted); - rSum += rAcc * weights[i]; - gSum += gAcc * weights[i]; - bSum += bAcc * weights[i]; - pixelSum += region.counted * weights[i]; + rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted); + gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted); + bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted); + pixelSum += region.counted; } if (pixelSum == 0.0) { LOG(RPiAgc, Warning) << "computeInitialY: pixelSum is zero"; diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h index 4e5f272fac78..939f97295a02 100644 --- a/src/ipa/rpi/controller/rpi/agc.h +++ b/src/ipa/rpi/controller/rpi/agc.h @@ -69,6 +69,7 @@ public: char const *name() const override; int read(const libcamera::YamlObject ¶ms) override; unsigned int getConvergenceFrames() const override; + std::vector<double> const &getWeights() const override; void setEv(double ev) override; void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override; void setMaxShutter(libcamera::utils::Duration maxShutter) override; diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp index 0d929cda6c4a..0e022c2aeed3 100644 --- a/src/ipa/rpi/vc4/vc4.cpp +++ b/src/ipa/rpi/vc4/vc4.cpp @@ -211,13 +211,25 @@ RPiController::StatisticsPtr IpaVc4::platformProcessStats(Span<uint8_t> mem) stats->awb_stats[i].counted, stats->awb_stats[i].notcounted }); - statistics->agcRegions.init(hw.agcRegions); - for (i = 0; i < statistics->agcRegions.numRegions(); i++) - statistics->agcRegions.set(i, { { stats->agc_stats[i].r_sum << scale, - stats->agc_stats[i].g_sum << scale, - stats->agc_stats[i].b_sum << scale }, - stats->agc_stats[i].counted, - stats->awb_stats[i].notcounted }); + RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>( + controller_.getAlgorithm("agc")); + if (!agc) { + LOG(IPARPI, Debug) << "No AGC algorithm - not copying statistics"; + statistics->agcRegions.init(0); + } else { + statistics->agcRegions.init(hw.agcRegions); + const std::vector<double> &weights = agc->getWeights(); + for (i = 0; i < statistics->agcRegions.numRegions(); i++) { + uint64_t rSum = (stats->agc_stats[i].r_sum << scale) * weights[i]; + uint64_t gSum = (stats->agc_stats[i].g_sum << scale) * weights[i]; + uint64_t bSum = (stats->agc_stats[i].b_sum << scale) * weights[i]; + uint32_t counted = stats->agc_stats[i].counted * weights[i]; + uint32_t notcounted = stats->agc_stats[i].notcounted * weights[i]; + statistics->agcRegions.set(i, { { rSum, gSum, bSum }, + counted, + notcounted }); + } + } statistics->focusRegions.init(hw.focusRegions); for (i = 0; i < statistics->focusRegions.numRegions(); i++)