[v2,32/32] DNI: Move all queue/algo logic into FcLogic class
diff mbox series

Message ID 20260325151416.2114564-33-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • rkisp1: pipeline rework for PFC
Related show

Commit Message

Stefan Klug March 25, 2026, 3:14 p.m. UTC
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 <stefan.klug@ideasonboard.com>

---

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

Patch
diff mbox series

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 <libcamera/base/log.h>
+
+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 <list>
+#include <stdint.h>
+#include <vector>
+
+#include <libcamera/base/log.h>
+#include <libcamera/controls.h>
+
+#include "algorithm.h"
+#include "fc_queue.h"
+
+namespace libcamera {
+
+LOG_DECLARE_CATEGORY(FCLogic)
+
+namespace ipa {
+
+template<typename FrameContext>
+class FCLogic;
+
+template<typename _Module>
+class FCLogic
+{
+public:
+	using Module = _Module;
+	using Algo = Algorithm<Module>;
+
+	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<Algo *>(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<typename Module::FrameContext> 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<typename FrameContext>
 class FCQueue;
 
+template<typename FrameContext>
+class FCLogic;
+
 struct FrameContext {
 	uint32_t frame() const { return frame_; }
 
 private:
-	template<typename T> friend class FCQueue;
+	template<typename T>
+	friend class FCQueue;
+	template<typename T>
+	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<IPAFrameContext> 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<Module> 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<unsigned int> &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<Algorithm *>(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, &params);
+	fcLogic_.prepareFrame(frame, &params);
 
 	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<int32_t>();
+	frameContext.sensor.gain =
+		context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
+
+	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<rkisp1_stat_buffer *>(
 			mappedBuffers_.at(bufferId).planes()[0].data());
 
-	frameContext.sensor.exposure =
-		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
-	frameContext.sensor.gain =
-		context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
-
-	ControlList metadata(controls::controls);
-
-	for (const auto &a : algorithms()) {
-		Algorithm *algo = static_cast<Algorithm *>(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);