[12/16] libcamera: software_isp: Introduce arguments for statistics buffers
diff mbox series

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

Commit Message

Milan Zamazal Aug. 12, 2024, 11:50 a.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>
---
 .../libcamera/internal/software_isp/software_isp.h  |  3 ++-
 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                |  2 +-
 src/libcamera/software_isp/debayer_cpu.cpp          | 11 +++--------
 src/libcamera/software_isp/debayer_cpu.h            |  2 +-
 src/libcamera/software_isp/software_isp.cpp         | 13 ++++++++++---
 src/libcamera/software_isp/swstats_cpu.cpp          |  6 +++---
 src/libcamera/software_isp/swstats_cpu.h            |  4 ++--
 11 files changed, 32 insertions(+), 25 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 8956978c..ae64b247 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -89,7 +89,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);
 
diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
index 86dd988f..7b85c454 100644
--- a/include/libcamera/ipa/soft.mojom
+++ b/include/libcamera/ipa/soft.mojom
@@ -25,10 +25,11 @@  interface IPASoftInterface {
 
 	[async] queueRequest(uint32 frame, libcamera.ControlList sensorControls);
 	prepare(uint32 frame, uint32 paramsBufferId);
-	[async] processStats(uint32 frame, uint32 bufferId, libcamera.ControlList sensorControls);
+	[async] processStats(uint32 frame, uint32 statsBufferId, libcamera.ControlList sensorControls);
 };
 
 interface IPASoftEventInterface {
 	setSensorControls(libcamera.ControlList sensorControls);
+	statsProcessed(uint32 statsBufferId);
 	setIspParams(uint32 paramsBufferId);
 };
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index 50f8f194..3c95c4d9 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -266,7 +266,7 @@  void IPASoftSimple::prepare(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);
@@ -285,6 +285,7 @@  void IPASoftSimple::processStats(
 	ControlList metadata(controls::controls);
 	for (auto const &algo : algorithms())
 		algo->process(context_, frame, frameContext, stats_, 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 ebead8f0..d9791ab1 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -295,7 +295,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 setSensorControls(const ControlList &sensorControls);
 };
 
@@ -901,10 +901,11 @@  void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
 		pipe->completeRequest(request);
 }
 
-void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
+void SimpleCameraData::ispStatsReady(uint32_t frame,
+				     const uint32_t statsBufferId)
 {
 	swIsp_->processStats(
-		frame, bufferId,
+		frame, statsBufferId,
 		delayedCtrls_->get(frame));
 }
 
diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
index 52df8c23..af54bb18 100644
--- a/src/libcamera/software_isp/debayer.cpp
+++ b/src/libcamera/software_isp/debayer.cpp
@@ -94,9 +94,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 251b14fd..0be7c59a 100644
--- a/src/libcamera/software_isp/debayer.h
+++ b/src/libcamera/software_isp/debayer.h
@@ -41,7 +41,7 @@  public:
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;
 
 	virtual void process(uint32_t frame,
-			     const uint32_t paramsBufferId,
+			     const uint32_t statsBufferId, const uint32_t paramsBufferId,
 			     FrameBuffer *input, FrameBuffer *output) = 0;
 
 	virtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index d3831474..14bd58b5 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -742,7 +742,7 @@  static inline int64_t timeDiff(timespec &after, timespec &before)
 }
 
 void DebayerCpu::process(uint32_t frame,
-			 const uint32_t paramsBufferId,
+			 const uint32_t statsBufferId, const uint32_t paramsBufferId,
 			 FrameBuffer *input, FrameBuffer *output)
 {
 	timespec frameStartTime;
@@ -772,7 +772,7 @@  void DebayerCpu::process(uint32_t frame,
 		return;
 	}
 
-	stats_->startFrame();
+	stats_->startFrame(statsBufferId);
 
 	if (inputConfig_.patternSize.height == 2)
 		process2(in.planes()[0].data(), out.planes()[0].data());
@@ -799,12 +799,7 @@  void DebayerCpu::process(uint32_t frame,
 		}
 	}
 
-	/*
-	 * 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 f25520be..7fb399b0 100644
--- a/src/libcamera/software_isp/debayer_cpu.h
+++ b/src/libcamera/software_isp/debayer_cpu.h
@@ -39,7 +39,7 @@  public:
 	std::tuple<unsigned int, unsigned int>
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size);
 	void process(uint32_t frame,
-		     const uint32_t paramsBufferId,
+		     const uint32_t statsBufferId, const uint32_t paramsBufferId,
 		     FrameBuffer *input, FrameBuffer *output);
 	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 db77f6f9..5d9f5008 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -120,6 +120,7 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 		return;
 	}
 
+	ipa_->statsProcessed.connect(this, &SoftwareIsp::statsProcessed);
 	ipa_->setIspParams.connect(this, &SoftwareIsp::saveIspParams);
 	ipa_->setSensorControls.connect(this, &SoftwareIsp::setSensorCtrls);
 
@@ -386,8 +387,10 @@  void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *outpu
 	const uint32_t paramsBufferId = availableParams_.front();
 	availableParams_.pop();
 	ipa_->prepare(frame, paramsBufferId);
+	const uint32_t statsBufferId = 0;
 	debayer_->invokeMethod(&DebayerCpu::process,
-			       ConnectionTypeQueued, frame, paramsBufferId, input, output);
+			       ConnectionTypeQueued, frame,
+			       statsBufferId, paramsBufferId, input, output);
 }
 
 void SoftwareIsp::saveIspParams(uint32_t paramsBufferId)
@@ -405,9 +408,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 a9b1f35a..b8ee06a0 100644
--- a/src/libcamera/software_isp/swstats_cpu.cpp
+++ b/src/libcamera/software_isp/swstats_cpu.cpp
@@ -298,7 +298,7 @@  void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
  *
  * This may only be called after a successful setWindow() call.
  */
-void SwStatsCpu::startFrame(void)
+void SwStatsCpu::startFrame([[maybe_unused]] const uint32_t statsBufferId)
 {
 	if (window_.width == 0)
 		LOG(SwStatsCpu, Error) << "Calling startFrame() without setWindow()";
@@ -314,10 +314,10 @@  void SwStatsCpu::startFrame(void)
  *
  * 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)
 {
 	*sharedStats_ = stats_;
-	statsReady.emit(frame, bufferId);
+	statsReady.emit(frame, statsBufferId);
 }
 
 /**
diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h
index 26a2f462..402eef2d 100644
--- a/src/libcamera/software_isp/swstats_cpu.h
+++ b/src/libcamera/software_isp/swstats_cpu.h
@@ -40,8 +40,8 @@  public:
 
 	int configure(const StreamConfiguration &inputCfg);
 	void setWindow(const Rectangle &window);
-	void startFrame();
-	void finishFrame(uint32_t frame, uint32_t bufferId);
+	void startFrame(const uint32_t statsBufferId);
+	void finishFrame(uint32_t frame, const uint32_t statsBufferId);
 
 	void processLine0(unsigned int y, const uint8_t *src[])
 	{