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 */ - From patchwork Mon Oct 4 09:48:22 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: 14038 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 9DF76C3243 for ; Mon, 4 Oct 2021 09:48:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6C5B7691B8; Mon, 4 Oct 2021 11:48:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1633340917; bh=aZs5cb3UIqcfxO5bxh/cT5pGZJMPl8t/1xB8xduk254=; 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=BYhZ81zUOvDgmUcD6PLBfGmyhg3iNbf/9tBokrfXtmxB8Hj78usxQdeOXYiMMNx9u 9Kgu51WXqUDlPCT2TfO2myEaMq8EgvpRSzU4TteqEczZcL9A/kqAOUStlBNWvJ/gm5 p0dtRlmW599JgQlxNWkeHoKhVW3Cr9uRyscVYRwF9n0pYVkIel57Lla2tiCN1lHLY4 eKDuU6GlsYhJy6EvkWCaSBkCqNGJEuMoe04cXfvE6E+24gRuhayuI6+INd/KhC0aOt IJqnDJQybhkISNJe+gg11MmueuXTQGqnXFndi4sPtbUIPZ8u8mdpvdddRf+c/9t3im ke8RqE+UcpEZQ== Date: Mon, 4 Oct 2021 17:48:22 +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 2/3] ipu3: Change Macro migrated from Chrome OS to its std version accordingly Content-Disposition: inline Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: hanlinchen Change the Macro STDCOPY, MEMCPY_S and CLEAR to its std version to better suit the style. The patch also fix misusage of STDCOPY as memcpy, which leads to copying overflown in PA and SA results. Signed-off-by: Han-Lin Chen Reviewed-by: Umang Jain --- aiq/aiq_input_parameters.cpp | 50 +++++++++----------- aiq/aiq_results.cpp | 91 +++++++++++++++--------------------- 2 files changed, 58 insertions(+), 83 deletions(-) diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp index 56301f6..56e8513 100644 --- a/aiq/aiq_input_parameters.cpp +++ b/aiq/aiq_input_parameters.cpp @@ -14,11 +14,6 @@ #include -/* Macros used by imported code */ -#define STDCOPY(dst, src, size) std::copy((src), ((src) + (size)), (dst)) -#define MEMCPY_S(dest, dmax, src, smax) memcpy((dest), (src), std::min((size_t)(dmax), (size_t)(smax))) -#define CLEAR(x) memset(&(x), 0, sizeof(x)) - namespace libcamera { LOG_DEFINE_CATEGORY(AIQInputParameters) @@ -27,26 +22,26 @@ namespace ipa::ipu3::aiq { void AiqInputParameters::init() { - CLEAR(aeInputParams); - CLEAR(afParams); - CLEAR(afBracketParams); - CLEAR(awbParams); - CLEAR(gbceParams); - CLEAR(paParams); - CLEAR(saParams); - CLEAR(sensorDescriptor); - CLEAR(exposureWindow); - CLEAR(exposureCoordinate); - CLEAR(aeFeatures); - CLEAR(aeManualLimits); - CLEAR(manualFocusParams); - CLEAR(focusRect); - CLEAR(manualCctRange); - CLEAR(manualWhiteCoordinate); - CLEAR(awbResults); - CLEAR(colorGains); - CLEAR(exposureParams); - CLEAR(sensorFrameParams); + memset(&aeInputParams, 0, sizeof(aeInputParams)); + memset(&afParams, 0, sizeof(afParams)); + memset(&afBracketParams, 0, sizeof(afBracketParams)); + memset(&awbParams, 0, sizeof(awbParams)); + memset(&gbceParams, 0, sizeof(gbceParams)); + memset(&paParams, 0, sizeof(paParams)); + memset(&saParams, 0, sizeof(saParams)); + memset(&sensorDescriptor, 0, sizeof(sensorDescriptor)); + memset(&exposureWindow, 0, sizeof(exposureWindow)); + memset(&exposureCoordinate, 0, sizeof(exposureCoordinate)); + memset(&aeFeatures, 0, sizeof(aeFeatures)); + memset(&aeManualLimits, 0, sizeof(aeManualLimits)); + memset(&manualFocusParams, 0, sizeof(manualFocusParams)); + memset(&focusRect, 0, sizeof(focusRect)); + memset(&manualCctRange, 0, sizeof(manualCctRange)); + memset(&manualWhiteCoordinate, 0, sizeof(manualWhiteCoordinate)); + memset(&awbResults, 0, sizeof(awbResults)); + memset(&colorGains, 0, sizeof(colorGains)); + memset(&exposureParams, 0, sizeof(exposureParams)); + memset(&sensorFrameParams, 0, sizeof(sensorFrameParams)); aeLock = false; awbLock = false; blackLevelLock = false; @@ -102,10 +97,7 @@ AiqInputParameters &AiqInputParameters::operator=(const AiqInputParameters &othe if (this == &other) return *this; - MEMCPY_S(this, - sizeof(AiqInputParameters), - &other, - sizeof(AiqInputParameters)); + memcpy(this, &other, sizeof(AiqInputParameters)); reset(); /* Exposure coordinate is nullptr in other than SPOT mode. */ diff --git a/aiq/aiq_results.cpp b/aiq/aiq_results.cpp index 9dda17c..f727f36 100644 --- a/aiq/aiq_results.cpp +++ b/aiq/aiq_results.cpp @@ -14,9 +14,6 @@ #include -/* Macros used by imported code */ -#define STDCOPY(dst, src, size) std::copy((src), ((src) + (size)), (dst)) - namespace libcamera { LOG_DEFINE_CATEGORY(AIQResults) @@ -111,17 +108,14 @@ void AiqResults::setAe(ia_aiq_ae_results *ae) ae->weight_grid->height; gridElements = std::clamp(gridElements, 1, MAX_AE_GRID_SIZE); - STDCOPY(ae_.weight_grid->weights, - ae->weight_grid->weights, - gridElements * sizeof(char)); + std::copy_n(ae->weight_grid->weights, gridElements, ae_.weight_grid->weights); } else { LOG(AIQResults, Error) << "Not copying AE Weight Grids"; } // Copy the flash info structure if (ae_.flashes && ae->flashes) { - STDCOPY((int8_t *)ae_.flashes, (int8_t *)ae->flashes, - NUM_FLASH_LEDS * sizeof(ia_aiq_flash_parameters)); + std::copy_n(ae->flashes, NUM_FLASH_LEDS, ae_.flashes); } else { LOG(AIQResults, Error) << "Not copying AE Flashes"; } @@ -172,20 +166,16 @@ void AiqResults::setGbce(ia_aiq_gbce_results *gbce) gbce_.gamma_lut_size = gbce->gamma_lut_size; - STDCOPY((int8_t *)gbce_.r_gamma_lut, (int8_t *)gbce->r_gamma_lut, - gbce->gamma_lut_size * sizeof(float)); - STDCOPY((int8_t *)gbce_.b_gamma_lut, (int8_t *)gbce->b_gamma_lut, - gbce->gamma_lut_size * sizeof(float)); - STDCOPY((int8_t *)gbce_.g_gamma_lut, (int8_t *)gbce->g_gamma_lut, - gbce->gamma_lut_size * sizeof(float)); + std::copy_n(gbce->r_gamma_lut, gbce->gamma_lut_size, gbce_.r_gamma_lut); + std::copy_n(gbce->b_gamma_lut, gbce->gamma_lut_size, gbce_.b_gamma_lut); + std::copy_n(gbce->g_gamma_lut, gbce->gamma_lut_size, gbce_.g_gamma_lut); } else { LOG(AIQResults, Error) << "Not copying Gamma LUT channels"; } if (gbce->tone_map_lut_size > 0) { gbce_.tone_map_lut_size = gbce->tone_map_lut_size; - STDCOPY((int8_t *)gbce_.tone_map_lut, (int8_t *)gbce->tone_map_lut, - gbce->tone_map_lut_size * sizeof(float)); + std::copy_n(gbce->tone_map_lut, gbce->tone_map_lut_size, gbce_.tone_map_lut); } else { LOG(AIQResults, Error) << "Not copying Tone Mapping Gain LUT"; } @@ -200,20 +190,20 @@ void AiqResults::setPa(ia_aiq_pa_results *pa) { ASSERT(pa); - STDCOPY(&pa_.color_conversion_matrix[0][0], &pa->color_conversion_matrix[0][0], - MAX_COLOR_CONVERSION_MATRIX * MAX_COLOR_CONVERSION_MATRIX * - sizeof(pa->color_conversion_matrix[0][0])); + std::copy_n(&pa->color_conversion_matrix[0][0], + MAX_COLOR_CONVERSION_MATRIX * MAX_COLOR_CONVERSION_MATRIX, + &pa_.color_conversion_matrix[0][0]); if (pa_.preferred_acm && pa->preferred_acm) { pa_.preferred_acm->sector_count = pa->preferred_acm->sector_count; - STDCOPY(pa_.preferred_acm->hue_of_sectors, - pa->preferred_acm->hue_of_sectors, - sizeof(*pa->preferred_acm->hue_of_sectors) * pa->preferred_acm->sector_count); + std::copy_n(pa->preferred_acm->hue_of_sectors, + pa->preferred_acm->sector_count, + pa_.preferred_acm->hue_of_sectors); - STDCOPY(pa_.preferred_acm->advanced_color_conversion_matrices[0][0], - pa->preferred_acm->advanced_color_conversion_matrices[0][0], - sizeof(*pa->preferred_acm->advanced_color_conversion_matrices) * pa->preferred_acm->sector_count); + std::copy_n(pa->preferred_acm->advanced_color_conversion_matrices[0][0], + pa->preferred_acm->sector_count, + pa_.preferred_acm->advanced_color_conversion_matrices[0][0]); } else { LOG(AIQResults, Error) << "Not copying PA hue of sectors"; } @@ -222,17 +212,17 @@ void AiqResults::setPa(ia_aiq_pa_results *pa) pa_.ir_weight->height = pa->ir_weight->height; pa_.ir_weight->width = pa->ir_weight->width; - STDCOPY(pa_.ir_weight->ir_weight_grid_R, - pa->ir_weight->ir_weight_grid_R, - sizeof(*pa->ir_weight->ir_weight_grid_R) * pa->ir_weight->height * pa->ir_weight->width); + std::copy_n(pa->ir_weight->ir_weight_grid_R, + pa->ir_weight->height * pa->ir_weight->width, + pa_.ir_weight->ir_weight_grid_R); - STDCOPY(pa_.ir_weight->ir_weight_grid_G, - pa->ir_weight->ir_weight_grid_G, - sizeof(*pa->ir_weight->ir_weight_grid_G) * pa->ir_weight->height * pa->ir_weight->width); + std::copy_n(pa->ir_weight->ir_weight_grid_G, + pa->ir_weight->height * pa->ir_weight->width, + pa_.ir_weight->ir_weight_grid_G); - STDCOPY(pa_.ir_weight->ir_weight_grid_B, - pa->ir_weight->ir_weight_grid_B, - sizeof(*pa->ir_weight->ir_weight_grid_B) * pa->ir_weight->height * pa->ir_weight->width); + std::copy_n(pa->ir_weight->ir_weight_grid_B, + pa->ir_weight->height * pa->ir_weight->width, + pa_.ir_weight->ir_weight_grid_B); } else { LOG(AIQResults, Error) << "Not copying IR weight"; } @@ -253,13 +243,13 @@ void AiqResults::setSa(ia_aiq_sa_results *sa) sa_.height = sa->height; sa_.lsc_update = sa->lsc_update; + uint32_t lscGridSize = sa_.width * sa_.height; /* Check against one of the vectors but resize applicable to all. */ - if (channelGr_.size() < (sa_.width * sa_.height)) { - int lscNewSize = sa_.width * sa_.height; - channelGr_.resize(lscNewSize); - channelGb_.resize(lscNewSize); - channelR_.resize(lscNewSize); - channelB_.resize(lscNewSize); + if (channelGr_.size() < lscGridSize) { + channelGr_.resize(lscGridSize); + channelGb_.resize(lscGridSize); + channelR_.resize(lscGridSize); + channelB_.resize(lscGridSize); /* Update the SA data pointers to new memory locations. */ sa_.channel_gr = channelGr_.data(); @@ -269,23 +259,16 @@ void AiqResults::setSa(ia_aiq_sa_results *sa) } if (sa->lsc_update) { - uint32_t memCopySize = sa->width * sa->height * sizeof(float); - - STDCOPY((int8_t *)sa_.channel_gr, (int8_t *)sa->channel_gb, - memCopySize); - STDCOPY((int8_t *)sa_.channel_gb, (int8_t *)sa->channel_gr, - memCopySize); - STDCOPY((int8_t *)sa_.channel_r, (int8_t *)sa->channel_r, - memCopySize); - STDCOPY((int8_t *)sa_.channel_b, (int8_t *)sa->channel_b, - memCopySize); + std::copy_n(sa->channel_gr, lscGridSize, sa_.channel_gr); + std::copy_n(sa->channel_gb, lscGridSize, sa_.channel_gb); + std::copy_n(sa->channel_r, lscGridSize, sa_.channel_r); + std::copy_n(sa->channel_b, lscGridSize, sa_.channel_b); } else { - LOG(AIQResults, Error) << "Not copying LSC tables."; + LOG(AIQResults, Debug) << "Not copying LSC tables."; } - STDCOPY(&sa_.light_source[0], - &sa->light_source[0], - CMC_NUM_LIGHTSOURCES * sizeof(sa->light_source[0])); + std::copy_n(&sa->light_source[0], CMC_NUM_LIGHTSOURCES, &sa_.light_source[0]); + sa_.scene_difficulty = sa->scene_difficulty; sa_.num_patches = sa->num_patches; sa_.covered_area = sa->covered_area; From patchwork Mon Oct 4 09:48:23 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: 14039 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 F2E9CC3243 for ; Mon, 4 Oct 2021 09:48:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C2D2C691BA; Mon, 4 Oct 2021 11:48:41 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1633340921; bh=CQCiNmFY6TypvPrj+vKdOrxnkdSy54b57Pb0TfSkJFA=; 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=1ULQG2llsUywAKunETAqH9cHLCF04R+YXHmlnmYyQigadTYDOZAFvjJ0M8UMXokJT NGcIuO1CczGwVQDHOxrHR5Cg80sKoxroHUpbQ1YlX0AHEao17oY0QMF9/Z9fUtDwUF CBw91utjIchwlswayBX0xss+odrXSVHvqmsfKfK+55bG1WAtV3BwAI2VYyo0896fBB KCCOUQv2gc0nhb8d6rSLY8zLBVz3vHPuqITBawrLdUYGjKBrEJyqDRkkfvdQaBQ1sN aOcPwhyU4G7+wSLz7/4uqizX+uV0XIWv7hEnwZm4Okeo1jrBu1bO69s10EMsmplA4y 6jB3VxE/jrC2Q== Date: Mon, 4 Oct 2021 17:48:23 +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 3/3] ipu3: Apply shading adapter as part of AIQ::run2a() Content-Disposition: inline Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: hanlinchen Apply shading adapter to correct lens shading for both camera. Signed-off-by: Han-Lin Chen --- aiq/aiq.cpp | 20 ++++++++++++++++++-- aiq/aiq.h | 4 +++- ipu3.cpp | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp index 708e9d6..0a92eaf 100644 --- a/aiq/aiq.cpp +++ b/aiq/aiq.cpp @@ -20,7 +20,8 @@ LOG_DEFINE_CATEGORY(AIQ) namespace ipa::ipu3::aiq { AIQ::AIQ() - : aiq_(nullptr) + : aiq_(nullptr), + sensor_frame_params_{} { LOG(AIQ, Info) << "Creating IA AIQ Wrapper"; } @@ -105,10 +106,19 @@ int AIQ::init(BinaryData &aiqb, BinaryData &nvm, BinaryData &aiqd) return 0; } -int AIQ::configure() +int AIQ::configure(const struct IPAConfigInfo &configInfo) { LOG(AIQ, Debug) << "Configure AIQ"; + sensor_frame_params_.horizontal_crop_offset = 0; + sensor_frame_params_.vertical_crop_offset = 0; + sensor_frame_params_.cropped_image_width = configInfo.sensorInfo.analogCrop.width; + sensor_frame_params_.cropped_image_height = configInfo.sensorInfo.analogCrop.height; + sensor_frame_params_.horizontal_scaling_numerator = 1; + sensor_frame_params_.horizontal_scaling_denominator = 1; + sensor_frame_params_.vertical_scaling_numerator = 1; + sensor_frame_params_.vertical_scaling_denominator = 1; + return 0; } @@ -154,6 +164,11 @@ int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, params.paParams.exposure_params = results.ae()->exposures[0].exposure; parameterAdapterRun(params.paParams, results); + params.saParams.frame_use = params.aeInputParams.frame_use; + params.saParams.sensor_frame_params = &sensor_frame_params_; + params.saParams.awb_results = results.awb(); + shadingAdapterRun(params.saParams, results); + afRun(params.afParams, results); return 0; @@ -328,6 +343,7 @@ int AIQ::shadingAdapterRun(ia_aiq_sa_input_params &saParams, { ia_aiq_sa_results *saResults = nullptr; ia_err err = ia_aiq_sa_run(aiq_, &saParams, &saResults); + if (err) { LOG(AIQ, Error) << "Failed to run shading adapter: " << decodeError(err); diff --git a/aiq/aiq.h b/aiq/aiq.h index fcd02d2..8a68827 100644 --- a/aiq/aiq.h +++ b/aiq/aiq.h @@ -35,7 +35,7 @@ public: ~AIQ(); int init(BinaryData &aiqb, BinaryData &nvm, BinaryData &aiqd); - int configure(); + int configure(const struct IPAConfigInfo &configInfo); int setStatistics(unsigned int frame, int64_t timestamp, AiqResults &results, const ipu3_uapi_stats_3a *stats); @@ -62,6 +62,8 @@ private: ia_cmc_t *iaCmc_; std::string version_; + ia_aiq_frame_params sensor_frame_params_; + IPAIPU3Stats *aiqStats_; }; diff --git a/ipu3.cpp b/ipu3.cpp index b60c58c..ed3c516 100644 --- a/ipu3.cpp +++ b/ipu3.cpp @@ -232,7 +232,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) int ret; - ret = aiq_.configure(); + ret = aiq_.configure(configInfo); if (ret) { LOG(IPAIPU3, Error) << "Failed to configure the AIQ"; return ret;