[PATCH 15/16] libcamera: software_isp: Share statistics buffers with IPA

Milan Zamazal mzamazal at redhat.com
Mon Aug 12 13:50:04 CEST 2024


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 at redhat.com>
---
 include/libcamera/ipa/soft.mojom            |  2 +-
 src/ipa/simple/soft_simple.cpp              | 43 +++++++++++----------
 src/libcamera/software_isp/debayer_cpu.h    |  7 ----
 src/libcamera/software_isp/software_isp.cpp |  2 +-
 src/libcamera/software_isp/swstats_cpu.cpp  | 37 ++++++------------
 src/libcamera/software_isp/swstats_cpu.h    | 12 +++---
 6 files changed, 43 insertions(+), 60 deletions(-)

diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
index 7b85c454..e5af50d6 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.ControlInfoMap sensorCtrlInfoMap)
 		=> (int32 ret);
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index 3c95c4d9..2769bce2 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -47,7 +47,7 @@ public:
 	~IPASoftSimple();
 
 	int init(const IPASettings &settings,
-		 const SharedFD &fdStats,
+		 const std::vector<SharedFD> &fdStats,
 		 const std::vector<SharedFD> &fdParams,
 		 const ControlInfoMap &sensorInfoMap) override;
 	int configure(const IPAConfigInfo &configInfo) override;
@@ -67,7 +67,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_;
 
@@ -77,14 +77,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 ControlInfoMap &sensorInfoMap)
 {
@@ -122,11 +122,21 @@ int IPASoftSimple::init(const IPASettings &settings,
 	if (ret)
 		return ret;
 
-	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) {
@@ -146,17 +156,6 @@ int IPASoftSimple::init(const IPASettings &settings,
 		paramsBuffers_[sharedFd.get()] = static_cast<DebayerParams *>(mem);
 	}
 
-	{
-		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);
-	}
-
 	/*
 	 * Check if the sensor driver supports the controls required by the
 	 * Soft IPA.
@@ -271,6 +270,8 @@ void IPASoftSimple::processStats(
 {
 	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>();
@@ -284,7 +285,7 @@ void IPASoftSimple::processStats(
 	 */
 	ControlList metadata(controls::controls);
 	for (auto const &algo : algorithms())
-		algo->process(context_, frame, frameContext, stats_, metadata);
+		algo->process(context_, frame, frameContext, stats, metadata);
 	statsProcessed.emit(statsBufferId);
 
 	/* Sanity check */
diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
index 7fb399b0..36300534 100644
--- a/src/libcamera/software_isp/debayer_cpu.h
+++ b/src/libcamera/software_isp/debayer_cpu.h
@@ -43,13 +43,6 @@ public:
 		     FrameBuffer *input, FrameBuffer *output);
 	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize);
 
-	/**
-	 * \brief Get the file descriptor for the statistics
-	 *
-	 * \return the file descriptor pointing to the statistics
-	 */
-	const SharedFD &getStatsFD() { return stats_->getStatsFD(); }
-
 	/**
 	 * \brief Get the output frame size
 	 *
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index c4ad22be..025093a9 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -113,7 +113,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 		ipa_->configurationFile(sensor->model() + ".yaml", "uncalibrated.yaml");
 
 	int ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },
-			     debayer_->getStatsFD(),
+			     fdStats,
 			     fdParams,
 			     sensor->controls());
 	if (ret) {
diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
index cb411a18..2c7acf47 100644
--- a/src/libcamera/software_isp/swstats_cpu.cpp
+++ b/src/libcamera/software_isp/swstats_cpu.cpp
@@ -36,20 +36,6 @@ namespace libcamera {
  * instead of processing the whole frame.
  */
 
-/**
- * \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
@@ -161,12 +147,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_.sumR_ += sumR;       \
-	stats_.sumG_ += sumG;       \
-	stats_.sumB_ += sumB;
+#define SWSTATS_FINISH_LINE_STATS()   \
+	stats_->stats->sumR_ += sumR; \
+	stats_->stats->sumG_ += sumG; \
+	stats_->stats->sumB_ += sumB;
 
 void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])
 {
@@ -302,15 +288,17 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
  *
  * This may only be called after a successful setWindow() call.
  */
-void SwStatsCpu::startFrame([[maybe_unused]] const uint32_t statsBufferId)
+void SwStatsCpu::startFrame(const uint32_t statsBufferId)
 {
 	if (window_.width == 0)
 		LOG(SwStatsCpu, Error) << "Calling startFrame() without setWindow()";
 
-	stats_.sumR_ = 0;
-	stats_.sumB_ = 0;
-	stats_.sumG_ = 0;
-	stats_.yHistogram.fill(0);
+	auto &s = sharedStats_->at(statsBufferId);
+	stats_ = std::make_unique<SwIspStatsRef>(s);
+	stats_->stats->sumR_ = 0;
+	stats_->stats->sumB_ = 0;
+	stats_->stats->sumG_ = 0;
+	stats_->stats->yHistogram.fill(0);
 }
 
 /**
@@ -320,7 +308,6 @@ void SwStatsCpu::startFrame([[maybe_unused]] const uint32_t statsBufferId)
  */
 void SwStatsCpu::finishFrame(uint32_t frame, const uint32_t statsBufferId)
 {
-	*(sharedStats_->at(statsBufferId)) = stats_;
 	statsReady.emit(frame, statsBufferId);
 }
 
diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h
index fbdea4a3..2b4a98c2 100644
--- a/src/libcamera/software_isp/swstats_cpu.h
+++ b/src/libcamera/software_isp/swstats_cpu.h
@@ -12,6 +12,7 @@
 #pragma once
 
 #include <map>
+#include <memory>
 #include <stdint.h>
 
 #include <libcamera/base/signal.h>
@@ -33,10 +34,6 @@ public:
 	SwStatsCpu(std::unique_ptr<std::map<uint32_t, SharedMemObject<SwIspStats>>> sharedStats);
 	~SwStatsCpu() = default;
 
-	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);
@@ -92,7 +89,12 @@ private:
 	unsigned int xShift_;
 
 	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_;
 };
 
 } /* namespace libcamera */
-- 
2.44.1



More information about the libcamera-devel mailing list