| Message ID | 20210809092007.79067-5-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi JM Thank you for the patch On 8/9/21 2:50 PM, Jean-Michel Hautbois wrote: > Use the Context class and algorithm interface to properly call the AWB > algorithm from IPAIPU3. > We need to pass the BDS grid size calculated, pass those through the Context > class in a dedicated Awb structure. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > .../ipu3/{ipu3_awb.cpp => algorithms/awb.cpp} | 42 ++++++++++++------- > src/ipa/ipu3/{ipu3_awb.h => algorithms/awb.h} | 29 ++++++------- > src/ipa/ipu3/algorithms/meson.build | 1 + > src/ipa/ipu3/ipa_context.h | 12 ++++++ > src/ipa/ipu3/ipu3.cpp | 30 ++++++------- > src/ipa/ipu3/meson.build | 1 - > 6 files changed, 66 insertions(+), 49 deletions(-) > rename src/ipa/ipu3/{ipu3_awb.cpp => algorithms/awb.cpp} (92%) > rename src/ipa/ipu3/{ipu3_awb.h => algorithms/awb.h} (79%) > > diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > similarity index 92% > rename from src/ipa/ipu3/ipu3_awb.cpp > rename to src/ipa/ipu3/algorithms/awb.cpp > index 043c3838..92a41b65 100644 > --- a/src/ipa/ipu3/ipu3_awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -2,9 +2,9 @@ > /* > * Copyright (C) 2021, Ideas On Board > * > - * ipu3_awb.cpp - AWB control algorithm > + * awb.cpp - AWB control algorithm > */ > -#include "ipu3_awb.h" > +#include "awb.h" > > #include <cmath> > #include <numeric> > @@ -14,7 +14,7 @@ > > namespace libcamera { > > -namespace ipa::ipu3 { > +namespace ipa::ipu3::algorithms { > > LOG_DEFINE_CATEGORY(IPU3Awb) > > @@ -134,7 +134,7 @@ static const struct ipu3_uapi_ccm_mat_config imguCssCcmDefault = { > 0, 0, 8191, 0 > }; > > -IPU3Awb::IPU3Awb() > +Awb::Awb() > : Algorithm() > { > asyncResults_.blueGain = 1.0; > @@ -143,17 +143,19 @@ IPU3Awb::IPU3Awb() > asyncResults_.temperatureK = 4500; > } > > -IPU3Awb::~IPU3Awb() > +Awb::~Awb() > { > } > > -void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid) > +int Awb::configure(IPAContext &context) Mapping old Initialise() directly to Algorithm::configure() ? If there is a reason, I would expect something about it in the commit message maybe? > { > + ipu3_uapi_params ¶ms = context.params; > + Size &bdsOutputSize = context.awb.grid.bdsOutputSize; > + > params.use.acc_awb = 1; > params.acc_param.awb.config = imguCssAwbDefaults; > > - awbGrid_ = bdsGrid; > - params.acc_param.awb.config.grid = awbGrid_; > + awbGrid_ = context.awb.grid.bdsGrid; > > params.use.acc_bnr = 1; > params.acc_param.bnr = imguCssBnrDefaults; > @@ -174,6 +176,8 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, st > params.acc_param.ccm = imguCssCcmDefault; > > zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY); > + > + return 0; > } > > /** > @@ -190,7 +194,7 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, st > * More detailed information can be found in: > * https://en.wikipedia.org/wiki/Color_temperature#Approximation > */ > -uint32_t IPU3Awb::estimateCCT(double red, double green, double blue) > +uint32_t Awb::estimateCCT(double red, double green, double blue) > { > /* Convert the RGB values to CIE tristimulus values (XYZ) */ > double X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue); > @@ -207,7 +211,7 @@ uint32_t IPU3Awb::estimateCCT(double red, double green, double blue) > } > > /* Generate an RGB vector with the average values for each region */ > -void IPU3Awb::generateZones(std::vector<RGB> &zones) > +void Awb::generateZones(std::vector<RGB> &zones) > { > for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { > RGB zone; > @@ -224,7 +228,7 @@ void IPU3Awb::generateZones(std::vector<RGB> &zones) > } > > /* Translate the IPU3 statistics into the default statistics region array */ > -void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) > +void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) > { > uint32_t regionWidth = round(awbGrid_.width / static_cast<double>(kAwbStatsSizeX)); > uint32_t regionHeight = round(awbGrid_.height / static_cast<double>(kAwbStatsSizeY)); > @@ -256,7 +260,7 @@ void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) > } > } > > -void IPU3Awb::clearAwbStats() > +void Awb::clearAwbStats() > { > for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { > awbStats_[i].bSum = 0; > @@ -267,7 +271,7 @@ void IPU3Awb::clearAwbStats() > } > } > > -void IPU3Awb::awbGreyWorld() > +void Awb::awbGreyWorld() > { > LOG(IPU3Awb, Debug) << "Grey world AWB"; > /* > @@ -307,7 +311,7 @@ void IPU3Awb::awbGreyWorld() > asyncResults_.blueGain = blueGain; > } > > -void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > +void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > { > ASSERT(stats->stats_3a_status.awb_en); > zones_.clear(); > @@ -322,7 +326,7 @@ void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > } > } > > -void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms) > +void Awb::updateWbParameters(ipu3_uapi_params ¶ms) > { > /* > * Green gains should not be touched and considered 1. > @@ -340,6 +344,12 @@ void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms) > params.acc_param.ccm = imguCssCcmDefault; > } > > -} /* namespace ipa::ipu3 */ > +void Awb::process(IPAContext &context) > +{ > + calculateWBGains(context.stats); > + updateWbParameters(context.params); > +} > + > +} /* namespace ipa::ipu3::algorithms */ > > } /* namespace libcamera */ > diff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/algorithms/awb.h > similarity index 79% > rename from src/ipa/ipu3/ipu3_awb.h > rename to src/ipa/ipu3/algorithms/awb.h > index 8b05ac03..37690b37 100644 > --- a/src/ipa/ipu3/ipu3_awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -2,10 +2,10 @@ > /* > * Copyright (C) 2021, Ideas On Board > * > - * ipu3_awb.h - IPU3 AWB control algorithm > + * awb.h - IPU3 AWB control algorithm > */ > -#ifndef __LIBCAMERA_IPU3_AWB_H__ > -#define __LIBCAMERA_IPU3_AWB_H__ > +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ > +#define __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ > > #include <vector> > > @@ -13,26 +13,24 @@ > > #include <libcamera/geometry.h> > > -#include "algorithms/algorithm.h" > -#include "libipa/algorithm.h" > +#include "algorithm.h" > > namespace libcamera { > > -namespace ipa::ipu3 { > +namespace ipa::ipu3::algorithms { > > /* Region size for the statistics generation algorithm */ > static constexpr uint32_t kAwbStatsSizeX = 16; > static constexpr uint32_t kAwbStatsSizeY = 12; > > -class IPU3Awb : public ipa::Algorithm > +class Awb : public Algorithm > { > public: > - IPU3Awb(); > - ~IPU3Awb(); > + Awb(); > + ~Awb(); > > - void initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid); > - void calculateWBGains(const ipu3_uapi_stats_3a *stats); > - void updateWbParameters(ipu3_uapi_params ¶ms); > + int configure(IPAContext &context) override; > + void process(IPAContext &context) override; hmm, no initialise() here? Doesn't seem sane to have a direct call to configure(). I wonder, as in the last patch where there was only Initialise() and no over-ridden configure()/process(). > > struct Ipu3AwbCell { > unsigned char greenRedAvg; > @@ -73,6 +71,9 @@ public: > }; > > private: > + void calculateWBGains(const ipu3_uapi_stats_3a *stats); > + void updateWbParameters(ipu3_uapi_params ¶ms); > + > void generateZones(std::vector<RGB> &zones); > void generateAwbStats(const ipu3_uapi_stats_3a *stats); > void clearAwbStats(); > @@ -86,7 +87,7 @@ private: > AwbStatus asyncResults_; > }; > > -} /* namespace ipa::ipu3 */ > +} /* namespace ipa::ipu3::algorithms */ > > } /* namespace libcamera*/ > -#endif /* __LIBCAMERA_IPU3_AWB_H__ */ > +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ */ > diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build > index f71d1e61..df36d719 100644 > --- a/src/ipa/ipu3/algorithms/meson.build > +++ b/src/ipa/ipu3/algorithms/meson.build > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: CC0-1.0 > > ipu3_ipa_algorithms = files([ > + 'awb.cpp', > 'contrast.cpp', > ]) > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 5d717be5..c43b275b 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -11,6 +11,8 @@ > > #include <linux/intel-ipu3.h> > > +#include <libcamera/geometry.h> > + > namespace libcamera { > > namespace ipa::ipu3 { > @@ -21,6 +23,16 @@ struct IPAContext { > > /* Output Parameters which will be written to the hardware */ > ipu3_uapi_params params; > + > + /* AWB specific parameters to share */ > + struct Awb { > + struct Grid { > + /* BDS grid plane config used by the kernel */ > + ipu3_uapi_grid_config bdsGrid; > + /* BDS output size configured by the pipeline handler */ > + Size bdsOutputSize; > + } grid; > + } awb; > }; > > } /* namespace ipa::ipu3 */ > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 7035802f..82506461 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -23,11 +23,11 @@ > #include "libcamera/internal/framebuffer.h" > > #include "algorithms/algorithm.h" > +#include "algorithms/awb.h" > #include "algorithms/contrast.h" > #include "ipa_context.h" > > #include "ipu3_agc.h" > -#include "ipu3_awb.h" > #include "libipa/camera_sensor_helper.h" > > static constexpr uint32_t kMaxCellWidthPerSet = 160; > @@ -81,8 +81,6 @@ private: > uint32_t minGain_; > uint32_t maxGain_; > > - /* Interface to the AWB algorithm */ > - std::unique_ptr<IPU3Awb> awbAlgo_; > /* Interface to the AEC/AGC algorithm */ > std::unique_ptr<IPU3Agc> agcAlgo_; > /* Interface to the Camera Helper */ > @@ -105,6 +103,7 @@ int IPAIPU3::init(const IPASettings &settings) > } > > /* Construct our Algorithms */ > + algorithms_.emplace_back(new algorithms::Awb()); > algorithms_.emplace_back(new algorithms::Contrast()); > > /* Reset all the hardware settings */ > @@ -138,7 +137,7 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize) > uint32_t minError = std::numeric_limits<uint32_t>::max(); > Size best; > Size bestLog2; > - bdsGrid_ = {}; > + ipu3_uapi_grid_config &bdsGrid = context_.awb.grid.bdsGrid; > > for (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) { > uint32_t width = std::min(kMaxCellWidthPerSet, > @@ -162,14 +161,14 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize) > } > } > > - bdsGrid_.width = best.width >> bestLog2.width; > - bdsGrid_.block_width_log2 = bestLog2.width; > - bdsGrid_.height = best.height >> bestLog2.height; > - bdsGrid_.block_height_log2 = bestLog2.height; > + bdsGrid.width = best.width >> bestLog2.width; > + bdsGrid.block_width_log2 = bestLog2.width; > + bdsGrid.height = best.height >> bestLog2.height; > + bdsGrid.block_height_log2 = bestLog2.height; > > LOG(IPAIPU3, Debug) << "Best grid found is: (" > - << (int)bdsGrid_.width << " << " << (int)bdsGrid_.block_width_log2 << ") x (" > - << (int)bdsGrid_.height << " << " << (int)bdsGrid_.block_height_log2 << ")"; > + << (int)bdsGrid.width << " << " << (int)bdsGrid.block_width_log2 << ") x (" > + << (int)bdsGrid.height << " << " << (int)bdsGrid.block_height_log2 << ")"; > } > > int IPAIPU3::configure(const IPAConfigInfo &configInfo) > @@ -211,13 +210,13 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) > > defVBlank_ = itVBlank->second.def().get<int32_t>(); > > + /* Prepare AWB parameters */ > calculateBdsGrid(configInfo.bdsOutputSize); > + context_.awb.grid.bdsOutputSize = configInfo.bdsOutputSize; > > configureAlgorithms(); > > - awbAlgo_ = std::make_unique<IPU3Awb>(); > - awbAlgo_->initialise(context_.params, configInfo.bdsOutputSize, bdsGrid_); > - > + bdsGrid_ = context_.awb.grid.bdsGrid; > agcAlgo_ = std::make_unique<IPU3Agc>(); > agcAlgo_->initialise(bdsGrid_, sensorInfo_); > > @@ -293,9 +292,6 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame, > > void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) > { > - if (agcAlgo_->updateControls()) > - awbAlgo_->updateWbParameters(context_.params); > - > *params = context_.params; > > IPU3Action op; > @@ -349,8 +345,6 @@ void IPAIPU3::parseStatistics(unsigned int frame, > agcAlgo_->process(stats, exposure_, gain); > gain_ = camHelper_->gainCode(gain); > > - awbAlgo_->calculateWBGains(stats); > - > if (agcAlgo_->updateControls()) > setControls(frame); > > diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build > index fcb27d68..d1126947 100644 > --- a/src/ipa/ipu3/meson.build > +++ b/src/ipa/ipu3/meson.build > @@ -7,7 +7,6 @@ ipa_name = 'ipa_ipu3' > ipu3_ipa_sources = files([ > 'ipu3.cpp', > 'ipu3_agc.cpp', > - 'ipu3_awb.cpp', > ]) > > ipu3_ipa_sources += ipu3_ipa_algorithms
On 09/08/2021 10:20, Jean-Michel Hautbois wrote: > Use the Context class and algorithm interface to properly call the AWB s/Context/IPAContext/ s/algorithm/Algorithm/ and instead of 'properly call' I'd say "to call the AWB algorithm using the new modular interface" > algorithm from IPAIPU3. > We need to pass the BDS grid size calculated, pass those through the Context > class in a dedicated Awb structure. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > .../ipu3/{ipu3_awb.cpp => algorithms/awb.cpp} | 42 ++++++++++++------- > src/ipa/ipu3/{ipu3_awb.h => algorithms/awb.h} | 29 ++++++------- > src/ipa/ipu3/algorithms/meson.build | 1 + > src/ipa/ipu3/ipa_context.h | 12 ++++++ > src/ipa/ipu3/ipu3.cpp | 30 ++++++------- > src/ipa/ipu3/meson.build | 1 - > 6 files changed, 66 insertions(+), 49 deletions(-) > rename src/ipa/ipu3/{ipu3_awb.cpp => algorithms/awb.cpp} (92%) > rename src/ipa/ipu3/{ipu3_awb.h => algorithms/awb.h} (79%) > > diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > similarity index 92% > rename from src/ipa/ipu3/ipu3_awb.cpp > rename to src/ipa/ipu3/algorithms/awb.cpp > index 043c3838..92a41b65 100644 > --- a/src/ipa/ipu3/ipu3_awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -2,9 +2,9 @@ > /* > * Copyright (C) 2021, Ideas On Board > * > - * ipu3_awb.cpp - AWB control algorithm > + * awb.cpp - AWB control algorithm > */ > -#include "ipu3_awb.h" > +#include "awb.h" > > #include <cmath> > #include <numeric> > @@ -14,7 +14,7 @@ > > namespace libcamera { > > -namespace ipa::ipu3 { > +namespace ipa::ipu3::algorithms { > > LOG_DEFINE_CATEGORY(IPU3Awb) > > @@ -134,7 +134,7 @@ static const struct ipu3_uapi_ccm_mat_config imguCssCcmDefault = { > 0, 0, 8191, 0 > }; > > -IPU3Awb::IPU3Awb() > +Awb::Awb() > : Algorithm() > { > asyncResults_.blueGain = 1.0; > @@ -143,17 +143,19 @@ IPU3Awb::IPU3Awb() > asyncResults_.temperatureK = 4500; > } > > -IPU3Awb::~IPU3Awb() > +Awb::~Awb() > { > } > > -void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid) > +int Awb::configure(IPAContext &context) > { > + ipu3_uapi_params ¶ms = context.params; > + Size &bdsOutputSize = context.awb.grid.bdsOutputSize; > + We'll need to give more thought as to what parameters are coming in as Configuration parameters, and they might get passed in (perhaps as a separate structure, or perhaps still using the IPAContext). But we've already discussed whether the Grid configuration would be something calculated by the top level ipu3.cpp and passed in, or if it should be it's own 'algorithm/module' and handled as part of the processing chain. But for now, I think this is fine. > params.use.acc_awb = 1; > params.acc_param.awb.config = imguCssAwbDefaults; > > - awbGrid_ = bdsGrid; > - params.acc_param.awb.config.grid = awbGrid_; > + awbGrid_ = context.awb.grid.bdsGrid; does this Awb class still need a local copy of the grid in awbGrid_ ? Perhaps it does if it's something that will be passed in, and potentially not available in the IPAContext at process() time ... but at the moment, it 'could' always get it from the Context ... > > params.use.acc_bnr = 1; > params.acc_param.bnr = imguCssBnrDefaults; > @@ -174,6 +176,8 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, st > params.acc_param.ccm = imguCssCcmDefault; > > zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY); > + > + return 0; > } > > /** > @@ -190,7 +194,7 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, st > * More detailed information can be found in: > * https://en.wikipedia.org/wiki/Color_temperature#Approximation > */ > -uint32_t IPU3Awb::estimateCCT(double red, double green, double blue) > +uint32_t Awb::estimateCCT(double red, double green, double blue) > { > /* Convert the RGB values to CIE tristimulus values (XYZ) */ > double X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue); > @@ -207,7 +211,7 @@ uint32_t IPU3Awb::estimateCCT(double red, double green, double blue) > } > > /* Generate an RGB vector with the average values for each region */ > -void IPU3Awb::generateZones(std::vector<RGB> &zones) > +void Awb::generateZones(std::vector<RGB> &zones) > { > for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { > RGB zone; > @@ -224,7 +228,7 @@ void IPU3Awb::generateZones(std::vector<RGB> &zones) > } > > /* Translate the IPU3 statistics into the default statistics region array */ > -void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) > +void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) > { > uint32_t regionWidth = round(awbGrid_.width / static_cast<double>(kAwbStatsSizeX)); > uint32_t regionHeight = round(awbGrid_.height / static_cast<double>(kAwbStatsSizeY)); > @@ -256,7 +260,7 @@ void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) > } > } > > -void IPU3Awb::clearAwbStats() > +void Awb::clearAwbStats() > { > for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { > awbStats_[i].bSum = 0; > @@ -267,7 +271,7 @@ void IPU3Awb::clearAwbStats() > } > } > > -void IPU3Awb::awbGreyWorld() > +void Awb::awbGreyWorld() > { > LOG(IPU3Awb, Debug) << "Grey world AWB"; > /* > @@ -307,7 +311,7 @@ void IPU3Awb::awbGreyWorld() > asyncResults_.blueGain = blueGain; > } > > -void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > +void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > { > ASSERT(stats->stats_3a_status.awb_en); > zones_.clear(); > @@ -322,7 +326,7 @@ void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > } > } > > -void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms) > +void Awb::updateWbParameters(ipu3_uapi_params ¶ms) > { > /* > * Green gains should not be touched and considered 1. > @@ -340,6 +344,12 @@ void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms) > params.acc_param.ccm = imguCssCcmDefault; > } > > -} /* namespace ipa::ipu3 */ > +void Awb::process(IPAContext &context) > +{ > + calculateWBGains(context.stats); > + updateWbParameters(context.params); > +} > + > +} /* namespace ipa::ipu3::algorithms */ > > } /* namespace libcamera */ > diff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/algorithms/awb.h > similarity index 79% > rename from src/ipa/ipu3/ipu3_awb.h > rename to src/ipa/ipu3/algorithms/awb.h > index 8b05ac03..37690b37 100644 > --- a/src/ipa/ipu3/ipu3_awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -2,10 +2,10 @@ > /* > * Copyright (C) 2021, Ideas On Board > * > - * ipu3_awb.h - IPU3 AWB control algorithm > + * awb.h - IPU3 AWB control algorithm > */ > -#ifndef __LIBCAMERA_IPU3_AWB_H__ > -#define __LIBCAMERA_IPU3_AWB_H__ > +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ > +#define __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ > > #include <vector> > > @@ -13,26 +13,24 @@ > > #include <libcamera/geometry.h> > > -#include "algorithms/algorithm.h" > -#include "libipa/algorithm.h" > +#include "algorithm.h" > > namespace libcamera { > > -namespace ipa::ipu3 { > +namespace ipa::ipu3::algorithms { > > /* Region size for the statistics generation algorithm */ > static constexpr uint32_t kAwbStatsSizeX = 16; > static constexpr uint32_t kAwbStatsSizeY = 12; > > -class IPU3Awb : public ipa::Algorithm > +class Awb : public Algorithm > { > public: > - IPU3Awb(); > - ~IPU3Awb(); > + Awb(); > + ~Awb(); > > - void initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid); > - void calculateWBGains(const ipu3_uapi_stats_3a *stats); > - void updateWbParameters(ipu3_uapi_params ¶ms); > + int configure(IPAContext &context) override; > + void process(IPAContext &context) override; > > struct Ipu3AwbCell { > unsigned char greenRedAvg; > @@ -73,6 +71,9 @@ public: > }; > > private: > + void calculateWBGains(const ipu3_uapi_stats_3a *stats); > + void updateWbParameters(ipu3_uapi_params ¶ms); > + > void generateZones(std::vector<RGB> &zones); > void generateAwbStats(const ipu3_uapi_stats_3a *stats); > void clearAwbStats(); > @@ -86,7 +87,7 @@ private: > AwbStatus asyncResults_; > }; > > -} /* namespace ipa::ipu3 */ > +} /* namespace ipa::ipu3::algorithms */ > > } /* namespace libcamera*/ > -#endif /* __LIBCAMERA_IPU3_AWB_H__ */ > +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ */ > diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build > index f71d1e61..df36d719 100644 > --- a/src/ipa/ipu3/algorithms/meson.build > +++ b/src/ipa/ipu3/algorithms/meson.build > @@ -1,5 +1,6 @@ > # SPDX-License-Identifier: CC0-1.0 > > ipu3_ipa_algorithms = files([ > + 'awb.cpp', > 'contrast.cpp', > ]) > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 5d717be5..c43b275b 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -11,6 +11,8 @@ > > #include <linux/intel-ipu3.h> > > +#include <libcamera/geometry.h> > + > namespace libcamera { > > namespace ipa::ipu3 { > @@ -21,6 +23,16 @@ struct IPAContext { > > /* Output Parameters which will be written to the hardware */ > ipu3_uapi_params params; > + > + /* AWB specific parameters to share */ > + struct Awb { > + struct Grid { > + /* BDS grid plane config used by the kernel */ > + ipu3_uapi_grid_config bdsGrid; > + /* BDS output size configured by the pipeline handler */ > + Size bdsOutputSize; > + } grid; Discussed off-list, but re-discussing here for the list: I think the BDS grid sounds like it's own module. It references 'BDS' which is a distinct entity on the IPU3, and it does a calculation at configure time. The Grid isn't a property of the AWB, but something that it uses, (as does the AGC, I believe). So I'd expect to see this stored outside of the Awb context structure. However - As this is currently calculated /inside/ the Awb algorithm module - I'm fine with keeping it here to show that. If or rather when, the grid calculation gets moved out - I would expect that this will likely move out of this structure and to a structure that matches the component which generates it. > + } awb; > }; > > } /* namespace ipa::ipu3 */ > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 7035802f..82506461 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -23,11 +23,11 @@ > #include "libcamera/internal/framebuffer.h" > > #include "algorithms/algorithm.h" > +#include "algorithms/awb.h" > #include "algorithms/contrast.h" > #include "ipa_context.h" > > #include "ipu3_agc.h" > -#include "ipu3_awb.h" > #include "libipa/camera_sensor_helper.h" > > static constexpr uint32_t kMaxCellWidthPerSet = 160; > @@ -81,8 +81,6 @@ private: > uint32_t minGain_; > uint32_t maxGain_; > > - /* Interface to the AWB algorithm */ > - std::unique_ptr<IPU3Awb> awbAlgo_; > /* Interface to the AEC/AGC algorithm */ > std::unique_ptr<IPU3Agc> agcAlgo_; > /* Interface to the Camera Helper */ > @@ -105,6 +103,7 @@ int IPAIPU3::init(const IPASettings &settings) > } > > /* Construct our Algorithms */ Perhaps in the previous patch we should state here that algorithms will be initialised, configured, and processed in the order that they are added to this list. > + algorithms_.emplace_back(new algorithms::Awb()); > algorithms_.emplace_back(new algorithms::Contrast()); > > /* Reset all the hardware settings */ > @@ -138,7 +137,7 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize) Oh! The grid *isn't* calculated in the Awb currently, so it really isn't an Awb structure ? It's just that the Awb uses the structure ? is that right? It looks like this is something that the IPU3 top level code does currently - and really could be moved out to an 'algorithm' to handle during the Configure stage. I would be quite tempted to see that done as a patch /before/ converting the Awb given it's a pre-requisite to handling the Awb ... It might also show that we should pass "const IPAConfigInfo &configInfo" into the Configure() operations of the modules too. Alternatively, if you really believe this is part of Awb and not it's own 'module' - should/could it move into the Configure phase of the Awb /in this patch/ ? If the grid configuration is used only for Awb, then it can be part of the Awb configure. If it is used for any other algorithm stage (and I mean on the IPU3, not just our current algorithms) then I think it deserves it's own module. > uint32_t minError = std::numeric_limits<uint32_t>::max(); > Size best; > Size bestLog2; > - bdsGrid_ = {}; > + ipu3_uapi_grid_config &bdsGrid = context_.awb.grid.bdsGrid; > > for (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) { > uint32_t width = std::min(kMaxCellWidthPerSet, > @@ -162,14 +161,14 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize) > } > } > > - bdsGrid_.width = best.width >> bestLog2.width; > - bdsGrid_.block_width_log2 = bestLog2.width; > - bdsGrid_.height = best.height >> bestLog2.height; > - bdsGrid_.block_height_log2 = bestLog2.height; > + bdsGrid.width = best.width >> bestLog2.width; > + bdsGrid.block_width_log2 = bestLog2.width; > + bdsGrid.height = best.height >> bestLog2.height; > + bdsGrid.block_height_log2 = bestLog2.height; > > LOG(IPAIPU3, Debug) << "Best grid found is: (" > - << (int)bdsGrid_.width << " << " << (int)bdsGrid_.block_width_log2 << ") x (" > - << (int)bdsGrid_.height << " << " << (int)bdsGrid_.block_height_log2 << ")"; > + << (int)bdsGrid.width << " << " << (int)bdsGrid.block_width_log2 << ") x (" > + << (int)bdsGrid.height << " << " << (int)bdsGrid.block_height_log2 << ")"; > } > > int IPAIPU3::configure(const IPAConfigInfo &configInfo) > @@ -211,13 +210,13 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) > > defVBlank_ = itVBlank->second.def().get<int32_t>(); > > + /* Prepare AWB parameters */ > calculateBdsGrid(configInfo.bdsOutputSize); > + context_.awb.grid.bdsOutputSize = configInfo.bdsOutputSize; This looks like evidence suggesting we need an algorithm to handle the configuration of the BDS which stores it's 'output' in our IPAContext as a BDS specific data structure. > > configureAlgorithms(); > > - awbAlgo_ = std::make_unique<IPU3Awb>(); > - awbAlgo_->initialise(context_.params, configInfo.bdsOutputSize, bdsGrid_); > - > + bdsGrid_ = context_.awb.grid.bdsGrid; Is this just a copy of the grid in the top level IPU3 framework ? I suspect it shouldn't be needed when we have it on the IPAContext... Is it still used somehow? Anyway, that might be all irrelevant if you create a BDS algorithm that runs first. > agcAlgo_ = std::make_unique<IPU3Agc>(); > agcAlgo_->initialise(bdsGrid_, sensorInfo_); > > @@ -293,9 +292,6 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame, > > void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) > { > - if (agcAlgo_->updateControls()) > - awbAlgo_->updateWbParameters(context_.params); > - > *params = context_.params; > > IPU3Action op; > @@ -349,8 +345,6 @@ void IPAIPU3::parseStatistics(unsigned int frame, > agcAlgo_->process(stats, exposure_, gain); > gain_ = camHelper_->gainCode(gain); > > - awbAlgo_->calculateWBGains(stats); > - Very happy to see these algorithm specific hooks being removed though. > if (agcAlgo_->updateControls()) > setControls(frame); > > diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build > index fcb27d68..d1126947 100644 > --- a/src/ipa/ipu3/meson.build > +++ b/src/ipa/ipu3/meson.build > @@ -7,7 +7,6 @@ ipa_name = 'ipa_ipu3' > ipu3_ipa_sources = files([ > 'ipu3.cpp', > 'ipu3_agc.cpp', > - 'ipu3_awb.cpp', > ]) > > ipu3_ipa_sources += ipu3_ipa_algorithms >
On 09/08/2021 11:20, Umang Jain wrote: > Hi JM > > Thank you for the patch > <snip> >> +Awb::~Awb() >> { >> } >> -void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size >> &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid) >> +int Awb::configure(IPAContext &context) > Mapping old Initialise() directly to Algorithm::configure() ? If there > is a reason, I would expect something about it in the commit message maybe? >> { >> + ipu3_uapi_params ¶ms = context.params; <snip> >> +class Awb : public Algorithm >> { >> public: >> - IPU3Awb(); >> - ~IPU3Awb(); >> + Awb(); >> + ~Awb(); >> - void initialise(ipu3_uapi_params ¶ms, const Size >> &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid); >> - void calculateWBGains(const ipu3_uapi_stats_3a *stats); >> - void updateWbParameters(ipu3_uapi_params ¶ms); >> + int configure(IPAContext &context) override; >> + void process(IPAContext &context) override; > hmm, no initialise() here? Doesn't seem sane to have a direct call to > configure(). I wonder, as in the last patch where there was only > Initialise() and no over-ridden configure()/process(). Yes, I think we should explicitly state in the commit message that as part of the conversion, the initialisation phase of the algorithm is now handled as part of the configuration phase in the new algorithm structure. -- Kieran
diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp similarity index 92% rename from src/ipa/ipu3/ipu3_awb.cpp rename to src/ipa/ipu3/algorithms/awb.cpp index 043c3838..92a41b65 100644 --- a/src/ipa/ipu3/ipu3_awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -2,9 +2,9 @@ /* * Copyright (C) 2021, Ideas On Board * - * ipu3_awb.cpp - AWB control algorithm + * awb.cpp - AWB control algorithm */ -#include "ipu3_awb.h" +#include "awb.h" #include <cmath> #include <numeric> @@ -14,7 +14,7 @@ namespace libcamera { -namespace ipa::ipu3 { +namespace ipa::ipu3::algorithms { LOG_DEFINE_CATEGORY(IPU3Awb) @@ -134,7 +134,7 @@ static const struct ipu3_uapi_ccm_mat_config imguCssCcmDefault = { 0, 0, 8191, 0 }; -IPU3Awb::IPU3Awb() +Awb::Awb() : Algorithm() { asyncResults_.blueGain = 1.0; @@ -143,17 +143,19 @@ IPU3Awb::IPU3Awb() asyncResults_.temperatureK = 4500; } -IPU3Awb::~IPU3Awb() +Awb::~Awb() { } -void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid) +int Awb::configure(IPAContext &context) { + ipu3_uapi_params ¶ms = context.params; + Size &bdsOutputSize = context.awb.grid.bdsOutputSize; + params.use.acc_awb = 1; params.acc_param.awb.config = imguCssAwbDefaults; - awbGrid_ = bdsGrid; - params.acc_param.awb.config.grid = awbGrid_; + awbGrid_ = context.awb.grid.bdsGrid; params.use.acc_bnr = 1; params.acc_param.bnr = imguCssBnrDefaults; @@ -174,6 +176,8 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, st params.acc_param.ccm = imguCssCcmDefault; zones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY); + + return 0; } /** @@ -190,7 +194,7 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, st * More detailed information can be found in: * https://en.wikipedia.org/wiki/Color_temperature#Approximation */ -uint32_t IPU3Awb::estimateCCT(double red, double green, double blue) +uint32_t Awb::estimateCCT(double red, double green, double blue) { /* Convert the RGB values to CIE tristimulus values (XYZ) */ double X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue); @@ -207,7 +211,7 @@ uint32_t IPU3Awb::estimateCCT(double red, double green, double blue) } /* Generate an RGB vector with the average values for each region */ -void IPU3Awb::generateZones(std::vector<RGB> &zones) +void Awb::generateZones(std::vector<RGB> &zones) { for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { RGB zone; @@ -224,7 +228,7 @@ void IPU3Awb::generateZones(std::vector<RGB> &zones) } /* Translate the IPU3 statistics into the default statistics region array */ -void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) +void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) { uint32_t regionWidth = round(awbGrid_.width / static_cast<double>(kAwbStatsSizeX)); uint32_t regionHeight = round(awbGrid_.height / static_cast<double>(kAwbStatsSizeY)); @@ -256,7 +260,7 @@ void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats) } } -void IPU3Awb::clearAwbStats() +void Awb::clearAwbStats() { for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { awbStats_[i].bSum = 0; @@ -267,7 +271,7 @@ void IPU3Awb::clearAwbStats() } } -void IPU3Awb::awbGreyWorld() +void Awb::awbGreyWorld() { LOG(IPU3Awb, Debug) << "Grey world AWB"; /* @@ -307,7 +311,7 @@ void IPU3Awb::awbGreyWorld() asyncResults_.blueGain = blueGain; } -void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) +void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) { ASSERT(stats->stats_3a_status.awb_en); zones_.clear(); @@ -322,7 +326,7 @@ void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) } } -void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms) +void Awb::updateWbParameters(ipu3_uapi_params ¶ms) { /* * Green gains should not be touched and considered 1. @@ -340,6 +344,12 @@ void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms) params.acc_param.ccm = imguCssCcmDefault; } -} /* namespace ipa::ipu3 */ +void Awb::process(IPAContext &context) +{ + calculateWBGains(context.stats); + updateWbParameters(context.params); +} + +} /* namespace ipa::ipu3::algorithms */ } /* namespace libcamera */ diff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/algorithms/awb.h similarity index 79% rename from src/ipa/ipu3/ipu3_awb.h rename to src/ipa/ipu3/algorithms/awb.h index 8b05ac03..37690b37 100644 --- a/src/ipa/ipu3/ipu3_awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -2,10 +2,10 @@ /* * Copyright (C) 2021, Ideas On Board * - * ipu3_awb.h - IPU3 AWB control algorithm + * awb.h - IPU3 AWB control algorithm */ -#ifndef __LIBCAMERA_IPU3_AWB_H__ -#define __LIBCAMERA_IPU3_AWB_H__ +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ +#define __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ #include <vector> @@ -13,26 +13,24 @@ #include <libcamera/geometry.h> -#include "algorithms/algorithm.h" -#include "libipa/algorithm.h" +#include "algorithm.h" namespace libcamera { -namespace ipa::ipu3 { +namespace ipa::ipu3::algorithms { /* Region size for the statistics generation algorithm */ static constexpr uint32_t kAwbStatsSizeX = 16; static constexpr uint32_t kAwbStatsSizeY = 12; -class IPU3Awb : public ipa::Algorithm +class Awb : public Algorithm { public: - IPU3Awb(); - ~IPU3Awb(); + Awb(); + ~Awb(); - void initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid); - void calculateWBGains(const ipu3_uapi_stats_3a *stats); - void updateWbParameters(ipu3_uapi_params ¶ms); + int configure(IPAContext &context) override; + void process(IPAContext &context) override; struct Ipu3AwbCell { unsigned char greenRedAvg; @@ -73,6 +71,9 @@ public: }; private: + void calculateWBGains(const ipu3_uapi_stats_3a *stats); + void updateWbParameters(ipu3_uapi_params ¶ms); + void generateZones(std::vector<RGB> &zones); void generateAwbStats(const ipu3_uapi_stats_3a *stats); void clearAwbStats(); @@ -86,7 +87,7 @@ private: AwbStatus asyncResults_; }; -} /* namespace ipa::ipu3 */ +} /* namespace ipa::ipu3::algorithms */ } /* namespace libcamera*/ -#endif /* __LIBCAMERA_IPU3_AWB_H__ */ +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AWB_H__ */ diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build index f71d1e61..df36d719 100644 --- a/src/ipa/ipu3/algorithms/meson.build +++ b/src/ipa/ipu3/algorithms/meson.build @@ -1,5 +1,6 @@ # SPDX-License-Identifier: CC0-1.0 ipu3_ipa_algorithms = files([ + 'awb.cpp', 'contrast.cpp', ]) diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index 5d717be5..c43b275b 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -11,6 +11,8 @@ #include <linux/intel-ipu3.h> +#include <libcamera/geometry.h> + namespace libcamera { namespace ipa::ipu3 { @@ -21,6 +23,16 @@ struct IPAContext { /* Output Parameters which will be written to the hardware */ ipu3_uapi_params params; + + /* AWB specific parameters to share */ + struct Awb { + struct Grid { + /* BDS grid plane config used by the kernel */ + ipu3_uapi_grid_config bdsGrid; + /* BDS output size configured by the pipeline handler */ + Size bdsOutputSize; + } grid; + } awb; }; } /* namespace ipa::ipu3 */ diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 7035802f..82506461 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -23,11 +23,11 @@ #include "libcamera/internal/framebuffer.h" #include "algorithms/algorithm.h" +#include "algorithms/awb.h" #include "algorithms/contrast.h" #include "ipa_context.h" #include "ipu3_agc.h" -#include "ipu3_awb.h" #include "libipa/camera_sensor_helper.h" static constexpr uint32_t kMaxCellWidthPerSet = 160; @@ -81,8 +81,6 @@ private: uint32_t minGain_; uint32_t maxGain_; - /* Interface to the AWB algorithm */ - std::unique_ptr<IPU3Awb> awbAlgo_; /* Interface to the AEC/AGC algorithm */ std::unique_ptr<IPU3Agc> agcAlgo_; /* Interface to the Camera Helper */ @@ -105,6 +103,7 @@ int IPAIPU3::init(const IPASettings &settings) } /* Construct our Algorithms */ + algorithms_.emplace_back(new algorithms::Awb()); algorithms_.emplace_back(new algorithms::Contrast()); /* Reset all the hardware settings */ @@ -138,7 +137,7 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize) uint32_t minError = std::numeric_limits<uint32_t>::max(); Size best; Size bestLog2; - bdsGrid_ = {}; + ipu3_uapi_grid_config &bdsGrid = context_.awb.grid.bdsGrid; for (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) { uint32_t width = std::min(kMaxCellWidthPerSet, @@ -162,14 +161,14 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize) } } - bdsGrid_.width = best.width >> bestLog2.width; - bdsGrid_.block_width_log2 = bestLog2.width; - bdsGrid_.height = best.height >> bestLog2.height; - bdsGrid_.block_height_log2 = bestLog2.height; + bdsGrid.width = best.width >> bestLog2.width; + bdsGrid.block_width_log2 = bestLog2.width; + bdsGrid.height = best.height >> bestLog2.height; + bdsGrid.block_height_log2 = bestLog2.height; LOG(IPAIPU3, Debug) << "Best grid found is: (" - << (int)bdsGrid_.width << " << " << (int)bdsGrid_.block_width_log2 << ") x (" - << (int)bdsGrid_.height << " << " << (int)bdsGrid_.block_height_log2 << ")"; + << (int)bdsGrid.width << " << " << (int)bdsGrid.block_width_log2 << ") x (" + << (int)bdsGrid.height << " << " << (int)bdsGrid.block_height_log2 << ")"; } int IPAIPU3::configure(const IPAConfigInfo &configInfo) @@ -211,13 +210,13 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) defVBlank_ = itVBlank->second.def().get<int32_t>(); + /* Prepare AWB parameters */ calculateBdsGrid(configInfo.bdsOutputSize); + context_.awb.grid.bdsOutputSize = configInfo.bdsOutputSize; configureAlgorithms(); - awbAlgo_ = std::make_unique<IPU3Awb>(); - awbAlgo_->initialise(context_.params, configInfo.bdsOutputSize, bdsGrid_); - + bdsGrid_ = context_.awb.grid.bdsGrid; agcAlgo_ = std::make_unique<IPU3Agc>(); agcAlgo_->initialise(bdsGrid_, sensorInfo_); @@ -293,9 +292,6 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame, void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) { - if (agcAlgo_->updateControls()) - awbAlgo_->updateWbParameters(context_.params); - *params = context_.params; IPU3Action op; @@ -349,8 +345,6 @@ void IPAIPU3::parseStatistics(unsigned int frame, agcAlgo_->process(stats, exposure_, gain); gain_ = camHelper_->gainCode(gain); - awbAlgo_->calculateWBGains(stats); - if (agcAlgo_->updateControls()) setControls(frame); diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build index fcb27d68..d1126947 100644 --- a/src/ipa/ipu3/meson.build +++ b/src/ipa/ipu3/meson.build @@ -7,7 +7,6 @@ ipa_name = 'ipa_ipu3' ipu3_ipa_sources = files([ 'ipu3.cpp', 'ipu3_agc.cpp', - 'ipu3_awb.cpp', ]) ipu3_ipa_sources += ipu3_ipa_algorithms
Use the Context class and algorithm interface to properly call the AWB algorithm from IPAIPU3. We need to pass the BDS grid size calculated, pass those through the Context class in a dedicated Awb structure. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- .../ipu3/{ipu3_awb.cpp => algorithms/awb.cpp} | 42 ++++++++++++------- src/ipa/ipu3/{ipu3_awb.h => algorithms/awb.h} | 29 ++++++------- src/ipa/ipu3/algorithms/meson.build | 1 + src/ipa/ipu3/ipa_context.h | 12 ++++++ src/ipa/ipu3/ipu3.cpp | 30 ++++++------- src/ipa/ipu3/meson.build | 1 - 6 files changed, 66 insertions(+), 49 deletions(-) rename src/ipa/ipu3/{ipu3_awb.cpp => algorithms/awb.cpp} (92%) rename src/ipa/ipu3/{ipu3_awb.h => algorithms/awb.h} (79%)