[{"id":38302,"web_url":"https://patchwork.libcamera.org/comment/38302/","msgid":"<851pi9c660.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-02-25T13:19:03","subject":"Re: [PATCH v3 3/4] software_isp: debayer_cpu: Add multi-threading\n\tsupport","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Hans,\n\nthank you for the update.\n\nHans de Goede <johannes.goede@oss.qualcomm.com> writes:\n\n> Add CPU soft ISP multi-threading support.\n>\n> Benchmark results for the Arduino Uno-Q with a weak CPU which is good for\n> performance testing, all numbers with an IMX219 running at\n> 3280x2464 -> 3272x2464:\n>\n> 1 thread : 147ms / frame, ~6.5 fps\n> 2 threads:  80ms / frame, ~12.5 fps\n> 3 threads:  65ms / frame, ~15 fps\n>\n> Adding a 4th thread does not improve performance.\n>\n> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>\n> ---\n> Changes in v3:\n> - Adjust for DebayerCpuThread now inheriting from Thread\n> - Use for (auto &thread : threads_)\n>\n> Changes in v2:\n> - Adjust to use the new DebayerCpuThread class introduced in the v2 patch-series\n> - Re-use threads instead of starting new threads every frame\n> ---\n>  src/libcamera/software_isp/debayer_cpu.cpp | 45 ++++++++++++++++++++--\n>  src/libcamera/software_isp/debayer_cpu.h   |  6 +++\n>  2 files changed, 48 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index 36b7881b..cf16f00b 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -73,6 +73,7 @@ DebayerCpuThread::DebayerCpuThread(DebayerCpu *debayer, unsigned int threadIndex\n>  \t  debayer_(debayer), threadIndex_(threadIndex),\n>  \t  enableInputMemcpy_(enableInputMemcpy)\n>  {\n> +\tmoveToThread(this);\n>  }\n>  \n>  /**\n> @@ -104,8 +105,10 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfigurat\n>  \tbool enableInputMemcpy =\n>  \t\tconfiguration.option<bool>({ \"software_isp\", \"copy_input_buffer\" }).value_or(true);\n>  \n> -\t/* Just one thread object for now, which will be called inline rather than async */\n> -\tthreads_.resize(1);\n> +\tunsigned int threadCount =\n> +\t\tconfiguration.option<unsigned int>({ \"software_isp\", \"threads\" }).value_or(2);\n\nThe configuration option should be documented in\nruntime_configuration.rst.\n\nAlso, I wouldn't mind making the default value more prominently visible,\nlike mentioning it in the documentation and/or defining it as a named\nconstant.\n\n> +\tthreadCount = std::clamp(threadCount, 1u, 8u);\n> +\tthreads_.resize(threadCount);\n>  \n>  \tfor (unsigned int i = 0; i < threads_.size(); i++)\n>  \t\tthreads_[i] = std::make_unique<DebayerCpuThread>(this, i, enableInputMemcpy);\n> @@ -743,6 +746,11 @@ void DebayerCpuThread::process(uint32_t frame, const uint8_t *src, uint8_t *dst)\n>  \t\tprocess2(frame, src, dst);\n>  \telse\n>  \t\tprocess4(frame, src, dst);\n> +\n> +\tdebayer_->workPendingMutex_.lock();\n> +\tdebayer_->workPending_ &= ~(1 << threadIndex_);\n> +\tdebayer_->workPendingMutex_.unlock();\n> +\tdebayer_->workPendingCv_.notify_one();\n>  }\n>  \n>  void DebayerCpuThread::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> @@ -982,7 +990,21 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n>  \n>  \tstats_->startFrame(frame);\n>  \n> -\tthreads_[0]->process(frame, in.planes()[0].data(), out.planes()[0].data());\n> +\tworkPendingMutex_.lock();\n> +\tworkPending_ = (1 << threads_.size()) - 1;\n> +\tworkPendingMutex_.unlock();\n> +\n> +\tfor (auto &thread : threads_)\n> +\t\tthread->invokeMethod(&DebayerCpuThread::process,\n> +\t\t\t\t     ConnectionTypeQueued, frame,\n> +\t\t\t\t     in.planes()[0].data(), out.planes()[0].data());\n> +\n> +\t{\n> +\t\tMutexLocker locker(workPendingMutex_);\n> +\t\tworkPendingCv_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(workPendingMutex_) {\n> +\t\t\treturn workPending_ == 0;\n> +\t\t});\n> +\t}\n>  \n>  \tmetadata.planes()[0].bytesused = out.planes()[0].size();\n>  \n> @@ -1001,6 +1023,23 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n>  \tinputBufferReady.emit(input);\n>  }\n>  \n> +int DebayerCpu::start()\n> +{\n> +\tfor (auto &thread : threads_)\n> +\t\tthread->start();\n> +\n> +\treturn 0;\n> +}\n> +\n> +void DebayerCpu::stop()\n> +{\n> +\tfor (auto &thread : threads_)\n> +\t\tthread->exit();\n> +\n> +\tfor (auto &thread : threads_)\n> +\t\tthread->wait();\n> +}\n> +\n>  SizeRange DebayerCpu::sizes(PixelFormat inputFormat, const Size &inputSize)\n>  {\n>  \tSize patternSize = this->patternSize(inputFormat);\n> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> index 1074bc9c..eb52f101 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -16,6 +16,7 @@\n>  #include <vector>\n>  \n>  #include <libcamera/base/object.h>\n> +#include <libcamera/base/mutex.h>\n>  \n>  #include \"libcamera/internal/bayer_format.h\"\n>  #include \"libcamera/internal/global_configuration.h\"\n> @@ -41,6 +42,8 @@ public:\n>  \tstd::tuple<unsigned int, unsigned int>\n>  \tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size);\n>  \tvoid process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params);\n> +\tint start();\n> +\tvoid stop();\n>  \tSizeRange sizes(PixelFormat inputFormat, const Size &inputSize);\n>  \tconst SharedFD &getStatsFD() { return stats_->getStatsFD(); }\n>  \n> @@ -147,6 +150,9 @@ private:\n>  \tstd::unique_ptr<SwStatsCpu> stats_;\n>  \tunsigned int xShift_; /* Offset of 0/1 applied to window_.x */\n>  \n> +\tunsigned int workPending_ LIBCAMERA_TSA_GUARDED_BY(workPendingMutex_);\n> +\tMutex workPendingMutex_;\n> +\tConditionVariable workPendingCv_;\n>  \tstd::vector<std::unique_ptr<DebayerCpuThread>>threads_;\n>  };","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 66424C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 25 Feb 2026 13:19:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5ED42622AE;\n\tWed, 25 Feb 2026 14:19:12 +0100 (CET)","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 0649861FA0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 25 Feb 2026 14:19:09 +0100 (CET)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-657-MBHJWOBTPsen3z11XhxyZA-1; Wed, 25 Feb 2026 08:19:07 -0500","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-483101623e9so58701935e9.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 25 Feb 2026 05:19:06 -0800 (PST)","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\tffacd0b85a97d-43970bf9feasm33910933f8f.6.2026.02.25.05.19.03\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 25 Feb 2026 05:19:04 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"YHO+hy3+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1772025548;\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=3cQ//TijcDUEy2Gx0iGew3usUtYok4YZXRS4tWMLDCw=;\n\tb=YHO+hy3+tzRNgiz0Gyr3uyXUG6EesIyMvkAeM6185+QVP8za4qVvMk6Q8UQ0tVyMY4vwgQ\n\tzNnv8To97ExJj1rSQs4HCxg/ISMm53KECgurV/PmUnrcaPSwtoEoecJK5ozVsrzMVJJmz2\n\tdTv85X/rOkMSn3yMnJLL46UR6TN6ZLo=","X-MC-Unique":"MBHJWOBTPsen3z11XhxyZA-1","X-Mimecast-MFC-AGG-ID":"MBHJWOBTPsen3z11XhxyZA_1772025546","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1772025545; x=1772630345;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=3cQ//TijcDUEy2Gx0iGew3usUtYok4YZXRS4tWMLDCw=;\n\tb=Dc3RY4MJFkf9kXEy+XJGqwia7qm0DAF5uWi5Ok8UDlfgYbbkl/gQEGiS3PecRWV1As\n\tJRcWT5shGOABicrI/BBeSSHzl8+7vHdpvEWeDrL+qLa5yEwV3BfAeLbnkud7QCwmVoSS\n\tOz2JLx9PvvKF5YrpKBKUebIpyyVYRtNmkFn/TIP3U4eygBXB8xkzg8kEfex+nktx7p4o\n\t4b4Gt3UAMJEaDHB6WmQqUstW1efHZmFCC34kRzcmO83ZD5uUSt5HOMpo69ZiGm7WFveG\n\tgf22qw9Nvnyky/UhOX7+tH3MD9r8pxm7CHyedonRnmkD0A036RvqIDn93IjOtbOb8ubo\n\tdoWA==","X-Gm-Message-State":"AOJu0YztJcn3Y6lj/cYd0BoEQkEF3VXQMNrjjFgi4v9dFYeeJDMAPShA\n\tfLJX6DBEeY15o+2w1BVrPtAMH3XylScanC54HislskeUstBGxx+B/NMJdGQMLaoCeyChNRVa5j3\n\tzlcuWq+N/lE/ofhFmO1uN77sBYapImel8oXNq0XJRTvtSjYCkwUFPqPYmbEd4IDRy6Zuj+WiKDK\n\ted1KEHfvaqQj1W3lXHeFNNMKBJ1+0He2mxAMmbf26WKbkPLMS+du+Ce9cFVyc=","X-Gm-Gg":"ATEYQzyAiAK7lG3gROwxiRH2SLwxUArsaAzivApV0DluY4pi1CCT1NRYKBOOZeYX2BW\n\tdoRA5hVzgB4xJu9/jqpNzimMJGz3TLEbGNQxDFWcHmp+DH5Vq2TbouJdhooP1dVwlBoYkzEg1xr\n\tFZA8H1ve4vS/YkAs/NJogdRCHACO8jKHJb9EWQNAzzEhSIpbBo8UvKGkC2zzym87GAf1Lh/jCyO\n\tK1GnMQf19FL9l4/+NjkVi21VkOtR2PRWKToIQ5elJ3Og9PIW6rN1f51KFgvkCX6AsjZVkjWiyTf\n\tJpJVGwcCKwIc0mC9krR5eJd6eEv221a4+uOYglxbMcgqjlmDC5v1oWuk5frb4oobepIVN2Kt3Lc\n\t8nXfIXhZfPclceZblnjuXQOXjoxb80C02cjQIWhk/hNKUNgBjYfqxtQYCSua46O12zIyGZ0Omku\n\tQ=","X-Received":["by 2002:a05:600c:8011:b0:477:7bca:8b34 with SMTP id\n\t5b1f17b1804b1-483a95eb3eemr248848675e9.6.1772025545349; \n\tWed, 25 Feb 2026 05:19:05 -0800 (PST)","by 2002:a05:600c:8011:b0:477:7bca:8b34 with SMTP id\n\t5b1f17b1804b1-483a95eb3eemr248848135e9.6.1772025544793; \n\tWed, 25 Feb 2026 05:19:04 -0800 (PST)"],"From":"Milan Zamazal <mzamazal@redhat.com>","To":"Hans de Goede <johannes.goede@oss.qualcomm.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 3/4] software_isp: debayer_cpu: Add multi-threading\n\tsupport","In-Reply-To":"<20260224193745.106186-4-johannes.goede@oss.qualcomm.com> (Hans\n\tde Goede's message of \"Tue, 24 Feb 2026 20:37:44 +0100\")","References":"<20260224193745.106186-1-johannes.goede@oss.qualcomm.com>\n\t<20260224193745.106186-4-johannes.goede@oss.qualcomm.com>","Date":"Wed, 25 Feb 2026 14:19:03 +0100","Message-ID":"<851pi9c660.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":"UonoZaMTWcTinRkF2lPM1o_ciqVfbIZlDHMXPr4RT0U_1772025546","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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]