From patchwork Mon Oct 4 09:48:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han-lin Chen X-Patchwork-Id: 14037 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 4C12CC3243 for ; Mon, 4 Oct 2021 09:48:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 11D96691BE; Mon, 4 Oct 2021 11:48:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1633340914; bh=neqq77iJV8bLuFmcGciceErKBHn2+dmG7Xu3ReNiHVI=; h=Date:In-Reply-To:References:To:List-Id:List-Post:From: List-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help: Subject:From; b=a/yPMdRVEArbK+AWS+jPml6B8Ad7eiVHvr3YHO/gfkAPeEqiFHVrXMDsfvsx0euUR 8lGI2W8CUz2dUzxtIFwDZbZ/sjTtvtR7SXBppUJvz+u0lo5fwWMZY0bb25b742B5yr SM8JCnaHXnS9h+x7BHkxNqeheX/XXzyWrWYRUzCEzUJGojhuxTLHid+3GeK1FmTCCt Q+XP1POc0dgYBP6l4aHN8Fxqm2yprx2qUx0vk6QQO34a5LYi8gN4ix2nhCov+Cmi9Y ttsJYx2sjcJQd1SyjtqTeeblpXHcx3Yd5tAp6JkD8+F/xiJ4yFDv44C/k9bWYLI6yE m91lCPK1hWjnw== Date: Mon, 4 Oct 2021 17:48:21 +0800 In-Reply-To: <20211004094823.260789-1-hanlinchen@google.com> References: <20211004094823.260789-1-hanlinchen@google.com> To: libcamera-devel@lists.libcamera.org MIME-Version: 1.0 Message-ID: List-Id: List-Post: X-Patchwork-Original-From: Han-Lin Chen via libcamera-devel From: Han-lin Chen Precedence: list X-Mailman-Version: 2.1.29 X-BeenThere: libcamera-devel@lists.libcamera.org List-Subscribe: , List-Unsubscribe: , List-Archive: Reply-To: Han-Lin Chen List-Help: Subject: [libcamera-devel] [PATCH 1/3] ipu3: Remove the usage of SharedItemPool Content-Disposition: inline Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: hanlinchen 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 Reviewed-by: Umang Jain --- 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 @@ 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>("RgbsGridBuffPool"); - afFilterBuffPool_ = std::make_shared>("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 rgbsGrid = nullptr; - std::shared_ptr 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 rgbsGrid = nullptr; - std::shared_ptr 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 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 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 #include - -#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> afFilterBuffPool_; - std::shared_ptr> 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 - -#include - -#include "shared_item_pool.h" - -namespace libcamera { - -LOG_DEFINE_CATEGORY(SharedItemPool) - -template -SharedItemPool::SharedItemPool(const char *name) - : allocated_(nullptr), capacity_(0), deleter_(this), poolName_(name), - resetter_(nullptr) -{ -} - -template -SharedItemPool::~SharedItemPool() -{ - deInit(); -} - -template -int SharedItemPool::init(int32_t capacity, void (*resetter)(ItemType *)) -{ - if (capacity_ != 0) { - LOG(SharedItemPool, Error) << "Pool initialized already"; - return -ENOSYS; - } - std::lock_guard 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 -bool SharedItemPool::isFull() -{ - std::lock_guard l(mutex_); - bool ret = (available_.size() == capacity_); - return ret; -} - -template -int SharedItemPool::deInit() -{ - std::lock_guard 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 -int SharedItemPool::acquireItem(std::shared_ptr &item) -{ - item.reset(); - std::lock_guard l(mutex_); - if (available_.empty()) { - LOG(SharedItemPool, Error) << "Shared pool " << poolName_ - << "is empty"; - return -ENOSYS; - } - - std::shared_ptr sh(available_[0], deleter_); - available_.erase(available_.begin()); - item = sh; - LOG(SharedItemPool, Debug) << "Shared pool " << poolName_ - << "acquire items " << sh.get(); - return 0; -} - -template -size_t SharedItemPool::availableItems() -{ - std::lock_guard l(mutex_); - size_t ret = available_.size(); - return ret; -} - -template -int SharedItemPool::_releaseItem(ItemType *item) -{ - std::lock_guard l(mutex_); - if (resetter_) - resetter_(item); - - LOG(SharedItemPool, Debug) << "Shared pool " << poolName_ - << "returning item " << item; - - available_.push_back(item); - return 0; -} - -template class SharedItemPool; -template class SharedItemPool; -} /* 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 -#include -#include -#include - -/** - * \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 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 &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 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 */ -