[{"id":36317,"web_url":"https://patchwork.libcamera.org/comment/36317/","msgid":"<85zf9qdh7t.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-10-16T15:00:38","subject":"Re: [PATCH v3 07/39] libcamera: software_isp: Move benchmark code\n\tto its own class","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Bryan O'Donoghue writes:\n\n> From: Hans de Goede \n>\n> Move the code for the builtin benchmark to its own small\n> Benchmark class.\n>\n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Milan Zamazal \n> Signed-off-by: Hans de Goede \n> [bod: Fixed up some drift in this patch since initial propostion]\n> Signed-off-by: Bryan O'Donoghue \n> ---\n> .../internal/software_isp/benchmark.h | 39 ++++++++\n> .../internal/software_isp/meson.build | 1 +\n> src/libcamera/software_isp/benchmark.cpp | 92 +++++++++++++++++++\n> src/libcamera/software_isp/debayer_cpu.cpp | 45 +--------\n> src/libcamera/software_isp/debayer_cpu.h | 6 +-\n> src/libcamera/software_isp/meson.build | 1 +\n> 6 files changed, 138 insertions(+), 46 deletions(-)\n> create mode 100644 include/libcamera/internal/software_isp/benchmark.h\n> create mode 100644 src/libcamera/software_isp/benchmark.cpp\n>\n> diff --git a/include/libcamera/internal/software_isp/benchmark.h b/include/libcamera/internal/software_isp/benchmark.h\n> new file mode 100644\n> index 00000000..0680d6cd\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp/benchmark.h\n> @@ -0,0 +1,39 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Authors:\n> + * Hans de Goede \n> + *\n> + * Simple builtin benchmark to measure software ISP processing times\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n\ntime.h include should be removed from its original location in\ndebayer_cpu.cpp as it's no longer needed there.\n\n> +#include \n> +#include \"libcamera/internal/global_configuration.h\"\n> +\n> +namespace libcamera {\n> +\n> +class Benchmark\n> +{\n> +public:\n> +\tBenchmark(const GlobalConfiguration &configuration);\n> +\t~Benchmark();\n> +\n> +\tvoid startFrame(void);\n> +\tvoid finishFrame(void);\n> +\n> +private:\n> +\ttimespec frameStartTime_;\n> +\tbool measure;\n> +\t/* Skip 30 frames for things to stabilize then measure 30 frames */\n> +\tunsigned int encounteredFrames_ = 0;\n> +\tint64_t frameProcessTime_ = 0;\n> +\tunsigned int skipBeforeMeasure_ = 30;\n> +\tunsigned int framesToMeasure_ = 30;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n> index ea3f3f1c..df7c3b97 100644\n> --- a/include/libcamera/internal/software_isp/meson.build\n> +++ b/include/libcamera/internal/software_isp/meson.build\n> @@ -1,6 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_internal_headers += files([\n> + 'benchmark.h',\n> 'debayer_params.h',\n> 'software_isp.h',\n> 'swisp_stats.h',\n> diff --git a/src/libcamera/software_isp/benchmark.cpp b/src/libcamera/software_isp/benchmark.cpp\n> new file mode 100644\n> index 00000000..1a00ae56\n> --- /dev/null\n> +++ b/src/libcamera/software_isp/benchmark.cpp\n> @@ -0,0 +1,92 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Authors:\n> + * Hans de Goede \n> + *\n> + * Simple builtin benchmark to measure software ISP processing times\n> + */\n> +\n> +#include \"libcamera/internal/software_isp/benchmark.h\"\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(Benchmark)\n> +\n> +/**\n> + * \\class Benchmark\n> + * \\brief Simple builtin benchmark\n> + *\n> + * Simple builtin benchmark to measure software ISP processing times.\n> + */\n> +\n> +/**\n> + * \\brief Constructs a Benchmark object\n> + */\n> +Benchmark::Benchmark(const GlobalConfiguration &configuration)\n> +{\n> +\tskipBeforeMeasure_ = configuration.option(\n> +\t\t\t\t\t\t{ \"software_isp\", \"measure\", \"skip\" })\n> +\t\t\t\t\t\t\t.value_or(skipBeforeMeasure_);\n> +\tframesToMeasure_ = configuration.option(\n> +\t\t\t\t\t\t{ \"software_isp\", \"measure\", \"number\" })\n> +\t\t\t\t\t\t\t.value_or(framesToMeasure_);\n> +}\n> +\n> +Benchmark::~Benchmark()\n> +{\n> +}\n> +\n> +static inline int64_t timeDiff(timespec &after, timespec &before)\n> +{\n> +\treturn (after.tv_sec - before.tv_sec) * 1000000000LL +\n> +\t (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;\n> +}\n> +\n> +/**\n> + * \\brief Start measuring process time for a single frame\n> + *\n> + * Call this function before processing frame data to start measuring\n> + * the process time for a frame.\n> + */\n> +void Benchmark::startFrame(void)\n> +{\n> +\tmeasure = framesToMeasure_ > 0 &&\n> +\t\t encounteredFrames_ < skipBeforeMeasure_ + framesToMeasure_ &&\n> +\t\t ++encounteredFrames_ > skipBeforeMeasure_;\n> +\n> +\tif (measure) {\n> +\t\tframeStartTime_ = {};\n> +\t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime_);\n> +\t}\n> +}\n> +\n> +/**\n> + * \\brief Finish measuring process time for a single frame\n> + *\n> + * Call this function after processing frame data to finish measuring\n> + * the process time for a frame.\n> + *\n> + * This function will log frame processing time information after\n> + * Benchmark::kLastFrameToMeasure frames have been processed.\n> + */\n> +void Benchmark::finishFrame(void)\n> +{\n> +\tif (measure) {\n> +\t\ttimespec frameEndTime = {};\n> +\t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameEndTime);\n> +\t\tframeProcessTime_ += timeDiff(frameEndTime, frameStartTime_);\n> +\t\tif (encounteredFrames_ == skipBeforeMeasure_ + framesToMeasure_) {\n> +\t\t\tLOG(Benchmark, Info)\n> +\t\t\t\t<< \"Processed \" << framesToMeasure_\n> +\t\t\t\t<< \" frames in \" << frameProcessTime_ / 1000 << \"us, \"\n> +\t\t\t\t<< frameProcessTime_ / (1000 * framesToMeasure_)\n> +\t\t\t\t<< \" us/frame\";\n> +\t\t}\n> +\t}\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index c2fb11ba..df77deb6 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -42,7 +42,7 @@ namespace libcamera {\n> * \\param[in] configuration The global configuration\n> */\n> DebayerCpu::DebayerCpu(std::unique_ptr stats, const GlobalConfiguration &configuration)\n> -\t: stats_(std::move(stats))\n> +\t: stats_(std::move(stats)), bench_(configuration)\n> {\n> \t/*\n> \t * Reading from uncached buffers may be very slow.\n> @@ -58,13 +58,6 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats, const GlobalConfigurat\n> \tenableInputMemcpy_ =\n> \t\tconfiguration.option({ \"software_isp\", \"copy_input_buffer\" }).value_or(true);\n> \n> -\tskipBeforeMeasure_ = configuration.option(\n> -\t\t\t\t\t\t { \"software_isp\", \"measure\", \"skip\" })\n> -\t\t\t\t .value_or(skipBeforeMeasure_);\n> -\tframesToMeasure_ = configuration.option(\n> -\t\t\t\t\t\t{ \"software_isp\", \"measure\", \"number\" })\n> -\t\t\t\t .value_or(framesToMeasure_);\n> -\n> \t/* Initialize color lookup tables */\n> \tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n> \t\tred_[i] = green_[i] = blue_[i] = i;\n> @@ -571,9 +564,6 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> \t\t\tlineBuffers_[i].resize(lineBufferLength_);\n> \t}\n> \n> -\tencounteredFrames_ = 0;\n> -\tframeProcessTime_ = 0;\n> -\n> \treturn 0;\n> }\n> \n> @@ -765,27 +755,9 @@ void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> \t}\n> }\n> \n> -namespace {\n> -\n> -inline int64_t timeDiff(timespec &after, timespec &before)\n> -{\n> -\treturn (after.tv_sec - before.tv_sec) * 1000000000LL +\n> -\t (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;\n> -}\n> -\n> -} /* namespace */\n> -\n> void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params)\n> {\n> -\ttimespec frameStartTime;\n> -\n> -\tbool measure = framesToMeasure_ > 0 &&\n> -\t\t encounteredFrames_ < skipBeforeMeasure_ + framesToMeasure_ &&\n> -\t\t ++encounteredFrames_ > skipBeforeMeasure_;\n> -\tif (measure) {\n> -\t\tframeStartTime = {};\n> -\t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);\n> -\t}\n> +\tbench_.startFrame();\n> \n> \tstd::vector dmaSyncers;\n> \tfor (const FrameBuffer::Plane &plane : input->planes())\n> @@ -839,18 +811,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n> \tdmaSyncers.clear();\n> \n> \t/* Measure before emitting signals */\n> -\tif (measure) {\n> -\t\ttimespec frameEndTime = {};\n> -\t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameEndTime);\n> -\t\tframeProcessTime_ += timeDiff(frameEndTime, frameStartTime);\n> -\t\tif (encounteredFrames_ == skipBeforeMeasure_ + framesToMeasure_) {\n> -\t\t\tLOG(Debayer, Info)\n> -\t\t\t\t<< \"Processed \" << framesToMeasure_\n> -\t\t\t\t<< \" frames in \" << frameProcessTime_ / 1000 << \"us, \"\n> -\t\t\t\t<< frameProcessTime_ / (1000 * framesToMeasure_)\n> -\t\t\t\t<< \" us/frame\";\n> -\t\t}\n> -\t}\n> +\tbench_.finishFrame();\n> \n> \t/*\n> \t * Buffer ids are currently not used, so pass zeros as its parameter.\n> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> index 1cd411f2..aff32491 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -17,6 +17,7 @@\n> \n> #include \n> \n> +#include \"libcamera/internal/software_isp/benchmark.h\"\n> #include \"libcamera/internal/bayer_format.h\"\n> #include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/software_isp/swstats_cpu.h\"\n> @@ -161,10 +162,7 @@ private:\n> \tunsigned int xShift_; /* Offset of 0/1 applied to window_.x */\n> \tbool enableInputMemcpy_;\n> \tbool swapRedBlueGains_;\n> -\tunsigned int encounteredFrames_;\n> -\tint64_t frameProcessTime_;\n> -\tunsigned int skipBeforeMeasure_ = 30;\n> -\tunsigned int framesToMeasure_ = 30;\n> +\tBenchmark bench_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n> index aac7eda7..59fa5f02 100644\n> --- a/src/libcamera/software_isp/meson.build\n> +++ b/src/libcamera/software_isp/meson.build\n> @@ -8,6 +8,7 @@ if not softisp_enabled\n> endif\n> \n> libcamera_internal_sources += files([\n> + 'benchmark.cpp',\n> 'debayer.cpp',\n> 'debayer_cpu.cpp',\n> 'software_isp.cpp',","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D6997C3259\n\tfor ;\n\tThu, 16 Oct 2025 15:00:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 062126068C;\n\tThu, 16 Oct 2025 17:00:50 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E2AE605D7\n\tfor ;\n\tThu, 16 Oct 2025 17:00:48 +0200 (CEST)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-15-zIm4ApZNM8a1YGVZYaE8yw-1; Thu, 16 Oct 2025 11:00:45 -0400","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-b3c72638b5dso122441866b.3\n\tfor ;\n\tThu, 16 Oct 2025 08:00:45 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-b5cccda99adsm538538466b.41.2025.10.16.08.00.40\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 16 Oct 2025 08:00:40 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"NYwc4RyW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1760626847;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=prbCpTQbaP7nZvZb7KXvfMhDUIeFJaetZkjZ+gy+iuM=;\n\tb=NYwc4RyWVZKrT5qtZyK4WMgIQF1wUJ3WotK0DmjtzOPPtg5g5G6AhXcyg3GUyPBezoy19m\n\tReYxLU1gUPzTrhoR//25AWLC95f8F3JlKNKY9VG6uTtlXo0D6UuHiFth0r/vmHHBTd3iWa\n\te//QEikWb5xA/ud7LEmvsShJRm2OQEY=","X-MC-Unique":"zIm4ApZNM8a1YGVZYaE8yw-1","X-Mimecast-MFC-AGG-ID":"zIm4ApZNM8a1YGVZYaE8yw_1760626845","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1760626844; x=1761231644;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=prbCpTQbaP7nZvZb7KXvfMhDUIeFJaetZkjZ+gy+iuM=;\n\tb=xKPunePkUGiedcUM/BVo98nnvCETRn/Zyfn7edwYGWHncz+f+PVmsmMAr7K96/g3LW\n\tTufF6RgIl0iQaT1BEjVIbO8Ll6pMWVQd9GND8O0ZRC0mvXD/iR9YhVCqYCSnh7ZX8PpH\n\tEo9zlO2Zy1gJfoOJm2oxTwUilcMQ0/GjpPwXE6TOkWX7BJ82r7/pLQ9bIX/zbBWZ47Au\n\tRnavwWimomPNtN735zjPXZ5AC9B1wy2f9xUnnltIN9tFD97hgkQXaPlHYbU8xIhcxANd\n\t7BWQG3xzHXg4M/fRUcFaQc4oi8/52wG4hKqbcT7CIx1pz2FLhT9dO38lBUVaodPfm/lm\n\tRuBw==","X-Gm-Message-State":"AOJu0YxXMNVGlpb3idNL7I0M3Q76igQgXXTKab2u31DtmYHT6qF4fhaQ\n\t6OxDQrC/al+9mQ15e/5lQeSbsk+8kNRPjkGlhIX+nTA844r6j/THoJMqs2fOjqbE3gVkIABMf+s\n\tqyu0gmcOSh9TpLsZESEliAjcj4M1OQzZqz/9OyZdd2grNJeA6mmTpS1PiXk5+FgeoEll+SXrNu0\n\ttcD82Qnpw=","X-Gm-Gg":"ASbGncudhIJ+oTcvLxKZDEsj3YGHqN08gCZV9D5TRjCvvthBMmDG2O6OwXDQmbVFeQ2\n\tmN/7yNEvsNAlAXS6xv05C1+GzwTksMO/PQl5pCLCxJnp3A1jXjWIF83XxVcZB4bm8n9B4C7oo3x\n\tg3XcAl/ULFJ1QBpDA+4Jdkw9CpWlzhs2YIRX8rw9vUGdiNWOkU4iJzmIO1zGHi4rsx8+Hf9GY8H\n\tYF2fi2j3vNnYHjUz5oEXd3zBHvERHvO3Q+aeKQe8wdkUMomkEbuu4Kj+IK0XUOociVjES9XbWY1\n\tqM9TZ+3BPFilYhZU7R01T903eGBtX2t1BEwbdC4xY09yIvxJ8L1ylkkjYx9BXpvJUI7+R40YsXJ\n\tZR4+M3Z5MsYUrBWtX5gyw0hhkzE3At8PfH08RtxmMHhdMsnkUH3GB","X-Received":["by 2002:a17:907:3f0f:b0:b3f:9e3d:daa4 with SMTP id\n\ta640c23a62f3a-b6475511ad9mr27466766b.65.1760626843849; \n\tThu, 16 Oct 2025 08:00:43 -0700 (PDT)","by 2002:a17:907:3f0f:b0:b3f:9e3d:daa4 with SMTP id\n\ta640c23a62f3a-b6475511ad9mr27448666b.65.1760626841758; \n\tThu, 16 Oct 2025 08:00:41 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IFON8kicVc6CzlJoFHXhRiFtZRtBV3C61aAg7FQfa0H3naQ/gDsHumbf9reCQNYwRFidHIwfA==","From":"Milan Zamazal ","To":"Bryan O'Donoghue ","Cc":"libcamera-devel@lists.libcamera.org, hdegoede@redhat.com,\n\tbod.linux@nxsw.ie, Kieran Bingham ","Subject":"Re: [PATCH v3 07/39] libcamera: software_isp: Move benchmark code\n\tto its own class","In-Reply-To":"<20251015012251.17508-8-bryan.odonoghue@linaro.org> (Bryan\n\tO'Donoghue's message of \"Wed, 15 Oct 2025 02:22:19 +0100\")","References":"<20251015012251.17508-1-bryan.odonoghue@linaro.org>\n\t<20251015012251.17508-8-bryan.odonoghue@linaro.org>","Date":"Thu, 16 Oct 2025 17:00:38 +0200","Message-ID":"<85zf9qdh7t.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"r8Cuep_xX9IUNl31QFxYwGlopT3EtrOx6Q7_XkYl5Wo_1760626845","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]