[RFC,v3,11/17] libcamera: software_isp: Share parameters buffers with debayering

26836 diff mbox series

diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
index e3419305b..9ea0ed4db 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -89,7 +89,6 @@  public:
 	Signal<const ControlList &> setSensorControls;
 
 private:
-	void saveIspParams(const uint32_t paramsBufferId);
 	void paramsBufferReady(const uint32_t paramsBufferId);
 	bool allocateParamsBuffers(const unsigned int bufferCount);
 	void setSensorCtrls(const ControlList &sensorControls);
@@ -99,7 +98,6 @@  private:
 	std::unique_ptr<Debayer> debayer_;
 	Thread ispWorkerThread_;
 	std::map<uint32_t, SharedMemObject<DebayerParams>> sharedParams_;
-	DebayerParams debayerParams_;
 	std::vector<uint32_t> availableParams_;
 	DmaBufAllocator dmaHeap_;
 	bool ccmEnabled_;
diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
index 7b1be52b2..28e04d0df 100644
--- a/src/libcamera/software_isp/debayer.cpp
+++ b/src/libcamera/software_isp/debayer.cpp
@@ -11,6 +11,8 @@ 
 
 #include "debayer.h"
 
+#include <sys/mman.h>
+
 namespace libcamera {
 
 /**
@@ -54,15 +56,29 @@  LOG_DEFINE_CATEGORY(Debayer)
 
 /**
  * \brief Construct a Debayer object
+ * \param[in] paramsBuffers ids and SharedFDs of parameter buffers
  * \param[in] cm The camera manager
  */
-Debayer::Debayer(const CameraManager &cm)
+Debayer::Debayer(const std::map<uint32_t, SharedFD> &paramsBuffers, const CameraManager &cm)
 	: bench_(cm, "Debayer")
 {
+	for (auto &[bufferId, sharedFd] : paramsBuffers) {
+		void *mem = mmap(nullptr, sizeof(DebayerParams),
+				 PROT_READ | PROT_WRITE, MAP_SHARED,
+				 sharedFd.get(), 0);
+		if (mem == MAP_FAILED) {
+			LOG(Debayer, Error) << "Unable to map Parameters";
+			return;
+		}
+
+		paramsBuffers_[bufferId] = static_cast<DebayerParams *>(mem);
+	}
 }
 
 Debayer::~Debayer()
 {
+	for (auto &item : paramsBuffers_)
+		munmap(item.second, sizeof(DebayerParams));
 }
 
 /**
@@ -104,16 +120,12 @@  Debayer::~Debayer()
  */
 
 /**
- * \fn void Debayer::process(uint32_t frame, const uint32_t paramsBufferId, FrameBuffer *input, FrameBuffer *output, DebayerParams params)
+ * \fn void Debayer::process(uint32_t frame, const uint32_t paramsBufferId, FrameBuffer *input, FrameBuffer *output)
  * \brief Process the bayer data into the requested format
  * \param[in] frame The frame number
  * \param[in] paramsBufferId The id of the params buffer in use
  * \param[in] input The input buffer
  * \param[in] output The output buffer
- * \param[in] params The parameters to be used in debayering
- *
- * \note DebayerParams is passed by value deliberately so that a copy is passed
- * when this is run in another thread by invokeMethod().
  */
 
 /**
@@ -240,6 +252,11 @@  Debayer::~Debayer()
  * reversed.
  */
 
+/**
+ * \var Debayer::paramsBuffers_
+ * \brief Ring of debayering parameters buffers
+ */
+
 /**
  * \var Debayer::bench_
  * \brief Benchmarking utility instance for performance measurements
diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
index 9a97851b7..6cefa4c01 100644
--- a/src/libcamera/software_isp/debayer.h
+++ b/src/libcamera/software_isp/debayer.h
@@ -1,7 +1,7 @@ 
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 /*
  * Copyright (C) 2023, Linaro Ltd
- * Copyright (C) 2023, Red Hat Inc.
+ * Copyright (C) 2023-2026 Red Hat Inc.
  *
  * Authors:
  * Hans de Goede <hdegoede@redhat.com>
@@ -35,7 +35,8 @@  LOG_DECLARE_CATEGORY(Debayer)
 class Debayer : public Object
 {
 public:
-	Debayer(const CameraManager &cm);
+	Debayer(const std::map<uint32_t, SharedFD> &paramsBuffers,
+		const CameraManager &cm);
 	virtual ~Debayer() = 0;
 
 	virtual int configure(const StreamConfiguration &inputCfg,
@@ -49,8 +50,7 @@  public:
 
 	virtual void process(uint32_t frame,
 			     const uint32_t paramsBufferId,
-			     FrameBuffer *input, FrameBuffer *output,
-			     const DebayerParams &params) = 0;
+			     FrameBuffer *input, FrameBuffer *output) = 0;
 	virtual int start() { return 0; }
 	virtual void stop() {}
 
@@ -83,6 +83,7 @@  public:
 	PixelFormat inputPixelFormat_;
 	PixelFormat outputPixelFormat_;
 	bool swapRedBlueGains_;
+	std::map<uint32_t, DebayerParams *> paramsBuffers_;
 	Benchmark bench_;
 
 private:
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index 51b58fce5..7813eb695 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -12,12 +12,12 @@ 
 #include "debayer_cpu.h"
 
 #include <algorithm>
-#include <stdlib.h>
 #include <sys/ioctl.h>
 #include <utility>
 
 #include <linux/dma-buf.h>
 
+#include <libcamera/base/shared_fd.h>
 #include <libcamera/base/thread.h>
 
 #include <libcamera/formats.h>
@@ -90,10 +90,13 @@  DebayerCpuThread::DebayerCpuThread(DebayerCpu *debayer, unsigned int threadIndex
 /**
  * \brief Constructs a DebayerCpu object
  * \param[in] stats Pointer to the stats object to use
+ * \param[in] paramsBuffers SharedFDs of parameter buffers
  * \param[in] cm The camera manager
  */
-DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const CameraManager &cm)
-	: Debayer(cm), stats_(std::move(stats))
+DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats,
+		       const std::map<uint32_t, SharedFD> &paramsBuffers,
+		       const CameraManager &cm)
+	: Debayer(paramsBuffers, cm), stats_(std::move(stats))
 {
 	/*
 	 * Reading from uncached buffers may be very slow.
@@ -879,13 +882,13 @@  void DebayerCpuThread::process4(uint32_t frame, const uint8_t *src, uint8_t *dst
 	}
 }
 
-void DebayerCpu::updateGammaTable(const DebayerParams &params)
+void DebayerCpu::updateGammaTable(const DebayerParams *params)
 {
-	const RGB<float> blackLevel = params.blackLevel;
+	const RGB<float> blackLevel = params->blackLevel;
 	/* Take let's say the green channel black level */
 	const unsigned int blackIndex = blackLevel[1] * gammaTable_.size();
-	const float gamma = params.gamma;
-	const float contrastExp = params.contrastExp;
+	const float gamma = params->gamma;
+	const float contrastExp = params->contrastExp;
 
 	const float divisor = gammaTable_.size() - blackIndex - 1.0;
 	for (unsigned int i = blackIndex; i < gammaTable_.size(); i++) {
@@ -909,12 +912,12 @@  void DebayerCpu::updateGammaTable(const DebayerParams &params)
 		  gammaTable_[blackIndex]);
 }
 
-void DebayerCpu::updateLookupTables(const DebayerParams &params)
+void DebayerCpu::updateLookupTables(const DebayerParams *params)
 {
 	const bool gammaUpdateNeeded =
-		params.gamma != params_.gamma ||
-		params.blackLevel != params_.blackLevel ||
-		params.contrastExp != params_.contrastExp;
+		params->gamma != params_.gamma ||
+		params->blackLevel != params_.blackLevel ||
+		params->contrastExp != params_.contrastExp;
 	if (gammaUpdateNeeded)
 		updateGammaTable(params);
 
@@ -925,7 +928,7 @@  void DebayerCpu::updateLookupTables(const DebayerParams &params)
 	const double div = static_cast<double>(kRGBLookupSize) / gammaTableSize;
 	if (ccmEnabled_) {
 		if (gammaUpdateNeeded ||
-		    matrixChanged(params.combinedMatrix, params_.combinedMatrix)) {
+		    matrixChanged(params->combinedMatrix, params_.combinedMatrix)) {
 			auto &red = swapRedBlueGains_ ? blueCcm_ : redCcm_;
 			auto &green = greenCcm_;
 			auto &blue = swapRedBlueGains_ ? redCcm_ : blueCcm_;
@@ -933,21 +936,21 @@  void DebayerCpu::updateLookupTables(const DebayerParams &params)
 			const unsigned int greenIndex = 1;
 			const unsigned int blueIndex = swapRedBlueGains_ ? 0 : 2;
 			for (unsigned int i = 0; i < kRGBLookupSize; i++) {
-				red[i].r = std::round(i * params.combinedMatrix[redIndex][0]);
-				red[i].g = std::round(i * params.combinedMatrix[greenIndex][0]);
-				red[i].b = std::round(i * params.combinedMatrix[blueIndex][0]);
-				green[i].r = std::round(i * params.combinedMatrix[redIndex][1]);
-				green[i].g = std::round(i * params.combinedMatrix[greenIndex][1]);
-				green[i].b = std::round(i * params.combinedMatrix[blueIndex][1]);
-				blue[i].r = std::round(i * params.combinedMatrix[redIndex][2]);
-				blue[i].g = std::round(i * params.combinedMatrix[greenIndex][2]);
-				blue[i].b = std::round(i * params.combinedMatrix[blueIndex][2]);
+				red[i].r = std::round(i * params->combinedMatrix[redIndex][0]);
+				red[i].g = std::round(i * params->combinedMatrix[greenIndex][0]);
+				red[i].b = std::round(i * params->combinedMatrix[blueIndex][0]);
+				green[i].r = std::round(i * params->combinedMatrix[redIndex][1]);
+				green[i].g = std::round(i * params->combinedMatrix[greenIndex][1]);
+				green[i].b = std::round(i * params->combinedMatrix[blueIndex][1]);
+				blue[i].r = std::round(i * params->combinedMatrix[redIndex][2]);
+				blue[i].g = std::round(i * params->combinedMatrix[greenIndex][2]);
+				blue[i].b = std::round(i * params->combinedMatrix[blueIndex][2]);
 				gammaLut_[i] = gammaTable_[i / div];
 			}
 		}
 	} else {
-		if (gammaUpdateNeeded || params.gains != params_.gains) {
-			auto &gains = params.gains;
+		if (gammaUpdateNeeded || params->gains != params_.gains) {
+			auto &gains = params->gains;
 			auto &red = swapRedBlueGains_ ? blue_ : red_;
 			auto &green = green_;
 			auto &blue = swapRedBlueGains_ ? red_ : blue_;
@@ -962,18 +965,17 @@  void DebayerCpu::updateLookupTables(const DebayerParams &params)
 	}
 
 	LOG(Debayer, Debug)
-		<< "Debayer parameters: blackLevel=" << params.blackLevel
-		<< "; gamma=" << params.gamma
-		<< "; contrastExp=" << params.contrastExp
-		<< "; gains=" << params.gains
-		<< "; matrix=" << params.combinedMatrix;
+		<< "Debayer parameters: blackLevel=" << params->blackLevel
+		<< "; gamma=" << params->gamma
+		<< "; contrastExp=" << params->contrastExp
+		<< "; gains=" << params->gains
+		<< "; matrix=" << params->combinedMatrix;
 
-	params_ = params;
+	params_ = *params;
 }
 
 void DebayerCpu::process(uint32_t frame, const uint32_t paramsBufferId,
-			 FrameBuffer *input, FrameBuffer *output,
-			 const DebayerParams &params)
+			 FrameBuffer *input, FrameBuffer *output)
 {
 	bench_.startFrame();
 
@@ -981,8 +983,8 @@  void DebayerCpu::process(uint32_t frame, const uint32_t paramsBufferId,
 
 	dmaSyncBegin(dmaSyncers, input, output);
 
+	DebayerParams *params = paramsBuffers_.at(paramsBufferId);
 	updateLookupTables(params);
-
 	paramsBufferReady.emit(paramsBufferId);
 
 	/* Copy metadata from the input buffer */
diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
index 1b4e8548a..56d9087da 100644
--- a/src/libcamera/software_isp/debayer_cpu.h
+++ b/src/libcamera/software_isp/debayer_cpu.h
@@ -17,6 +17,7 @@ 
 
 #include <libcamera/base/mutex.h>
 #include <libcamera/base/object.h>
+#include <libcamera/base/shared_fd.h>
 
 #include <libcamera/camera_manager.h>
 
@@ -32,7 +33,9 @@  class DebayerCpuThread;
 class DebayerCpu : public Debayer
 {
 public:
-	DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const CameraManager &cm);
+	DebayerCpu(std::unique_ptr<SwStatsCpu> stats,
+		   const std::map<uint32_t, SharedFD> &paramsBuffers,
+		   const CameraManager &cm);
 	~DebayerCpu();
 
 	int configure(const StreamConfiguration &inputCfg,
@@ -43,8 +46,7 @@  public:
 	std::tuple<unsigned int, unsigned int>
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) override;
 	void process(uint32_t frame, const uint32_t paramsBufferId,
-		     FrameBuffer *input, FrameBuffer *output,
-		     const DebayerParams &params) override;
+		     FrameBuffer *input, FrameBuffer *output) override;
 	int start() override;
 	void stop() override;
 	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) override;
@@ -119,8 +121,8 @@  private:
 	int setDebayerFunctions(PixelFormat inputFormat,
 				PixelFormat outputFormat,
 				bool ccmEnabled);
-	void updateGammaTable(const DebayerParams &params);
-	void updateLookupTables(const DebayerParams &params);
+	void updateGammaTable(const DebayerParams *params);
+	void updateLookupTables(const DebayerParams *params);
 
 	static constexpr unsigned int kRGBLookupSize = 256;
 	static constexpr unsigned int kGammaLookupSize = 1024;
diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
index 60fd5463f..77696f1e7 100644
--- a/src/libcamera/software_isp/debayer_egl.cpp
+++ b/src/libcamera/software_isp/debayer_egl.cpp
@@ -37,10 +37,13 @@  namespace libcamera {
 /**
  * \brief Construct a DebayerEGL object
  * \param[in] stats Statistics processing object
+ * \param[in] paramsBuffers SharedFDs of parameter buffers
  * \param[in] cm The camera manager
  */
-DebayerEGL::DebayerEGL(std::unique_ptr<SwStatsCpu> stats, const CameraManager &cm)
-	: Debayer(cm), stats_(std::move(stats))
+DebayerEGL::DebayerEGL(std::unique_ptr<SwStatsCpu> stats,
+		       const std::map<uint32_t, SharedFD> &paramsBuffers,
+		       const CameraManager &cm)
+	: Debayer(paramsBuffers, cm), stats_(std::move(stats))
 {
 }
 
@@ -547,8 +550,7 @@  int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const Debaye
 }
 
 void DebayerEGL::process(uint32_t frame, const uint32_t paramsBufferId,
-			 FrameBuffer *input, FrameBuffer *output,
-			 const DebayerParams &params)
+			 FrameBuffer *input, FrameBuffer *output)
 {
 	bench_.startFrame();
 
@@ -561,11 +563,13 @@  void DebayerEGL::process(uint32_t frame, const uint32_t paramsBufferId,
 	std::optional<MappedFrameBuffer> inMapped;
 	std::optional<DmaSyncer> inDmaSyncer;
 
+	DebayerParams params = *paramsBuffers_.at(paramsBufferId);
+	paramsBufferReady.emit(paramsBufferId);
+
 	if (debayerGPU(input, output, params, &inMapped, &inDmaSyncer)) {
 		LOG(Debayer, Error) << "debayerGPU failed";
 		goto error;
 	}
-	paramsBufferReady.emit(paramsBufferId);
 
 	metadata.planes()[0].bytesused = output->planes()[0].length;
 
@@ -596,7 +600,6 @@  void DebayerEGL::process(uint32_t frame, const uint32_t paramsBufferId,
 	return;
 
 error:
-	paramsBufferReady.emit(paramsBufferId);
 	bench_.finishFrame();
 	metadata.status = FrameMetadata::FrameError;
 	return;
diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
index a3a4448e7..43e3dcb15 100644
--- a/src/libcamera/software_isp/debayer_egl.h
+++ b/src/libcamera/software_isp/debayer_egl.h
@@ -39,7 +39,9 @@  class CameraManager;
 class DebayerEGL : public Debayer
 {
 public:
-	DebayerEGL(std::unique_ptr<SwStatsCpu> stats, const CameraManager &cm);
+	DebayerEGL(std::unique_ptr<SwStatsCpu> stats,
+		   const std::map<uint32_t, SharedFD> &paramsBuffers,
+		   const CameraManager &cm);
 	~DebayerEGL();
 
 	int configure(const StreamConfiguration &inputCfg,
@@ -52,8 +54,7 @@  public:
 	std::tuple<unsigned int, unsigned int> strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) override;
 
 	void process(uint32_t frame, const uint32_t paramsBufferId,
-		     FrameBuffer *input, FrameBuffer *output,
-		     const DebayerParams &params) override;
+		     FrameBuffer *input, FrameBuffer *output) override;
 	int start() override;
 	void stop() override;
 
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 7f3bfe812..dbe4cc270 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -106,6 +106,10 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe,
 	}
 	stats->statsReady.connect(this, &SoftwareIsp::statsReady);
 
+	std::map<uint32_t, SharedFD> fdParams;
+	for (auto &[bufferId, item] : sharedParams_)
+		fdParams[bufferId] = item.fd();
+
 #if HAVE_DEBAYER_EGL
 	const GlobalConfiguration &configuration = cm._d()->configuration();
 	std::optional<std::string> softISPMode = configuration.option<std::string>({ "software_isp", "mode" });
@@ -120,15 +124,14 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe,
 	}
 
 	if (!softISPMode || softISPMode == "gpu")
-		debayer_ = std::make_unique<DebayerEGL>(std::move(stats), cm);
+		debayer_ = std::make_unique<DebayerEGL>(std::move(stats), fdParams,
+							cm);
 
 #endif
 	if (!debayer_)
-		debayer_ = std::make_unique<DebayerCpu>(std::move(stats), cm);
+		debayer_ = std::make_unique<DebayerCpu>(std::move(stats), fdParams,
+							cm);
 
-	std::map<uint32_t, SharedFD> fdParams;
-	for (auto &[bufferId, item] : sharedParams_)
-		fdParams[bufferId] = item.fd();
 	debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady);
 	debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);
 	debayer_->paramsBufferReady.connect(this, &SoftwareIsp::paramsBufferReady);
@@ -168,7 +171,6 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe,
 		return;
 	}
 
-	ipa_->paramsComputed.connect(this, &SoftwareIsp::saveIspParams);
 	ipa_->metadataReady.connect(this,
 				    [this](uint32_t frame, const ControlList &metadata) {
 					    metadataReady.emit(frame, metadata);
@@ -444,16 +446,11 @@  int SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output
 	ipa_->computeParams(frame, paramsBufferId);
 	debayer_->invokeMethod(&Debayer::process,
 			       ConnectionTypeQueued, frame, paramsBufferId,
-			       input, output, debayerParams_);
+			       input, output);
 
 	return 0;
 }
 
-void SoftwareIsp::saveIspParams(const uint32_t paramsBufferId)
-{
-	debayerParams_ = *sharedParams_.at(paramsBufferId);
-}
-
 void SoftwareIsp::paramsBufferReady(const uint32_t paramsBufferId)
 {
 	availableParams_.push_back(paramsBufferId);