From patchwork Wed Mar 25 15:14:04 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26374 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 3D12DC3316 for ; Wed, 25 Mar 2026 15:16:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id ACA6B62CE8; Wed, 25 Mar 2026 16:16:12 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="fQuqcEG5"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6229B62BB6 for ; Wed, 25 Mar 2026 16:16:11 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 37C8E1B98; Wed, 25 Mar 2026 16:14:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451693; bh=mo4fhPqKCeRqTMq1VS82qLaUbLgfc6H1VAwfvMjhmow=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fQuqcEG5z+zzXPO0FMlSZlYqYVKHzCyYieo2EKfoTBMCSWlShHjNtsQqBq9k8pKss Tpgkc0w1ACz19tmgbL+JbjLuWh3cDd15DytdqSAHP9Fukw0EoA4eVxRshTxH6YHV7X kHUJER/HQ6fFJGQs+7XDGzIEfTeJYJwpB/U0T0f8= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 32/32] DNI: Move all queue/algo logic into FcLogic class Date: Wed, 25 Mar 2026 16:14:04 +0100 Message-ID: <20260325151416.2114564-33-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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" This is a prototype to toy around with the idea of moving all the typical IPA lgoc into an own FcLogic class (better names welcome). This could further reduce the boilerplate necessary to implement a typical IPA. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- src/ipa/libipa/algorithm.h | 5 + src/ipa/libipa/fc_logic.cpp | 20 ++++ src/ipa/libipa/fc_logic.h | 151 ++++++++++++++++++++++++++ src/ipa/libipa/fc_queue.h | 8 +- src/ipa/libipa/meson.build | 2 + src/ipa/rkisp1/algorithms/algorithm.h | 5 + src/ipa/rkisp1/ipa_context.h | 5 +- src/ipa/rkisp1/rkisp1.cpp | 61 ++++------- 8 files changed, 212 insertions(+), 45 deletions(-) create mode 100644 src/ipa/libipa/fc_logic.cpp create mode 100644 src/ipa/libipa/fc_logic.h diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h index 9a19dbd61b31..2d7372d2ff5e 100644 --- a/src/ipa/libipa/algorithm.h +++ b/src/ipa/libipa/algorithm.h @@ -38,6 +38,11 @@ public: return 0; } + virtual bool enabled() + { + return true; + } + virtual void queueRequest([[maybe_unused]] typename Module::Context &context, [[maybe_unused]] const uint32_t frame, [[maybe_unused]] typename Module::FrameContext &frameContext, diff --git a/src/ipa/libipa/fc_logic.cpp b/src/ipa/libipa/fc_logic.cpp new file mode 100644 index 000000000000..8b1917dfd40e --- /dev/null +++ b/src/ipa/libipa/fc_logic.cpp @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Google Inc. + * + * IPA Frame context queue + */ + +#include "fc_logic.h" + +#include + +namespace libcamera { + +LOG_DEFINE_CATEGORY(FCLogic) + +namespace ipa { + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/fc_logic.h b/src/ipa/libipa/fc_logic.h new file mode 100644 index 000000000000..be51b09643b8 --- /dev/null +++ b/src/ipa/libipa/fc_logic.h @@ -0,0 +1,151 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Google Inc. + * + * IPA Frame context queue + */ + +#pragma once + +#include +#include +#include + +#include +#include + +#include "algorithm.h" +#include "fc_queue.h" + +namespace libcamera { + +LOG_DECLARE_CATEGORY(FCLogic) + +namespace ipa { + +template +class FCLogic; + +template +class FCLogic +{ +public: + using Module = _Module; + using Algo = Algorithm; + + FCLogic(unsigned int size, Module *module, typename Module::Context &context) + : module_(module), contexts_(size), context_(context) + { + clear(); + } + + void clear() + { + for (typename Module::FrameContext &ctx : contexts_) { + ctx.frame_ = 0; + } + initialized_ = false; + lastPrepared_ = 0; + } + + typename Module::FrameContext &initFrameContext(unsigned int frame, const ControlList &controls = {}) + { + typename Module::FrameContext &fc = contexts_[frame % contexts_.size()]; + FrameContext &frameContext = fc; + + /* + * 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(FCLogic, Fatal) << "Frame context for " << frame + << " has been overwritten by " + << frameContext.frame_; + + if (initialized_ && frame == frameContext.frame_) { + if (!controls.empty()) { + /* Too late to apply the controls. Store them for later. */ + LOG(FCLogic, Warning) + << "Request underrun. Controls for frame " + << frame << " are delayed "; + controlsToApply_.merge(controls, + ControlList::MergePolicy::OverwriteExisting); + } + + return fc; + } + + const ControlList *controls2 = &controls; + if (!controlsToApply_.empty()) { + LOG(FCLogic, Debug) << "Applied late controls on frame" << frame; + controlsToApply_.merge(controls, ControlList::MergePolicy::OverwriteExisting); + controls2 = &controlsToApply_; + } + + fc = {}; + frameContext.frame_ = frame; + for (auto const &a : module_->algorithms()) { + Algo *algo = static_cast(a.get()); + if (!algo->enabled()) + continue; + algo->queueRequest(context_, fc.frame(), fc, *controls2); + } + + initialized_ = true; + controlsToApply_.clear(); + + return fc; + } + + void prepareFrame(const uint32_t frame, typename Module::Params *params) + { + while (lastPrepared_ + 1 < frame) { + uint32_t f = lastPrepared_ + 1; + LOG(FCLogic, Warning) << "Collect skipped params for frame " << f; + prepareFrame(f, params); + } + + typename Module::FrameContext &frameContext = initFrameContext(frame); + for (auto const &algo : module_->algorithms()) + algo->prepare(context_, frameContext.frame(), frameContext, params); + + lastPrepared_ = std::max(lastPrepared_, frame); + } + + void processStats(const uint32_t frame, const typename Module::Stats *stats, ControlList &metadata) + { + /* + * If we apply stats on an uninitialized frame the activeState + * might end up very wrong (imagine exposureTime initialized to + * 0 but used together with the agc stats to upadet activeState. + */ + if (!isInitialize(frame)) + LOG(FCLogic, Error) << "Process stats called on an uninitialized FC " << frame; + + typename Module::FrameContext &frameContext = initFrameContext(frame); + for (auto const &algo : module_->algorithms()) + algo->process(context_, frameContext.frame(), frameContext, stats, metadata); + } + +private: + bool isInitialize(const uint32_t frame) + { + FrameContext &frameContext = contexts_[frame % contexts_.size()]; + return initialized_ && frame == frameContext.frame_; + } + + const Module *module_; + std::vector contexts_; + typename Module::Context &context_; + ControlList controlsToApply_; + uint32_t lastPrepared_; + bool initialized_; +}; + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index 633bf646d88d..34811cfb3514 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -22,11 +22,17 @@ namespace ipa { template class FCQueue; +template +class FCLogic; + struct FrameContext { uint32_t frame() const { return frame_; } private: - template friend class FCQueue; + template + friend class FCQueue; + template + friend class FCLogic; uint32_t frame_; }; diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build index 963c5ee73063..918279a89d27 100644 --- a/src/ipa/libipa/meson.build +++ b/src/ipa/libipa/meson.build @@ -9,6 +9,7 @@ libipa_headers = files([ 'camera_sensor_helper.h', 'colours.h', 'exposure_mode_helper.h', + 'fc_logic.h', 'fc_queue.h', 'fixedpoint.h', 'histogram.h', @@ -30,6 +31,7 @@ libipa_sources = files([ 'camera_sensor_helper.cpp', 'colours.cpp', 'exposure_mode_helper.cpp', + 'fc_logic.cpp', 'fc_queue.cpp', 'fixedpoint.cpp', 'histogram.cpp', diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h index 715cfcd8298b..8b6644c0ee26 100644 --- a/src/ipa/rkisp1/algorithms/algorithm.h +++ b/src/ipa/rkisp1/algorithms/algorithm.h @@ -23,6 +23,11 @@ public: { } + virtual bool enabled() override + { + return !disabled_; + } + bool disabled_; bool supportsRaw_; }; diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index e61391bb1510..43ac9415a0d5 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -236,8 +236,7 @@ struct IPAFrameContext : public FrameContext { }; struct IPAContext { - IPAContext(unsigned int frameContextSize) - : frameContexts(frameContextSize) + IPAContext() { } @@ -246,8 +245,6 @@ struct IPAContext { IPASessionConfiguration configuration; IPAActiveState activeState; - FCQueue frameContexts; - ControlInfoMap::Map ctrlMap; DebugMetadata debugMetadata; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 863cea13dfa3..baebaa8ceb77 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -32,6 +32,7 @@ #include "libcamera/internal/yaml_parser.h" #include "algorithms/algorithm.h" +#include "libipa/fc_logic.h" #include "ipa_context.h" #include "params.h" @@ -78,7 +79,7 @@ protected: std::string logPrefix() const override; private: - uint32_t computeParamsInternal(IPAFrameContext &frameContext, const uint32_t bufferId); + uint32_t computeParamsInternal(const uint32_t frame, const uint32_t bufferId); void updateControls(const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, @@ -92,6 +93,7 @@ private: /* Local parameter storage */ struct IPAContext context_; + FCLogic fcLogic_; }; namespace { @@ -132,12 +134,8 @@ const ControlInfoMap::Map rkisp1Controls{ } /* namespace */ IPARkISP1::IPARkISP1() - : context_(kMaxFrameContexts) + : Module(), context_(), fcLogic_(kMaxFrameContexts, this, context_) { - context_.frameContexts.setInitCallback( - [this](IPAFrameContext &fc, const ControlList &c) { - this->initializeFrameContext(fc, c); - }); } std::string IPARkISP1::logPrefix() const @@ -225,15 +223,15 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, void IPARkISP1::start(const ControlList &controls, const uint32_t paramBufferId, StartResult *result) { - IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(0, controls); - result->paramBufferBytesUsed = computeParamsInternal(frameContext, paramBufferId); + IPAFrameContext &frameContext = fcLogic_.initFrameContext(0, controls); + result->paramBufferBytesUsed = computeParamsInternal(0, paramBufferId); result->controls = getSensorControls(frameContext); result->code = 0; } void IPARkISP1::stop() { - context_.frameContexts.clear(); + fcLogic_.clear(); } int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, @@ -257,7 +255,7 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, /* Clear the IPA context before the streaming session. */ context_.configuration = {}; context_.activeState = {}; - context_.frameContexts.clear(); + fcLogic_.clear(); context_.configuration.paramFormat = ipaConfig.paramFormat; @@ -345,20 +343,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids) void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls) { context_.debugMetadata.enableByControl(controls); - context_.frameContexts.getOrInitContext(frame, controls); + fcLogic_.initFrameContext(frame, controls); } -void IPARkISP1::initializeFrameContext(IPAFrameContext &fc, const ControlList &controls) -{ - for (const auto &a : algorithms()) { - Algorithm *algo = static_cast(a.get()); - if (algo->disabled_) - continue; - algo->queueRequest(context_, fc.frame(), fc, controls); - } -} - -uint32_t IPARkISP1::computeParamsInternal(IPAFrameContext &frameContext, const uint32_t bufferId) +uint32_t IPARkISP1::computeParamsInternal(const uint32_t frame, const uint32_t bufferId) { if (bufferId == 0) return 0; @@ -366,17 +354,16 @@ uint32_t IPARkISP1::computeParamsInternal(IPAFrameContext &frameContext, const u RkISP1Params params(context_.configuration.paramFormat, mappedBuffers_.at(bufferId).planes()[0]); - for (const auto &algo : algorithms()) - algo->prepare(context_, frameContext.frame(), frameContext, ¶ms); + fcLogic_.prepareFrame(frame, ¶ms); return params.bytesused(); } void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) { - IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); + uint32_t size = computeParamsInternal(frame, bufferId); - uint32_t size = computeParamsInternal(frameContext, bufferId); + IPAFrameContext &frameContext = fcLogic_.initFrameContext(frame); ControlList ctrls = getSensorControls(frameContext); setSensorControls.emit(frame, ctrls); @@ -387,7 +374,13 @@ void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, const ControlList &sensorControls) { - IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); + IPAFrameContext &frameContext = fcLogic_.initFrameContext(frame); + frameContext.sensor.exposure = + sensorControls.get(V4L2_CID_EXPOSURE).get(); + frameContext.sensor.gain = + context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get()); + + ControlList metadata(controls::controls); /* * In raw capture mode, the ISP is bypassed and no statistics buffer is @@ -398,19 +391,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, stats = reinterpret_cast( mappedBuffers_.at(bufferId).planes()[0].data()); - frameContext.sensor.exposure = - sensorControls.get(V4L2_CID_EXPOSURE).get(); - frameContext.sensor.gain = - context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get()); - - ControlList metadata(controls::controls); - - for (const auto &a : algorithms()) { - Algorithm *algo = static_cast(a.get()); - if (algo->disabled_) - continue; - algo->process(context_, frame, frameContext, stats, metadata); - } + fcLogic_.processStats(frame, stats, metadata); context_.debugMetadata.moveEntries(metadata); metadataReady.emit(frame, metadata);