From patchwork Thu Oct 7 07:21:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanlin Chen X-Patchwork-Id: 14067 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 937DCC323E for ; Thu, 7 Oct 2021 07:22:14 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5F940691BA; Thu, 7 Oct 2021 09:22:14 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="DdVJjtZy"; dkim-atps=neutral Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 61C9C69189 for ; Thu, 7 Oct 2021 09:22:12 +0200 (CEST) Received: by mail-pg1-x531.google.com with SMTP id r2so4821475pgl.10 for ; Thu, 07 Oct 2021 00:22:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=mqoroVLP45q/3Elje5qFmKzQRIGkFEMA0ANJVAprPW0=; b=DdVJjtZy6kF1fcXDmtaHC5d+b4u9UqTssT09MNz3FqZzbvcheX5LLQdRMpFTnpaV84 CVo2xIs1eNJoqtByOGT9JbHPJXmJJ29FYWlEB61+XH64K3+ELWvJIiR6YGCo1tzFqTNJ WFZuE1/UlDyWc5dWw9GjNXq+xSJ8WAWTBqONU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=mqoroVLP45q/3Elje5qFmKzQRIGkFEMA0ANJVAprPW0=; b=00BdLGmk6yH1/jW56v/FCya7w/fTtQHEya36FCzxId7cExGIGZPV6rQKAvGzuAADEg TSAbYK2IROV428dpFUVQ1t/IuD4aAdeiIxUta85N+LgGpHkiFSsKjTfD87RqHjtfrL6O LRlRKs7klGLW6GvZMwy5UdLTzEcfUlH7Mp5Yha20g3EDdj2cqxfkrR9hX4OsLUz6XvRa 8qDMEF0afBKBqFKyRSuPAj2eHRwRWcmgpP/BrnYkye5+z07I0Oa8TxOITXKEx3rXQZZC DHqnlnrM07z6Ysw5LaE8SpoCey1UbmgLp92HwBOGXCuQ+NbgRjSut/nn7V1fUcELa3S3 4Wtw== X-Gm-Message-State: AOAM532+aBBrELXCxSgV1hdautuXSy6GROpzXAemvfZbsmHJLe8kn+Jo U3tGecmIxhTsoTI/2N/E/slxyhcw3BWMdEVuFxQ= X-Google-Smtp-Source: ABdhPJyFSs9+ovFAMtOvAY/ra530X/XtNHOYq5xiTEufXs0nkSE0+8nv/Yntl05B8Ydlh6qjVi1CTw== X-Received: by 2002:a63:df16:: with SMTP id u22mr2124239pgg.150.1633591330269; Thu, 07 Oct 2021 00:22:10 -0700 (PDT) Received: from localhost ([2401:fa00:1:10:db0a:f55:d7cb:95ae]) by smtp.gmail.com with UTF8SMTPSA id x193sm12148156pfd.57.2021.10.07.00.22.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Oct 2021 00:22:10 -0700 (PDT) From: Han-Lin Chen To: libcamera-devel@lists.libcamera.org Date: Thu, 7 Oct 2021 15:21:45 +0800 Message-Id: <20211007072147.1289490-1-hanlinchen@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/3] ipu3: Remove the usage of SharedItemPool X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 Acked-by: Kieran Bingham --- 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 Thu Oct 7 07:21:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanlin Chen X-Patchwork-Id: 14068 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 118B9C323E for ; Thu, 7 Oct 2021 07:22:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CA650691C0; Thu, 7 Oct 2021 09:22:15 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="Qm8cSoV6"; dkim-atps=neutral Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BCB4C691B9 for ; Thu, 7 Oct 2021 09:22:14 +0200 (CEST) Received: by mail-pf1-x42d.google.com with SMTP id k26so4575909pfi.5 for ; Thu, 07 Oct 2021 00:22:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=zzpINsVDQqtol6mpAOE9QUl+unDwpbdAubYUqnR7CZo=; b=Qm8cSoV6f7GaJl0F8gvdSx3q0pEm/QK6Ycw6thgj97G01qUi/m2+EnxqEl0cDGstWj j0OaaduBnAaWMO5aP5stzebNIPdXXIKkJI+umqTpr7NvB6aM7WlL4X9bgeHrTxdiZcbI 0lOLCu1d2WPZmP7dhOmIrEeAzhFe63TZluxJY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=zzpINsVDQqtol6mpAOE9QUl+unDwpbdAubYUqnR7CZo=; b=vDaSSazTgJPSHjlbrpiNjPzaVSecJzrsFpWqaewiOUDG+cpVtloMvNVfNtaBjtQSgw qeGOdBfkMPp3ODm9/Esdo0dD/Cxm7lDLfD1UTl+0wOOayKvKZU3WEQvSLRcbZEkStssS hiI2GBMHkiPDOhQxo7lEe0eXdPM/n/TQvk7PxI8XvWt/dNMaFkgQloMhmAnO3xNhBSr+ uBidigk60svUXpcQ7nXt9I6/b4wNZqCQyeJVdd3mSQedFJT3ccNEpeJnK1cjV4oc7IN/ j87FEEHNQULjQ0fL3RVhiyiSgyM1JNNkyLSjRT1saPVdtd3msEQFpuDrIP00UpFiBnoq 1hXA== X-Gm-Message-State: AOAM5339+9Nl6AfZq6efdVOOidHmQAVH1sd7F46uUEPcqdxsrqmzF6qK lpWQbl8WJ0b+WZRSkOO1VI/7+WWof6qQim/lAK0= X-Google-Smtp-Source: ABdhPJwLAAvE7FhdCJ9aoGN0gxXcEdoffPk7WonBKfp+ukiFGj+KQ6T7Ay1duGctG8a+9Kr/w6/BLg== X-Received: by 2002:a63:392:: with SMTP id 140mr2182448pgd.189.1633591332873; Thu, 07 Oct 2021 00:22:12 -0700 (PDT) Received: from localhost ([2401:fa00:1:10:db0a:f55:d7cb:95ae]) by smtp.gmail.com with UTF8SMTPSA id w15sm12037750pfc.220.2021.10.07.00.22.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Oct 2021 00:22:12 -0700 (PDT) From: Han-Lin Chen To: libcamera-devel@lists.libcamera.org Date: Thu, 7 Oct 2021 15:21:46 +0800 Message-Id: <20211007072147.1289490-2-hanlinchen@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20211007072147.1289490-1-hanlinchen@chromium.org> References: <20211007072147.1289490-1-hanlinchen@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/3] ipu3: Change Macro migrated from Chrome OS to its std version accordingly X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 Reviewed-by: Kieran Bingham --- 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..8a53849 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); + aeInputParams = {}; + afParams = {}; + afBracketParams = {}; + awbParams = {}; + gbceParams = {}; + paParams = {}; + saParams = {}; + sensorDescriptor = {}; + exposureWindow = {}; + exposureCoordinate = {}; + aeFeatures = {}; + aeManualLimits = {}; + manualFocusParams = {}; + focusRect = {}; + manualCctRange = {}; + manualWhiteCoordinate = {}; + awbResults = {}; + colorGains = {}; + exposureParams = {}; + 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 Thu Oct 7 07:21:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanlin Chen X-Patchwork-Id: 14069 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 C774CC323E for ; Thu, 7 Oct 2021 07:22:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 87A7D691BD; Thu, 7 Oct 2021 09:22:18 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="eT6muaGE"; dkim-atps=neutral Received: from mail-pg1-x544.google.com (mail-pg1-x544.google.com [IPv6:2607:f8b0:4864:20::544]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0AA6569189 for ; Thu, 7 Oct 2021 09:22:17 +0200 (CEST) Received: by mail-pg1-x544.google.com with SMTP id h3so4840918pgb.7 for ; Thu, 07 Oct 2021 00:22:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=JUPdGar9kUJMjHrK5iHzWnm1VLkae7BcpyAAR4GhvVc=; b=eT6muaGEq6ET4W+CdWQ/E+nfui4A5iv2V7987gyZfGPLIhCi66GVhhhVHVCKIxMCfS V7vaXwE6TZnVzdKHLUarX3CYrYwVSjWI8YtvxPh61NoEtrAKTTtoobZF1zJAvpJmlBdY /rqMlfI8WYt3kh6ylfj9gtsbJUojTekkXm4AA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=JUPdGar9kUJMjHrK5iHzWnm1VLkae7BcpyAAR4GhvVc=; b=F+k1v3UbG5KNPa5T9nUO6TvFNvpS7P6a+LN42iWv3A1ljQgT75K1P7Cgrqef+KaIuw 0xPXSGL6w2BIbuRQccXnS0IL9F/QipOicvmO21flVb+AY9fSItghapqv5RU0ETHSY+5k jvcJEpnM4k/iy39Q5i38dSVxDzzlWZwjJQ0/RpJW8v2XOwRhlTxgk8EOxsqORrnlYu6W QeIGyXU2km6qUnzJSPWlccQsd0spWB+/Hdqh/2kRteE0XXsQhLvjlKcnT0fkBcpZsWum STUPEC8Yr74EJX7fKyFajsJX7r+LhaenMDMX85qCmv0g8oCDEdTGY5nFuwWU8Jq2yOXE w2FQ== X-Gm-Message-State: AOAM530Po2yqsbg2OuptoWCO+mxXLpVkcHdui+8BnOwzKmHTtC1jNzYn CPI2dg9XYobbPfQsC2N9DbBzy1TD/8l/ZHSUz04= X-Google-Smtp-Source: ABdhPJwjNVj7kuXDuvlR3Sm4cfHlrT18/OGJ2FYL5KFeq2PKWcjzNJUanvpFlboFOM48mP6qZt7iRA== X-Received: by 2002:a62:7904:0:b0:44c:ca08:1956 with SMTP id u4-20020a627904000000b0044cca081956mr250175pfc.47.1633591335327; Thu, 07 Oct 2021 00:22:15 -0700 (PDT) Received: from localhost ([2401:fa00:1:10:db0a:f55:d7cb:95ae]) by smtp.gmail.com with UTF8SMTPSA id t28sm8227975pfq.158.2021.10.07.00.22.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Oct 2021 00:22:15 -0700 (PDT) From: Han-Lin Chen To: libcamera-devel@lists.libcamera.org Date: Thu, 7 Oct 2021 15:21:47 +0800 Message-Id: <20211007072147.1289490-3-hanlinchen@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20211007072147.1289490-1-hanlinchen@chromium.org> References: <20211007072147.1289490-1-hanlinchen@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 3/3] ipu3: Apply shading adapter as part of AIQ::run2a() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Apply shading adapter to correct lens shading for both camera. Signed-off-by: Han-Lin Chen Reviewed-by: Umang Jain --- aiq/aiq.cpp | 3 +++ aiq/aiq_input_parameters.cpp | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp index 708e9d6..24c61cb 100644 --- a/aiq/aiq.cpp +++ b/aiq/aiq.cpp @@ -154,6 +154,9 @@ int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, params.paParams.exposure_params = results.ae()->exposures[0].exposure; parameterAdapterRun(params.paParams, results); + params.saParams.awb_results = results.awb(); + shadingAdapterRun(params.saParams, results); + afRun(params.afParams, results); return 0; diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp index 8a53849..36e2b07 100644 --- a/aiq/aiq_input_parameters.cpp +++ b/aiq/aiq_input_parameters.cpp @@ -89,6 +89,15 @@ int AiqInputParameters::configure(const IPAConfigInfo &configInfo) /* Guess from hal-configs-nautilus/files/camera3_profiles.xml#263 */ sensorDescriptor.coarse_integration_time_max_margin = 10; + sensorFrameParams.horizontal_crop_offset = 0; + sensorFrameParams.vertical_crop_offset = 0; + sensorFrameParams.cropped_image_width = configInfo.sensorInfo.analogCrop.width; + sensorFrameParams.cropped_image_height = configInfo.sensorInfo.analogCrop.height; + sensorFrameParams.horizontal_scaling_numerator = 1; + sensorFrameParams.horizontal_scaling_denominator = 1; + sensorFrameParams.vertical_scaling_numerator = 1; + sensorFrameParams.vertical_scaling_denominator = 1; + return 0; } @@ -165,6 +174,9 @@ void AiqInputParameters::setAeAwbAfDefaults() gbceParams.tone_map_level = ia_aiq_tone_map_level_default; gbceParams.frame_use = ia_aiq_frame_use_still; gbceParams.ev_shift = 0; + + /* SA Params */ + saParams.frame_use = ia_aiq_frame_use_still; } } /* namespace ipa::ipu3::aiq */