[RFC,v2,10/14] libcamera: software_isp: Introduce arguments for statistics buffers
diff mbox series

Message ID 20260216203034.27558-11-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
Statistics in software ISP is held in two fixed buffers, copying the
statistics from the working one to the shared one in
SwIspStats::finishFrame.  This patch introduces support for a ring of
statistics buffers that are never copied, similarly to parameters
buffers and to how hardware pipelines operate.  This is to address
software ISP TODO #2.

The patch adds a new argument statsBufferId for the future parameters
buffer ids passed to the calls.  The buffer ids must be passed to the
following groups of methods:

- SwStatsCpu::startFrame, to reset the statistics.

- Debayer::process, to signal what statistics buffer is started and
  stopped being used to gather the statistics.

- SoftwareIsp::statsProcessed, a new signal to indicate that the given
  statistics buffer contents is no longer needed.

The type of the buffer id statistics is set to uint32_t because:

- It can be used in mojom.
- The same type is used for parameters buffers.
- It is consistent with the similar types in the hardware pipelines.
- It covers file descriptor number range, which will be used as buffer
  ids.

Already provided statistics buffer arguments are renamed from bufferId
to statsBufferId for consistency and to avoid confusion between
statistics and parameters buffer id arguments.

This patch doesn't do more than adding the arguments, to keep the patch
simple.  The buffer handling will be implemented in the followup
patches.

Statistics and parameters buffers use is coupled to much extent.  It can
be tempting to perhaps use the same buffer ids and perhaps some common
signals.  But it's better to keep them separate to avoid contingent
future refactoring in case this coupling becomes less tight in future.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 .../internal/software_isp/software_isp.h      |  3 ++-
 .../internal/software_isp/swstats_cpu.h       |  4 ++--
 include/libcamera/ipa/soft.mojom              |  3 ++-
 src/ipa/simple/soft_simple.cpp                |  3 ++-
 src/libcamera/pipeline/simple/simple.cpp      |  7 ++++---
 src/libcamera/software_isp/debayer.cpp        |  3 ++-
 src/libcamera/software_isp/debayer.h          |  1 +
 src/libcamera/software_isp/debayer_cpu.cpp    | 16 +++++++--------
 src/libcamera/software_isp/debayer_cpu.h      |  4 +++-
 src/libcamera/software_isp/debayer_egl.cpp    |  7 +++++--
 src/libcamera/software_isp/debayer_egl.h      |  7 +++++--
 src/libcamera/software_isp/software_isp.cpp   | 13 +++++++++---
 src/libcamera/software_isp/swstats_cpu.cpp    | 20 ++++++++++---------
 13 files changed, 56 insertions(+), 35 deletions(-)

Patch
diff mbox series

diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
index 65348e213..519dfc71a 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -93,7 +93,8 @@  private:
 	void saveIspParams(uint32_t paramsBufferId);
 	void releaseIspParams(uint32_t paramsBufferId);
 	void setSensorCtrls(const ControlList &sensorControls);
-	void statsReady(uint32_t frame, uint32_t bufferId);
+	void statsReady(uint32_t frame, const uint32_t statsBufferId);
+	void statsProcessed(const uint32_t statsBufferId);
 	void inputReady(FrameBuffer *input);
 	void outputReady(FrameBuffer *output);
 	std::unique_ptr<Debayer> debayer_;
diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h
index 64b3e23f5..ae8ce1e59 100644
--- a/include/libcamera/internal/software_isp/swstats_cpu.h
+++ b/include/libcamera/internal/software_isp/swstats_cpu.h
@@ -53,8 +53,8 @@  public:
 
 	int configure(const StreamConfiguration &inputCfg);
 	void setWindow(const Rectangle &window);
-	void startFrame(uint32_t frame);
-	void finishFrame(uint32_t frame, uint32_t bufferId);
+	void startFrame(uint32_t frame, const uint32_t statsBufferId);
+	void finishFrame(uint32_t frame, const uint32_t statsBufferId);
 	void processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input);
 
 	void processLine0(uint32_t frame, unsigned int y, const uint8_t *src[])
diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
index 4738a1b46..5ebe2864c 100644
--- a/include/libcamera/ipa/soft.mojom
+++ b/include/libcamera/ipa/soft.mojom
@@ -27,12 +27,13 @@  interface IPASoftInterface {
 	[async] queueRequest(uint32 frame, libcamera.ControlList sensorControls);
 	[async] computeParams(uint32 frame, uint32 paramsBufferId);
 	[async] processStats(uint32 frame,
-			     uint32 bufferId,
+			     uint32 statsBufferId,
 			     libcamera.ControlList sensorControls);
 };
 
 interface IPASoftEventInterface {
 	setSensorControls(libcamera.ControlList sensorControls);
+	statsProcessed(uint32 statsBufferId);
 	setIspParams(uint32 paramsBufferId);
 	metadataReady(uint32 frame, libcamera.ControlList metadata);
 };
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index f212fabe3..4a681babe 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -300,7 +300,7 @@  void IPASoftSimple::computeParams(const uint32_t frame,
 }
 
 void IPASoftSimple::processStats(const uint32_t frame,
-				 [[maybe_unused]] const uint32_t bufferId,
+				 const uint32_t statsBufferId,
 				 const ControlList &sensorControls)
 {
 	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
@@ -314,6 +314,7 @@  void IPASoftSimple::processStats(const uint32_t frame,
 	for (auto const &algo : algorithms())
 		algo->process(context_, frame, frameContext, stats_, metadata);
 	metadataReady.emit(frame, metadata);
+	statsProcessed.emit(statsBufferId);
 
 	/* Sanity check */
 	if (!sensorControls.contains(V4L2_CID_EXPOSURE) ||
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index dc9176c29..9e2c2a493 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -369,7 +369,7 @@  private:
 	void conversionInputDone(FrameBuffer *buffer);
 	void conversionOutputDone(FrameBuffer *buffer);
 
-	void ispStatsReady(uint32_t frame, uint32_t bufferId);
+	void ispStatsReady(uint32_t frame, const uint32_t statsBufferId);
 	void metadataReady(uint32_t frame, const ControlList &metadata);
 	void setSensorControls(const ControlList &sensorControls);
 };
@@ -1025,9 +1025,10 @@  void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
 		tryCompleteRequest(request);
 }
 
-void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
+void SimpleCameraData::ispStatsReady(uint32_t frame,
+				     const uint32_t statsBufferId)
 {
-	swIsp_->processStats(frame, bufferId,
+	swIsp_->processStats(frame, statsBufferId,
 			     delayedCtrls_->get(frame));
 }
 
diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
index ad47c1e8d..f3f241e14 100644
--- a/src/libcamera/software_isp/debayer.cpp
+++ b/src/libcamera/software_isp/debayer.cpp
@@ -122,9 +122,10 @@  Debayer::~Debayer()
  */
 
 /**
- * \fn void Debayer::process(uint32_t frame, const uint32_t paramsBufferId, FrameBuffer *input, FrameBuffer *output)
+ * \fn void Debayer::process(uint32_t frame, const uint32_t statsBufferId, 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] statsBufferId The id of the stats buffer to use
  * \param[in] paramsBufferId The id of the params buffer in use
  * \param[in] input The input buffer
  * \param[in] output The output buffer
diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
index a66d0cbbe..9f08a7f4e 100644
--- a/src/libcamera/software_isp/debayer.h
+++ b/src/libcamera/software_isp/debayer.h
@@ -49,6 +49,7 @@  public:
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;
 
 	virtual void process(uint32_t frame,
+			     const uint32_t statsBufferId,
 			     const uint32_t paramsBufferId,
 			     FrameBuffer *input, FrameBuffer *output) = 0;
 	virtual int start() { return 0; }
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index 927ac7d10..6f2d70617 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -846,8 +846,11 @@  void DebayerCpu::updateLookupTables(const DebayerParams *params)
 	params_ = *params;
 }
 
-void DebayerCpu::process(uint32_t frame, const uint32_t paramsBufferId,
-			 FrameBuffer *input, FrameBuffer *output)
+void DebayerCpu::process(uint32_t frame,
+			 const uint32_t statsBufferId,
+			 const uint32_t paramsBufferId,
+			 FrameBuffer *input,
+			 FrameBuffer *output)
 {
 	bench_.startFrame();
 
@@ -873,7 +876,7 @@  void DebayerCpu::process(uint32_t frame, const uint32_t paramsBufferId,
 		return;
 	}
 
-	stats_->startFrame(frame);
+	stats_->startFrame(frame, statsBufferId);
 
 	if (inputConfig_.patternSize.height == 2)
 		process2(frame, in.planes()[0].data(), out.planes()[0].data());
@@ -887,12 +890,7 @@  void DebayerCpu::process(uint32_t frame, const uint32_t paramsBufferId,
 	/* Measure before emitting signals */
 	bench_.finishFrame();
 
-	/*
-	 * Buffer ids are currently not used, so pass zeros as its parameter.
-	 *
-	 * \todo Pass real bufferId once stats buffer passing is changed.
-	 */
-	stats_->finishFrame(frame, 0);
+	stats_->finishFrame(frame, statsBufferId);
 	outputBufferReady.emit(output);
 	inputBufferReady.emit(input);
 }
diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
index 5e3670b37..8d38fa43f 100644
--- a/src/libcamera/software_isp/debayer_cpu.h
+++ b/src/libcamera/software_isp/debayer_cpu.h
@@ -43,8 +43,10 @@  public:
 	std::tuple<unsigned int, unsigned int>
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size);
 	void process(uint32_t frame,
+		     const uint32_t statsBufferId,
 		     const uint32_t paramsBufferId,
-		     FrameBuffer *input, FrameBuffer *output);
+		     FrameBuffer *input,
+		     FrameBuffer *output);
 	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize);
 	const SharedFD &getStatsFD() { return stats_->getStatsFD(); }
 
diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
index c24590994..5715b80ae 100644
--- a/src/libcamera/software_isp/debayer_egl.cpp
+++ b/src/libcamera/software_isp/debayer_egl.cpp
@@ -536,8 +536,11 @@  int DebayerEGL::debayerGPU(MappedFrameBuffer &in, int out_fd, const DebayerParam
 	return 0;
 }
 
-void DebayerEGL::process(uint32_t frame, const uint32_t paramsBufferId,
-			 FrameBuffer *input, FrameBuffer *output)
+void DebayerEGL::process(uint32_t frame,
+			 [[maybe_unused]] const uint32_t statsBufferId,
+			 const uint32_t paramsBufferId,
+			 FrameBuffer *input,
+			 FrameBuffer *output)
 {
 	bench_.startFrame();
 
diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
index 435ff7d31..3504c0d4f 100644
--- a/src/libcamera/software_isp/debayer_egl.h
+++ b/src/libcamera/software_isp/debayer_egl.h
@@ -52,8 +52,11 @@  public:
 	std::vector<PixelFormat> formats(PixelFormat input);
 	std::tuple<unsigned int, unsigned int> strideAndFrameSize(const PixelFormat &outputFormat, const Size &size);
 
-	void process(uint32_t frame, const uint32_t paramsBufferId,
-		     FrameBuffer *input, FrameBuffer *output);
+	void process(uint32_t frame,
+		     const uint32_t statsBufferId,
+		     const uint32_t paramsBufferId,
+		     FrameBuffer *input,
+		     FrameBuffer *output);
 	int start();
 	void stop();
 
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 230396bee..04aaf7a8c 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -164,6 +164,7 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 		return;
 	}
 
+	ipa_->statsProcessed.connect(this, &SoftwareIsp::statsProcessed);
 	ipa_->setIspParams.connect(this, &SoftwareIsp::saveIspParams);
 	ipa_->metadataReady.connect(this,
 				    [this](uint32_t frame, const ControlList &metadata) {
@@ -429,8 +430,10 @@  void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *outpu
 	const uint32_t paramsBufferId = availableParams_.front();
 	availableParams_.pop();
 	ipa_->computeParams(frame, paramsBufferId);
+	const uint32_t statsBufferId = 0;
 	debayer_->invokeMethod(&Debayer::process,
-			       ConnectionTypeQueued, frame, paramsBufferId, input, output);
+			       ConnectionTypeQueued, frame,
+			       statsBufferId, paramsBufferId, input, output);
 }
 
 void SoftwareIsp::saveIspParams(uint32_t paramsBufferId)
@@ -448,9 +451,13 @@  void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)
 	setSensorControls.emit(sensorControls);
 }
 
-void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)
+void SoftwareIsp::statsReady(uint32_t frame, const uint32_t statsBufferId)
+{
+	ispStatsReady.emit(frame, statsBufferId);
+}
+
+void SoftwareIsp::statsProcessed([[maybe_unused]] const uint32_t statsBufferId)
 {
-	ispStatsReady.emit(frame, bufferId);
 }
 
 void SoftwareIsp::inputReady(FrameBuffer *input)
diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
index 5c3011a7f..a3b11c443 100644
--- a/src/libcamera/software_isp/swstats_cpu.cpp
+++ b/src/libcamera/software_isp/swstats_cpu.cpp
@@ -321,10 +321,11 @@  void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
 /**
  * \brief Reset state to start statistics gathering for a new frame
  * \param[in] frame The frame number
+ * \param[in] statsBufferId ID of the statistics buffer
  *
  * This may only be called after a successful setWindow() call.
  */
-void SwStatsCpu::startFrame(uint32_t frame)
+void SwStatsCpu::startFrame(uint32_t frame, [[maybe_unused]] const uint32_t statsBufferId)
 {
 	if (frame % kStatPerNumFrames)
 		return;
@@ -339,15 +340,16 @@  void SwStatsCpu::startFrame(uint32_t frame)
 /**
  * \brief Finish statistics calculation for the current frame
  * \param[in] frame The frame number
- * \param[in] bufferId ID of the statistics buffer
+ * \param[in] statsBufferId ID of the statistics buffer
  *
  * This may only be called after a successful setWindow() call.
  */
-void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId)
+void SwStatsCpu::finishFrame(uint32_t frame,
+			     const uint32_t statsBufferId)
 {
 	stats_.valid = frame % kStatPerNumFrames == 0;
 	*sharedStats_ = stats_;
-	statsReady.emit(frame, bufferId);
+	statsReady.emit(frame, statsBufferId);
 }
 
 /**
@@ -512,20 +514,20 @@  void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in)
 /**
  * \brief Calculate statistics for a frame in one go
  * \param[in] frame The frame number
- * \param[in] bufferId ID of the statistics buffer
+ * \param[in] statsBufferId ID of the statistics buffer
  * \param[in] input The frame to process
  *
  * This may only be called after a successful setWindow() call.
  */
-void SwStatsCpu::processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input)
+void SwStatsCpu::processFrame(uint32_t frame, uint32_t statsBufferId, FrameBuffer *input)
 {
 	if (frame % kStatPerNumFrames) {
-		finishFrame(frame, bufferId);
+		finishFrame(frame, statsBufferId);
 		return;
 	}
 
 	bench_.startFrame();
-	startFrame(frame);
+	startFrame(frame, statsBufferId);
 
 	MappedFrameBuffer in(input, MappedFrameBuffer::MapFlag::Read);
 	if (!in.isValid()) {
@@ -534,7 +536,7 @@  void SwStatsCpu::processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *in
 	}
 
 	(this->*processFrame_)(in);
-	finishFrame(frame, bufferId);
+	finishFrame(frame, statsBufferId);
 	bench_.finishFrame();
 }