libcamera: software_isp: Move benchmark code to its own class

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>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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 */

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',

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 */

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())
@@ -840,18 +812,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.

src/libcamera/software_isp/debayer_cpu.h

@@ -19,6 +19,7 @@
 
 #include "libcamera/internal/bayer_format.h"
 #include "libcamera/internal/global_configuration.h"
+#include "libcamera/internal/software_isp/benchmark.h"
 #include "libcamera/internal/software_isp/swstats_cpu.h"
 
 #include "debayer.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 */

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',