[v2,2/2] software_isp: benchmark: Print what is being benchmarked

26159 diff mbox series

With the GPU accelerated softISP 2 separate benchmark results are printed,
1 for the generation of the output images on the GPU and a separate one
for generating the statistics on the CPU.

Add a new name argument to the Benchmark class descriptor and print this
out when printing the benchmark result.

Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
Changes in v2:
- Add include <string> to benchmark.h
- Use ": name_(name)" in the Benchmark constructor to init name
---
 include/libcamera/internal/software_isp/benchmark.h | 4 +++-
 src/libcamera/software_isp/benchmark.cpp            | 5 +++--
 src/libcamera/software_isp/debayer.cpp              | 3 ++-
 src/libcamera/software_isp/swstats_cpu.cpp          | 2 +-
 4 files changed, 9 insertions(+), 5 deletions(-)

Quoting Hans de Goede (2026-02-15 09:44:18)
> With the GPU accelerated softISP 2 separate benchmark results are printed,
> 1 for the generation of the output images on the GPU and a separate one
> for generating the statistics on the CPU.
> 
> Add a new name argument to the Benchmark class descriptor and print this
> out when printing the benchmark result.
> 
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

I'd really like to get this data into completion metadata sometime, so
it's easy to plot and graph in camshark while the stream is running for
instance.

But I think we'd need easier to extend string controls, or perhaps we'd
have to make a softisp/gpuisp control namespace for these specifically.
Not sure what would fit better, but that's all only things that would be
'on top' here anyway so:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> ---
> Changes in v2:
> - Add include <string> to benchmark.h
> - Use ": name_(name)" in the Benchmark constructor to init name
> ---
>  include/libcamera/internal/software_isp/benchmark.h | 4 +++-
>  src/libcamera/software_isp/benchmark.cpp            | 5 +++--
>  src/libcamera/software_isp/debayer.cpp              | 3 ++-
>  src/libcamera/software_isp/swstats_cpu.cpp          | 2 +-
>  4 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/benchmark.h b/include/libcamera/internal/software_isp/benchmark.h
> index 46bdb86d..5526abc5 100644
> --- a/include/libcamera/internal/software_isp/benchmark.h
> +++ b/include/libcamera/internal/software_isp/benchmark.h
> @@ -11,6 +11,7 @@
>  #pragma once
>  
>  #include <stdint.h>
> +#include <string>
>  #include <time.h>
>  #include <libcamera/base/log.h>
>  #include "libcamera/internal/global_configuration.h"
> @@ -20,13 +21,14 @@ namespace libcamera {
>  class Benchmark
>  {
>  public:
> -       Benchmark(const GlobalConfiguration &configuration);
> +       Benchmark(const GlobalConfiguration &configuration, const std::string &name);
>         ~Benchmark();
>  
>         void startFrame(void);
>         void finishFrame(void);
>  
>  private:
> +       std::string name_;
>         timespec frameStartTime_;
>         bool measure_;
>         /* Skip 30 frames for things to stabilize then measure 30 frames */
> diff --git a/src/libcamera/software_isp/benchmark.cpp b/src/libcamera/software_isp/benchmark.cpp
> index 4ffb6773..36c49770 100644
> --- a/src/libcamera/software_isp/benchmark.cpp
> +++ b/src/libcamera/software_isp/benchmark.cpp
> @@ -26,7 +26,8 @@ LOG_DEFINE_CATEGORY(Benchmark)
>  /**
>   * \brief Constructs a Benchmark object
>   */
> -Benchmark::Benchmark(const GlobalConfiguration &configuration)
> +Benchmark::Benchmark(const GlobalConfiguration &configuration, const std::string &name)
> +       : name_(name)
>  {
>         skipBeforeMeasure_ = configuration.option<unsigned int>(
>                                                 { "software_isp", "measure", "skip" })
> @@ -81,7 +82,7 @@ void Benchmark::finishFrame(void)
>                 frameProcessTime_ += timeDiff(frameEndTime, frameStartTime_);
>                 if (encounteredFrames_ == skipBeforeMeasure_ + framesToMeasure_) {
>                         LOG(Benchmark, Info)
> -                               << "Processed " << framesToMeasure_
> +                               << name_ << " processed " << framesToMeasure_
>                                 << " frames in " << frameProcessTime_ / 1000 << "us, "
>                                 << frameProcessTime_ / (1000 * framesToMeasure_)
>                                 << " us/frame";
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index dccdd86b..a6bceb58 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -58,7 +58,8 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(Debayer)
>  
> -Debayer::Debayer(const GlobalConfiguration &configuration) : bench_(configuration)
> +Debayer::Debayer(const GlobalConfiguration &configuration)
> +       : bench_(configuration, "Debayer")
>  {
>  }
>  
> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
> index 5c3011a7..1cedcfbc 100644
> --- a/src/libcamera/software_isp/swstats_cpu.cpp
> +++ b/src/libcamera/software_isp/swstats_cpu.cpp
> @@ -155,7 +155,7 @@ namespace libcamera {
>  LOG_DEFINE_CATEGORY(SwStatsCpu)
>  
>  SwStatsCpu::SwStatsCpu(const GlobalConfiguration &configuration)
> -       : sharedStats_("softIsp_stats"), bench_(configuration)
> +       : sharedStats_("softIsp_stats"), bench_(configuration, "CPU stats")
>  {
>         if (!sharedStats_)
>                 LOG(SwStatsCpu, Error)
> -- 
> 2.52.0
>

Hi Hans,

Hans de Goede <johannes.goede@oss.qualcomm.com> writes:

> With the GPU accelerated softISP 2 separate benchmark results are printed,
> 1 for the generation of the output images on the GPU and a separate one
> for generating the statistics on the CPU.
>
> Add a new name argument to the Benchmark class descriptor and print this
> out when printing the benchmark result.

This was missing, good to see it added.

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
> Changes in v2:
> - Add include <string> to benchmark.h
> - Use ": name_(name)" in the Benchmark constructor to init name
> ---
>  include/libcamera/internal/software_isp/benchmark.h | 4 +++-
>  src/libcamera/software_isp/benchmark.cpp            | 5 +++--
>  src/libcamera/software_isp/debayer.cpp              | 3 ++-
>  src/libcamera/software_isp/swstats_cpu.cpp          | 2 +-
>  4 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/internal/software_isp/benchmark.h b/include/libcamera/internal/software_isp/benchmark.h
> index 46bdb86d..5526abc5 100644
> --- a/include/libcamera/internal/software_isp/benchmark.h
> +++ b/include/libcamera/internal/software_isp/benchmark.h
> @@ -11,6 +11,7 @@
>  #pragma once
>  
>  #include <stdint.h>
> +#include <string>
>  #include <time.h>
>  #include <libcamera/base/log.h>
>  #include "libcamera/internal/global_configuration.h"
> @@ -20,13 +21,14 @@ namespace libcamera {
>  class Benchmark
>  {
>  public:
> -	Benchmark(const GlobalConfiguration &configuration);
> +	Benchmark(const GlobalConfiguration &configuration, const std::string &name);
>  	~Benchmark();
>  
>  	void startFrame(void);
>  	void finishFrame(void);
>  
>  private:
> +	std::string name_;
>  	timespec frameStartTime_;
>  	bool measure_;
>  	/* Skip 30 frames for things to stabilize then measure 30 frames */
> diff --git a/src/libcamera/software_isp/benchmark.cpp b/src/libcamera/software_isp/benchmark.cpp
> index 4ffb6773..36c49770 100644
> --- a/src/libcamera/software_isp/benchmark.cpp
> +++ b/src/libcamera/software_isp/benchmark.cpp
> @@ -26,7 +26,8 @@ LOG_DEFINE_CATEGORY(Benchmark)
>  /**
>   * \brief Constructs a Benchmark object
>   */
> -Benchmark::Benchmark(const GlobalConfiguration &configuration)
> +Benchmark::Benchmark(const GlobalConfiguration &configuration, const std::string &name)
> +	: name_(name)
>  {
>  	skipBeforeMeasure_ = configuration.option<unsigned int>(
>  						{ "software_isp", "measure", "skip" })
> @@ -81,7 +82,7 @@ void Benchmark::finishFrame(void)
>  		frameProcessTime_ += timeDiff(frameEndTime, frameStartTime_);
>  		if (encounteredFrames_ == skipBeforeMeasure_ + framesToMeasure_) {
>  			LOG(Benchmark, Info)
> -				<< "Processed " << framesToMeasure_
> +				<< name_ << " processed " << framesToMeasure_
>  				<< " frames in " << frameProcessTime_ / 1000 << "us, "
>  				<< frameProcessTime_ / (1000 * framesToMeasure_)
>  				<< " us/frame";
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index dccdd86b..a6bceb58 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -58,7 +58,8 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(Debayer)
>  
> -Debayer::Debayer(const GlobalConfiguration &configuration) : bench_(configuration)
> +Debayer::Debayer(const GlobalConfiguration &configuration)
> +	: bench_(configuration, "Debayer")
>  {
>  }
>  
> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
> index 5c3011a7..1cedcfbc 100644
> --- a/src/libcamera/software_isp/swstats_cpu.cpp
> +++ b/src/libcamera/software_isp/swstats_cpu.cpp
> @@ -155,7 +155,7 @@ namespace libcamera {
>  LOG_DEFINE_CATEGORY(SwStatsCpu)
>  
>  SwStatsCpu::SwStatsCpu(const GlobalConfiguration &configuration)
> -	: sharedStats_("softIsp_stats"), bench_(configuration)
> +	: sharedStats_("softIsp_stats"), bench_(configuration, "CPU stats")
>  {
>  	if (!sharedStats_)
>  		LOG(SwStatsCpu, Error)