ipu3: Remove the usage of SharedItemPool

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@chromium.org>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

5 files changed, 47 insertions, 383 deletions

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 */
-