[RFC,v2,13/14] libcamera: software_isp: Share statistics buffers with IPA
diff mbox series

Message ID 20260216203034.27558-14-mzamazal@redhat.com
State New
Headers show
Series
  • Software ISP: Share params and stats buffers
Related show

Commit Message

Milan Zamazal Feb. 16, 2026, 8:30 p.m. UTC
The last step to complete statistics buffer sharing is to pass all the
allocated statistics buffers to the IPA and refer to them using their ids.

This allows to remove the buffer copying in SwStatsCpu::finishFrame.
In order to track the current statistics buffer in SwStatsCpu instead,
we change SwStatsCpu::stats_ to a wrapper structure.  This is because we
need a reference to a shared mem object but a class attribute cannot be
a dynamically assigned reference.  This hack works around the problem.

We can also remove now the methods that served for handling the former
single buffer shared between debayering and IPA.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 .../internal/software_isp/swstats_cpu.h       | 12 +++---
 include/libcamera/ipa/soft.mojom              |  2 +-
 src/ipa/simple/soft_simple.cpp                | 43 ++++++++++---------
 src/libcamera/software_isp/debayer.cpp        | 10 -----
 src/libcamera/software_isp/debayer.h          |  2 -
 src/libcamera/software_isp/debayer_cpu.h      |  1 -
 src/libcamera/software_isp/debayer_egl.h      |  1 -
 src/libcamera/software_isp/software_isp.cpp   |  2 +-
 src/libcamera/software_isp/swstats_cpu.cpp    | 35 +++++----------
 9 files changed, 42 insertions(+), 66 deletions(-)

Patch
diff mbox series

diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h
index 050b13ec1..2592da94a 100644
--- a/include/libcamera/internal/software_isp/swstats_cpu.h
+++ b/include/libcamera/internal/software_isp/swstats_cpu.h
@@ -12,6 +12,7 @@ 
 #pragma once
 
 #include <map>
+#include <memory>
 #include <stdint.h>
 
 #include <libcamera/base/signal.h>
@@ -46,10 +47,6 @@  public:
 	 */
 	static constexpr uint32_t kStatPerNumFrames = 4;
 
-	bool isValid() const { return sharedStats_->begin()->second.fd().isValid(); }
-
-	const SharedFD &getStatsFD() { return sharedStats_->begin()->second.fd(); }
-
 	const Size &patternSize() { return patternSize_; }
 
 	int configure(const StreamConfiguration &inputCfg);
@@ -118,7 +115,12 @@  private:
 	unsigned int stride_;
 
 	std::unique_ptr<std::map<uint32_t, SharedMemObject<SwIspStats>>> sharedStats_;
-	SwIspStats stats_;
+	struct SwIspStatsRef {
+		SharedMemObject<SwIspStats> &stats;
+		SwIspStatsRef(SharedMemObject<SwIspStats> &_stats)
+			: stats(_stats) {};
+	};
+	std::unique_ptr<SwIspStatsRef> stats_;
 	Benchmark bench_;
 };
 
diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
index 5ebe2864c..129f2f6b4 100644
--- a/include/libcamera/ipa/soft.mojom
+++ b/include/libcamera/ipa/soft.mojom
@@ -14,7 +14,7 @@  struct IPAConfigInfo {
 
 interface IPASoftInterface {
 	init(libcamera.IPASettings settings,
-	     libcamera.SharedFD fdStats,
+	     array<libcamera.SharedFD> fdStats,
 	     array<libcamera.SharedFD> fdParams,
 	     libcamera.IPACameraSensorInfo sensorInfo,
 	     libcamera.ControlInfoMap sensorControls)
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index 4a681babe..b36894d5d 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -52,7 +52,7 @@  public:
 	~IPASoftSimple();
 
 	int init(const IPASettings &settings,
-		 const SharedFD &fdStats,
+		 const std::vector<SharedFD> &fdStats,
 		 const std::vector<SharedFD> &fdParams,
 		 const IPACameraSensorInfo &sensorInfo,
 		 const ControlInfoMap &sensorControls,
@@ -77,7 +77,7 @@  private:
 	void updateExposure(double exposureMSV);
 
 	std::map<unsigned int, DebayerParams *> paramsBuffers_;
-	SwIspStats *stats_;
+	std::map<unsigned int, SwIspStats *> statsBuffers_;
 	std::unique_ptr<CameraSensorHelper> camHelper_;
 	ControlInfoMap sensorInfoMap_;
 
@@ -87,14 +87,14 @@  private:
 
 IPASoftSimple::~IPASoftSimple()
 {
-	if (stats_)
-		munmap(stats_, sizeof(SwIspStats));
+	for (auto &item : statsBuffers_)
+		munmap(item.second, sizeof(SwIspStats));
 	for (auto &item : paramsBuffers_)
 		munmap(item.second, sizeof(DebayerParams));
 }
 
 int IPASoftSimple::init(const IPASettings &settings,
-			const SharedFD &fdStats,
+			const std::vector<SharedFD> &fdStats,
 			const std::vector<SharedFD> &fdParams,
 			const IPACameraSensorInfo &sensorInfo,
 			const ControlInfoMap &sensorControls,
@@ -138,11 +138,21 @@  int IPASoftSimple::init(const IPASettings &settings,
 		return ret;
 
 	*ccmEnabled = context_.ccmEnabled;
-	stats_ = nullptr;
+	for (auto &sharedFd : fdStats) {
+		if (!sharedFd.isValid()) {
+			LOG(IPASoft, Error) << "Invalid Statistics handle";
+			return -ENODEV;
+		}
 
-	if (!fdStats.isValid()) {
-		LOG(IPASoft, Error) << "Invalid Statistics handle";
-		return -ENODEV;
+		void *mem = mmap(nullptr, sizeof(SwIspStats), PROT_READ,
+				 MAP_SHARED, sharedFd.get(), 0);
+		if (mem == MAP_FAILED) {
+			LOG(IPASoft, Error) << "Unable to map Statistics";
+			return -errno;
+		}
+
+		ASSERT(sharedFd.get() >= 0);
+		statsBuffers_[sharedFd.get()] = static_cast<SwIspStats *>(mem);
 	}
 
 	for (auto &sharedFd : fdParams) {
@@ -168,17 +178,6 @@  int IPASoftSimple::init(const IPASettings &settings,
 		paramsBuffers_[sharedFd.get()] = params;
 	}
 
-	{
-		void *mem = mmap(nullptr, sizeof(SwIspStats), PROT_READ,
-				 MAP_SHARED, fdStats.get(), 0);
-		if (mem == MAP_FAILED) {
-			LOG(IPASoft, Error) << "Unable to map Statistics";
-			return -errno;
-		}
-
-		stats_ = static_cast<SwIspStats *>(mem);
-	}
-
 	ControlInfoMap::Map ctrlMap = context_.ctrlMap;
 	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
 
@@ -305,6 +304,8 @@  void IPASoftSimple::processStats(const uint32_t frame,
 {
 	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
 
+	const SwIspStats *stats = statsBuffers_.at(statsBufferId);
+
 	frameContext.sensor.exposure =
 		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
 	int32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
@@ -312,7 +313,7 @@  void IPASoftSimple::processStats(const uint32_t frame,
 
 	ControlList metadata(controls::controls);
 	for (auto const &algo : algorithms())
-		algo->process(context_, frame, frameContext, stats_, metadata);
+		algo->process(context_, frame, frameContext, stats, metadata);
 	metadataReady.emit(frame, metadata);
 	statsProcessed.emit(statsBufferId);
 
diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
index f3f241e14..3a9049d0b 100644
--- a/src/libcamera/software_isp/debayer.cpp
+++ b/src/libcamera/software_isp/debayer.cpp
@@ -143,16 +143,6 @@  Debayer::~Debayer()
  * \return The valid size ranges or an empty range if there are none
  */
 
-/**
- * \fn const SharedFD &Debayer::getStatsFD()
- * \brief Get the file descriptor for the statistics
- *
- * This file descriptor provides access to the output statistics buffer
- * associated with the current debayering process.
- *
- * \return The file descriptor pointing to the statistics data
- */
-
 /**
  * \fn unsigned int Debayer::frameSize()
  * \brief Get the output frame size
diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
index 9f08a7f4e..e640c95a0 100644
--- a/src/libcamera/software_isp/debayer.h
+++ b/src/libcamera/software_isp/debayer.h
@@ -57,8 +57,6 @@  public:
 
 	virtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;
 
-	virtual const SharedFD &getStatsFD() = 0;
-
 	unsigned int frameSize() { return outputConfig_.frameSize; }
 
 	Signal<FrameBuffer *> inputBufferReady;
diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
index 8d38fa43f..7f4428bc0 100644
--- a/src/libcamera/software_isp/debayer_cpu.h
+++ b/src/libcamera/software_isp/debayer_cpu.h
@@ -48,7 +48,6 @@  public:
 		     FrameBuffer *input,
 		     FrameBuffer *output);
 	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize);
-	const SharedFD &getStatsFD() { return stats_->getStatsFD(); }
 
 private:
 	/**
diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
index 3504c0d4f..344c4a4d2 100644
--- a/src/libcamera/software_isp/debayer_egl.h
+++ b/src/libcamera/software_isp/debayer_egl.h
@@ -60,7 +60,6 @@  public:
 	int start();
 	void stop();
 
-	const SharedFD &getStatsFD() { return stats_->getStatsFD(); }
 	unsigned int frameSize();
 
 	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize);
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index f3ee5773b..047d0fb22 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -154,7 +154,7 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 	}
 
 	ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },
-			 debayer_->getStatsFD(),
+			 fdStats,
 			 fdParams,
 			 sensorInfo,
 			 sensor->controls(),
diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
index b9f36b383..22164e33d 100644
--- a/src/libcamera/software_isp/swstats_cpu.cpp
+++ b/src/libcamera/software_isp/swstats_cpu.cpp
@@ -48,20 +48,6 @@  namespace libcamera {
  * exchange.
  */
 
-/**
- * \fn bool SwStatsCpu::isValid() const
- * \brief Gets whether the statistics object is valid
- *
- * \return True if it's valid, false otherwise
- */
-
-/**
- * \fn const SharedFD &SwStatsCpu::getStatsFD()
- * \brief Get the file descriptor for the statistics
- *
- * \return The file descriptor
- */
-
 /**
  * \fn const Size &SwStatsCpu::patternSize()
  * \brief Get the pattern size
@@ -183,12 +169,12 @@  static constexpr unsigned int kBlueYMul = 29; /* 0.114 * 256 */
 	yVal = r * kRedYMul;               \
 	yVal += g * kGreenYMul;            \
 	yVal += b * kBlueYMul;             \
-	stats_.yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;
+	stats_->stats->yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;
 
-#define SWSTATS_FINISH_LINE_STATS() \
-	stats_.sum_.r() += sumR;    \
-	stats_.sum_.g() += sumG;    \
-	stats_.sum_.b() += sumB;
+#define SWSTATS_FINISH_LINE_STATS()      \
+	stats_->stats->sum_.r() += sumR; \
+	stats_->stats->sum_.g() += sumG; \
+	stats_->stats->sum_.b() += sumB;
 
 void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])
 {
@@ -326,7 +312,7 @@  void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
  *
  * This may only be called after a successful setWindow() call.
  */
-void SwStatsCpu::startFrame(uint32_t frame, [[maybe_unused]] const uint32_t statsBufferId)
+void SwStatsCpu::startFrame(uint32_t frame, const uint32_t statsBufferId)
 {
 	if (frame % kStatPerNumFrames)
 		return;
@@ -334,8 +320,10 @@  void SwStatsCpu::startFrame(uint32_t frame, [[maybe_unused]] const uint32_t stat
 	if (window_.width == 0)
 		LOG(SwStatsCpu, Error) << "Calling startFrame() without setWindow()";
 
-	stats_.sum_ = RGB<uint64_t>({ 0, 0, 0 });
-	stats_.yHistogram.fill(0);
+	auto &s = sharedStats_->at(statsBufferId);
+	stats_ = std::make_unique<SwIspStatsRef>(s);
+	stats_->stats->sum_ = RGB<uint64_t>({ 0, 0, 0 });
+	stats_->stats->yHistogram.fill(0);
 }
 
 /**
@@ -348,8 +336,7 @@  void SwStatsCpu::startFrame(uint32_t frame, [[maybe_unused]] const uint32_t stat
 void SwStatsCpu::finishFrame(uint32_t frame,
 			     const uint32_t statsBufferId)
 {
-	stats_.valid = frame % kStatPerNumFrames == 0;
-	*(sharedStats_->at(statsBufferId)) = stats_;
+	stats_->stats->valid = frame % kStatPerNumFrames == 0;
 	statsReady.emit(frame, statsBufferId);
 }