[{"id":38215,"web_url":"https://patchwork.libcamera.org/comment/38215/","msgid":"<85pl63batb.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-02-17T16:21:20","subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","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 patches.\n\nHans de Goede writes:\n\n> Move the storage used to accumulate the RGB sums and the Y histogram\n> out of the SwStatsCpu class and into the callers.\n>\n> The idea is to allow a single SwStatsCpu object to be shared between\n> multiple threads each processing part of the image, with finishFrame()\n> accumulating the per thread data into the final stats for the entire\n> frame.\n>\n> This is a preparation patch for making DebayerCpu support multi-threading\n> and this could also be used to make processFrame() multi-threaded.\n>\n> Benchmarking with the GPU-ISP which does separate swstats benchmarking,\n> on the Uno-Q which has a weak CPU which is good for performance testing,\n> shows 20-21ms to generate stats for a 3272x2464 frame both before and\n> after this change.\n>\n> Signed-off-by: Hans de Goede \n> ---\n> .../internal/software_isp/swstats_cpu.h | 29 ++++-----\n> src/libcamera/software_isp/debayer_cpu.cpp | 12 ++--\n> src/libcamera/software_isp/debayer_cpu.h | 1 +\n> src/libcamera/software_isp/swstats_cpu.cpp | 65 +++++++++++++------\n> 4 files changed, 65 insertions(+), 42 deletions(-)\n>\n> diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h\n> index 64b3e23f..a157afe8 100644\n> --- a/include/libcamera/internal/software_isp/swstats_cpu.h\n> +++ b/include/libcamera/internal/software_isp/swstats_cpu.h\n> @@ -53,11 +53,11 @@ public:\n> \n> \tint configure(const StreamConfiguration &inputCfg);\n> \tvoid setWindow(const Rectangle &window);\n> -\tvoid startFrame(uint32_t frame);\n> -\tvoid finishFrame(uint32_t frame, uint32_t bufferId);\n> +\tvoid startFrame(uint32_t frame, struct SwIspStats statsBuffer[], unsigned int statsBufferCount);\n\n`struct' keyword not needed (here and elsewhere).\n\nIs there any reason not to use std::vector (+ to omit statsBufferCount\nargument then)?\n\n> +\tvoid finishFrame(uint32_t frame, uint32_t bufferId, struct SwIspStats statsBuffer[], unsigned int statsBufferCount);\n> \tvoid processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input);\n> \n> -\tvoid processLine0(uint32_t frame, unsigned int y, const uint8_t *src[])\n> +\tvoid processLine0(uint32_t frame, unsigned int y, const uint8_t *src[], SwIspStats *stats)\n\nWould it be perhaps better to use references rather than pointers for `stats'?\n\n> \t{\n> \t\tif (frame % kStatPerNumFrames)\n> \t\t\treturn;\n> @@ -66,10 +66,10 @@ public:\n> \t\t y >= (window_.y + window_.height))\n> \t\t\treturn;\n> \n> -\t\t(this->*stats0_)(src);\n> +\t\t(this->*stats0_)(src, stats);\n> \t}\n> \n> -\tvoid processLine2(uint32_t frame, unsigned int y, const uint8_t *src[])\n> +\tvoid processLine2(uint32_t frame, unsigned int y, const uint8_t *src[], SwIspStats *stats)\n> \t{\n> \t\tif (frame % kStatPerNumFrames)\n> \t\t\treturn;\n> @@ -78,27 +78,27 @@ public:\n> \t\t y >= (window_.y + window_.height))\n> \t\t\treturn;\n> \n> -\t\t(this->*stats2_)(src);\n> +\t\t(this->*stats2_)(src, stats);\n> \t}\n> \n> \tSignal statsReady;\n> \n> private:\n> -\tusing statsProcessFn = void (SwStatsCpu::*)(const uint8_t *src[]);\n> -\tusing processFrameFn = void (SwStatsCpu::*)(MappedFrameBuffer &in);\n> +\tusing statsProcessFn = void (SwStatsCpu::*)(const uint8_t *src[], SwIspStats *stats);\n> +\tusing processFrameFn = void (SwStatsCpu::*)(MappedFrameBuffer &in, SwIspStats *stats);\n> \n> \tint setupStandardBayerOrder(BayerFormat::Order order);\n> \t/* Bayer 8 bpp unpacked */\n> -\tvoid statsBGGR8Line0(const uint8_t *src[]);\n> +\tvoid statsBGGR8Line0(const uint8_t *src[], SwIspStats *stats);\n> \t/* Bayer 10 bpp unpacked */\n> -\tvoid statsBGGR10Line0(const uint8_t *src[]);\n> +\tvoid statsBGGR10Line0(const uint8_t *src[], SwIspStats *stats);\n> \t/* Bayer 12 bpp unpacked */\n> -\tvoid statsBGGR12Line0(const uint8_t *src[]);\n> +\tvoid statsBGGR12Line0(const uint8_t *src[], SwIspStats *stats);\n> \t/* Bayer 10 bpp packed */\n> -\tvoid statsBGGR10PLine0(const uint8_t *src[]);\n> -\tvoid statsGBRG10PLine0(const uint8_t *src[]);\n> +\tvoid statsBGGR10PLine0(const uint8_t *src[], SwIspStats *stats);\n> +\tvoid statsGBRG10PLine0(const uint8_t *src[], SwIspStats *stats);\n> \n> -\tvoid processBayerFrame2(MappedFrameBuffer &in);\n> +\tvoid processBayerFrame2(MappedFrameBuffer &in, SwIspStats *stats);\n> \n> \tprocessFrameFn processFrame_;\n> \n> @@ -117,7 +117,6 @@ private:\n> \tunsigned int stride_;\n> \n> \tSharedMemObject sharedStats_;\n> -\tSwIspStats stats_;\n> \tBenchmark bench_;\n> };\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index d0988357..97c1959a 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -673,7 +673,7 @@ void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> \tfor (unsigned int y = 0; y < yEnd; y += 2) {\n> \t\tshiftLinePointers(linePointers, src);\n> \t\tmemcpyNextLine(linePointers);\n> -\t\tstats_->processLine0(frame, y, linePointers);\n> +\t\tstats_->processLine0(frame, y, linePointers, &statsBuffer_);\n> \t\t(this->*debayer0_)(dst, linePointers);\n> \t\tsrc += inputConfig_.stride;\n> \t\tdst += outputConfig_.stride;\n> @@ -688,7 +688,7 @@ void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> \tif (window_.y == 0) {\n> \t\tshiftLinePointers(linePointers, src);\n> \t\tmemcpyNextLine(linePointers);\n> -\t\tstats_->processLine0(frame, yEnd, linePointers);\n> +\t\tstats_->processLine0(frame, yEnd, linePointers, &statsBuffer_);\n> \t\t(this->*debayer0_)(dst, linePointers);\n> \t\tsrc += inputConfig_.stride;\n> \t\tdst += outputConfig_.stride;\n> @@ -724,7 +724,7 @@ void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> \tfor (unsigned int y = 0; y < window_.height; y += 4) {\n> \t\tshiftLinePointers(linePointers, src);\n> \t\tmemcpyNextLine(linePointers);\n> -\t\tstats_->processLine0(frame, y, linePointers);\n> +\t\tstats_->processLine0(frame, y, linePointers, &statsBuffer_);\n> \t\t(this->*debayer0_)(dst, linePointers);\n> \t\tsrc += inputConfig_.stride;\n> \t\tdst += outputConfig_.stride;\n> @@ -737,7 +737,7 @@ void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> \n> \t\tshiftLinePointers(linePointers, src);\n> \t\tmemcpyNextLine(linePointers);\n> -\t\tstats_->processLine2(frame, y, linePointers);\n> +\t\tstats_->processLine2(frame, y, linePointers, &statsBuffer_);\n> \t\t(this->*debayer2_)(dst, linePointers);\n> \t\tsrc += inputConfig_.stride;\n> \t\tdst += outputConfig_.stride;\n> @@ -866,7 +866,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n> \t\treturn;\n> \t}\n> \n> -\tstats_->startFrame(frame);\n> +\tstats_->startFrame(frame, &statsBuffer_, 1);\n> \n> \tif (inputConfig_.patternSize.height == 2)\n> \t\tprocess2(frame, in.planes()[0].data(), out.planes()[0].data());\n> @@ -885,7 +885,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n> \t *\n> \t * \\todo Pass real bufferId once stats buffer passing is changed.\n> \t */\n> -\tstats_->finishFrame(frame, 0);\n> +\tstats_->finishFrame(frame, 0, &statsBuffer_, 1);\n> \toutputBufferReady.emit(output);\n> \tinputBufferReady.emit(input);\n> }\n> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> index 7a651746..8abf5168 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -135,6 +135,7 @@ private:\n> \tLookupTable gammaLut_;\n> \tbool ccmEnabled_;\n> \tDebayerParams params_;\n> +\tSwIspStats statsBuffer_;\n> \n> \tdebayerFn debayer0_;\n> \tdebayerFn debayer1_;\n> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> index 5c3011a7..23842f6c 100644\n> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> @@ -182,14 +182,14 @@ static constexpr unsigned int kBlueYMul = 29; /* 0.114 * 256 */\n> \tyVal = r * kRedYMul; \\\n> \tyVal += g * kGreenYMul; \\\n> \tyVal += b * kBlueYMul; \\\n> -\tstats_.yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;\n> +\tstats->yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;\n> \n> #define SWSTATS_FINISH_LINE_STATS() \\\n> -\tstats_.sum_.r() += sumR; \\\n> -\tstats_.sum_.g() += sumG; \\\n> -\tstats_.sum_.b() += sumB;\n> +\tstats->sum_.r() += sumR; \\\n> +\tstats->sum_.g() += sumG; \\\n> +\tstats->sum_.b() += sumB;\n> \n> -void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])\n> +void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[], SwIspStats *stats)\n> {\n> \tconst uint8_t *src0 = src[1] + window_.x;\n> \tconst uint8_t *src1 = src[2] + window_.x;\n> @@ -214,7 +214,7 @@ void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])\n> \tSWSTATS_FINISH_LINE_STATS()\n> }\n> \n> -void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[])\n> +void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[], SwIspStats *stats)\n> {\n> \tconst uint16_t *src0 = (const uint16_t *)src[1] + window_.x;\n> \tconst uint16_t *src1 = (const uint16_t *)src[2] + window_.x;\n> @@ -240,7 +240,7 @@ void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[])\n> \tSWSTATS_FINISH_LINE_STATS()\n> }\n> \n> -void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[])\n> +void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[], SwIspStats *stats)\n> {\n> \tconst uint16_t *src0 = (const uint16_t *)src[1] + window_.x;\n> \tconst uint16_t *src1 = (const uint16_t *)src[2] + window_.x;\n> @@ -266,7 +266,7 @@ void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[])\n> \tSWSTATS_FINISH_LINE_STATS()\n> }\n> \n> -void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n> +void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[], SwIspStats *stats)\n> {\n> \tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> \tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> @@ -292,7 +292,7 @@ void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n> \tSWSTATS_FINISH_LINE_STATS()\n> }\n> \n> -void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n> +void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[], SwIspStats *stats)\n> {\n> \tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> \tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> @@ -321,10 +321,13 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n> /**\n> * \\brief Reset state to start statistics gathering for a new frame\n> * \\param[in] frame The frame number\n> + * \\param[in] statsBuffer Array of buffers storing stats\n> + * \\param[in] statsBufferCount number of buffers in the statsBuffer array\n> *\n> * This may only be called after a successful setWindow() call.\n> */\n> -void SwStatsCpu::startFrame(uint32_t frame)\n> +void SwStatsCpu::startFrame(uint32_t frame,\n> +\t\t\t struct SwIspStats statsBuffer[], unsigned int statsBufferCount)\n> {\n> \tif (frame % kStatPerNumFrames)\n> \t\treturn;\n> @@ -332,21 +335,39 @@ void SwStatsCpu::startFrame(uint32_t frame)\n> \tif (window_.width == 0)\n> \t\tLOG(SwStatsCpu, Error) << \"Calling startFrame() without setWindow()\";\n> \n> -\tstats_.sum_ = RGB({ 0, 0, 0 });\n> -\tstats_.yHistogram.fill(0);\n> +\tfor (unsigned int i = 0; i < statsBufferCount; i++) {\n> +\t\tstatsBuffer[i].sum_ = RGB({ 0, 0, 0 });\n> +\t\tstatsBuffer[i].yHistogram.fill(0);\n> +\t}\n> }\n> \n> /**\n> * \\brief Finish statistics calculation for the current frame\n> * \\param[in] frame The frame number\n> * \\param[in] bufferId ID of the statistics buffer\n> + * \\param[in] statsBuffer Array of buffers storing stats\n> + * \\param[in] statsBufferCount number of buffers in the statsBuffer array\n> *\n> * This may only be called after a successful setWindow() call.\n> */\n> -void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId)\n> +void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId,\n> +\t\t\t struct SwIspStats statsBuffer[], unsigned int statsBufferCount)\n> {\n> -\tstats_.valid = frame % kStatPerNumFrames == 0;\n> -\t*sharedStats_ = stats_;\n> +\tif (frame % kStatPerNumFrames) {\n> +\t\tsharedStats_->valid = false;\n> +\t\tstatsReady.emit(frame, bufferId);\n> +\t\treturn;\n\nUsing such a shortcut rather than\n\n if (stats_.valid) {\n ...\n }\n\nfeels error-prone but up to the maintainers whether they like it or not.\n\n> +\t}\n> +\n> +\tsharedStats_->sum_ = RGB({ 0, 0, 0 });\n> +\tsharedStats_->yHistogram.fill(0);\n> +\tfor (unsigned int i = 0; i < statsBufferCount; i++) {\n> +\t\tsharedStats_->sum_ += statsBuffer[i].sum_;\n> +\t\tfor (unsigned int j = 0; j < SwIspStats::kYHistogramSize; j++)\n> +\t\t\tsharedStats_->yHistogram[j] += statsBuffer[i].yHistogram[j];\n> +\t}\n> +\n> +\tsharedStats_->valid = true;\n> \tstatsReady.emit(frame, bufferId);\n> }\n> \n> @@ -487,7 +508,7 @@ void SwStatsCpu::setWindow(const Rectangle &window)\n> \twindow_.height &= ~(patternSize_.height - 1);\n> }\n> \n> -void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in)\n> +void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in, SwIspStats *stats)\n> {\n> \tconst uint8_t *src = in.planes()[0].data();\n> \tconst uint8_t *linePointers[3];\n> @@ -504,7 +525,7 @@ void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in)\n> \t\t/* linePointers[0] is not used by any stats0_ functions */\n> \t\tlinePointers[1] = src;\n> \t\tlinePointers[2] = src + stride_;\n> -\t\t(this->*stats0_)(linePointers);\n> +\t\t(this->*stats0_)(linePointers, stats);\n> \t\tsrc += stride_ * 2;\n> \t}\n> }\n> @@ -520,12 +541,14 @@ void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in)\n> void SwStatsCpu::processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input)\n> {\n> \tif (frame % kStatPerNumFrames) {\n> -\t\tfinishFrame(frame, bufferId);\n> +\t\tfinishFrame(frame, bufferId, NULL, 0);\n\nnullptr (or an empty std::vector)\n\n> \t\treturn;\n> \t}\n> \n> +\tSwIspStats stats;\n> +\n> \tbench_.startFrame();\n> -\tstartFrame(frame);\n> +\tstartFrame(frame, &stats, 1);\n\nWould it be useful to add a comment somewhere that this method is not\ninterested in multihreading? Maybe obvious but still... These two ways\nof computing stats (CPU, multi-threaded, startFrame+finishFrame; GPU,\nsingle-go, processFrame) are somewhat confusing (not a fault of this\npatch).\n\n> \tMappedFrameBuffer in(input, MappedFrameBuffer::MapFlag::Read);\n> \tif (!in.isValid()) {\n> @@ -533,8 +556,8 @@ void SwStatsCpu::processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *in\n> \t\treturn;\n> \t}\n> \n> -\t(this->*processFrame_)(in);\n> -\tfinishFrame(frame, bufferId);\n> +\t(this->*processFrame_)(in, &stats);\n> +\tfinishFrame(frame, bufferId, &stats, 1);\n> \tbench_.finishFrame();\n> }","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 5714FC31E9\n\tfor ;\n\tTue, 17 Feb 2026 16:21:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9570B62210;\n\tTue, 17 Feb 2026 17:21:30 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 732CD61FA0\n\tfor ;\n\tTue, 17 Feb 2026 17:21:29 +0100 (CET)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-208-rNw__vqYOVCMv1D9V_KKHg-1; Tue, 17 Feb 2026 11:21:25 -0500","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-48069a43217so43787215e9.1\n\tfor ;\n\tTue, 17 Feb 2026 08:21:25 -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\t5b1f17b1804b1-48370a3ee00sm110107295e9.1.2026.02.17.08.21.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 17 Feb 2026 08:21:21 -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=\"e50dUXQA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1771345288;\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=XAOIBKyTKgVszgKcaF1y4hD/X/yJ5etRSeWlT2OYsgc=;\n\tb=e50dUXQAa0kdiFfLh8FN2tGH4gSM+rNIioi0+ejA4kzEKQmRBo6N7LQ5k1lpaO4bQKfC5d\n\t5qbzVwx1F5NiAmv/Hc2oae/wNi9+e8CJWOs11CZv2mMZYRP1H+giBNbPvSI6kRzotexW/5\n\teoAvEmZYwOLT6QIiSlg9ZphUGMsKlFY=","X-MC-Unique":"rNw__vqYOVCMv1D9V_KKHg-1","X-Mimecast-MFC-AGG-ID":"rNw__vqYOVCMv1D9V_KKHg_1771345284","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1771345283; x=1771950083;\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=XAOIBKyTKgVszgKcaF1y4hD/X/yJ5etRSeWlT2OYsgc=;\n\tb=sR9EliEmNdpvlT1Fmm5Y5SmyzS/vGNR25GTbGNO7QV2duL8keIZMw1TNbaKfUbaVx/\n\tgi0BDZYF5VLeBv3qPDltJSFfB6iMczhruu4fxTDH0vgAYVar3s1iEcBuSvLNK9L0mD1H\n\tkBLjqO+YSLmaSVtFxB2X+jZVOUj7gxeakSxJIg8V0xdFEmWa/wy4chxaFr1wTjTQhQef\n\t8F7ojWOyRxvJEJd3qkwitSmAHc0c2tMu9E6P86yTH13+bbJrZesuDK0PBeAYy2kBTaLv\n\tfkV3JFiH+iZC/ltafkPe7z3EbxSYIGVmw8xnEkoHvqAUg1kKH0R7PrWBDge7nE4kS2U9\n\tIsHQ==","X-Gm-Message-State":"AOJu0Yym8soT/Gr6mSkh7ubh5MimKiu52WhnVtpgndDVeJC+1N77ODM+\n\tORi6FzjjACbmaCFFL2lNZp2QSf2kp5zyg0GmmBBuB1JugT4qGWOP/DDIPWzo+muixmuaMCZSyf+\n\tlueEPC4NWrSMaPyMnlhm/JCr1j7IK/B37+ibv/Og5F2uWQGV8MMfVhIsnxLf2LqY04n6AWuEqDN\n\t9v3QNtvEZJbEwCEEnwnS+nO3SQMxLLbdZ1j8aAVBikv2irF/yS9oVkpxveaMg=","X-Gm-Gg":"AZuq6aLwa2RwYHW9o59Y1p+ggAqG6UHsNhxQeDGRah2oZUaR+MaKbjlXKURCrLBVzPZ\n\tIUVSRlCgnQdMPTWF/QxQw9Ap0Jsl9JPZ2618esZWLuxCDXo8XfQXQhIPMNrsOW9ZJONehcXrWSX\n\tukPtt/VECiB4mWKvjO9Jh5b7LuTbQowmEVdHRNq8z4zwz04iO3NtGzEHFAix0hoZS7/amNMi/qN\n\tW2BR9ZhiaE/TOyLwGmstc8rTi5F1KtiVYC3Y1lYHmotHO/yJVwAx5RsyeUiVJKv7Xw9tVFXOn/n\n\t6u5EAW2n/hcvyMn3ZNdb3xbl0Tsw59CnLZrNgbCnOjHPfSArLTtfSWtxE//w/tYTxfrX7tIcJre\n\tUnvsoJDT3/SBniHF8eCdOlcsAJnyQF+rAh9CDVeSm5mj4985KMi82GFWhv+43RWWHwUVdlIP/Fp\n\tI=","X-Received":["by 2002:a05:600c:c4a5:b0:47e:e71a:e13a with SMTP id\n\t5b1f17b1804b1-48379bedc20mr157267025e9.32.1771345283273; \n\tTue, 17 Feb 2026 08:21:23 -0800 (PST)","by 2002:a05:600c:c4a5:b0:47e:e71a:e13a with SMTP id\n\t5b1f17b1804b1-48379bedc20mr157266485e9.32.1771345282601; \n\tTue, 17 Feb 2026 08:21:22 -0800 (PST)"],"From":"Milan Zamazal ","To":"Hans de Goede ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","In-Reply-To":"<20260216190204.106922-2-johannes.goede@oss.qualcomm.com> (Hans\n\tde Goede's message of \"Mon, 16 Feb 2026 20:02:00 +0100\")","References":"<20260216190204.106922-1-johannes.goede@oss.qualcomm.com>\n\t<20260216190204.106922-2-johannes.goede@oss.qualcomm.com>","Date":"Tue, 17 Feb 2026 17:21:20 +0100","Message-ID":"<85pl63batb.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":"Sm8g4gCoteAN_oCYJ8emqmCVWkdVPz3BpW7PH1IX_pw_1771345284","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\" "}},{"id":38252,"web_url":"https://patchwork.libcamera.org/comment/38252/","msgid":"<20260219141122.GO520738@killaraus.ideasonboard.com>","date":"2026-02-19T14:11:22","subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Feb 17, 2026 at 05:21:20PM +0100, Milan Zamazal wrote:\n> Hans de Goede writes:\n> \n> > Move the storage used to accumulate the RGB sums and the Y histogram\n> > out of the SwStatsCpu class and into the callers.\n> >\n> > The idea is to allow a single SwStatsCpu object to be shared between\n> > multiple threads each processing part of the image, with finishFrame()\n> > accumulating the per thread data into the final stats for the entire\n> > frame.\n\nI'm not sure why it has to be done this way. In patch 5/5, where you\nintroduce multi-threading, you still create a single instance of the\nSwStatsCpu class. You could allocate the array of stats data internally\nin the class. Why did you pick this design ?\n\n> > This is a preparation patch for making DebayerCpu support multi-threading\n> > and this could also be used to make processFrame() multi-threaded.\n> >\n> > Benchmarking with the GPU-ISP which does separate swstats benchmarking,\n> > on the Uno-Q which has a weak CPU which is good for performance testing,\n> > shows 20-21ms to generate stats for a 3272x2464 frame both before and\n> > after this change.\n> >\n> > Signed-off-by: Hans de Goede \n> > ---\n> > .../internal/software_isp/swstats_cpu.h | 29 ++++-----\n> > src/libcamera/software_isp/debayer_cpu.cpp | 12 ++--\n> > src/libcamera/software_isp/debayer_cpu.h | 1 +\n> > src/libcamera/software_isp/swstats_cpu.cpp | 65 +++++++++++++------\n> > 4 files changed, 65 insertions(+), 42 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h\n> > index 64b3e23f..a157afe8 100644\n> > --- a/include/libcamera/internal/software_isp/swstats_cpu.h\n> > +++ b/include/libcamera/internal/software_isp/swstats_cpu.h\n> > @@ -53,11 +53,11 @@ public:\n> > \n> > \tint configure(const StreamConfiguration &inputCfg);\n> > \tvoid setWindow(const Rectangle &window);\n> > -\tvoid startFrame(uint32_t frame);\n> > -\tvoid finishFrame(uint32_t frame, uint32_t bufferId);\n> > +\tvoid startFrame(uint32_t frame, struct SwIspStats statsBuffer[], unsigned int statsBufferCount);\n> \n> `struct' keyword not needed (here and elsewhere).\n> \n> Is there any reason not to use std::vector (+ to omit statsBufferCount\n> argument then)?\n\nOr the Span class. Pointer + size is a no-go anywhere in libcamera.\n\n> > +\tvoid finishFrame(uint32_t frame, uint32_t bufferId, struct SwIspStats statsBuffer[], unsigned int statsBufferCount);\n> > \tvoid processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input);\n> > \n> > -\tvoid processLine0(uint32_t frame, unsigned int y, const uint8_t *src[])\n> > +\tvoid processLine0(uint32_t frame, unsigned int y, const uint8_t *src[], SwIspStats *stats)\n> \n> Would it be perhaps better to use references rather than pointers for `stats'?\n\nWe typically use const references for input parameters that can't be\nnull, and pointers for input parameters that can be null and for output\nparameters (I think that's even documented in coding-style.rst).\n\n> > \t{\n> > \t\tif (frame % kStatPerNumFrames)\n> > \t\t\treturn;\n> > @@ -66,10 +66,10 @@ public:\n> > \t\t y >= (window_.y + window_.height))\n> > \t\t\treturn;\n> > \n> > -\t\t(this->*stats0_)(src);\n> > +\t\t(this->*stats0_)(src, stats);\n> > \t}\n> > \n> > -\tvoid processLine2(uint32_t frame, unsigned int y, const uint8_t *src[])\n> > +\tvoid processLine2(uint32_t frame, unsigned int y, const uint8_t *src[], SwIspStats *stats)\n> > \t{\n> > \t\tif (frame % kStatPerNumFrames)\n> > \t\t\treturn;\n> > @@ -78,27 +78,27 @@ public:\n> > \t\t y >= (window_.y + window_.height))\n> > \t\t\treturn;\n> > \n> > -\t\t(this->*stats2_)(src);\n> > +\t\t(this->*stats2_)(src, stats);\n> > \t}\n> > \n> > \tSignal statsReady;\n> > \n> > private:\n> > -\tusing statsProcessFn = void (SwStatsCpu::*)(const uint8_t *src[]);\n> > -\tusing processFrameFn = void (SwStatsCpu::*)(MappedFrameBuffer &in);\n> > +\tusing statsProcessFn = void (SwStatsCpu::*)(const uint8_t *src[], SwIspStats *stats);\n> > +\tusing processFrameFn = void (SwStatsCpu::*)(MappedFrameBuffer &in, SwIspStats *stats);\n> > \n> > \tint setupStandardBayerOrder(BayerFormat::Order order);\n> > \t/* Bayer 8 bpp unpacked */\n> > -\tvoid statsBGGR8Line0(const uint8_t *src[]);\n> > +\tvoid statsBGGR8Line0(const uint8_t *src[], SwIspStats *stats);\n> > \t/* Bayer 10 bpp unpacked */\n> > -\tvoid statsBGGR10Line0(const uint8_t *src[]);\n> > +\tvoid statsBGGR10Line0(const uint8_t *src[], SwIspStats *stats);\n> > \t/* Bayer 12 bpp unpacked */\n> > -\tvoid statsBGGR12Line0(const uint8_t *src[]);\n> > +\tvoid statsBGGR12Line0(const uint8_t *src[], SwIspStats *stats);\n> > \t/* Bayer 10 bpp packed */\n> > -\tvoid statsBGGR10PLine0(const uint8_t *src[]);\n> > -\tvoid statsGBRG10PLine0(const uint8_t *src[]);\n> > +\tvoid statsBGGR10PLine0(const uint8_t *src[], SwIspStats *stats);\n> > +\tvoid statsGBRG10PLine0(const uint8_t *src[], SwIspStats *stats);\n> > \n> > -\tvoid processBayerFrame2(MappedFrameBuffer &in);\n> > +\tvoid processBayerFrame2(MappedFrameBuffer &in, SwIspStats *stats);\n> > \n> > \tprocessFrameFn processFrame_;\n> > \n> > @@ -117,7 +117,6 @@ private:\n> > \tunsigned int stride_;\n> > \n> > \tSharedMemObject sharedStats_;\n> > -\tSwIspStats stats_;\n> > \tBenchmark bench_;\n> > };\n> > \n> > diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> > index d0988357..97c1959a 100644\n> > --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> > +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> > @@ -673,7 +673,7 @@ void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> > \tfor (unsigned int y = 0; y < yEnd; y += 2) {\n> > \t\tshiftLinePointers(linePointers, src);\n> > \t\tmemcpyNextLine(linePointers);\n> > -\t\tstats_->processLine0(frame, y, linePointers);\n> > +\t\tstats_->processLine0(frame, y, linePointers, &statsBuffer_);\n> > \t\t(this->*debayer0_)(dst, linePointers);\n> > \t\tsrc += inputConfig_.stride;\n> > \t\tdst += outputConfig_.stride;\n> > @@ -688,7 +688,7 @@ void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> > \tif (window_.y == 0) {\n> > \t\tshiftLinePointers(linePointers, src);\n> > \t\tmemcpyNextLine(linePointers);\n> > -\t\tstats_->processLine0(frame, yEnd, linePointers);\n> > +\t\tstats_->processLine0(frame, yEnd, linePointers, &statsBuffer_);\n> > \t\t(this->*debayer0_)(dst, linePointers);\n> > \t\tsrc += inputConfig_.stride;\n> > \t\tdst += outputConfig_.stride;\n> > @@ -724,7 +724,7 @@ void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> > \tfor (unsigned int y = 0; y < window_.height; y += 4) {\n> > \t\tshiftLinePointers(linePointers, src);\n> > \t\tmemcpyNextLine(linePointers);\n> > -\t\tstats_->processLine0(frame, y, linePointers);\n> > +\t\tstats_->processLine0(frame, y, linePointers, &statsBuffer_);\n> > \t\t(this->*debayer0_)(dst, linePointers);\n> > \t\tsrc += inputConfig_.stride;\n> > \t\tdst += outputConfig_.stride;\n> > @@ -737,7 +737,7 @@ void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n> > \n> > \t\tshiftLinePointers(linePointers, src);\n> > \t\tmemcpyNextLine(linePointers);\n> > -\t\tstats_->processLine2(frame, y, linePointers);\n> > +\t\tstats_->processLine2(frame, y, linePointers, &statsBuffer_);\n> > \t\t(this->*debayer2_)(dst, linePointers);\n> > \t\tsrc += inputConfig_.stride;\n> > \t\tdst += outputConfig_.stride;\n> > @@ -866,7 +866,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n> > \t\treturn;\n> > \t}\n> > \n> > -\tstats_->startFrame(frame);\n> > +\tstats_->startFrame(frame, &statsBuffer_, 1);\n> > \n> > \tif (inputConfig_.patternSize.height == 2)\n> > \t\tprocess2(frame, in.planes()[0].data(), out.planes()[0].data());\n> > @@ -885,7 +885,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n> > \t *\n> > \t * \\todo Pass real bufferId once stats buffer passing is changed.\n> > \t */\n> > -\tstats_->finishFrame(frame, 0);\n> > +\tstats_->finishFrame(frame, 0, &statsBuffer_, 1);\n> > \toutputBufferReady.emit(output);\n> > \tinputBufferReady.emit(input);\n> > }\n> > diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> > index 7a651746..8abf5168 100644\n> > --- a/src/libcamera/software_isp/debayer_cpu.h\n> > +++ b/src/libcamera/software_isp/debayer_cpu.h\n> > @@ -135,6 +135,7 @@ private:\n> > \tLookupTable gammaLut_;\n> > \tbool ccmEnabled_;\n> > \tDebayerParams params_;\n> > +\tSwIspStats statsBuffer_;\n> > \n> > \tdebayerFn debayer0_;\n> > \tdebayerFn debayer1_;\n> > diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> > index 5c3011a7..23842f6c 100644\n> > --- a/src/libcamera/software_isp/swstats_cpu.cpp\n> > +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> > @@ -182,14 +182,14 @@ static constexpr unsigned int kBlueYMul = 29; /* 0.114 * 256 */\n> > \tyVal = r * kRedYMul; \\\n> > \tyVal += g * kGreenYMul; \\\n> > \tyVal += b * kBlueYMul; \\\n> > -\tstats_.yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;\n> > +\tstats->yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;\n> > \n> > #define SWSTATS_FINISH_LINE_STATS() \\\n> > -\tstats_.sum_.r() += sumR; \\\n> > -\tstats_.sum_.g() += sumG; \\\n> > -\tstats_.sum_.b() += sumB;\n> > +\tstats->sum_.r() += sumR; \\\n> > +\tstats->sum_.g() += sumG; \\\n> > +\tstats->sum_.b() += sumB;\n> > \n> > -void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])\n> > +void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[], SwIspStats *stats)\n> > {\n> > \tconst uint8_t *src0 = src[1] + window_.x;\n> > \tconst uint8_t *src1 = src[2] + window_.x;\n> > @@ -214,7 +214,7 @@ void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])\n> > \tSWSTATS_FINISH_LINE_STATS()\n> > }\n> > \n> > -void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[])\n> > +void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[], SwIspStats *stats)\n> > {\n> > \tconst uint16_t *src0 = (const uint16_t *)src[1] + window_.x;\n> > \tconst uint16_t *src1 = (const uint16_t *)src[2] + window_.x;\n> > @@ -240,7 +240,7 @@ void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[])\n> > \tSWSTATS_FINISH_LINE_STATS()\n> > }\n> > \n> > -void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[])\n> > +void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[], SwIspStats *stats)\n> > {\n> > \tconst uint16_t *src0 = (const uint16_t *)src[1] + window_.x;\n> > \tconst uint16_t *src1 = (const uint16_t *)src[2] + window_.x;\n> > @@ -266,7 +266,7 @@ void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[])\n> > \tSWSTATS_FINISH_LINE_STATS()\n> > }\n> > \n> > -void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n> > +void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[], SwIspStats *stats)\n> > {\n> > \tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> > \tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> > @@ -292,7 +292,7 @@ void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n> > \tSWSTATS_FINISH_LINE_STATS()\n> > }\n> > \n> > -void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n> > +void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[], SwIspStats *stats)\n> > {\n> > \tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> > \tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> > @@ -321,10 +321,13 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n> > /**\n> > * \\brief Reset state to start statistics gathering for a new frame\n> > * \\param[in] frame The frame number\n> > + * \\param[in] statsBuffer Array of buffers storing stats\n> > + * \\param[in] statsBufferCount number of buffers in the statsBuffer array\n> > *\n> > * This may only be called after a successful setWindow() call.\n> > */\n> > -void SwStatsCpu::startFrame(uint32_t frame)\n> > +void SwStatsCpu::startFrame(uint32_t frame,\n> > +\t\t\t struct SwIspStats statsBuffer[], unsigned int statsBufferCount)\n> > {\n> > \tif (frame % kStatPerNumFrames)\n> > \t\treturn;\n> > @@ -332,21 +335,39 @@ void SwStatsCpu::startFrame(uint32_t frame)\n> > \tif (window_.width == 0)\n> > \t\tLOG(SwStatsCpu, Error) << \"Calling startFrame() without setWindow()\";\n> > \n> > -\tstats_.sum_ = RGB({ 0, 0, 0 });\n> > -\tstats_.yHistogram.fill(0);\n> > +\tfor (unsigned int i = 0; i < statsBufferCount; i++) {\n> > +\t\tstatsBuffer[i].sum_ = RGB({ 0, 0, 0 });\n> > +\t\tstatsBuffer[i].yHistogram.fill(0);\n> > +\t}\n> > }\n> > \n> > /**\n> > * \\brief Finish statistics calculation for the current frame\n> > * \\param[in] frame The frame number\n> > * \\param[in] bufferId ID of the statistics buffer\n> > + * \\param[in] statsBuffer Array of buffers storing stats\n> > + * \\param[in] statsBufferCount number of buffers in the statsBuffer array\n> > *\n> > * This may only be called after a successful setWindow() call.\n> > */\n> > -void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId)\n> > +void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId,\n> > +\t\t\t struct SwIspStats statsBuffer[], unsigned int statsBufferCount)\n> > {\n> > -\tstats_.valid = frame % kStatPerNumFrames == 0;\n> > -\t*sharedStats_ = stats_;\n> > +\tif (frame % kStatPerNumFrames) {\n> > +\t\tsharedStats_->valid = false;\n> > +\t\tstatsReady.emit(frame, bufferId);\n> > +\t\treturn;\n> \n> Using such a shortcut rather than\n> \n> if (stats_.valid) {\n> ...\n> }\n> \n> feels error-prone but up to the maintainers whether they like it or not.\n> \n> > +\t}\n> > +\n> > +\tsharedStats_->sum_ = RGB({ 0, 0, 0 });\n> > +\tsharedStats_->yHistogram.fill(0);\n> > +\tfor (unsigned int i = 0; i < statsBufferCount; i++) {\n> > +\t\tsharedStats_->sum_ += statsBuffer[i].sum_;\n> > +\t\tfor (unsigned int j = 0; j < SwIspStats::kYHistogramSize; j++)\n> > +\t\t\tsharedStats_->yHistogram[j] += statsBuffer[i].yHistogram[j];\n> > +\t}\n> > +\n> > +\tsharedStats_->valid = true;\n> > \tstatsReady.emit(frame, bufferId);\n> > }\n> > \n> > @@ -487,7 +508,7 @@ void SwStatsCpu::setWindow(const Rectangle &window)\n> > \twindow_.height &= ~(patternSize_.height - 1);\n> > }\n> > \n> > -void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in)\n> > +void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in, SwIspStats *stats)\n> > {\n> > \tconst uint8_t *src = in.planes()[0].data();\n> > \tconst uint8_t *linePointers[3];\n> > @@ -504,7 +525,7 @@ void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in)\n> > \t\t/* linePointers[0] is not used by any stats0_ functions */\n> > \t\tlinePointers[1] = src;\n> > \t\tlinePointers[2] = src + stride_;\n> > -\t\t(this->*stats0_)(linePointers);\n> > +\t\t(this->*stats0_)(linePointers, stats);\n> > \t\tsrc += stride_ * 2;\n> > \t}\n> > }\n> > @@ -520,12 +541,14 @@ void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in)\n> > void SwStatsCpu::processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input)\n> > {\n> > \tif (frame % kStatPerNumFrames) {\n> > -\t\tfinishFrame(frame, bufferId);\n> > +\t\tfinishFrame(frame, bufferId, NULL, 0);\n> \n> nullptr (or an empty std::vector)\n> \n> > \t\treturn;\n> > \t}\n> > \n> > +\tSwIspStats stats;\n> > +\n> > \tbench_.startFrame();\n> > -\tstartFrame(frame);\n> > +\tstartFrame(frame, &stats, 1);\n> \n> Would it be useful to add a comment somewhere that this method is not\n> interested in multihreading? Maybe obvious but still... These two ways\n> of computing stats (CPU, multi-threaded, startFrame+finishFrame; GPU,\n> single-go, processFrame) are somewhat confusing (not a fault of this\n> patch).\n> \n> > \tMappedFrameBuffer in(input, MappedFrameBuffer::MapFlag::Read);\n> > \tif (!in.isValid()) {\n> > @@ -533,8 +556,8 @@ void SwStatsCpu::processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *in\n> > \t\treturn;\n> > \t}\n> > \n> > -\t(this->*processFrame_)(in);\n> > -\tfinishFrame(frame, bufferId);\n> > +\t(this->*processFrame_)(in, &stats);\n> > +\tfinishFrame(frame, bufferId, &stats, 1);\n> > \tbench_.finishFrame();\n> > }","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 B74ECC0DA4\n\tfor ;\n\tThu, 19 Feb 2026 14:11:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 161BA62248;\n\tThu, 19 Feb 2026 15:11:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06D23620C9\n\tfor ;\n\tThu, 19 Feb 2026 15:11:26 +0100 (CET)","from killaraus.ideasonboard.com (unknown [83.245.237.175])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 582A24D3;\n\tThu, 19 Feb 2026 15:10:32 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UEN5oLLi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1771510232;\n\tbh=R9wTlWh1uR7W6NoloxEQPw/2x2wRrAFounUiiXJbXgg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UEN5oLLinz99xTGcHA9nPu49sBOE6yJJMAhb/5m9laTRKjf4X/cZF0G3NtSuQn1Pm\n\tcfJup8blK/bKjRUXeSA6FmbHXjODkjnKH9/EeKG0fwe+OL8b/kztyj7qJPrRjvOX5f\n\tQOcGSxudtj/zLhP3N0U7Ka/wGqfEHceagedHYFW4=","Date":"Thu, 19 Feb 2026 15:11:22 +0100","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"Hans de Goede ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","Message-ID":"<20260219141122.GO520738@killaraus.ideasonboard.com>","References":"<20260216190204.106922-1-johannes.goede@oss.qualcomm.com>\n\t<20260216190204.106922-2-johannes.goede@oss.qualcomm.com>\n\t<85pl63batb.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<85pl63batb.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","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\" "}},{"id":38258,"web_url":"https://patchwork.libcamera.org/comment/38258/","msgid":"<7ae2ce5f-c3c6-4389-8781-32152460a22e@oss.qualcomm.com>","date":"2026-02-19T16:37:02","subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","submitter":{"id":242,"url":"https://patchwork.libcamera.org/api/people/242/","name":"Hans de Goede","email":"johannes.goede@oss.qualcomm.com"},"content":"Hi,\n\nOn 19-Feb-26 15:11, Laurent Pinchart wrote:\n> On Tue, Feb 17, 2026 at 05:21:20PM +0100, Milan Zamazal wrote:\n>> Hans de Goede writes:\n>>\n>>> Move the storage used to accumulate the RGB sums and the Y histogram\n>>> out of the SwStatsCpu class and into the callers.\n>>>\n>>> The idea is to allow a single SwStatsCpu object to be shared between\n>>> multiple threads each processing part of the image, with finishFrame()\n>>> accumulating the per thread data into the final stats for the entire\n>>> frame.\n> \n> I'm not sure why it has to be done this way. In patch 5/5, where you\n> introduce multi-threading, you still create a single instance of the\n> SwStatsCpu class. You could allocate the array of stats data internally\n> in the class. Why did you pick this design ?\n\nYes a single SwStatsCpu object, but an array of SwIspStats structs,\none for each thread to store the statistics for its part of\nthe entire input buffer before adding them.\n\nThe DebayerCpu code needs access to the array of SwIspStats structs\nsince each thread passes a pointer to its thread specific SwIspStats\nstruct. This is done to avoid cache-line bouncing which would happen\nif multiple-threads were incrementing the values in a shared struct.\n\nI can move the allocation of the SwIspStats structs to the SwStatsCpu\nclass, but then either the processLine0() method needs to take\nan index to know which of the N SwIspStats structs to use, or it\nneeds to expose the array of SwIspStats structs which it allocated.\n\nI think that if we move the SwIspStats struct array allocation\ninto SwStatsCpu, then passing an index of which buffer to use\nto processLine0() makes the most sense.\n\nOr we can just leave this as is.\n\nI don't have any preference either way, please let me know how you\nwant to proceed.\n\nRegards,\n\nHans","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 DB520C31E9\n\tfor ;\n\tThu, 19 Feb 2026 16:37:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 34FE762262;\n\tThu, 19 Feb 2026 17:37:08 +0100 (CET)","from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com\n\t[205.220.180.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D50B3620C9\n\tfor ;\n\tThu, 19 Feb 2026 17:37:06 +0100 (CET)","from pps.filterd (m0279871.ppops.net [127.0.0.1])\n\tby mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id\n\t61JAePWe2004670 for ;\n\tThu, 19 Feb 2026 16:37:05 GMT","from mail-qk1-f200.google.com (mail-qk1-f200.google.com\n\t[209.85.222.200])\n\tby mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4cdn1qjnkv-1\n\t(version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT)\n\tfor ;\n\tThu, 19 Feb 2026 16:37:05 +0000 (GMT)","by mail-qk1-f200.google.com with SMTP id\n\taf79cd13be357-8c881d0c617so808833385a.1\n\tfor ;\n\tThu, 19 Feb 2026 08:37:05 -0800 (PST)","from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec?\n\t(2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:c32:7800:5bfa:a036:83f0:f9ec])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-b8fc735d2dcsm594637066b.12.2026.02.19.08.37.02\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 19 Feb 2026 08:37:03 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=qualcomm.com header.i=@qualcomm.com\n\theader.b=\"QVLbta+D\"; dkim=pass (2048-bit key;\n\tunprotected) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com\n\theader.b=\"hUEFpWLc\"; dkim-atps=neutral","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h=\n\tcc:content-transfer-encoding:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to; s=qcppdkim1; bh=\n\tpOcF7Njoosz9Y0ZeqE5w5vWoxMrvBxBNzUkTOWdVxPg=; b=QVLbta+DhK7nvDki\n\t3TjjSVe0JE79/sJrXli6W4o4m3FYK7QwF8cvGeS2YmH2eltPTpQoxSU7LJ3bO4v8\n\tJaZGYuOMPh4RVmNxsjcACmxoacPtUXRIzave5AfBrXX3oF43deZP8DHVxuJ8fLPl\n\tMKOvhDsBgrJZl95PWr4Lq8ReVbUT873SmbUCBOashofClJB131xrFFva2kIG34KT\n\taJAoaKrgifFVOxItdlVo5ZXdB3KQdyR3aVXffj2kgnZtG71+UjYQgLb6vcZkSmtK\n\tmMnn7SNJD2eG8TD8xoCcyTemIIiazc5ij4w1sQxuw5tPWUEb1ZSlyVFhPu8gDwjL\n\t2hYfzg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=oss.qualcomm.com; s=google; t=1771519024; x=1772123824;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:content-language:references\n\t:cc:to:subject:from:user-agent:mime-version:date:message-id:from:to\n\t:cc:subject:date:message-id:reply-to;\n\tbh=pOcF7Njoosz9Y0ZeqE5w5vWoxMrvBxBNzUkTOWdVxPg=;\n\tb=hUEFpWLc/UL69DRrfeiOpA5uvC6gu20G27ivKpyk9E0sDnCBGamvueUciispuxDm0R\n\tp6Z/4lpQiJ16HG2TsWSud2N6KK3rbFrHBQdWBwEyTw+aj1i8OsAG90V4aNSVtsSQ3XHv\n\tyardH3vzi3kpwYmbrBq0GqmF+g44aiJ/LxvFV0gvKZ1x3Tvscj0DcR7FJUrEcCawCS4S\n\tcwScf4svkk5nhDR7GT0RrKcnm2gAKs/HqB0r/eGnaZmcwN0p4u+yxFfMIZHRZevJXaBk\n\tpxu6/gsbS/4sCt259kNPc8M65bA+6OQQOzxNJvNrP4ceDBq4bnSqedSCyrsSNopleuI+\n\tQzqg=="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1771519024; x=1772123824;\n\th=content-transfer-encoding:in-reply-to:content-language:references\n\t:cc:to:subject:from:user-agent:mime-version:date:message-id:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=pOcF7Njoosz9Y0ZeqE5w5vWoxMrvBxBNzUkTOWdVxPg=;\n\tb=vc2VXEK/ust+fnPZ64fmG/EtdMwm3ZNj/y0jMwqCNQlp74Ei6VtzW4WXPqH50qSCCO\n\tG5getyZU/BlfJd7ZbzFtXM16XubJ49OBseHy3iheD4qlbE+tpLs5g22hzBBIK7a0g2IH\n\t9hh02YZH4G8f5Sp5DYeJeAb53NPtvxDCYwPx18TCfZjpJwntRburpR3MGcPhJAwkPoP4\n\tzp4lrkXwZvIsz0oeABhGXFO0/k8KGPPI8fDmBZfevFV1Sn/cm2yE2IjP1v9K8NongW6C\n\tLakLoUCB0XtbgaqI5ksCuMYNwNVSOK9+hn27FUuM9+mIqCfuTd4RqJetMN68cCfd+rmp\n\tksoQ==","X-Gm-Message-State":"AOJu0Ywmljt5XeHY6jjgmqCmffZZUG772BDUDKEzwj6as7y83q9cU8GJ\n\tViEv4B1CFEqYIpRnGA6aCYpOVHSk4KeUaqoBMOBPAfsJpOKiKV3zN0FNZ1Kw+3fg2cQn0PS3lLW\n\taapBXnduFxl1C1j7PyOmLxA113+I5UU7VZg/WxNEmvCQtoPjDMlF6m4oGXLgDtGeP6YWecZp2OB\n\tC5","X-Gm-Gg":"AZuq6aIs1FcRTP0Or/rao/mV7dZXIDVw11ICCORa4ctrHvnw8/3Xsv1wKtXzzKQ3aEO\n\tmKEeA1XOrHYu6uT1CXZeC3YkygUkbEyTZtcxrrPDzZXzL7Dgf2xQeVaZ/NmlBX6Zww9zc7P5/zp\n\tf3XIX/t1RucNEGl/mqm6E1EJqTxWEdSBov8rvENk8y70TMQSxxd0YsKh0mMtWOM8CKmxEVlEZ/b\n\t1TWFFNUhH6wGiFIbwc6lBd57X24ZmqPElbhp7or1IDfs5IuqXJ7bAviYEHRjzjvzfd5ANj8nOww\n\tvr69b533hny/e1V540OVCf50twXk+Ulmw6ZI60fRlrgY3rZg8oqbpfMSKTHTr7ONmruq+LGObhG\n\t2HeZBzzjfjVaCarOQLoa9LiaZxaDD6rjkmGEdFQYdNpfBDqt26HyfktdaJZtsAu4ii8AByhpNBn\n\tjIcw/SlLWR7i2CARVzqZPjl/ZqRfWMcHSZB2YJoLYGmSf/859SZwLjWyFy26hXKo43umOWVHzqO\n\tYxufcgulioFqrCc","X-Received":["by 2002:a05:620a:ac0f:b0:8cb:4cb0:8d51 with SMTP id\n\taf79cd13be357-8cb4cb091d0mr1850461985a.61.1771519024466; \n\tThu, 19 Feb 2026 08:37:04 -0800 (PST)","by 2002:a05:620a:ac0f:b0:8cb:4cb0:8d51 with SMTP id\n\taf79cd13be357-8cb4cb091d0mr1850457285a.61.1771519023720; \n\tThu, 19 Feb 2026 08:37:03 -0800 (PST)"],"Message-ID":"<7ae2ce5f-c3c6-4389-8781-32152460a22e@oss.qualcomm.com>","Date":"Thu, 19 Feb 2026 17:37:02 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","From":"Hans de Goede ","Subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","To":"Laurent Pinchart ,\n\tMilan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260216190204.106922-1-johannes.goede@oss.qualcomm.com>\n\t<20260216190204.106922-2-johannes.goede@oss.qualcomm.com>\n\t<85pl63batb.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<20260219141122.GO520738@killaraus.ideasonboard.com>","Content-Language":"en-US, nl","In-Reply-To":"<20260219141122.GO520738@killaraus.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Proofpoint-ORIG-GUID":"ROkonFQgI0Zhm6SleSjntUc7Kps-t45M","X-Proofpoint-GUID":"ROkonFQgI0Zhm6SleSjntUc7Kps-t45M","X-Authority-Analysis":"v=2.4 cv=c7umgB9l c=1 sm=1 tr=0 ts=69973c31 cx=c_pps\n\ta=hnmNkyzTK/kJ09Xio7VxxA==:117 a=xqWC_Br6kY4A:10 a=IkcTkHD0fZMA:10\n\ta=HzLeVaNsDn8A:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22\n\ta=Mpw57Om8IfrbqaoTuvik:22 a=GgsMoib0sEa3-_RKJdDe:22 a=EUspDBNiAAAA:8\n\ta=8yFJivqdQUEtBJpK3BcA:9 a=QEXdDO2ut3YA:10 a=PEH46H7Ffwr30OY-TuGO:22","X-Proofpoint-Spam-Details-Enc":"AW1haW4tMjYwMjE5MDE0OSBTYWx0ZWRfX1nK6fRBygQt+\n\tOL0/VrgBq2ZfxGj+68pZinfJFkjyYIrHXjaSMXWENgY02jQsxYSxzlGNwKAcypjfqtg2oTf6ne3\n\tlsEV5aCCD7rUGiy+yYmK0ZqDeSPUlK5wesCX1o9eGFBNBiLbn2aK8rFiRAQq6aa7NQ6qaEPv3sT\n\tiCb/9UD/JoWGTW3vCtUjibaD6krMI5UwAumrhkajiFvhSwMTcf57yS/eQlyUkbLgAPopGffrHS7\n\t9OD76X5GYZQA22XgfuowAFCZsF9IBZclBxzN0uS9Iho6BvTfL/8IYP8cMoiRecmuHmpaYHj/v9Q\n\tAXkdBBAJYNofKMkeXEf6qFXcysGORULce/LSoxR8kjk9qBFHCnVjVEIgGzcQalDd/tCX5NM4q0W\n\tl0L7mZvLbcoNmZz8F9zRWA1yAyEWh82riWYPXOlDNrZXWGlDnGuTq04ysUOW+Zb0ZxeW2VOywqc\n\tTmgArxfZblw7xlFiiFg==","X-Proofpoint-Virus-Version":"vendor=baseguard\n\tengine=ICAP:2.0.293, Aquarius:18.0.1121, Hydra:6.1.51,\n\tFMLib:17.12.100.49\n\tdefinitions=2026-02-19_04,2026-02-19_01,2025-10-01_01","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\timpostorscore=0 lowpriorityscore=0 adultscore=0 malwarescore=0\n\tsuspectscore=0\n\tpriorityscore=1501 spamscore=0 phishscore=0 clxscore=1015 bulkscore=0\n\tclassifier=typeunknown authscore=0 authtc= authcc= route=outbound\n\tadjust=0\n\treason=mlx scancount=1 engine=8.22.0-2601150000\n\tdefinitions=main-2602190149","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\" "}},{"id":38261,"web_url":"https://patchwork.libcamera.org/comment/38261/","msgid":"<20260219172348.GB1619026@killaraus.ideasonboard.com>","date":"2026-02-19T17:23:48","subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Feb 19, 2026 at 05:37:02PM +0100, Hans de Goede wrote:\n> Hi,\n> \n> On 19-Feb-26 15:11, Laurent Pinchart wrote:\n> > On Tue, Feb 17, 2026 at 05:21:20PM +0100, Milan Zamazal wrote:\n> >> Hans de Goede writes:\n> >>\n> >>> Move the storage used to accumulate the RGB sums and the Y histogram\n> >>> out of the SwStatsCpu class and into the callers.\n> >>>\n> >>> The idea is to allow a single SwStatsCpu object to be shared between\n> >>> multiple threads each processing part of the image, with finishFrame()\n> >>> accumulating the per thread data into the final stats for the entire\n> >>> frame.\n> > \n> > I'm not sure why it has to be done this way. In patch 5/5, where you\n> > introduce multi-threading, you still create a single instance of the\n> > SwStatsCpu class. You could allocate the array of stats data internally\n> > in the class. Why did you pick this design ?\n> \n> Yes a single SwStatsCpu object, but an array of SwIspStats structs,\n> one for each thread to store the statistics for its part of\n> the entire input buffer before adding them.\n> \n> The DebayerCpu code needs access to the array of SwIspStats structs\n> since each thread passes a pointer to its thread specific SwIspStats\n> struct. This is done to avoid cache-line bouncing which would happen\n> if multiple-threads were incrementing the values in a shared struct.\n> \n> I can move the allocation of the SwIspStats structs to the SwStatsCpu\n> class, but then either the processLine0() method needs to take\n> an index to know which of the N SwIspStats structs to use, or it\n> needs to expose the array of SwIspStats structs which it allocated.\n> \n> I think that if we move the SwIspStats struct array allocation\n> into SwStatsCpu, then passing an index of which buffer to use\n> to processLine0() makes the most sense.\n> \n> Or we can just leave this as is.\n> \n> I don't have any preference either way, please let me know how you\n> want to proceed.\n\nLet's focus on the discussion in 5/5 first, as I think it will influence\nhow you will end up storing and passing data around.","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 A53DEC31E9\n\tfor ;\n\tThu, 19 Feb 2026 17:23:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A0586225A;\n\tThu, 19 Feb 2026 18:23:53 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ABA94620C9\n\tfor ;\n\tThu, 19 Feb 2026 18:23:52 +0100 (CET)","from killaraus.ideasonboard.com (unknown [83.245.237.175])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 212FE3A4;\n\tThu, 19 Feb 2026 18:22:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mSTbW6G6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1771521779;\n\tbh=4vPVYVw+kH0qoFaDgxRnBOLDIoGoapW7tJCkawlvn0g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mSTbW6G6j6kilv28dsTUTxI2fM45CKd2lv252v5gDT+WDeBQZCniiGn0ERddhGFFB\n\tONNCVsWmgeNOMV1oc2PqMedWU5cZfoIVXQGJwZcF6opPekWHCRdQRgT+iywp+ufAxJ\n\tnP4rwzszHhdKWDIqnsnubKQGcircLoEcJo6zM+RU=","Date":"Thu, 19 Feb 2026 18:23:48 +0100","From":"Laurent Pinchart ","To":"Hans de Goede ","Cc":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","Message-ID":"<20260219172348.GB1619026@killaraus.ideasonboard.com>","References":"<20260216190204.106922-1-johannes.goede@oss.qualcomm.com>\n\t<20260216190204.106922-2-johannes.goede@oss.qualcomm.com>\n\t<85pl63batb.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<20260219141122.GO520738@killaraus.ideasonboard.com>\n\t<7ae2ce5f-c3c6-4389-8781-32152460a22e@oss.qualcomm.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<7ae2ce5f-c3c6-4389-8781-32152460a22e@oss.qualcomm.com>","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\" "}},{"id":38273,"web_url":"https://patchwork.libcamera.org/comment/38273/","msgid":"<70aeaeb6-c3a0-4d28-8dc1-b6d233bba65e@oss.qualcomm.com>","date":"2026-02-23T15:06:18","subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","submitter":{"id":242,"url":"https://patchwork.libcamera.org/api/people/242/","name":"Hans de Goede","email":"johannes.goede@oss.qualcomm.com"},"content":"Hi,\n\nOn 17-Feb-26 5:21 PM, Milan Zamazal wrote:\n> Hi Hans,\n> \n> thank you for the patches.\n> \n> Hans de Goede writes:\n> \n>> Move the storage used to accumulate the RGB sums and the Y histogram\n>> out of the SwStatsCpu class and into the callers.\n>>\n>> The idea is to allow a single SwStatsCpu object to be shared between\n>> multiple threads each processing part of the image, with finishFrame()\n>> accumulating the per thread data into the final stats for the entire\n>> frame.\n>>\n>> This is a preparation patch for making DebayerCpu support multi-threading\n>> and this could also be used to make processFrame() multi-threaded.\n>>\n>> Benchmarking with the GPU-ISP which does separate swstats benchmarking,\n>> on the Uno-Q which has a weak CPU which is good for performance testing,\n>> shows 20-21ms to generate stats for a 3272x2464 frame both before and\n>> after this change.\n>>\n>> Signed-off-by: Hans de Goede \n>> ---\n>> .../internal/software_isp/swstats_cpu.h | 29 ++++-----\n>> src/libcamera/software_isp/debayer_cpu.cpp | 12 ++--\n>> src/libcamera/software_isp/debayer_cpu.h | 1 +\n>> src/libcamera/software_isp/swstats_cpu.cpp | 65 +++++++++++++------\n>> 4 files changed, 65 insertions(+), 42 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h\n>> index 64b3e23f..a157afe8 100644\n>> --- a/include/libcamera/internal/software_isp/swstats_cpu.h\n>> +++ b/include/libcamera/internal/software_isp/swstats_cpu.h\n>> @@ -53,11 +53,11 @@ public:\n>> \n>> \tint configure(const StreamConfiguration &inputCfg);\n>> \tvoid setWindow(const Rectangle &window);\n>> -\tvoid startFrame(uint32_t frame);\n>> -\tvoid finishFrame(uint32_t frame, uint32_t bufferId);\n>> +\tvoid startFrame(uint32_t frame, struct SwIspStats statsBuffer[], unsigned int statsBufferCount);\n> \n> `struct' keyword not needed (here and elsewhere).\n\nAck.\n\n> Is there any reason not to use std::vector (+ to omit statsBufferCount\n> argument then)?\n\nAck, v2 will use std::vector.\n\n>> +\tvoid finishFrame(uint32_t frame, uint32_t bufferId, struct SwIspStats statsBuffer[], unsigned int statsBufferCount);\n>> \tvoid processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input);\n>> \n>> -\tvoid processLine0(uint32_t frame, unsigned int y, const uint8_t *src[])\n>> +\tvoid processLine0(uint32_t frame, unsigned int y, const uint8_t *src[], SwIspStats *stats)\n> \n> Would it be perhaps better to use references rather than pointers for `stats'?\n\nAck, v2 will use references.\n\n...\n\n>> +void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId,\n>> +\t\t\t struct SwIspStats statsBuffer[], unsigned int statsBufferCount)\n>> {\n>> -\tstats_.valid = frame % kStatPerNumFrames == 0;\n>> -\t*sharedStats_ = stats_;\n>> +\tif (frame % kStatPerNumFrames) {\n>> +\t\tsharedStats_->valid = false;\n>> +\t\tstatsReady.emit(frame, bufferId);\n>> +\t\treturn;\n> \n> Using such a shortcut rather than\n> \n> if (stats_.valid) {\n> ...\n> }\n> \n> feels error-prone but up to the maintainers whether they like it or not.\n\nAck, fixed for v2.\n\nRegards,\n\nHans","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 74728BE175\n\tfor ;\n\tMon, 23 Feb 2026 15:06:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 89393621CE;\n\tMon, 23 Feb 2026 16:06:25 +0100 (CET)","from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com\n\t[205.220.168.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 81C0E621CE\n\tfor ;\n\tMon, 23 Feb 2026 16:06:23 +0100 (CET)","from pps.filterd (m0279867.ppops.net [127.0.0.1])\n\tby mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id\n\t61NAYSt93409213 for ;\n\tMon, 23 Feb 2026 15:06:21 GMT","from mail-ua1-f69.google.com (mail-ua1-f69.google.com\n\t[209.85.222.69])\n\tby mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4cgn8trrhb-1\n\t(version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT)\n\tfor ;\n\tMon, 23 Feb 2026 15:06:21 +0000 (GMT)","by mail-ua1-f69.google.com with SMTP id\n\ta1e0cc1a2514c-948b988578fso72374884241.0\n\tfor ;\n\tMon, 23 Feb 2026 07:06:21 -0800 (PST)","from [10.40.99.10] ([78.108.130.194])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-b9084c82495sm329461066b.20.2026.02.23.07.06.19\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 23 Feb 2026 07:06:19 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=qualcomm.com header.i=@qualcomm.com\n\theader.b=\"ZnGDPlOd\"; dkim=pass (2048-bit key;\n\tunprotected) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com\n\theader.b=\"TJzpierW\"; dkim-atps=neutral","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h=\n\tcc:content-transfer-encoding:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to; s=qcppdkim1; bh=\n\tIkW5J6Dj9+IGCcS3IB0b+8XGqaKrAh5IHASRo85x/HU=; b=ZnGDPlOdEYeyEwQh\n\tm8MJynjqSxCEgS+m4gkQxH9RdLhhYVqCKRsoDgQfNVeimfIWZokPjPuSZ1znjLHJ\n\t4locUCZ5HcXg0WeDvWfZgitMbFGqyhryye797MxIz/AzaPImRXNLbHhE3BV7hXSG\n\tTR0B3iVgPe1ZIFITvY7Py1uMbtoCmQaCfIWo6Y5s/Yw02ZklNQR9PgiYrOJPZghD\n\t7KHSkZ1jQD9qaoWnCFRidSsKK8/4YYuWyEo3I1NSRxPu+e24UIm6mpim8cbxbKx0\n\tb3pisEHX8N8DvyzCqt4aKxIfQbpaghCmtMLP+tDTlaPD4XQQIrbbz6f+4CKgmhoN\n\tfB1E+Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=oss.qualcomm.com; s=google; t=1771859181; x=1772463981;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:content-language:references\n\t:cc:to:subject:from:user-agent:mime-version:date:message-id:from:to\n\t:cc:subject:date:message-id:reply-to;\n\tbh=IkW5J6Dj9+IGCcS3IB0b+8XGqaKrAh5IHASRo85x/HU=;\n\tb=TJzpierWLyUP3UcvFxPgU5m8c4jyU1QviFnzKEdEhk8ZvGbK/XOx1SAnXxEcmd5dzl\n\tIJO/lwXmqitMM/jRAYQVYayy3DywljYd4bq/3g05cfbDa//onMC7MZAE1IY3m5554kcy\n\trq3KUFuTM4uLNVzZneyxcRgrgKzErdglouvwlq336ohLy/hgXgTJTUWXoiSxoIn4ccsm\n\twFqrn7UeTjt6z/mNpTYj3lIyOSR4ZtEAnOEDxv6EvSFhwj/2WEcJ+vlbUz/JK9nQ9fl/\n\tms/FHQ4FEkqCcd6rwYXZYQNXQKJnqGrG7hjumQ6ByXLbVx5EYmFeGpMMBnsMTRVwRSAg\n\tYxcQ=="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1771859181; x=1772463981;\n\th=content-transfer-encoding:in-reply-to:content-language:references\n\t:cc:to:subject:from:user-agent:mime-version:date:message-id:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=IkW5J6Dj9+IGCcS3IB0b+8XGqaKrAh5IHASRo85x/HU=;\n\tb=JuLkYcZ7Af1Wun+CzOwq1oIu4DBf6rNMVMCaluB2z+d/pjsDdVLSY9vSe18WmPXBpl\n\tpDo73qowE4vlM4tvRB2v+A3sUcHmJzjWfrMH5j22T4r4eeqVWBhpdMBbjVnsqVc2T5bC\n\tgQeG1Y2YgRlRIauDVpynykvV5pMxc1XbC+gLxSdR2nJqraKHPL3p5fYYvSg2bLqXgv08\n\teAbu/4nLCjLC9cs9lR4dfWpv9tE8twu3CihMLgF8xWIIfpx3PeKAGCcfH2n5hfu+F+fb\n\tZ1ywjnn9x180M289vTNKVx+MM7QdH1DEJr6MikyZyqQIb8wpN1ayG/0qP9Oy877F4jzl\n\tiFAA==","X-Gm-Message-State":"AOJu0Ywm3BX+VW/n/9hCRG7vf/V+knH1zcvLZ8whqNpdrj8OBB/IDYzm\n\tS7kIqnRf+52Qdg7aCBYTpnc7IACIaIcQ6yyBW/lMzumS4owHC3RdjVfBSReRR6vZObJ8yH7ltWB\n\tGM7EFrWO9aUT5eEmfnEO/Lt8DTr4e4PhjltPvQ/9Po1fXFVSRYKM9UEk6pWd29vLWwBT1sJqiym\n\tUI","X-Gm-Gg":"AZuq6aLiBg8mLsm9lWQT3PEfRDHa5gGWKZgHUcgCBNMd7Y9RBfokCneziT8JXCUNEVR\n\tZfUfoGBMd4BIUFvgQZx2lSLjjnZm3+mVjs//4c0je5TDSh7gS+yYyv0u+JgzQXdnIfMMm9qdsuS\n\twFgUm91Uu6n+XbzPocb8olXjWUIXq/rSf6kt6O3sSTfNv5hnTF2crqboMBM1s8FET4pm0+ZPmo7\n\tf2OrK07FRNgY0W8rWCJGRqEWwj8w0M9ni85YQCSFhkR93Wk1oi0FPpYVOVEJMij/3Qd6ZlAyWQe\n\taIxg92JPRTkUEmjCmdm1F5syrhnkeaH1FpbvPD39xQN3LaNz245ZI/ITEG82dQ67Grk4cWECjxI\n\txsPhkrnFGAMSlfZ8JvZf81hnXg1cVFtwQGS/XoF/4","X-Received":["by 2002:a05:6102:3a07:b0:5fd:eb2d:e714 with SMTP id\n\tada2fe7eead31-5feb307ab52mr4578638137.22.1771859180662; \n\tMon, 23 Feb 2026 07:06:20 -0800 (PST)","by 2002:a05:6102:3a07:b0:5fd:eb2d:e714 with SMTP id\n\tada2fe7eead31-5feb307ab52mr4578597137.22.1771859180212; \n\tMon, 23 Feb 2026 07:06:20 -0800 (PST)"],"Message-ID":"<70aeaeb6-c3a0-4d28-8dc1-b6d233bba65e@oss.qualcomm.com>","Date":"Mon, 23 Feb 2026 16:06:18 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","From":"Hans de Goede ","Subject":"Re: [PATCH 1/5] software_isp: swstats_cpu: Move accumulator storage\n\tout of the class","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260216190204.106922-1-johannes.goede@oss.qualcomm.com>\n\t<20260216190204.106922-2-johannes.goede@oss.qualcomm.com>\n\t<85pl63batb.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Content-Language":"en-US, nl","In-Reply-To":"<85pl63batb.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Authority-Analysis":"v=2.4 cv=X7Jf6WTe c=1 sm=1 tr=0 ts=699c6ced cx=c_pps\n\ta=UbhLPJ621ZpgOD2l3yZY1w==:117 a=rrvG0T/C2D967D07Ol03YQ==:17\n\ta=IkcTkHD0fZMA:10 a=HzLeVaNsDn8A:10 a=s4-Qcg_JpJYA:10\n\ta=VkNPw1HP01LnGYTKEx00:22 a=u7WPNUs3qKkmUXheDGA7:22\n\ta=eoimf2acIAo5FJnRuUoq:22\n\ta=EUspDBNiAAAA:8 a=VyRaFgKKN1J4zzXDvEYA:9 a=QEXdDO2ut3YA:10\n\ta=TOPH6uDL9cOC6tEoww4z:22","X-Proofpoint-ORIG-GUID":"LgNpVQvx6a9RdzJYP5X0GrU6Hfi_ffWB","X-Proofpoint-GUID":"LgNpVQvx6a9RdzJYP5X0GrU6Hfi_ffWB","X-Proofpoint-Spam-Details-Enc":"AW1haW4tMjYwMjIzMDEyOCBTYWx0ZWRfX+/71If5TpyWM\n\teLTyWUCRhhcx47IlluHZQNTDzQZyvRd5H/8a1cfRN/jSmDFPFJVnipBavuaq+rrtA/ECRaJikmm\n\tKaw6H0yosQzNku4z21su40g+k26Hz2WhunT4bnFHAsD86l+q35T0HNRQQiP04DUVu4w3b8Ly7tW\n\tm0A9sNRFL75eGYLlAZbUemhnqhWCWHyT8yzxJ8avoeqGs8okdBLGC7fyfaohoEXG2TYQhTgYuYa\n\t3tBnQaIzPd+bVALVQzW/Y3DXpPKAJkmBIeR7vzqtmsNE6j3v9DynYp6XRD6CkQTx+j0QnAVeXa7\n\tviqMuBVVwemEqTCzn24wX+qhsVb7tyQQjrkh4P9nYRWkHOhDNpnR01S23RlLKF567E6USt14dES\n\tp2eXw7IGesl1i96pJWARzTsj6WTvJALsaj9qLs4c3oelexAMiJHdVDKLfG9OcthZiVto8m9/Oml\n\tIBYHAq7QYBl1vOJxZfQ==","X-Proofpoint-Virus-Version":"vendor=baseguard\n\tengine=ICAP:2.0.293, Aquarius:18.0.1121, Hydra:6.1.51,\n\tFMLib:17.12.100.49\n\tdefinitions=2026-02-23_03,2026-02-23_02,2025-10-01_01","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tmalwarescore=0 suspectscore=0 priorityscore=1501 clxscore=1015\n\tbulkscore=0\n\tadultscore=0 spamscore=0 impostorscore=0 lowpriorityscore=0\n\tphishscore=0\n\tclassifier=typeunknown authscore=0 authtc= authcc= route=outbound\n\tadjust=0\n\treason=mlx scancount=1 engine=8.22.0-2602130000\n\tdefinitions=main-2602230128","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\" "}}]