| Message ID | mailman.444.1633340913.837.libcamera-devel@lists.libcamera.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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
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 */ -