From patchwork Fri Aug 5 13:53:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 16979 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 759A7C3272 for ; Fri, 5 Aug 2022 13:53:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3371263338; Fri, 5 Aug 2022 15:53:28 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1659707608; bh=jgxINQqPxrEGz8R4obqe9QrNFtV7FFv7Vfvmjsq+R9s=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ECDw/u9BpuOj5amSSsMGg7lWBJL29kVx1wgIwngYMDKvcftFRGOm3u65BId8Zya4+ HWY7mXo+gSCB5xNxQ2oA5NxDDqxKJ/vvJ8oZCH4wzK/M3c+SCKyKHXdlH2mWPanPub zbksO/YQcxLseTDQAsiEqB7a8RNQZ7k+obxoFcacn9W4h22o2SV52s/CNjuT0sR9ER MPHazXR8qKppYd4Sczw1xORu9W4rpFT9SK4Fmy131cnWIsNRo2ojAy24WhvHh+Pk0b bt4tfmflLu0DdLRhrst/gFWfed705qDaoZ17nWtDFT4/ju+vfCT/rUiTNypXID/n73 lwGhPkc6ORrew== Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F2A2C603E4 for ; Fri, 5 Aug 2022 15:53:25 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by mail.gandi.net (Postfix) with ESMTPSA id 6E9CB1C0012; Fri, 5 Aug 2022 13:53:24 +0000 (UTC) To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Aug 2022 15:53:06 +0200 Message-Id: <20220805135312.47497-5-jacopo@jmondi.org> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220805135312.47497-1-jacopo@jmondi.org> References: <20220805135312.47497-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 04/10] ipa: libipa: Introduce FrameContextQueue 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: , X-Patchwork-Original-From: Jacopo Mondi via libcamera-devel From: Jacopo Mondi Reply-To: Jacopo Mondi Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Umang Jain Introduce a common implementation in libipa to represent the queue of frame contexts. Adjust the IPU3 IPAFrameContext to provide the basic class members required to work with the generic FCQueue implementation, before introducing an IPAFrameContext class in the next patches. Opportunely handle cleaning up the queue and the IPA context at IPA::stop() time. Signed-off-by: Umang Jain Signed-off-by: Kieran Bingham Signed-off-by: Jacopo Mondi Reviewed-by: Umang Jain --- src/ipa/ipu3/ipa_context.cpp | 20 +------ src/ipa/ipu3/ipa_context.h | 16 ++--- src/ipa/ipu3/ipu3.cpp | 18 +++--- src/ipa/libipa/fc_queue.cpp | 112 +++++++++++++++++++++++++++++++++++ src/ipa/libipa/fc_queue.h | 109 ++++++++++++++++++++++++++++++++++ src/ipa/libipa/meson.build | 2 + src/ipa/rkisp1/rkisp1.cpp | 11 ++-- 7 files changed, 250 insertions(+), 38 deletions(-) create mode 100644 src/ipa/libipa/fc_queue.cpp create mode 100644 src/ipa/libipa/fc_queue.h diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp index 13cdb835ef7f..9605c8a1106d 100644 --- a/src/ipa/ipu3/ipa_context.cpp +++ b/src/ipa/ipu3/ipa_context.cpp @@ -180,27 +180,10 @@ namespace libcamera::ipa::ipu3 { * struct ipu3_uapi_gamma_corr_lut for further details. */ -/** - * \brief Default constructor for IPAFrameContext - */ -IPAFrameContext::IPAFrameContext() = default; - -/** - * \brief Construct a IPAFrameContext instance - */ -IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls) - : frame(id), frameControls(reqControls) -{ - sensor = {}; -} - /** * \var IPAFrameContext::frame * \brief The frame number * - * \var IPAFrameContext::frameControls - * \brief Controls sent in by the application while queuing the request - * * \var IPAFrameContext::sensor * \brief Effective sensor values that were applied for the frame * @@ -209,6 +192,9 @@ IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls) * * \var IPAFrameContext::sensor.gain * \brief Analogue gain multiplier + * + * \var IPAFrameContext::error + * \brief The error flags for this frame context */ } /* namespace libcamera::ipa::ipu3 */ diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index 42e11141d3a1..dc5b7c30aaf9 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -8,22 +8,20 @@ #pragma once -#include - #include #include #include #include +#include + +#include namespace libcamera { namespace ipa::ipu3 { -/* Maximum number of frame contexts to be held */ -static constexpr uint32_t kMaxFrameContexts = 16; - struct IPASessionConfiguration { struct { ipu3_uapi_grid_config bdsGrid; @@ -77,23 +75,21 @@ struct IPAActiveState { }; struct IPAFrameContext { - IPAFrameContext(); - IPAFrameContext(uint32_t id, const ControlList &reqControls); + uint32_t frame; struct { uint32_t exposure; double gain; } sensor; - uint32_t frame; - ControlList frameControls; + Request::Errors error; }; struct IPAContext { IPASessionConfiguration configuration; IPAActiveState activeState; - std::array frameContexts; + FCQueue frameContexts; }; } /* namespace ipa::ipu3 */ diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 2f6bb672f7bb..209a6b040f8f 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -38,6 +38,8 @@ #include "algorithms/tone_mapping.h" #include "libipa/camera_sensor_helper.h" +#include "ipa_context.h" + /* Minimum grid width, expressed as a number of cells */ static constexpr uint32_t kMinGridWidth = 16; /* Maximum grid width, expressed as a number of cells */ @@ -348,6 +350,9 @@ int IPAIPU3::start() */ void IPAIPU3::stop() { + /* Clean the IPA context at the end of the streaming session. */ + context_.frameContexts.clear(); + context_ = {}; } /** @@ -454,11 +459,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, calculateBdsGrid(configInfo.bdsOutputSize); - /* Clean IPAActiveState at each reconfiguration. */ - context_.activeState = {}; - IPAFrameContext initFrameContext; - context_.frameContexts.fill(initFrameContext); - if (!validateSensorControls()) { LOG(IPAIPU3, Error) << "Sensor control validation failed."; return -EINVAL; @@ -569,7 +569,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, const ipu3_uapi_stats_3a *stats = reinterpret_cast(mem.data()); - IPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts]; + IPAFrameContext &frameContext = context_.frameContexts.get(frame); if (frameContext.frame != frame) LOG(IPAIPU3, Warning) << "Frame " << frame << " does not match its frame context"; @@ -618,7 +618,11 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) { /* \todo Start processing for 'frame' based on 'controls'. */ - context_.frameContexts[frame % kMaxFrameContexts] = { frame, controls }; + IPAFrameContext &frameContext = context_.frameContexts.initialise(frame); + + /* \todo Implement queueRequest to each algorithm. */ + (void)frameContext; + (void)controls; } /** diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp new file mode 100644 index 000000000000..37ffca60b992 --- /dev/null +++ b/src/ipa/libipa/fc_queue.cpp @@ -0,0 +1,112 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Google Inc. + * + * fc_queue.cpp - IPA Frame context queue + */ + +#include "fc_queue.h" + +#include + +namespace libcamera { + +LOG_DEFINE_CATEGORY(FCQueue) + +namespace ipa { + +/** + * \file fc_queue.h + * \brief Queue to access per-frame Context + */ + +/** + * \class FCQueue + * \brief A support class for queueing FrameContext instances through the IPA + * \tparam FrameContext The IPA specific FrameContext derived class type + * + * The frame context queue provides a simple circular buffer implementation to + * store IPA specific context for each frame through its lifetime within the + * IPA. + * + * FrameContext instances are expected to be referenced by a monotonically + * increasing sequence count corresponding to a frame sequence number. + * + * A FrameContext is initialised for a given frame when the corresponding + * Request is first queued into the IPA. From that point on the FrameContext can + * be obtained by the IPA and its algorithms by referencing it from the frame + * sequence number. + * + * A frame sequence number from the image source must correspond to the request + * sequence number for this implementation to be supported in an IPA. It is + * expected that the same sequence number will be used to reference the context + * of the Frame from the point of queueing the request, specifying controls for + * a given frame, and processing of any ISP related controls and statistics for + * the same corresponding image. + * + * IPA specific frame context implementations shall inherit from the + * IPAFrameContext base class to support the minimum required features for a + * FrameContext, including the frame number and the error flags that relate to + * the frame. + * + * FrameContexts are overwritten when they are recycled and re-initialised by + * the first access made on them by either initialise(frame) or get(frame). It + * is required that the number of available slots in the frame context queue is + * larger or equal to the maximum number of in-flight requests a pipeline can + * handle to avoid overwriting frame context instances that still have to be + * processed. + * + * In the case an application does not queue requests to the Camera fast + * enough, frames can be produced and processed by the IPA without a + * corresponding Request being queued. In this case the IPA algorithm + * will try to access the FrameContext with a call to get() before it + * had been initialized at queueRequest() time. In this case there + * is now way the controls associated with the late Request could be + * applied in time, as the frame as already been processed by the IPA, + * the PFCError flag is automatically raised on the FrameContext. + */ + +/** + * \fn FCQueue::clear() + * \brief Clear the context queue + * + * Reset the queue and ensure all FrameContext slots are re-initialised. + * IPA modules are expected to clear the frame context queue at the beginning of + * a new streaming session, in IPAModule::start(). + */ + +/** + * \fn FCQueue::initialise(uint32_t frame) + * \brief Initialize and return the Frame Context at slot specified by \a frame + * \param[in] frame The frame context sequence number + * + * The first call to obtain a FrameContext from the FCQueue should be handled + * through this call. The FrameContext will be initialised for the frame and + * returned to the caller if it was not already initialised. + * + * If the FrameContext was already initialized for this sequence, a warning will + * be reported and the previously initialized FrameContext is returned. + * + * Frame contexts are expected to be initialised when a Request is first passed + * to the IPA module in IPAModule::queueRequest(). + * + * \return A reference to the FrameContext for sequence \a frame + */ + +/** + * \fn FCQueue::get() + * \brief Obtain the Frame Context at slot specified by \a frame + * \param[in] frame The frame context sequence number + * + * Obtains an existing FrameContext from the queue and returns it to the caller. + * + * If the FrameContext is not correctly initialised for the \a frame, it will be + * initialised and a PFCError will be flagged on the context to be transferred + * to the Request when it completes. + * + * \return A reference to the FrameContext for sequence \a frame + */ + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h new file mode 100644 index 000000000000..023b52420fe7 --- /dev/null +++ b/src/ipa/libipa/fc_queue.h @@ -0,0 +1,109 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Google Inc. + * + * fc_queue.h - IPA Frame context queue + */ + +#pragma once + +#include + +#include + +#include + +namespace libcamera { + +LOG_DECLARE_CATEGORY(FCQueue) + +namespace ipa { + +/* + * Maximum number of frame contexts to be held onto + * + * \todo Should be platform-specific and match the pipeline depth + */ +static constexpr uint32_t kMaxFrameContexts = 16; + +template +class FCQueue : private std::array +{ +private: + void initialise(FrameContext &frameContext, const uint32_t frame) + { + frameContext = {}; + frameContext.frame = frame; + } + +public: + void clear() + { + this->fill({}); + } + + FrameContext &initialise(const uint32_t frame) + { + FrameContext &frameContext = this->at(frame % kMaxFrameContexts); + + /* + * Do not re-initialise if a get() call has already fetched this + * frame context to preseve the error flags already raised. + * + * \todo If the the sequence number of the context to initialise + * is smaller than the sequence number of the queue slot to use, + * it means that we had a serious request underrun and more + * frames than the queue size has been produced since the last + * time the application has queued a request. Does this deserve + * an error condition ? + */ + if (frame != 0 && frame <= frameContext.frame) + LOG(FCQueue, Warning) + << "Frame " << frame << " already initialised"; + else + initialise(frameContext, frame); + + return frameContext; + } + + FrameContext &get(uint32_t frame) + { + FrameContext &frameContext = this->at(frame % kMaxFrameContexts); + + /* + * If the IPA algorithms try to access a frame context slot which + * has been already overwritten by a newer context, it means the + * frame context queue has overflowed and the desired context + * has been forever lost. The pipeline handler shall avoid + * queueing more requests to the IPA than the frame context + * queue size. + */ + if (frame < frameContext.frame) + LOG(FCQueue, Fatal) + << "Frame " << frame << " has been overwritten"; + + if (frame == frameContext.frame) + return frameContext; + + LOG(FCQueue, Warning) + << "Obtained an uninitialised FrameContext for " << frame; + + initialise(frameContext, frame); + + /* + * The frame context has been retrieved before it was + * initialised through the initialise() call. This indicates an + * algorithm attempted to access a Frame context before it was + * queued to the IPA. + * + * Controls applied for this request may be left unhandled. + */ + frameContext.error |= Request::PFCError; + + return frameContext; + } +}; + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build index fb894bc614af..016b8e0ec9be 100644 --- a/src/ipa/libipa/meson.build +++ b/src/ipa/libipa/meson.build @@ -3,6 +3,7 @@ libipa_headers = files([ 'algorithm.h', 'camera_sensor_helper.h', + 'fc_queue.h', 'histogram.h', 'module.h', ]) @@ -10,6 +11,7 @@ libipa_headers = files([ libipa_sources = files([ 'algorithm.cpp', 'camera_sensor_helper.cpp', + 'fc_queue.cpp', 'histogram.cpp', 'module.cpp', ]) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 6cf4d1699ce5..af22dbeb3da5 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -49,7 +49,7 @@ public: int init(const IPASettings &settings, unsigned int hwRevision, ControlInfoMap *ipaControls) override; int start() override; - void stop() override {} + void stop() override; int configure(const IPACameraSensorInfo &info, const std::map &streamConfig, @@ -189,6 +189,12 @@ int IPARkISP1::start() return 0; } +void IPARkISP1::stop() +{ + /* Clean the IPA context at the end of the streaming session. */ + context_ = {}; +} + /** * \todo The RkISP1 pipeline currently provides an empty IPACameraSensorInfo * if the connected sensor does not provide enough information to properly @@ -228,9 +234,6 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, << "Exposure: " << minExposure << "-" << maxExposure << " Gain: " << minGain << "-" << maxGain; - /* Clean context at configuration */ - context_ = {}; - /* Set the hardware revision for the algorithms. */ context_.configuration.hw.revision = hwRevision_;