[libcamera-devel,1/3] ipu3: Remove the usage of SharedItemPool
diff mbox series

Message ID mailman.444.1633340913.837.libcamera-devel@lists.libcamera.org
State Superseded
Headers show
Series
  • Fix dark caputred image with close sourced IPU3 IPA
Related show

Commit Message

Poduval, Karthik via libcamera-devel Oct. 4, 2021, 9:48 a.m. UTC
From: hanlinchen <hanlinchen@google.com>

The SharedItemPool was migrated from Chrome OS to maintain RGBS and AF
grids in IPAIPU3Stats. The orginal reason is to reserve the validness
unitl the grids are processed in a different thread. However, it leads
to a subtle bug which recycles the buffer before sending into AIQ
libraries. Since the IPU3 IPA (Unlike Chrome OS) process IPAIPU3Stats
in a single thread, It would be simpler to maintain the grids as the
members of IPAIPU3Stats, and remove the usage of SharedItemPool.

Signed-off-by: Han-Lin Chen <hanlinchen@google.com>
---
 stats/ipa_ipu3_stats.cpp   | 145 ++++++-------------------------------
 stats/ipa_ipu3_stats.h     |  41 +++++++----
 stats/meson.build          |   1 -
 stats/shared_item_pool.cpp | 129 ---------------------------------
 stats/shared_item_pool.h   | 114 -----------------------------
 5 files changed, 47 insertions(+), 383 deletions(-)
 delete mode 100644 stats/shared_item_pool.cpp
 delete mode 100644 stats/shared_item_pool.h

Comments

Umang Jain Oct. 5, 2021, 7:03 a.m. UTC | #1
Hi Han-Lin,

Thank you for the patch.


> From: hanlinchen<hanlinchen@google.com>
>
> The SharedItemPool was migrated from Chrome OS to maintain RGBS and AF
> grids in IPAIPU3Stats. The orginal reason is to reserve the validness
> unitl the grids are processed in a different thread. However, it leads


Ah, I think we were not aware of any thread processing in statistics at 
that point. Yes, maintaining a buffer pool would have made more sense in 
that case.

> to a subtle bug which recycles the buffer before sending into AIQ
> libraries. Since the IPU3 IPA (Unlike Chrome OS) process IPAIPU3Stats
> in a single thread, It would be simpler to maintain the grids as the
> members of IPAIPU3Stats, and remove the usage of SharedItemPool.


Agreed.

>
> Signed-off-by: Han-Lin Chen<hanlinchen@google.com>
> ---
>   stats/ipa_ipu3_stats.cpp   | 145 ++++++-------------------------------
>   stats/ipa_ipu3_stats.h     |  41 +++++++----
>   stats/meson.build          |   1 -
>   stats/shared_item_pool.cpp | 129 ---------------------------------
>   stats/shared_item_pool.h   | 114 -----------------------------
>   5 files changed, 47 insertions(+), 383 deletions(-)
>   delete mode 100644 stats/shared_item_pool.cpp
>   delete mode 100644 stats/shared_item_pool.h
>
> diff --git a/stats/ipa_ipu3_stats.cpp b/stats/ipa_ipu3_stats.cpp
> index c14bd7e..6697c9b 100644
> --- a/stats/ipa_ipu3_stats.cpp
> +++ b/stats/ipa_ipu3_stats.cpp
> @@ -15,30 +15,22 @@ namespacelibcamera::ipa::ipu3  {
>   
>   LOG_DEFINE_CATEGORY(IPAIPU3Stats)
>   
> -IPAIPU3Stats::IPAIPU3Stats()
> -{
> -	aiqStatsInputParams_ = {};
> -
> -	/* \todo: Is this fine here or we need separate helper? */
> -	rgbsGridBuffPool_ =std::make_shared<SharedItemPool<ia_aiq_rgbs_grid>>("RgbsGridBuffPool");
> -	afFilterBuffPool_ =std::make_shared<SharedItemPool<ia_aiq_af_grid>>("AfFilterBuffPool");
> -#define PUBLIC_STATS_POOL_SIZE 9 /* comes from CrOS */
> -	int ret = allocateStatBufferPools(PUBLIC_STATS_POOL_SIZE);
> -	if (ret < 0)
> -		LOG(IPAIPU3Stats, Error) << "Failed to allocate stats grid buffers";
> -}
> -
> -IPAIPU3Stats::~IPAIPU3Stats()
> -{
> -	freeStatBufferPools();
> -	rgbsGridBuffPool_.reset();
> -	afFilterBuffPool_.reset();
> -}
> -
>   ia_aiq_statistics_input_params *
>   IPAIPU3Stats::getInputStatsParams(int  frame,aiq::AiqResults  *results,
>   				  const ipu3_uapi_stats_3a *stats)
>   {
> +	IPU3AllStats::ipu3_stats_all_stats  outStats = {};
> +	IPU3AllStats::ipu3_stats_get_3a(&outStats, stats);
> +
> +	rgbsGrid_.blocks_ptr = rgbsGridBlock_;
> +
> +	afGrid_.filter_response_1 = filterResponse1_;
> +	afGrid_.filter_response_2 = filterResponse2_;
> +
> +	IPU3AllStats::intel_skycam_statistics_convert(
> +			outStats.ia_css_4a_statistics, &rgbsGrid_, &afGrid_);
> +
> +	aiqStatsInputParams_ = {};
>   	aiqStatsInputParams_.frame_id = frame;
>   	aiqStatsInputParams_.frame_ae_parameters = results->ae();
>   	aiqStatsInputParams_.frame_af_parameters = results->af();
> @@ -47,117 +39,24 @@IPAIPU3Stats::getInputStatsParams(int  frame,aiq::AiqResults  *results,
>   	aiqStatsInputParams_.frame_sa_parameters = results->sa();
>   	aiqStatsInputParams_.camera_orientation = ia_aiq_camera_orientation_unknown;
>   
> -	IPU3AllStats::ipu3_stats_all_stats  outStats;
> -	memset(&outStats, 0, sizeof(IPU3AllStats::ipu3_stats_all_stats));
> -	IPU3AllStats::ipu3_stats_get_3a(&outStats, stats);
> -
> -	std::shared_ptr<ia_aiq_rgbs_grid> rgbsGrid = nullptr;
> -	std::shared_ptr<ia_aiq_af_grid> afGrid = nullptr;
> -	int ret = afFilterBuffPool_->acquireItem(afGrid);
> -	ret |= rgbsGridBuffPool_->acquireItem(rgbsGrid);
> -	if (ret != 0 || afGrid.get() == nullptr || rgbsGrid.get() == nullptr) {
> -		LOG(IPAIPU3Stats, Error) << "Failed to acquire 3A buffers from pools";
> -		return nullptr;
> -	}
> -
> -	IPU3AllStats::intel_skycam_statistics_convert(outStats.ia_css_4a_statistics,
> -						      rgbsGrid.get(), afGrid.get());
> -
> -	const ia_aiq_rgbs_grid *rgbsGridPtr = rgbsGrid.get();
> -	const ia_aiq_af_grid *afGridPtr = afGrid.get();
> -
> +	rgbsGridPtr_ = &rgbsGrid_;
>   	aiqStatsInputParams_.num_rgbs_grids = 1;
> -	aiqStatsInputParams_.rgbs_grids = &rgbsGridPtr;
> +	aiqStatsInputParams_.rgbs_grids = &rgbsGridPtr_;
> +
> +	afGridPtr_ = &afGrid_;
>   	aiqStatsInputParams_.num_af_grids = 1;
> -	aiqStatsInputParams_.af_grids = &afGridPtr;
> +	aiqStatsInputParams_.af_grids = &afGridPtr_;
>   
>   	aiqStatsInputParams_.hdr_rgbs_grid = nullptr;
>   	aiqStatsInputParams_.depth_grids = nullptr;
>   
> -	return &aiqStatsInputParams_;
> -}
> +	aiqStatsInputParams_.num_external_histograms = 0;
> +	aiqStatsInputParams_.external_histograms = nullptr;


We were not setting this earlier, so +1 on adding them

>   
> -intIPAIPU3Stats::allocateStatBufferPools(int  numBufs)
> -{
> -	int ret = afFilterBuffPool_->init(numBufs);
> -	ret |= rgbsGridBuffPool_->init(numBufs);
> -	if (ret != 0) {
> -		LOG(IPAIPU3Stats, Error) << "Failed to initialize 3A statistics pools";
> -		freeStatBufferPools();
> -		return -ENOMEM;
> -	}
> -#define IPU3_MAX_STATISTICS_WIDTH 80
> -#define IPU3_MAX_STATISTICS_HEIGHT 60
> -	int maxGridSize = IPU3_MAX_STATISTICS_WIDTH * IPU3_MAX_STATISTICS_HEIGHT;
> -	std::shared_ptr<ia_aiq_rgbs_grid> rgbsGrid = nullptr;
> -	std::shared_ptr<ia_aiq_af_grid> afGrid = nullptr;
> -
> -	for (int allocated = 0; allocated < numBufs; allocated++) {
> -		ret = afFilterBuffPool_->acquireItem(afGrid);
> -		ret |= rgbsGridBuffPool_->acquireItem(rgbsGrid);
> -
> -		if (ret != 0 || afGrid.get() == nullptr ||
> -		    rgbsGrid.get() == nullptr) {
> -			LOG(IPAIPU3Stats, Error) << "Failed to acquire memory from pools";
> -			freeStatBufferPools();
> -			return -ENOMEM;
> -		}
> -
> -		rgbsGrid->blocks_ptr = new rgbs_grid_block[maxGridSize];
> -		rgbsGrid->grid_height = 0;
> -		rgbsGrid->grid_width = 0;
> -
> -		afGrid->filter_response_1 = new int[maxGridSize];
> -		afGrid->filter_response_2 = new int[maxGridSize];
> -		afGrid->block_height = 0;
> -		afGrid->block_width = 0;
> -		afGrid->grid_height = 0;
> -		afGrid->grid_width = 0;
> -	}
> -
> -	return 0;
> -}
> +	/* \todo: Fill the face state after the integration of face detection. */
> +	aiqStatsInputParams_.faces = nullptr;


this too, I like to see more and more fields being set,

>   
> -voidIPAIPU3Stats::freeStatBufferPools()
> -{
> -	if (!afFilterBuffPool_->isFull() || !rgbsGridBuffPool_->isFull()) {
> -		/* We will leak if we errored out in allocateStatBufferPools*/
> -		if (!afFilterBuffPool_->isFull())
> -			LOG(IPAIPU3Stats, Warning) << "AfFilterBuffPool is leaking";
> -		if (!rgbsGridBuffPool_->isFull())
> -			LOG(IPAIPU3Stats, Warning) << "RgbsGridBuffPool is leaking";
> -	}
> -
> -	int ret;
> -	size_t availableItems = afFilterBuffPool_->availableItems();
> -	std::shared_ptr<ia_aiq_af_grid> afGrid = nullptr;
> -	for (size_t i = 0; i < availableItems; i++) {
> -		ret = afFilterBuffPool_->acquireItem(afGrid);
> -		if (ret == 0 && afGrid.get() != nullptr) {
> -			delete[] afGrid->filter_response_1;
> -			afGrid->filter_response_1 = nullptr;
> -			delete[] afGrid->filter_response_2;
> -			afGrid->filter_response_2 = nullptr;
> -		} else {
> -			LOG(IPAIPU3Stats, Warning)
> -				<< "Could not acquire AF filter response "
> -				<< i << "for deletion - leak?";
> -		}
> -	}
> -
> -	availableItems = rgbsGridBuffPool_->availableItems();
> -	std::shared_ptr<ia_aiq_rgbs_grid> rgbsGrid = nullptr;
> -	for (size_t i = 0; i < availableItems; i++) {
> -		ret = rgbsGridBuffPool_->acquireItem(rgbsGrid);
> -		if (ret == 0 && rgbsGrid.get() != nullptr) {
> -			delete[] rgbsGrid->blocks_ptr;
> -			rgbsGrid->blocks_ptr = nullptr;
> -		} else {
> -			LOG(IPAIPU3Stats, Warning)
> -				<< "Could not acquire RGBS grid " << i
> -				<< "for deletion - leak?";
> -		}
> -	}
> +	return &aiqStatsInputParams_;
>   }
>   
>   } /* namespacelibcamera::ipa::ipu3  */
> diff --git a/stats/ipa_ipu3_stats.h b/stats/ipa_ipu3_stats.h
> index 4320024..5f59558 100644
> --- a/stats/ipa_ipu3_stats.h
> +++ b/stats/ipa_ipu3_stats.h
> @@ -5,38 +5,47 @@
>    * IPAIPU3Stats.cpp: Generate statistics in IA AIQ consumable format.
>    */
>   
> -#include "aiq/aiq_results.h"
> -
>   #ifndef IPA_IPU3_STATS_H
>   #define IPA_IPU3_STATS_H
>   
> -#include <ia_imaging/ia_aiq_types.h>
>   #include <linux/intel-ipu3.h>
> -
> -#include "shared_item_pool.h"
> +#include "aiq/aiq_results.h"
>   
>   namespacelibcamera::ipa::ipu3  {
>   
> +constexpr uint32_t IPU3MaxStatisticsWidth = 80;
> +constexpr uint32_t IPU3MaxStatisticsHeight = 60;
> +constexpr uint32_t IPU3MaxStatisticsGridSize =
> +	IPU3MaxStatisticsWidth * IPU3MaxStatisticsHeight;
> +


Are these max width/height same as ia_aiq_init() call when initializing 
the library? I think so, probably it makes sense to centralize them 
somewhere.

The patch makes sense to me so,

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

Thank you for your work Han-Lin.

>   struct AiqResults;
>   
>   class IPAIPU3Stats
>   {
>   public:
> -	IPAIPU3Stats();
> -	~IPAIPU3Stats();
> +	IPAIPU3Stats() = default;
> +	~IPAIPU3Stats() = default;
>   
> -	ia_aiq_statistics_input_params *
> -	getInputStatsParams(int frame,
> -			aiq::AiqResults  *results,
> -			    const ipu3_uapi_stats_3a *stats);
> +	ia_aiq_statistics_input_params* getInputStatsParams(
> +			int frame,
> +			aiq::AiqResults  *results,
> +			const ipu3_uapi_stats_3a *stats);
>   
>   private:
> -	void freeStatBufferPools();
> -	int allocateStatBufferPools(int numBufs);
> +	ia_aiq_statistics_input_params aiqStatsInputParams_ = {};
> +
> +	/*!< ia_aiq_statistics_input_params pointer contents */
> +	const ia_aiq_rgbs_grid* rgbsGridPtr_ = nullptr;
> +	const ia_aiq_af_grid* afGridPtr_ = nullptr;
> +	ia_aiq_rgbs_grid rgbsGrid_ = {};
> +	ia_aiq_af_grid afGrid_ = {};
> +
> +	/*!< ia_aiq_rgbs_grid pointer contents */
> +	rgbs_grid_block rgbsGridBlock_[IPU3MaxStatisticsGridSize] = {};
>   
> -	ia_aiq_statistics_input_params aiqStatsInputParams_;
> -	std::shared_ptr<SharedItemPool<ia_aiq_af_grid>> afFilterBuffPool_;
> -	std::shared_ptr<SharedItemPool<ia_aiq_rgbs_grid>> rgbsGridBuffPool_;
> +	/*!< ia_aiq_af_grid pointer contents */
> +	int filterResponse1_[IPU3MaxStatisticsGridSize] = {};
> +	int filterResponse2_[IPU3MaxStatisticsGridSize] = {};
>   };
>   
>   } /* namespacelibcamera::ipa::ipu3  */
> diff --git a/stats/meson.build b/stats/meson.build
> index 74ce657..e28a6b9 100644
> --- a/stats/meson.build
> +++ b/stats/meson.build
> @@ -3,6 +3,5 @@
>   ipu3_ipa_files += files([
>       'ipa_ipu3_stats.cpp',
>       'ipu3_all_stats.cpp',
> -    'shared_item_pool.cpp',
>   ])
>   
> diff --git a/stats/shared_item_pool.cpp b/stats/shared_item_pool.cpp
> deleted file mode 100644
> index 326a400..0000000
> --- a/stats/shared_item_pool.cpp
> +++ /dev/null
> @@ -1,129 +0,0 @@
> -/* SPDX-License-Identifier: Apache-2.0 */
> -/*
> - * Copyright (C) 2014-2018 Intel Corporation
> - *
> - * This implementation is highly derived from ChromeOS:
> - *   platform2/camera/hal/intel/ipu3/common/SharedItemPool.cpp
> - */
> -
> -#include <ia_imaging/ia_aiq_types.h>
> -
> -#include <libcamera/base/log.h>
> -
> -#include "shared_item_pool.h"
> -
> -namespace libcamera {
> -
> -LOG_DEFINE_CATEGORY(SharedItemPool)
> -
> -template<class ItemType>
> -SharedItemPool<ItemType>::SharedItemPool(const char *name)
> -	: allocated_(nullptr), capacity_(0), deleter_(this), poolName_(name),
> -	  resetter_(nullptr)
> -{
> -}
> -
> -template<class ItemType>
> -SharedItemPool<ItemType>::~SharedItemPool()
> -{
> -	deInit();
> -}
> -
> -template<class ItemType>
> -int SharedItemPool<ItemType>::init(int32_t capacity, void (*resetter)(ItemType *))
> -{
> -	if (capacity_ != 0) {
> -		LOG(SharedItemPool, Error) << "Pool initialized already";
> -		return -ENOSYS;
> -	}
> -	std::lock_guard<std::mutex>  l(mutex_);
> -	resetter_ = resetter;
> -	capacity_ = capacity;
> -	available_.reserve(capacity);
> -	allocated_ = new ItemType[capacity];
> -
> -	for (int32_t i = 0; i < capacity; i++)
> -		available_.push_back(&allocated_[i]);
> -
> -	LOG(SharedItemPool, Debug) << "Shared pool " << poolName_ << "init with " << capacity << " items";
> -
> -	return 0;
> -}
> -
> -template<class ItemType>
> -bool SharedItemPool<ItemType>::isFull()
> -{
> -	std::lock_guard<std::mutex>  l(mutex_);
> -	bool ret = (available_.size() == capacity_);
> -	return ret;
> -}
> -
> -template<class ItemType>
> -int SharedItemPool<ItemType>::deInit()
> -{
> -	std::lock_guard<std::mutex>  l(mutex_);
> -	if (capacity_ == 0) {
> -		LOG(SharedItemPool, Debug) << "Shared pool " << poolName_
> -					   << " isn't initialized or already de-initialized";
> -		return 0;
> -	}
> -	if (available_.size() != capacity_) {
> -		LOG(SharedItemPool, Error) << "Not all items are returned "
> -					   << "when destroying pool " << poolName_
> -					   << "(" << available_.size() << "/" << capacity_ << ")";
> -	}
> -
> -	delete[] allocated_;
> -	allocated_ = nullptr;
> -	available_.clear();
> -	capacity_ = 0;
> -	LOG(SharedItemPool, Debug) << "Shared pool " << poolName_
> -				   << " deinit done";
> -
> -	return 0;
> -}
> -
> -template<class ItemType>
> -int SharedItemPool<ItemType>::acquireItem(std::shared_ptr<ItemType> &item)
> -{
> -	item.reset();
> -	std::lock_guard<std::mutex>  l(mutex_);
> -	if (available_.empty()) {
> -		LOG(SharedItemPool, Error) << "Shared pool " << poolName_
> -					   << "is empty";
> -		return -ENOSYS;
> -	}
> -
> -	std::shared_ptr<ItemType> sh(available_[0], deleter_);
> -	available_.erase(available_.begin());
> -	item = sh;
> -	LOG(SharedItemPool, Debug) << "Shared pool " << poolName_
> -				   << "acquire items " << sh.get();
> -	return 0;
> -}
> -
> -template<class ItemType>
> -size_t SharedItemPool<ItemType>::availableItems()
> -{
> -	std::lock_guard<std::mutex>  l(mutex_);
> -	size_t ret = available_.size();
> -	return ret;
> -}
> -
> -template<class ItemType>
> -int SharedItemPool<ItemType>::_releaseItem(ItemType *item)
> -{
> -	std::lock_guard<std::mutex>  l(mutex_);
> -	if (resetter_)
> -		resetter_(item);
> -
> -	LOG(SharedItemPool, Debug) << "Shared pool " << poolName_
> -				   << "returning item " << item;
> -
> -	available_.push_back(item);
> -	return 0;
> -}
> -
> -template class SharedItemPool<ia_aiq_rgbs_grid>;
> -template class SharedItemPool<ia_aiq_af_grid>;
> -} /* namespace libcamera */
> diff --git a/stats/shared_item_pool.h b/stats/shared_item_pool.h
> deleted file mode 100644
> index 89dc9b3..0000000
> --- a/stats/shared_item_pool.h
> +++ /dev/null
> @@ -1,114 +0,0 @@
> -/* SPDX-License-Identifier: Apache-2.0 */
> -/*
> - * Copyright (C) 2014-2018 Intel Corporation
> - *
> - * This implementation is highly derived from ChromeOS:
> - *   platform2/camera/hal/intel/ipu3/common/SharedItemPool.h
> - */
> -
> -#ifndef SHARED_ITEM_POOL_H
> -#define SHARED_ITEM_POOL_H
> -
> -#include <memory>
> -#include <mutex>
> -#include <pthread.h>
> -#include <vector>
> -
> -/**
> - * \class SharedItemPool
> - *
> - * Pool of ref counted items. This class creates a pool of items and manages
> - * the acquisition of them. When all references to this item have disappeared
> - * The item is returned to the pool.
> - *
> - * This class is thread safe, i.e. it can be called from  multiple threads.
> - * When the element is recycled to the pool it can be reset via a client
> - * provided method.
> - *
> - */
> -
> -namespace libcamera {
> -
> -template<class ItemType>
> -class SharedItemPool
> -{
> -public:
> -	SharedItemPool(const char *name = "Unnamed");
> -	~SharedItemPool();
> -
> -	/**
> -	 * Initializes the capacity of the pool. It allocates the objects.
> -	 * optionally it will take function to reset the item before recycling
> -	 * it to the pool.
> -	 * This method is thread safe.
> -	 *
> -	 * \param capacity[IN]: Number of items the pool will hold
> -	 * \param resetter[IN]: Function to reset the item before recycling to the
> -	 *                      pool.
> -	 * \return -ENOSYS if trying to initialize twice
> -	 * \return 0 If everything went ok.
> -	 */
> -	int init(int32_t capacity, void (*resetter)(ItemType *) = nullptr);
> -
> -	bool isFull();
> -
> -	/**
> -	 * Free the resources of the pool
> -	 *
> -	 * \return 0 on success
> -	 */
> -	int deInit();
> -
> -	/**
> -	 * Acquire an item from the pool.
> -	 * This method is thread safe. Access to the internal acquire/release
> -	 * methods are protected.
> -	 * BUT the thread-safety for the utilization of the item after it has been
> -	 * acquired is the user's responsibility.
> -	 * Be careful not to provide the same item to multiple threads that write
> -	 * into it.
> -	 *
> -	 * \param item[OUT] shared pointer to an item.
> -	 * \return 0 on success
> -	 */
> -	int acquireItem(std::shared_ptr<ItemType> &item);
> -
> -	/**
> -	 * Returns the number of currently available items
> -	 * It there would be issues acquiring the lock the method returns 0
> -	 * available items.
> -	 *
> -	 * \return item count
> -	 */
> -	size_t availableItems();
> -
> -private:
> -	int _releaseItem(ItemType *item);
> -
> -	class ItemDeleter
> -	{
> -	public:
> -		ItemDeleter(SharedItemPool *pool)
> -			: mPool(pool) {}
> -		void operator()(ItemType *item) const
> -		{
> -			mPool->_releaseItem(item);
> -		}
> -
> -	private:
> -		SharedItemPool *mPool;
> -	};
> -
> -	std::vector<ItemType *> available_; /* SharedItemPool doesn't have ownership */
> -	ItemType *allocated_;
> -	size_t capacity_;
> -	ItemDeleter deleter_;
> -	std::mutex  mutex_; /* protects available_, allocated_, capacity_ */
> -	const char *poolName_;
> -	void (*resetter_)(ItemType *);
> -};
> -
> -} /* namespace libcamera */
> -
> -#endif /* SHARED_ITEM_POOL_H */
> -
> -- 2.33.0.800.g4c38ced690-goog

Patch
diff mbox series

diff --git a/stats/ipa_ipu3_stats.cpp b/stats/ipa_ipu3_stats.cpp
index c14bd7e..6697c9b 100644
--- a/stats/ipa_ipu3_stats.cpp
+++ b/stats/ipa_ipu3_stats.cpp
@@ -15,30 +15,22 @@  namespace libcamera::ipa::ipu3 {
 
 LOG_DEFINE_CATEGORY(IPAIPU3Stats)
 
-IPAIPU3Stats::IPAIPU3Stats()
-{
-	aiqStatsInputParams_ = {};
-
-	/* \todo: Is this fine here or we need separate helper? */
-	rgbsGridBuffPool_ = std::make_shared<SharedItemPool<ia_aiq_rgbs_grid>>("RgbsGridBuffPool");
-	afFilterBuffPool_ = std::make_shared<SharedItemPool<ia_aiq_af_grid>>("AfFilterBuffPool");
-#define PUBLIC_STATS_POOL_SIZE 9 /* comes from CrOS */
-	int ret = allocateStatBufferPools(PUBLIC_STATS_POOL_SIZE);
-	if (ret < 0)
-		LOG(IPAIPU3Stats, Error) << "Failed to allocate stats grid buffers";
-}
-
-IPAIPU3Stats::~IPAIPU3Stats()
-{
-	freeStatBufferPools();
-	rgbsGridBuffPool_.reset();
-	afFilterBuffPool_.reset();
-}
-
 ia_aiq_statistics_input_params *
 IPAIPU3Stats::getInputStatsParams(int frame, aiq::AiqResults *results,
 				  const ipu3_uapi_stats_3a *stats)
 {
+	IPU3AllStats::ipu3_stats_all_stats outStats = {};
+	IPU3AllStats::ipu3_stats_get_3a(&outStats, stats);
+
+	rgbsGrid_.blocks_ptr = rgbsGridBlock_;
+
+	afGrid_.filter_response_1 = filterResponse1_;
+	afGrid_.filter_response_2 = filterResponse2_;
+
+	IPU3AllStats::intel_skycam_statistics_convert(
+			outStats.ia_css_4a_statistics, &rgbsGrid_, &afGrid_);
+
+	aiqStatsInputParams_ = {};
 	aiqStatsInputParams_.frame_id = frame;
 	aiqStatsInputParams_.frame_ae_parameters = results->ae();
 	aiqStatsInputParams_.frame_af_parameters = results->af();
@@ -47,117 +39,24 @@  IPAIPU3Stats::getInputStatsParams(int frame, aiq::AiqResults *results,
 	aiqStatsInputParams_.frame_sa_parameters = results->sa();
 	aiqStatsInputParams_.camera_orientation = ia_aiq_camera_orientation_unknown;
 
-	IPU3AllStats::ipu3_stats_all_stats outStats;
-	memset(&outStats, 0, sizeof(IPU3AllStats::ipu3_stats_all_stats));
-	IPU3AllStats::ipu3_stats_get_3a(&outStats, stats);
-
-	std::shared_ptr<ia_aiq_rgbs_grid> rgbsGrid = nullptr;
-	std::shared_ptr<ia_aiq_af_grid> afGrid = nullptr;
-	int ret = afFilterBuffPool_->acquireItem(afGrid);
-	ret |= rgbsGridBuffPool_->acquireItem(rgbsGrid);
-	if (ret != 0 || afGrid.get() == nullptr || rgbsGrid.get() == nullptr) {
-		LOG(IPAIPU3Stats, Error) << "Failed to acquire 3A buffers from pools";
-		return nullptr;
-	}
-
-	IPU3AllStats::intel_skycam_statistics_convert(outStats.ia_css_4a_statistics,
-						      rgbsGrid.get(), afGrid.get());
-
-	const ia_aiq_rgbs_grid *rgbsGridPtr = rgbsGrid.get();
-	const ia_aiq_af_grid *afGridPtr = afGrid.get();
-
+	rgbsGridPtr_ = &rgbsGrid_;
 	aiqStatsInputParams_.num_rgbs_grids = 1;
-	aiqStatsInputParams_.rgbs_grids = &rgbsGridPtr;
+	aiqStatsInputParams_.rgbs_grids = &rgbsGridPtr_;
+
+	afGridPtr_ = &afGrid_;
 	aiqStatsInputParams_.num_af_grids = 1;
-	aiqStatsInputParams_.af_grids = &afGridPtr;
+	aiqStatsInputParams_.af_grids = &afGridPtr_;
 
 	aiqStatsInputParams_.hdr_rgbs_grid = nullptr;
 	aiqStatsInputParams_.depth_grids = nullptr;
 
-	return &aiqStatsInputParams_;
-}
+	aiqStatsInputParams_.num_external_histograms = 0;
+	aiqStatsInputParams_.external_histograms = nullptr;
 
-int IPAIPU3Stats::allocateStatBufferPools(int numBufs)
-{
-	int ret = afFilterBuffPool_->init(numBufs);
-	ret |= rgbsGridBuffPool_->init(numBufs);
-	if (ret != 0) {
-		LOG(IPAIPU3Stats, Error) << "Failed to initialize 3A statistics pools";
-		freeStatBufferPools();
-		return -ENOMEM;
-	}
-#define IPU3_MAX_STATISTICS_WIDTH 80
-#define IPU3_MAX_STATISTICS_HEIGHT 60
-	int maxGridSize = IPU3_MAX_STATISTICS_WIDTH * IPU3_MAX_STATISTICS_HEIGHT;
-	std::shared_ptr<ia_aiq_rgbs_grid> rgbsGrid = nullptr;
-	std::shared_ptr<ia_aiq_af_grid> afGrid = nullptr;
-
-	for (int allocated = 0; allocated < numBufs; allocated++) {
-		ret = afFilterBuffPool_->acquireItem(afGrid);
-		ret |= rgbsGridBuffPool_->acquireItem(rgbsGrid);
-
-		if (ret != 0 || afGrid.get() == nullptr ||
-		    rgbsGrid.get() == nullptr) {
-			LOG(IPAIPU3Stats, Error) << "Failed to acquire memory from pools";
-			freeStatBufferPools();
-			return -ENOMEM;
-		}
-
-		rgbsGrid->blocks_ptr = new rgbs_grid_block[maxGridSize];
-		rgbsGrid->grid_height = 0;
-		rgbsGrid->grid_width = 0;
-
-		afGrid->filter_response_1 = new int[maxGridSize];
-		afGrid->filter_response_2 = new int[maxGridSize];
-		afGrid->block_height = 0;
-		afGrid->block_width = 0;
-		afGrid->grid_height = 0;
-		afGrid->grid_width = 0;
-	}
-
-	return 0;
-}
+	/* \todo: Fill the face state after the integration of face detection. */
+	aiqStatsInputParams_.faces = nullptr;
 
-void IPAIPU3Stats::freeStatBufferPools()
-{
-	if (!afFilterBuffPool_->isFull() || !rgbsGridBuffPool_->isFull()) {
-		/* We will leak if we errored out in allocateStatBufferPools*/
-		if (!afFilterBuffPool_->isFull())
-			LOG(IPAIPU3Stats, Warning) << "AfFilterBuffPool is leaking";
-		if (!rgbsGridBuffPool_->isFull())
-			LOG(IPAIPU3Stats, Warning) << "RgbsGridBuffPool is leaking";
-	}
-
-	int ret;
-	size_t availableItems = afFilterBuffPool_->availableItems();
-	std::shared_ptr<ia_aiq_af_grid> afGrid = nullptr;
-	for (size_t i = 0; i < availableItems; i++) {
-		ret = afFilterBuffPool_->acquireItem(afGrid);
-		if (ret == 0 && afGrid.get() != nullptr) {
-			delete[] afGrid->filter_response_1;
-			afGrid->filter_response_1 = nullptr;
-			delete[] afGrid->filter_response_2;
-			afGrid->filter_response_2 = nullptr;
-		} else {
-			LOG(IPAIPU3Stats, Warning)
-				<< "Could not acquire AF filter response "
-				<< i << "for deletion - leak?";
-		}
-	}
-
-	availableItems = rgbsGridBuffPool_->availableItems();
-	std::shared_ptr<ia_aiq_rgbs_grid> rgbsGrid = nullptr;
-	for (size_t i = 0; i < availableItems; i++) {
-		ret = rgbsGridBuffPool_->acquireItem(rgbsGrid);
-		if (ret == 0 && rgbsGrid.get() != nullptr) {
-			delete[] rgbsGrid->blocks_ptr;
-			rgbsGrid->blocks_ptr = nullptr;
-		} else {
-			LOG(IPAIPU3Stats, Warning)
-				<< "Could not acquire RGBS grid " << i
-				<< "for deletion - leak?";
-		}
-	}
+	return &aiqStatsInputParams_;
 }
 
 } /* namespace libcamera::ipa::ipu3 */
diff --git a/stats/ipa_ipu3_stats.h b/stats/ipa_ipu3_stats.h
index 4320024..5f59558 100644
--- a/stats/ipa_ipu3_stats.h
+++ b/stats/ipa_ipu3_stats.h
@@ -5,38 +5,47 @@ 
  * IPAIPU3Stats.cpp: Generate statistics in IA AIQ consumable format.
  */
 
-#include "aiq/aiq_results.h"
-
 #ifndef IPA_IPU3_STATS_H
 #define IPA_IPU3_STATS_H
 
-#include <ia_imaging/ia_aiq_types.h>
 #include <linux/intel-ipu3.h>
-
-#include "shared_item_pool.h"
+#include "aiq/aiq_results.h"
 
 namespace libcamera::ipa::ipu3 {
 
+constexpr uint32_t IPU3MaxStatisticsWidth = 80;
+constexpr uint32_t IPU3MaxStatisticsHeight = 60;
+constexpr uint32_t IPU3MaxStatisticsGridSize =
+	IPU3MaxStatisticsWidth * IPU3MaxStatisticsHeight;
+
 struct AiqResults;
 
 class IPAIPU3Stats
 {
 public:
-	IPAIPU3Stats();
-	~IPAIPU3Stats();
+	IPAIPU3Stats() = default;
+	~IPAIPU3Stats() = default;
 
-	ia_aiq_statistics_input_params *
-	getInputStatsParams(int frame,
-			    aiq::AiqResults *results,
-			    const ipu3_uapi_stats_3a *stats);
+	ia_aiq_statistics_input_params* getInputStatsParams(
+			int frame,
+			aiq::AiqResults *results,
+			const ipu3_uapi_stats_3a *stats);
 
 private:
-	void freeStatBufferPools();
-	int allocateStatBufferPools(int numBufs);
+	ia_aiq_statistics_input_params aiqStatsInputParams_ = {};
+
+	/*!< ia_aiq_statistics_input_params pointer contents */
+	const ia_aiq_rgbs_grid* rgbsGridPtr_ = nullptr;
+	const ia_aiq_af_grid* afGridPtr_ = nullptr;
+	ia_aiq_rgbs_grid rgbsGrid_ = {};
+	ia_aiq_af_grid afGrid_ = {};
+
+	/*!< ia_aiq_rgbs_grid pointer contents */
+	rgbs_grid_block rgbsGridBlock_[IPU3MaxStatisticsGridSize] = {};
 
-	ia_aiq_statistics_input_params aiqStatsInputParams_;
-	std::shared_ptr<SharedItemPool<ia_aiq_af_grid>> afFilterBuffPool_;
-	std::shared_ptr<SharedItemPool<ia_aiq_rgbs_grid>> rgbsGridBuffPool_;
+	/*!< ia_aiq_af_grid pointer contents */
+	int filterResponse1_[IPU3MaxStatisticsGridSize] = {};
+	int filterResponse2_[IPU3MaxStatisticsGridSize] = {};
 };
 
 } /* namespace libcamera::ipa::ipu3 */
diff --git a/stats/meson.build b/stats/meson.build
index 74ce657..e28a6b9 100644
--- a/stats/meson.build
+++ b/stats/meson.build
@@ -3,6 +3,5 @@ 
 ipu3_ipa_files += files([
     'ipa_ipu3_stats.cpp',
     'ipu3_all_stats.cpp',
-    'shared_item_pool.cpp',
 ])
 
diff --git a/stats/shared_item_pool.cpp b/stats/shared_item_pool.cpp
deleted file mode 100644
index 326a400..0000000
--- a/stats/shared_item_pool.cpp
+++ /dev/null
@@ -1,129 +0,0 @@ 
-/* SPDX-License-Identifier: Apache-2.0 */
-/*
- * Copyright (C) 2014-2018 Intel Corporation
- *
- * This implementation is highly derived from ChromeOS:
- *   platform2/camera/hal/intel/ipu3/common/SharedItemPool.cpp
- */
-
-#include <ia_imaging/ia_aiq_types.h>
-
-#include <libcamera/base/log.h>
-
-#include "shared_item_pool.h"
-
-namespace libcamera {
-
-LOG_DEFINE_CATEGORY(SharedItemPool)
-
-template<class ItemType>
-SharedItemPool<ItemType>::SharedItemPool(const char *name)
-	: allocated_(nullptr), capacity_(0), deleter_(this), poolName_(name),
-	  resetter_(nullptr)
-{
-}
-
-template<class ItemType>
-SharedItemPool<ItemType>::~SharedItemPool()
-{
-	deInit();
-}
-
-template<class ItemType>
-int SharedItemPool<ItemType>::init(int32_t capacity, void (*resetter)(ItemType *))
-{
-	if (capacity_ != 0) {
-		LOG(SharedItemPool, Error) << "Pool initialized already";
-		return -ENOSYS;
-	}
-	std::lock_guard<std::mutex> l(mutex_);
-	resetter_ = resetter;
-	capacity_ = capacity;
-	available_.reserve(capacity);
-	allocated_ = new ItemType[capacity];
-
-	for (int32_t i = 0; i < capacity; i++)
-		available_.push_back(&allocated_[i]);
-
-	LOG(SharedItemPool, Debug) << "Shared pool " << poolName_ << "init with " << capacity << " items";
-
-	return 0;
-}
-
-template<class ItemType>
-bool SharedItemPool<ItemType>::isFull()
-{
-	std::lock_guard<std::mutex> l(mutex_);
-	bool ret = (available_.size() == capacity_);
-	return ret;
-}
-
-template<class ItemType>
-int SharedItemPool<ItemType>::deInit()
-{
-	std::lock_guard<std::mutex> l(mutex_);
-	if (capacity_ == 0) {
-		LOG(SharedItemPool, Debug) << "Shared pool " << poolName_
-					   << " isn't initialized or already de-initialized";
-		return 0;
-	}
-	if (available_.size() != capacity_) {
-		LOG(SharedItemPool, Error) << "Not all items are returned "
-					   << "when destroying pool " << poolName_
-					   << "(" << available_.size() << "/" << capacity_ << ")";
-	}
-
-	delete[] allocated_;
-	allocated_ = nullptr;
-	available_.clear();
-	capacity_ = 0;
-	LOG(SharedItemPool, Debug) << "Shared pool " << poolName_
-				   << " deinit done";
-
-	return 0;
-}
-
-template<class ItemType>
-int SharedItemPool<ItemType>::acquireItem(std::shared_ptr<ItemType> &item)
-{
-	item.reset();
-	std::lock_guard<std::mutex> l(mutex_);
-	if (available_.empty()) {
-		LOG(SharedItemPool, Error) << "Shared pool " << poolName_
-					   << "is empty";
-		return -ENOSYS;
-	}
-
-	std::shared_ptr<ItemType> sh(available_[0], deleter_);
-	available_.erase(available_.begin());
-	item = sh;
-	LOG(SharedItemPool, Debug) << "Shared pool " << poolName_
-				   << "acquire items " << sh.get();
-	return 0;
-}
-
-template<class ItemType>
-size_t SharedItemPool<ItemType>::availableItems()
-{
-	std::lock_guard<std::mutex> l(mutex_);
-	size_t ret = available_.size();
-	return ret;
-}
-
-template<class ItemType>
-int SharedItemPool<ItemType>::_releaseItem(ItemType *item)
-{
-	std::lock_guard<std::mutex> l(mutex_);
-	if (resetter_)
-		resetter_(item);
-
-	LOG(SharedItemPool, Debug) << "Shared pool " << poolName_
-				   << "returning item " << item;
-
-	available_.push_back(item);
-	return 0;
-}
-
-template class SharedItemPool<ia_aiq_rgbs_grid>;
-template class SharedItemPool<ia_aiq_af_grid>;
-} /* namespace libcamera */
diff --git a/stats/shared_item_pool.h b/stats/shared_item_pool.h
deleted file mode 100644
index 89dc9b3..0000000
--- a/stats/shared_item_pool.h
+++ /dev/null
@@ -1,114 +0,0 @@ 
-/* SPDX-License-Identifier: Apache-2.0 */
-/*
- * Copyright (C) 2014-2018 Intel Corporation
- *
- * This implementation is highly derived from ChromeOS:
- *   platform2/camera/hal/intel/ipu3/common/SharedItemPool.h
- */
-
-#ifndef SHARED_ITEM_POOL_H
-#define SHARED_ITEM_POOL_H
-
-#include <memory>
-#include <mutex>
-#include <pthread.h>
-#include <vector>
-
-/**
- * \class SharedItemPool
- *
- * Pool of ref counted items. This class creates a pool of items and manages
- * the acquisition of them. When all references to this item have disappeared
- * The item is returned to the pool.
- *
- * This class is thread safe, i.e. it can be called from  multiple threads.
- * When the element is recycled to the pool it can be reset via a client
- * provided method.
- *
- */
-
-namespace libcamera {
-
-template<class ItemType>
-class SharedItemPool
-{
-public:
-	SharedItemPool(const char *name = "Unnamed");
-	~SharedItemPool();
-
-	/**
-	 * Initializes the capacity of the pool. It allocates the objects.
-	 * optionally it will take function to reset the item before recycling
-	 * it to the pool.
-	 * This method is thread safe.
-	 *
-	 * \param capacity[IN]: Number of items the pool will hold
-	 * \param resetter[IN]: Function to reset the item before recycling to the
-	 *                      pool.
-	 * \return -ENOSYS if trying to initialize twice
-	 * \return 0 If everything went ok.
-	 */
-	int init(int32_t capacity, void (*resetter)(ItemType *) = nullptr);
-
-	bool isFull();
-
-	/**
-	 * Free the resources of the pool
-	 *
-	 * \return 0 on success
-	 */
-	int deInit();
-
-	/**
-	 * Acquire an item from the pool.
-	 * This method is thread safe. Access to the internal acquire/release
-	 * methods are protected.
-	 * BUT the thread-safety for the utilization of the item after it has been
-	 * acquired is the user's responsibility.
-	 * Be careful not to provide the same item to multiple threads that write
-	 * into it.
-	 *
-	 * \param item[OUT] shared pointer to an item.
-	 * \return 0 on success
-	 */
-	int acquireItem(std::shared_ptr<ItemType> &item);
-
-	/**
-	 * Returns the number of currently available items
-	 * It there would be issues acquiring the lock the method returns 0
-	 * available items.
-	 *
-	 * \return item count
-	 */
-	size_t availableItems();
-
-private:
-	int _releaseItem(ItemType *item);
-
-	class ItemDeleter
-	{
-	public:
-		ItemDeleter(SharedItemPool *pool)
-			: mPool(pool) {}
-		void operator()(ItemType *item) const
-		{
-			mPool->_releaseItem(item);
-		}
-
-	private:
-		SharedItemPool *mPool;
-	};
-
-	std::vector<ItemType *> available_; /* SharedItemPool doesn't have ownership */
-	ItemType *allocated_;
-	size_t capacity_;
-	ItemDeleter deleter_;
-	std::mutex mutex_; /* protects available_, allocated_, capacity_ */
-	const char *poolName_;
-	void (*resetter_)(ItemType *);
-};
-
-} /* namespace libcamera */
-
-#endif /* SHARED_ITEM_POOL_H */
-