[4/8] libcamera: software_isp: Move benchmark code to its own class
diff mbox

Message ID 20241103152205.29219-5-hdegoede@redhat.com
State Superseded
Headers show

Commit Message

Hans de Goede Nov. 3, 2024, 3:22 p.m. UTC
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>
---
Changes since the RFC:
- Add doxygen documentation to for all the public methods
---
 .../internal/software_isp/benchmark.h         | 36 +++++++
 .../internal/software_isp/meson.build         |  1 +
 src/libcamera/software_isp/benchmark.cpp      | 93 +++++++++++++++++++
 src/libcamera/software_isp/debayer_cpu.cpp    | 32 +------
 src/libcamera/software_isp/debayer_cpu.h      |  7 +-
 src/libcamera/software_isp/meson.build        |  1 +
 6 files changed, 135 insertions(+), 35 deletions(-)
 create mode 100644 include/libcamera/internal/software_isp/benchmark.h
 create mode 100644 src/libcamera/software_isp/benchmark.cpp

Patch
diff mbox

diff --git a/include/libcamera/internal/software_isp/benchmark.h b/include/libcamera/internal/software_isp/benchmark.h
new file mode 100644
index 00000000..8af25015
--- /dev/null
+++ b/include/libcamera/internal/software_isp/benchmark.h
@@ -0,0 +1,36 @@ 
+/* 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>
+
+namespace libcamera {
+
+class Benchmark
+{
+public:
+	Benchmark();
+	~Benchmark();
+
+	void startFrame(void);
+	void finishFrame(void);
+
+private:
+	unsigned int measuredFrames_;
+	int64_t frameProcessTime_;
+	timespec frameStartTime_;
+	/* Skip 30 frames for things to stabilize then measure 30 frames */
+	static constexpr unsigned int kFramesToSkip = 30;
+	static constexpr unsigned int kLastFrameToMeasure = 60;
+};
+
+} /* 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..b3da3c41
--- /dev/null
+++ b/src/libcamera/software_isp/benchmark.cpp
@@ -0,0 +1,93 @@ 
+/* 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()
+	: measuredFrames_(0), frameProcessTime_(0)
+{
+}
+
+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)
+{
+	if (measuredFrames_ >= Benchmark::kLastFrameToMeasure)
+		return;
+
+	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 (measuredFrames_ >= Benchmark::kLastFrameToMeasure)
+		return;
+
+	measuredFrames_++;
+
+	if (measuredFrames_ <= Benchmark::kFramesToSkip)
+		return;
+
+	timespec frameEndTime = {};
+	clock_gettime(CLOCK_MONOTONIC_RAW, &frameEndTime);
+	frameProcessTime_ += timeDiff(frameEndTime, frameStartTime_);
+
+	if (measuredFrames_ == Benchmark::kLastFrameToMeasure) {
+		const unsigned int measuredFrames = Benchmark::kLastFrameToMeasure -
+						    Benchmark::kFramesToSkip;
+		LOG(Benchmark, Info)
+			<< "Processed " << measuredFrames
+			<< " frames in " << frameProcessTime_ / 1000 << "us, "
+			<< frameProcessTime_ / (1000 * measuredFrames)
+			<< " us/frame";
+	}
+}
+
+} /* namespace libcamera */
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index cf5ecdf7..897fb83b 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -528,9 +528,6 @@  int DebayerCpu::configure(const StreamConfiguration &inputCfg,
 			lineBuffers_[i].resize(lineBufferLength_);
 	}
 
-	measuredFrames_ = 0;
-	frameProcessTime_ = 0;
-
 	return 0;
 }
 
@@ -739,22 +736,11 @@  void syncBufferForCPU(FrameBuffer *buffer, uint64_t syncFlags)
 	}
 }
 
-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;
-
-	if (measuredFrames_ < DebayerCpu::kLastFrameToMeasure) {
-		frameStartTime = {};
-		clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);
-	}
+	bench_.startFrame();
 
 	syncBufferForCPU(input, DMA_BUF_SYNC_START | DMA_BUF_SYNC_READ);
 	syncBufferForCPU(output, DMA_BUF_SYNC_START | DMA_BUF_SYNC_WRITE);
@@ -790,21 +776,7 @@  void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output
 	syncBufferForCPU(input, DMA_BUF_SYNC_END | DMA_BUF_SYNC_READ);
 
 	/* Measure before emitting signals */
-	if (measuredFrames_ < DebayerCpu::kLastFrameToMeasure &&
-	    ++measuredFrames_ > DebayerCpu::kFramesToSkip) {
-		timespec frameEndTime = {};
-		clock_gettime(CLOCK_MONOTONIC_RAW, &frameEndTime);
-		frameProcessTime_ += timeDiff(frameEndTime, frameStartTime);
-		if (measuredFrames_ == DebayerCpu::kLastFrameToMeasure) {
-			const unsigned int measuredFrames = DebayerCpu::kLastFrameToMeasure -
-							    DebayerCpu::kFramesToSkip;
-			LOG(Debayer, Info)
-				<< "Processed " << measuredFrames
-				<< " frames in " << frameProcessTime_ / 1000 << "us, "
-				<< frameProcessTime_ / (1000 * measuredFrames)
-				<< " 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 feb0452e..c0d1b03d 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/software_isp/swstats_cpu.h"
 
@@ -153,11 +154,7 @@  private:
 	unsigned int xShift_; /* Offset of 0/1 applied to window_.x */
 	bool enableInputMemcpy_;
 	bool swapRedBlueGains_;
-	unsigned int measuredFrames_;
-	int64_t frameProcessTime_;
-	/* Skip 30 frames for things to stabilize then measure 30 frames */
-	static constexpr unsigned int kFramesToSkip = 30;
-	static constexpr unsigned int kLastFrameToMeasure = 60;
+	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',