[libcamera-devel] [PATCH v4 4/9] ipa: ipu3: Introduce modular algorithm
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Aug 19 17:31:24 CEST 2021
On 19/08/2021 15:57, Jean-Michel Hautbois wrote:
> Implement a new modular framework for algorithms with a common context
> structure that is passed to each algorithm through a common API.
>
> This patch:
> - removes all the local references from IPAIPU3 and uses IPAContext
> - implements the list of pointers and the loop at configure call on each
> algorithm
> - loops in fillParams on each prepare() call on the algorithm list
> - loops in prepareStats on each process() call on the algorithm list
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
> src/ipa/ipu3/algorithms/algorithm.cpp | 2 +-
> src/ipa/ipu3/algorithms/algorithm.h | 2 +-
> src/ipa/ipu3/ipa_context.h | 6 +++
> src/ipa/ipu3/ipu3.cpp | 56 +++++++++++++++++++++------
> 4 files changed, 52 insertions(+), 14 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/algorithm.cpp b/src/ipa/ipu3/algorithms/algorithm.cpp
> index ec3c152e..8291fb16 100644
> --- a/src/ipa/ipu3/algorithms/algorithm.cpp
> +++ b/src/ipa/ipu3/algorithms/algorithm.cpp
> @@ -92,7 +92,7 @@ void Algorithm::prepare([[maybe_unused]] IPAContext &context, [[maybe_unused]] i
> * Care shall be taken to ensure the ordering of access to the information
> * such that the algorithms use up to date state as required.
> */
> -void Algorithm::process([[maybe_unused]] IPAContext &context, [[maybe_unused]] const ipu3_uapi_stats_3a &stats)
> +void Algorithm::process([[maybe_unused]] IPAContext &context, [[maybe_unused]] const ipu3_uapi_stats_3a *&stats)
*& should be just *, and it should be * from the previous patch, so no
change expected here.
> {
> }
>
> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h
> index 89431005..df66fc8c 100644
> --- a/src/ipa/ipu3/algorithms/algorithm.h
> +++ b/src/ipa/ipu3/algorithms/algorithm.h
> @@ -23,7 +23,7 @@ public:
>
> virtual void prepare(IPAContext &context, ipu3_uapi_params ¶ms);
>
> - virtual void process(IPAContext &context, const ipu3_uapi_stats_3a &stats);
> + virtual void process(IPAContext &context, const ipu3_uapi_stats_3a *&stats);
> };
>
> } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 2706d3ca..a031ab83 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -10,11 +10,17 @@
>
> #include <linux/intel-ipu3.h>
>
> +#include <libcamera/geometry.h>
> +
> namespace libcamera {
>
> namespace ipa::ipu3 {
>
> struct IPASessionConfiguration {
> + struct {
> + ipu3_uapi_grid_config bdsGrid;
> + Size bdsOutputSize;
> + } grid;
> };
>
> struct IPAFrameContext {
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index a4359d3e..fa09d9f2 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -29,6 +29,7 @@
>
> #include "libcamera/internal/mapped_framebuffer.h"
>
> +#include "algorithms/algorithm.h"
> #include "ipu3_agc.h"
> #include "ipu3_awb.h"
> #include "libipa/camera_sensor_helper.h"
> @@ -82,6 +83,17 @@ static constexpr uint32_t kMaxCellHeightPerSet = 56;
> * are run. This needs to be turned into real per-frame data storage.
> */
>
> +/**
> + * \struct IPASessionConfiguration::grid
> + * \brief Grid configuration of the IPA
> + *
> + * \var IPASessionConfiguration::grid::bdsGrid
> + * \brief Bayer Down Scaler grid plane config used by the kernel
> + *
> + * \var IPASessionConfiguration::grid::bdsOutputSize
> + * \brief BDS output size configured by the pipeline handler
> + */
> +
> namespace libcamera {
>
> LOG_DEFINE_CATEGORY(IPAIPU3)
> @@ -137,10 +149,12 @@ private:
> /* Interface to the Camera Helper */
> std::unique_ptr<CameraSensorHelper> camHelper_;
>
> + /* Maintain the algorithms used by the IPA */
> + std::list<std::unique_ptr<ipa::ipu3::Algorithm>> algorithms_;
> +
> /* Local parameter storage */
> + struct IPAContext context_;
> struct ipu3_uapi_params params_;
> -
> - struct ipu3_uapi_grid_config bdsGrid_;
> };
>
> /**
> @@ -237,7 +251,9 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)
> uint32_t minError = std::numeric_limits<uint32_t>::max();
> Size best;
> Size bestLog2;
> - bdsGrid_ = {};
> +
> + /* Set the BDS output size in the IPAConfiguration structure */
> + context_.configuration.grid.bdsOutputSize = bdsOutputSize;
>
> for (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) {
> uint32_t width = std::min(kMaxCellWidthPerSet,
> @@ -261,14 +277,17 @@ 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;
> + struct ipu3_uapi_grid_config &bdsGrid = context_.configuration.grid.bdsGrid;
> + bdsGrid.x_start = 0;
> + bdsGrid.y_start = 0;
> + 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)
> @@ -310,15 +329,22 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
>
> defVBlank_ = itVBlank->second.def().get<int32_t>();
>
> + /* Clean context and IPU3 parameters at configuration */
> params_ = {};
> + context_ = {};
>
> calculateBdsGrid(configInfo.bdsOutputSize);
>
> - awbAlgo_ = std::make_unique<IPU3Awb>();
> - awbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);
> + for (auto const &algo : algorithms_) {
> + int ret = algo->configure(context_, configInfo);
> + if (ret)
> + return ret;
> + }
>
> + awbAlgo_ = std::make_unique<IPU3Awb>();
> + awbAlgo_->initialise(params_, context_.configuration.grid.bdsOutputSize, context_.configuration.grid.bdsGrid);
> agcAlgo_ = std::make_unique<IPU3Agc>();
> - agcAlgo_->initialise(bdsGrid_, sensorInfo_);
> + agcAlgo_->initialise(context_.configuration.grid.bdsGrid, sensorInfo_);
>
> return 0;
> }
> @@ -392,6 +418,9 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,
>
> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> {
> + for (auto const &algo : algorithms_)
> + algo->prepare(context_, params_);
Aha, here we should be calling:
algo->prepare(context_, params);
But as this is a transition with the existing algorithms also running,
perhaps you might want to call
algo->prepare(context_, ¶ms_);
> +
> if (agcAlgo_->updateControls())
> awbAlgo_->updateWbParameters(params_, agcAlgo_->gamma());
Until after this ^ is removed, at which point the loop calls to prepare
can go directly to params, and params_ can be removed entirely from the
class.
With that,
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> @@ -409,6 +438,9 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> {
> ControlList ctrls(controls::controls);
>
> + for (auto const &algo : algorithms_)
> + algo->process(context_, stats);
> +
> double gain = camHelper_->gain(gain_);
> agcAlgo_->process(stats, exposure_, gain);
> gain_ = camHelper_->gainCode(gain);
>
More information about the libcamera-devel
mailing list