| Message ID | 20250611013245.133785-5-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Wed, Jun 11, 2025 at 02:32:14AM +0100, Bryan O'Donoghue wrote: > 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> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > .../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 | 36 +------ > src/libcamera/software_isp/debayer_cpu.h | 7 +- > src/libcamera/software_isp/meson.build | 1 + > 6 files changed, 135 insertions(+), 39 deletions(-) > create mode 100644 include/libcamera/internal/software_isp/benchmark.h > create mode 100644 src/libcamera/software_isp/benchmark.cpp > > 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 This is a bit generic of a name for something specific to the soft ISP. Is it time to move the soft ISP to a namespace ? > +{ > +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 66f6038c..8d30bf4a 100644 > --- a/src/libcamera/software_isp/debayer_cpu.cpp > +++ b/src/libcamera/software_isp/debayer_cpu.cpp > @@ -554,9 +554,6 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg, > lineBuffers_[i].resize(lineBufferLength_); > } > > - measuredFrames_ = 0; > - frameProcessTime_ = 0; > - > return 0; > } > > @@ -746,24 +743,9 @@ void DebayerCpu::process4(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; > - > - if (measuredFrames_ < DebayerCpu::kLastFrameToMeasure) { > - frameStartTime = {}; > - clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime); > - } > + bench_.startFrame(); > > std::vector<DmaSyncer> dmaSyncers; > for (const FrameBuffer::Plane &plane : input->planes()) > @@ -817,21 +799,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output > dmaSyncers.clear(); > > /* 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 89a89893..182607cd 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" > > @@ -160,11 +161,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',
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 66f6038c..8d30bf4a 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -554,9 +554,6 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg, lineBuffers_[i].resize(lineBufferLength_); } - measuredFrames_ = 0; - frameProcessTime_ = 0; - return 0; } @@ -746,24 +743,9 @@ void DebayerCpu::process4(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; - - if (measuredFrames_ < DebayerCpu::kLastFrameToMeasure) { - frameStartTime = {}; - clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime); - } + bench_.startFrame(); std::vector<DmaSyncer> dmaSyncers; for (const FrameBuffer::Plane &plane : input->planes()) @@ -817,21 +799,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output dmaSyncers.clear(); /* 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 89a89893..182607cd 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" @@ -160,11 +161,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',