From patchwork Thu Oct 14 07:05:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanlin Chen X-Patchwork-Id: 14130 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 3C3CBBDC71 for ; Thu, 14 Oct 2021 07:05:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AAA5068F4F; Thu, 14 Oct 2021 09:05:45 +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="jS0DuYmG"; dkim-atps=neutral Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9069D68F4A for ; Thu, 14 Oct 2021 09:05:44 +0200 (CEST) Received: by mail-pj1-x102a.google.com with SMTP id q10-20020a17090a1b0a00b001a076a59640so5023722pjq.0 for ; Thu, 14 Oct 2021 00:05:44 -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=sYPRy0xyZzVfmoj8GbuEac7xn0uDgN/+T8aC4orB5cc=; b=jS0DuYmGb7WlIbgCeaKD2M4KCNJdCAhggTl7b40/Eq1qbSrI2DNzuj1RYn87Qv2yRq rSfdPHZu7f0e/oFqP4YUA/65Hc1TCwTPPbbzHFQ8Fb8EpdUZsSVVFou/5MhSuCh4lqw5 yVbQ54Fr5iKFQsvzaUs4ECaMJScdkSEPn+CMM= 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=sYPRy0xyZzVfmoj8GbuEac7xn0uDgN/+T8aC4orB5cc=; b=4+Ow9ME8lIVJr1AksGSfhf6TXOiPQv5hXOdAwCp+l8SJtaotPb5h7aegj+xql9xqdD 9FdyfzYXkg5/VwqFjTY3GFzZZSk0HIXkNbHlZX/Zl8YUIIRSn4DcwBidf6mh7lqMIuTD YLwblZkGQy6cDb0XbFnBtqavefwNfO4V7E/EFqgyxe1U9aUMe/15Xty7KWBnZRp+azwp BnH2vl1EBShdIQECPYlFYbBYajgrJ+52T/PZj2+Y4lOvqB9gD5UM6JVdCMoqZDdvE7Dt 2DD2kYBd/crWbdHQn2I3HrWQt6QiGhmseApvjzyZv2vxulcXrc+wlK7EdaaJac6eBHBT F0iw== X-Gm-Message-State: AOAM532gHngD9paFYMzHVcw+h3KW4t8bpbUMQ+Ig8/hvpGP/DR+LB9oH w+rLwV3FG9hvG/m2y7uSY5LXgJnR/jttU5lR X-Google-Smtp-Source: ABdhPJxqZRBYu4CkDXRG8BbHtReB60bvKCJewW73qKhgH6zhLkAqHKNBxy1b7SWl57Ih2I/n33/x9Q== X-Received: by 2002:a17:90a:8b04:: with SMTP id y4mr19061902pjn.142.1634195140738; Thu, 14 Oct 2021 00:05:40 -0700 (PDT) Received: from localhost ([2401:fa00:1:10:4dac:fd0c:fba7:b5ae]) by smtp.gmail.com with UTF8SMTPSA id m7sm1456814pgn.32.2021.10.14.00.05.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Oct 2021 00:05:40 -0700 (PDT) From: Han-Lin Chen To: libcamera-devel@lists.libcamera.org Date: Thu, 14 Oct 2021 15:05:31 +0800 Message-Id: <20211014070533.3720964-1-hanlinchen@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 14 07:05:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanlin Chen X-Patchwork-Id: 14131 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 51D4ABDC71 for ; Thu, 14 Oct 2021 07:05:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2135168F52; Thu, 14 Oct 2021 09:05:48 +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="nOBMYGrp"; dkim-atps=neutral Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 65F6B68F4D for ; Thu, 14 Oct 2021 09:05:46 +0200 (CEST) Received: by mail-pj1-x102c.google.com with SMTP id g13-20020a17090a3c8d00b00196286963b9so6227323pjc.3 for ; Thu, 14 Oct 2021 00:05:46 -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=oN5vcNhH75dAQR8GHsogOinBnVa6zaInbpTKnMN3SKE=; b=nOBMYGrpQn1U+KRUmZu27oyN4ucbEgN9aXUqzBNXhtVh1SqPvMiblu5cHXzpIP5cjr Bz41RENuT9L0Ax1Cy9C7y9RDzK4c8tBiEfSdijLIiy8ooK6FDK+H4bnlQv0dBCoHQNcS wRLBZosUnz8kxSWbYtYNjmyC948R4UDb6IXvA= 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=oN5vcNhH75dAQR8GHsogOinBnVa6zaInbpTKnMN3SKE=; b=S+8KwfjIjjG3M8EtId4UVWg+psVkVZP1Q2WA14tsxoAnIwyUicU2AWZlPW50tFJ8u5 uH55Q5kf7JV4nWMSQg7wLQTnoxRRrWMPsQCM359HoGDz1vzOL77kl4Vy6nru9tPRBVWb LkwvZvuIRAKoi9tI+y0syoaZ/epA5uwlc8pHwbciY13ziH4I39iT+LvApHFfT1K44RhR Mdze+X7WFr7kZ1Ys3FBNlAUYtTewEAXL2rSBQIH0iuHU4iby8IgsVFb1RFaQ+QD9XSDG Tqk4sRhyDb0q1uKLTC2iiFsy9hy7WJwPAaCsh9JckIObYRh3Vfl9+i+Fj4MTzIxD0qol qIgg== X-Gm-Message-State: AOAM5304wHzEN0U8hof05U7gyOCdrBFrYPy2MZ8ii7iBXmwqPze6545y XKwp5YwpemsZVEPyyyCO+GK+yEsQ0QVEVBjC X-Google-Smtp-Source: ABdhPJzj+PrlYceeCl6dmscBKdyVRMitRygxVXEwPYWgUFw8m/xKdm4SVRv7xInx07rtKUlFjOPm1Q== X-Received: by 2002:a17:90a:e10:: with SMTP id v16mr18988845pje.86.1634195143191; Thu, 14 Oct 2021 00:05:43 -0700 (PDT) Received: from localhost ([2401:fa00:1:10:4dac:fd0c:fba7:b5ae]) by smtp.gmail.com with UTF8SMTPSA id x65sm631693pjj.42.2021.10.14.00.05.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Oct 2021 00:05:42 -0700 (PDT) From: Han-Lin Chen To: libcamera-devel@lists.libcamera.org Date: Thu, 14 Oct 2021 15:05:32 +0800 Message-Id: <20211014070533.3720964-2-hanlinchen@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20211014070533.3720964-1-hanlinchen@chromium.org> References: <20211014070533.3720964-1-hanlinchen@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 14 07:05:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanlin Chen X-Patchwork-Id: 14132 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 117F9BDC71 for ; Thu, 14 Oct 2021 07:05:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D3DF868F54; Thu, 14 Oct 2021 09:05:49 +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="LU/IuFT8"; dkim-atps=neutral Received: from mail-pg1-x541.google.com (mail-pg1-x541.google.com [IPv6:2607:f8b0:4864:20::541]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A63168F54 for ; Thu, 14 Oct 2021 09:05:48 +0200 (CEST) Received: by mail-pg1-x541.google.com with SMTP id r2so4635480pgl.10 for ; Thu, 14 Oct 2021 00:05:47 -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=CpFwya8vFkhHzBzR4WOewPCe3sJl5KgyNve3eOrJDAo=; b=LU/IuFT8nfB93avDYmQNj5ZisVZ+87+IzC3SC+Bhqju9TlsXkPIgXSwAJTBIP8iHz/ klwhC9x+gVKpQ7ERhf0wyu4AhyR0gPVKSmbV29UIXsYhslnYw/ntCR0KkDKm6EaEUiM7 mbhHmvS0960L+YAxL5qKP3JP2FXmM8jJ/qGWE= 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=CpFwya8vFkhHzBzR4WOewPCe3sJl5KgyNve3eOrJDAo=; b=rrDuCDK+G/+hA5RIJtKY8VIeVF2WKVJya2e6+yejokx48PhfrMgtiahVSA7B5EgpAx K/0pjsZ9KBP0vTQufk9Rr7VjV5Qg1RMHp7vsNxWlCfNsZPcLIHArnHtabRyghR5SVPo/ pb/dE3SHo67+f7fY0r21BuIwZWPuutFUuJxWjEXVbUGl9ofh7Qsphi7S71chkQ3f7GDY nBAurHbPx9EFMGCIAK1nUDNW1XwWn8rheGjEe8lLR2AUbfJ+C3adlAekb6s91nex7pJw 0biPodjQOgnRvzfm/5VWBypgHOB8BCB4Z/yhErMu/qOS8k/eTYrFxxxM+OteId8cQm9L XpBg== X-Gm-Message-State: AOAM530+eyQPLid9cRtZ8dgJASwdt8MstiS5aInoEAnn41gtSkvQnA83 ELxY/iPxSEkv/xGKQE0Up9sxEOKtPl4FghNh X-Google-Smtp-Source: ABdhPJwUhPPyZZaPerXpDPCkYATWr1jN7Cuil68m4BFxf+f97rTYLVxiK0AX/yh6kCvT7eCpALFUow== X-Received: by 2002:a63:cf41:: with SMTP id b1mr2976484pgj.407.1634195145184; Thu, 14 Oct 2021 00:05:45 -0700 (PDT) Received: from localhost ([2401:fa00:1:10:4dac:fd0c:fba7:b5ae]) by smtp.gmail.com with UTF8SMTPSA id y3sm1399766pfo.188.2021.10.14.00.05.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Oct 2021 00:05:44 -0700 (PDT) From: Han-Lin Chen To: libcamera-devel@lists.libcamera.org Date: Thu, 14 Oct 2021 15:05:33 +0800 Message-Id: <20211014070533.3720964-3-hanlinchen@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20211014070533.3720964-1-hanlinchen@chromium.org> References: <20211014070533.3720964-1-hanlinchen@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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" Populate shading adapter input parameters and run it as a part of run2a() to correct lens shading for the camera. Signed-off-by: Han-Lin Chen --- 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 */