[RFC 3/4] libcamera: software_isp: Move benchmark code to its own class
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Oct 18 23:58:58 CEST 2024
Quoting Hans de Goede (2024-10-09 21:01:09)
> Move the code for the builtin benchmark to its own small
> Benchmark class.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
> src/libcamera/software_isp/benchmark.cpp | 78 ++++++++++++++++++++++
> src/libcamera/software_isp/benchmark.h | 36 ++++++++++
> src/libcamera/software_isp/debayer_cpu.cpp | 32 +--------
> src/libcamera/software_isp/debayer_cpu.h | 7 +-
> src/libcamera/software_isp/meson.build | 1 +
> 5 files changed, 119 insertions(+), 35 deletions(-)
> create mode 100644 src/libcamera/software_isp/benchmark.cpp
> create mode 100644 src/libcamera/software_isp/benchmark.h
>
> diff --git a/src/libcamera/software_isp/benchmark.cpp b/src/libcamera/software_isp/benchmark.cpp
> new file mode 100644
> index 00000000..eecad51c
> --- /dev/null
> +++ b/src/libcamera/software_isp/benchmark.cpp
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Red Hat Inc.
> + *
> + * Authors:
> + * Hans de Goede <hdegoede at redhat.com>
> + *
> + * Simple builtin benchmark to measure software ISP processing times
> + */
> +
> +#include "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;
> +}
> +
> +void Benchmark::startFrame(void)
I was /so/ close to merging the first 3 patches of this series (4/4
isn't "used" so I don't think it's so easy to just merge) - but doxygen
kicks out at the undocumented class entries:
https://gitlab.freedesktop.org/camera/libcamera/-/jobs/65324937
--
Kieran
> +{
> + if (measuredFrames_ >= Benchmark::kLastFrameToMeasure)
> + return;
> +
> + frameStartTime_ = {};
> + clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime_);
> +}
> +
> +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/benchmark.h b/src/libcamera/software_isp/benchmark.h
> new file mode 100644
> index 00000000..8af25015
> --- /dev/null
> +++ b/src/libcamera/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 at 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/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 2c47e7c6..59015479 100644
> --- a/src/libcamera/software_isp/debayer_cpu.h
> +++ b/src/libcamera/software_isp/debayer_cpu.h
> @@ -19,6 +19,7 @@
>
> #include "libcamera/internal/bayer_format.h"
>
> +#include "benchmark.h"
> #include "debayer.h"
> #include "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',
> --
> 2.46.2
>
More information about the libcamera-devel
mailing list