[v3,07/39] libcamera: software_isp: Move benchmark code to its own class
diff mbox series

Message ID 20251015012251.17508-8-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • Add GLES 2.0 GPUISP to libcamera
Related show

Commit Message

Bryan O'Donoghue Oct. 15, 2025, 1:22 a.m. UTC
From: Hans de Goede <hdegoede@redhat.com>

Move the code for the builtin benchmark to its own small
Benchmark class.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
[bod: Fixed up some drift in this patch since initial propostion]
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../internal/software_isp/benchmark.h         | 39 ++++++++
 .../internal/software_isp/meson.build         |  1 +
 src/libcamera/software_isp/benchmark.cpp      | 92 +++++++++++++++++++
 src/libcamera/software_isp/debayer_cpu.cpp    | 45 +--------
 src/libcamera/software_isp/debayer_cpu.h      |  6 +-
 src/libcamera/software_isp/meson.build        |  1 +
 6 files changed, 138 insertions(+), 46 deletions(-)
 create mode 100644 include/libcamera/internal/software_isp/benchmark.h
 create mode 100644 src/libcamera/software_isp/benchmark.cpp

Patch
diff mbox series

diff --git a/include/libcamera/internal/software_isp/benchmark.h b/include/libcamera/internal/software_isp/benchmark.h
new file mode 100644
index 00000000..0680d6cd
--- /dev/null
+++ b/include/libcamera/internal/software_isp/benchmark.h
@@ -0,0 +1,39 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2024, Red Hat Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ *
+ * Simple builtin benchmark to measure software ISP processing times
+ */
+
+#pragma once
+
+#include <stdint.h>
+#include <time.h>
+#include <libcamera/base/log.h>
+#include "libcamera/internal/global_configuration.h"
+
+namespace libcamera {
+
+class Benchmark
+{
+public:
+	Benchmark(const GlobalConfiguration &configuration);
+	~Benchmark();
+
+	void startFrame(void);
+	void finishFrame(void);
+
+private:
+	timespec frameStartTime_;
+	bool measure;
+	/* Skip 30 frames for things to stabilize then measure 30 frames */
+	unsigned int encounteredFrames_ = 0;
+	int64_t frameProcessTime_ = 0;
+	unsigned int skipBeforeMeasure_ = 30;
+	unsigned int framesToMeasure_ = 30;
+};
+
+} /* namespace libcamera */
diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build
index ea3f3f1c..df7c3b97 100644
--- a/include/libcamera/internal/software_isp/meson.build
+++ b/include/libcamera/internal/software_isp/meson.build
@@ -1,6 +1,7 @@ 
 # SPDX-License-Identifier: CC0-1.0
 
 libcamera_internal_headers += files([
+    'benchmark.h',
     'debayer_params.h',
     'software_isp.h',
     'swisp_stats.h',
diff --git a/src/libcamera/software_isp/benchmark.cpp b/src/libcamera/software_isp/benchmark.cpp
new file mode 100644
index 00000000..1a00ae56
--- /dev/null
+++ b/src/libcamera/software_isp/benchmark.cpp
@@ -0,0 +1,92 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2024, Red Hat Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ *
+ * Simple builtin benchmark to measure software ISP processing times
+ */
+
+#include "libcamera/internal/software_isp/benchmark.h"
+
+#include <libcamera/base/log.h>
+
+namespace libcamera {
+
+LOG_DEFINE_CATEGORY(Benchmark)
+
+/**
+ * \class Benchmark
+ * \brief Simple builtin benchmark
+ *
+ * Simple builtin benchmark to measure software ISP processing times.
+ */
+
+/**
+ * \brief Constructs a Benchmark object
+ */
+Benchmark::Benchmark(const GlobalConfiguration &configuration)
+{
+	skipBeforeMeasure_ = configuration.option<unsigned int>(
+						{ "software_isp", "measure", "skip" })
+							.value_or(skipBeforeMeasure_);
+	framesToMeasure_ = configuration.option<unsigned int>(
+						{ "software_isp", "measure", "number" })
+							.value_or(framesToMeasure_);
+}
+
+Benchmark::~Benchmark()
+{
+}
+
+static inline int64_t timeDiff(timespec &after, timespec &before)
+{
+	return (after.tv_sec - before.tv_sec) * 1000000000LL +
+	       (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;
+}
+
+/**
+ * \brief Start measuring process time for a single frame
+ *
+ * Call this function before processing frame data to start measuring
+ * the process time for a frame.
+ */
+void Benchmark::startFrame(void)
+{
+	measure = framesToMeasure_ > 0 &&
+		  encounteredFrames_ < skipBeforeMeasure_ + framesToMeasure_ &&
+		  ++encounteredFrames_ > skipBeforeMeasure_;
+
+	if (measure) {
+		frameStartTime_ = {};
+		clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime_);
+	}
+}
+
+/**
+ * \brief Finish measuring process time for a single frame
+ *
+ * Call this function after processing frame data to finish measuring
+ * the process time for a frame.
+ *
+ * This function will log frame processing time information after
+ * Benchmark::kLastFrameToMeasure frames have been processed.
+ */
+void Benchmark::finishFrame(void)
+{
+	if (measure) {
+		timespec frameEndTime = {};
+		clock_gettime(CLOCK_MONOTONIC_RAW, &frameEndTime);
+		frameProcessTime_ += timeDiff(frameEndTime, frameStartTime_);
+		if (encounteredFrames_ == skipBeforeMeasure_ + framesToMeasure_) {
+			LOG(Benchmark, Info)
+				<< "Processed " << framesToMeasure_
+				<< " frames in " << frameProcessTime_ / 1000 << "us, "
+				<< frameProcessTime_ / (1000 * framesToMeasure_)
+				<< " us/frame";
+		}
+	}
+}
+
+} /* namespace libcamera */
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index c2fb11ba..df77deb6 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -42,7 +42,7 @@  namespace libcamera {
  * \param[in] configuration The global configuration
  */
 DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfiguration &configuration)
-	: stats_(std::move(stats))
+	: stats_(std::move(stats)), bench_(configuration)
 {
 	/*
 	 * Reading from uncached buffers may be very slow.
@@ -58,13 +58,6 @@  DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfigurat
 	enableInputMemcpy_ =
 		configuration.option<bool>({ "software_isp", "copy_input_buffer" }).value_or(true);
 
-	skipBeforeMeasure_ = configuration.option<unsigned int>(
-						  { "software_isp", "measure", "skip" })
-				     .value_or(skipBeforeMeasure_);
-	framesToMeasure_ = configuration.option<unsigned int>(
-						{ "software_isp", "measure", "number" })
-				   .value_or(framesToMeasure_);
-
 	/* Initialize color lookup tables */
 	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
 		red_[i] = green_[i] = blue_[i] = i;
@@ -571,9 +564,6 @@  int DebayerCpu::configure(const StreamConfiguration &inputCfg,
 			lineBuffers_[i].resize(lineBufferLength_);
 	}
 
-	encounteredFrames_ = 0;
-	frameProcessTime_ = 0;
-
 	return 0;
 }
 
@@ -765,27 +755,9 @@  void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)
 	}
 }
 
-namespace {
-
-inline int64_t timeDiff(timespec &after, timespec &before)
-{
-	return (after.tv_sec - before.tv_sec) * 1000000000LL +
-	       (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;
-}
-
-} /* namespace */
-
 void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params)
 {
-	timespec frameStartTime;
-
-	bool measure = framesToMeasure_ > 0 &&
-		       encounteredFrames_ < skipBeforeMeasure_ + framesToMeasure_ &&
-		       ++encounteredFrames_ > skipBeforeMeasure_;
-	if (measure) {
-		frameStartTime = {};
-		clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);
-	}
+	bench_.startFrame();
 
 	std::vector<DmaSyncer> dmaSyncers;
 	for (const FrameBuffer::Plane &plane : input->planes())
@@ -839,18 +811,7 @@  void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output
 	dmaSyncers.clear();
 
 	/* Measure before emitting signals */
-	if (measure) {
-		timespec frameEndTime = {};
-		clock_gettime(CLOCK_MONOTONIC_RAW, &frameEndTime);
-		frameProcessTime_ += timeDiff(frameEndTime, frameStartTime);
-		if (encounteredFrames_ == skipBeforeMeasure_ + framesToMeasure_) {
-			LOG(Debayer, Info)
-				<< "Processed " << framesToMeasure_
-				<< " frames in " << frameProcessTime_ / 1000 << "us, "
-				<< frameProcessTime_ / (1000 * framesToMeasure_)
-				<< " us/frame";
-		}
-	}
+	bench_.finishFrame();
 
 	/*
 	 * Buffer ids are currently not used, so pass zeros as its parameter.
diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
index 1cd411f2..aff32491 100644
--- a/src/libcamera/software_isp/debayer_cpu.h
+++ b/src/libcamera/software_isp/debayer_cpu.h
@@ -17,6 +17,7 @@ 
 
 #include <libcamera/base/object.h>
 
+#include "libcamera/internal/software_isp/benchmark.h"
 #include "libcamera/internal/bayer_format.h"
 #include "libcamera/internal/global_configuration.h"
 #include "libcamera/internal/software_isp/swstats_cpu.h"
@@ -161,10 +162,7 @@  private:
 	unsigned int xShift_; /* Offset of 0/1 applied to window_.x */
 	bool enableInputMemcpy_;
 	bool swapRedBlueGains_;
-	unsigned int encounteredFrames_;
-	int64_t frameProcessTime_;
-	unsigned int skipBeforeMeasure_ = 30;
-	unsigned int framesToMeasure_ = 30;
+	Benchmark bench_;
 };
 
 } /* namespace libcamera */
diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build
index aac7eda7..59fa5f02 100644
--- a/src/libcamera/software_isp/meson.build
+++ b/src/libcamera/software_isp/meson.build
@@ -8,6 +8,7 @@  if not softisp_enabled
 endif
 
 libcamera_internal_sources += files([
+    'benchmark.cpp',
     'debayer.cpp',
     'debayer_cpu.cpp',
     'software_isp.cpp',