[{"id":35998,"web_url":"https://patchwork.libcamera.org/comment/35998/","msgid":"<853489mj87.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-09-26T13:33:28","subject":"Re: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once\n\tevery 4th frame","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 patch.\n\nHans de Goede <hansg@kernel.org> writes:\n\n> Run sw-statistics once every 4th frame, \n\nI still miss any explanation why exactly every 4th frame.\n\n> instead of every frame. There are 2 reasons for this:\n>\n> 1. There really is no need to have statistics for every frame \n\nWell, depends on fps, I'd say.  Arguably, with slow fps rates,\neverything is and can be slow, but still.\n\n> and only doing this every 4th frame helps save some CPU time.\n>\n> 2. The generic nature of the simple pipeline-handler, so no information\n> about possible CSI receiver frame-delays. In combination with the software\n> ISP often being used with sensors without sensor info in the sensor-helper\n> code, so no reliable control-delay information means that the software ISP\n> is prone to AGC oscillation. Skipping statistics gathering also means\n> skipping running the AGC algorithm slowing it down, avoiding this\n> oscillation.\n\nThe last sentence is not very clear to me.\n\n> Note ideally the AGC oscillation problem would be fixed by adding sensor\n> metadata support all through the stack so that the exact gain and exposure\n> used for a specific frame are reliably provided by the sensor metadata.\n>\n> Signed-off-by: Hans de Goede <hansg@kernel.org>\n> ---\n>  src/libcamera/software_isp/debayer_cpu.cpp | 25 +++++++++++++---------\n>  src/libcamera/software_isp/debayer_cpu.h   |  4 ++--\n>  src/libcamera/software_isp/swstats_cpu.cpp |  5 +++++\n>  src/libcamera/software_isp/swstats_cpu.h   |  3 +++\n>  4 files changed, 25 insertions(+), 12 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index bfa60888..9010333e 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -655,7 +655,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>  \tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n>  }\n>  \n> -void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n> +void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n>  {\n>  \tunsigned int yEnd = window_.y + window_.height;\n>  \t/* Holds [0] previous- [1] current- [2] next-line */\n> @@ -681,7 +681,8 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>  \tfor (unsigned int y = window_.y; y < yEnd; y += 2) {\n>  \t\tshiftLinePointers(linePointers, src);\n>  \t\tmemcpyNextLine(linePointers);\n> -\t\tstats_->processLine0(y, linePointers);\n> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n> +\t\t\tstats_->processLine0(y, linePointers);\n>  \t\t(this->*debayer0_)(dst, linePointers);\n>  \t\tsrc += inputConfig_.stride;\n>  \t\tdst += outputConfig_.stride;\n> @@ -696,7 +697,8 @@ void DebayerCpu::process2(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(yEnd, linePointers);\n> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n> +\t\t\tstats_->processLine0(yEnd, linePointers);\n>  \t\t(this->*debayer0_)(dst, linePointers);\n>  \t\tsrc += inputConfig_.stride;\n>  \t\tdst += outputConfig_.stride;\n> @@ -710,7 +712,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>  \t}\n>  }\n>  \n> -void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n> +void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n>  {\n>  \tconst unsigned int yEnd = window_.y + window_.height;\n>  \t/*\n> @@ -733,7 +735,8 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>  \tfor (unsigned int y = window_.y; y < yEnd; y += 4) {\n>  \t\tshiftLinePointers(linePointers, src);\n>  \t\tmemcpyNextLine(linePointers);\n> -\t\tstats_->processLine0(y, linePointers);\n> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n> +\t\t\tstats_->processLine0(y, linePointers);\n>  \t\t(this->*debayer0_)(dst, linePointers);\n>  \t\tsrc += inputConfig_.stride;\n>  \t\tdst += outputConfig_.stride;\n> @@ -746,7 +749,8 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>  \n>  \t\tshiftLinePointers(linePointers, src);\n>  \t\tmemcpyNextLine(linePointers);\n> -\t\tstats_->processLine2(y, linePointers);\n> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n> +\t\t\tstats_->processLine2(y, linePointers);\n>  \t\t(this->*debayer2_)(dst, linePointers);\n>  \t\tsrc += inputConfig_.stride;\n>  \t\tdst += outputConfig_.stride;\n> @@ -821,12 +825,13 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n>  \t\treturn;\n>  \t}\n>  \n> -\tstats_->startFrame();\n> +\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n> +\t\tstats_->startFrame();\n>  \n>  \tif (inputConfig_.patternSize.height == 2)\n> -\t\tprocess2(in.planes()[0].data(), out.planes()[0].data());\n> +\t\tprocess2(frame, in.planes()[0].data(), out.planes()[0].data());\n>  \telse\n> -\t\tprocess4(in.planes()[0].data(), out.planes()[0].data());\n> +\t\tprocess4(frame, in.planes()[0].data(), out.planes()[0].data());\n>  \n>  \tmetadata.planes()[0].bytesused = out.planes()[0].size();\n>  \n> @@ -851,7 +856,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, true);\n> +\tstats_->finishFrame(frame, 0, frame % SwStatsCpu::kStatPerNumFrames == 0);\n\nThis leads to a crash in my environment, after 4 frames:\n\ncam: ../include/libcamera/controls.h:188: T libcamera::ControlValue::get() const [with T = int; typename std::enable_if<((! libcamera::details::is_span<U>::value) && (! std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1> >::type>::value)), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `type_ == details::control_type<std::remove_cv_t<T>>::value' failed.\n\nThread 5 \"cam\" received signal SIGABRT, Aborted.\n[...]\n#5  0x0000fffff4b99324 in libcamera::ControlValue::get<int, decltype(nullptr)> (this=<optimized out>) at ../include/libcamera/controls.h:188\n#6  libcamera::ipa::soft::IPASoftSimple::processStats (this=0xfffff0009f40, frame=4, bufferId=<optimized out>, sensorControls=...) at ../src/ipa/simple/soft_simple.cpp:312\n[...]\n\nThis omits IPASoftSimple::processStats, which I don't think is optional,\nit does (somewhat misleadingly) more than just stats processing.\n\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 9d343e46..03e0d784 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -133,8 +133,8 @@ private:\n>  \tvoid setupInputMemcpy(const uint8_t *linePointers[]);\n>  \tvoid shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src);\n>  \tvoid memcpyNextLine(const uint8_t *linePointers[]);\n> -\tvoid process2(const uint8_t *src, uint8_t *dst);\n> -\tvoid process4(const uint8_t *src, uint8_t *dst);\n> +\tvoid process2(uint32_t frame, const uint8_t *src, uint8_t *dst);\n> +\tvoid process4(uint32_t frame, const uint8_t *src, uint8_t *dst);\n>  \n>  \t/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */\n>  \tstatic constexpr unsigned int kMaxLineBuffers = 5;\n> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> index da91f912..35ba0a46 100644\n> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> @@ -89,6 +89,11 @@ namespace libcamera {\n>   * \\brief Signals that the statistics are ready\n>   */\n>  \n> +/**\n> + * \\var SwStatsCpu::kStatPerNumFrames\n> + * \\brief Run stats once every kStatPerNumFrames frames\n> + */\n> +\n>  /**\n>   * \\typedef SwStatsCpu::statsProcessFn\n>   * \\brief Called when there is data to get statistics from\n> diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h\n> index 6ac3c4de..ea0e6d5a 100644\n> --- a/src/libcamera/software_isp/swstats_cpu.h\n> +++ b/src/libcamera/software_isp/swstats_cpu.h\n> @@ -32,6 +32,9 @@ public:\n>  \tSwStatsCpu();\n>  \t~SwStatsCpu() = default;\n>  \n> +\t/* Run stats once every 4 frames */\n> +\tstatic constexpr uint32_t kStatPerNumFrames = 4;\n> +\n>  \tbool isValid() const { return sharedStats_.fd().isValid(); }\n>  \n>  \tconst SharedFD &getStatsFD() { return sharedStats_.fd(); }","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 74A2AC328C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 26 Sep 2025 13:33:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 19AE16B5F3;\n\tFri, 26 Sep 2025 15:33:37 +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 4DDAD613AB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Sep 2025 15:33:35 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-531-bEYt8FF4O7Cm2Zba9k-Thw-1; Fri, 26 Sep 2025 09:33:32 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-45de07b831dso13861805e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Sep 2025 06:33:32 -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\t5b1f17b1804b1-46e2ab31f62sm118302445e9.15.2025.09.26.06.33.29\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 26 Sep 2025 06:33:29 -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=\"VQ7lMcaG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1758893614;\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=2AQtCYvkzMIHOuGWWXENCw7RHTzDGWX05kgJ97wKO2U=;\n\tb=VQ7lMcaGt7AITF18xUy0KIupG3rTyjzDpj+Qh0mYspfelSToc8UE8fUw/regcRGog7910o\n\tsrxG+oU7pHSAuaEjShRAsK5lLrgtHSa+mvffyqRq4l4Ev81HCdj9G0YqCn185FBk6lUFap\n\tana1kl2hVK/lksLtubqjsDoGD1PAJkc=","X-MC-Unique":"bEYt8FF4O7Cm2Zba9k-Thw-1","X-Mimecast-MFC-AGG-ID":"bEYt8FF4O7Cm2Zba9k-Thw_1758893611","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1758893611; x=1759498411;\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=2AQtCYvkzMIHOuGWWXENCw7RHTzDGWX05kgJ97wKO2U=;\n\tb=Xn/UEs4CkUsUf1/7S/7xS/Kq03xlpEyba3SpYNrZvUs6oIeNidvu//atP3M0nYh8Cl\n\t7BsxgRQ10/jk6NS5l6myPzskhjGs2UEaITnbUFm+TddyGr9HAD8iT5DKv1yD4FfwL7nP\n\t8lkwGNzOvtHJv7uIpO3juzvxSG0FKHb/3O23gPJEj736NM4QJ3OTVgyPtaA21UX7KiHq\n\tyjeYpCenmBOghg7XkRXXPFzXvRT0w1/8iipMQPAX43JqKV6o49z/ntgS9B/Jp2mUkQVF\n\tGpKZ9kzSApVkyz8L+VB/GRt9npA9uOaLrXYSncggd/8lih1NbDHH+vb9cCjwLaeUlqwD\n\tqDNw==","X-Gm-Message-State":"AOJu0YyKqxDTdpb/Ff7VZtqjfvSBrzo7yOFLMX723F2JMmn39i5DSnFg\n\tjgjfde2/HsnrxFayDbnAKM2gnDGGZBqBiyZ/8HsfU1c0IB8nwxFFLG7GlME9+8jktjUXPT2sN9z\n\twxe2XX1IVdZHJxGfdWtE3QmJqBLS4u6Zh6tEjbsyGTHKvSLBhc/mZqpxjt8IbwI30qS2a+7avMP\n\tItH3onBX2Ktbj6656S7l2iaM0Ei5JQ8PnTKtvIH/0grl9Ra1R6nk2QgA9+qQE=","X-Gm-Gg":"ASbGnctZElxRn9HHm8Nx/Op5BUT1cDeSbYo5QRRmCxsE7rDAly2r0/I8/7Ng4FZTPS6\n\tNB4LpHj4LqLgpTTsokR9h5e7ZznSRmJXj5S46g8m2AnTCkOYZLWSUUWVkJqTTu5MQvxX9DsRAu0\n\t19tfjsPjbceIuIFP6T7bTJV2IGlOPK/CM54lDFckhXtGCYZfGRRkNBytIgYf1U92bwS44wIvrlb\n\t9D4s83bgzJ35k/LSVubqEudt3OjBE8XgxlNK1QhIMR64BsLqwIY9tSWDe3WXbJjmguu51rkw1Pv\n\tnnWt8GEFro9i1WqluDszw0vRLv5Kip4Co+bkvZ5PiZGfIKngzqGhw1vWkGDWwjWtdmmv+UBKyN1\n\tk5VuYaJ0ksFC5/Vc2xQ==","X-Received":["by 2002:a05:600c:8b5b:b0:46e:3dcb:d9a3 with SMTP id\n\t5b1f17b1804b1-46e3dcbdc9bmr19776875e9.12.1758893610711; \n\tFri, 26 Sep 2025 06:33:30 -0700 (PDT)","by 2002:a05:600c:8b5b:b0:46e:3dcb:d9a3 with SMTP id\n\t5b1f17b1804b1-46e3dcbdc9bmr19776355e9.12.1758893610048; \n\tFri, 26 Sep 2025 06:33:30 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEabfUeOafeX66TVa92H/hii4fBQLa5lvVy+jRKI3M+hbXg3LD12K+veI0E4P1894p18gcRcw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Hans de Goede <hansg@kernel.org>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once\n\tevery 4th frame","In-Reply-To":"<20250925221708.7471-7-hansg@kernel.org> (Hans de Goede's\n\tmessage of \"Fri, 26 Sep 2025 00:17:08 +0200\")","References":"<20250925221708.7471-1-hansg@kernel.org>\n\t<20250925221708.7471-7-hansg@kernel.org>","Date":"Fri, 26 Sep 2025 15:33:28 +0200","Message-ID":"<853489mj87.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":"0yIP9luEai3q5LCVuedS4xb6r7drQiGPX2WB5OXYhrs_1758893611","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>"}},{"id":35999,"web_url":"https://patchwork.libcamera.org/comment/35999/","msgid":"<c70f2f85-1bab-4614-93b4-66d14e71aa9d@ideasonboard.com>","date":"2025-09-26T14:21:14","subject":"Re: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once\n\tevery 4th frame","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 09. 26. 15:33 keltezéssel, Milan Zamazal írta:\n> Hi Hans,\n> \n> thank you for the patch.\n> \n> Hans de Goede <hansg@kernel.org> writes:\n> \n>> Run sw-statistics once every 4th frame,\n> \n> I still miss any explanation why exactly every 4th frame.\n> \n>> instead of every frame. There are 2 reasons for this:\n>>\n>> 1. There really is no need to have statistics for every frame\n> \n> Well, depends on fps, I'd say.  Arguably, with slow fps rates,\n> everything is and can be slow, but still.\n> \n>> and only doing this every 4th frame helps save some CPU time.\n>>\n>> 2. The generic nature of the simple pipeline-handler, so no information\n>> about possible CSI receiver frame-delays. In combination with the software\n>> ISP often being used with sensors without sensor info in the sensor-helper\n>> code, so no reliable control-delay information means that the software ISP\n>> is prone to AGC oscillation. Skipping statistics gathering also means\n>> skipping running the AGC algorithm slowing it down, avoiding this\n>> oscillation.\n> \n> The last sentence is not very clear to me.\n> \n>> Note ideally the AGC oscillation problem would be fixed by adding sensor\n>> metadata support all through the stack so that the exact gain and exposure\n>> used for a specific frame are reliably provided by the sensor metadata.\n>>\n>> Signed-off-by: Hans de Goede <hansg@kernel.org>\n>> ---\n>>   src/libcamera/software_isp/debayer_cpu.cpp | 25 +++++++++++++---------\n>>   src/libcamera/software_isp/debayer_cpu.h   |  4 ++--\n>>   src/libcamera/software_isp/swstats_cpu.cpp |  5 +++++\n>>   src/libcamera/software_isp/swstats_cpu.h   |  3 +++\n>>   4 files changed, 25 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n>> index bfa60888..9010333e 100644\n>> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n>> @@ -655,7 +655,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>>   \tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n>>   }\n>>   \n>> -void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>> +void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n>>   {\n>>   \tunsigned int yEnd = window_.y + window_.height;\n>>   \t/* Holds [0] previous- [1] current- [2] next-line */\n>> @@ -681,7 +681,8 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>   \tfor (unsigned int y = window_.y; y < yEnd; y += 2) {\n>>   \t\tshiftLinePointers(linePointers, src);\n>>   \t\tmemcpyNextLine(linePointers);\n>> -\t\tstats_->processLine0(y, linePointers);\n>> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>> +\t\t\tstats_->processLine0(y, linePointers);\n>>   \t\t(this->*debayer0_)(dst, linePointers);\n>>   \t\tsrc += inputConfig_.stride;\n>>   \t\tdst += outputConfig_.stride;\n>> @@ -696,7 +697,8 @@ void DebayerCpu::process2(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(yEnd, linePointers);\n>> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>> +\t\t\tstats_->processLine0(yEnd, linePointers);\n>>   \t\t(this->*debayer0_)(dst, linePointers);\n>>   \t\tsrc += inputConfig_.stride;\n>>   \t\tdst += outputConfig_.stride;\n>> @@ -710,7 +712,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>   \t}\n>>   }\n>>   \n>> -void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>> +void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n>>   {\n>>   \tconst unsigned int yEnd = window_.y + window_.height;\n>>   \t/*\n>> @@ -733,7 +735,8 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>>   \tfor (unsigned int y = window_.y; y < yEnd; y += 4) {\n>>   \t\tshiftLinePointers(linePointers, src);\n>>   \t\tmemcpyNextLine(linePointers);\n>> -\t\tstats_->processLine0(y, linePointers);\n>> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>> +\t\t\tstats_->processLine0(y, linePointers);\n>>   \t\t(this->*debayer0_)(dst, linePointers);\n>>   \t\tsrc += inputConfig_.stride;\n>>   \t\tdst += outputConfig_.stride;\n>> @@ -746,7 +749,8 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>>   \n>>   \t\tshiftLinePointers(linePointers, src);\n>>   \t\tmemcpyNextLine(linePointers);\n>> -\t\tstats_->processLine2(y, linePointers);\n>> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>> +\t\t\tstats_->processLine2(y, linePointers);\n>>   \t\t(this->*debayer2_)(dst, linePointers);\n>>   \t\tsrc += inputConfig_.stride;\n>>   \t\tdst += outputConfig_.stride;\n>> @@ -821,12 +825,13 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n>>   \t\treturn;\n>>   \t}\n>>   \n>> -\tstats_->startFrame();\n>> +\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>> +\t\tstats_->startFrame();\n>>   \n>>   \tif (inputConfig_.patternSize.height == 2)\n>> -\t\tprocess2(in.planes()[0].data(), out.planes()[0].data());\n>> +\t\tprocess2(frame, in.planes()[0].data(), out.planes()[0].data());\n>>   \telse\n>> -\t\tprocess4(in.planes()[0].data(), out.planes()[0].data());\n>> +\t\tprocess4(frame, in.planes()[0].data(), out.planes()[0].data());\n>>   \n>>   \tmetadata.planes()[0].bytesused = out.planes()[0].size();\n>>   \n>> @@ -851,7 +856,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, true);\n>> +\tstats_->finishFrame(frame, 0, frame % SwStatsCpu::kStatPerNumFrames == 0);\n> \n> This leads to a crash in my environment, after 4 frames:\n> \n> cam: ../include/libcamera/controls.h:188: T libcamera::ControlValue::get() const [with T = int; typename std::enable_if<((! libcamera::details::is_span<U>::value) && (! std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1> >::type>::value)), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `type_ == details::control_type<std::remove_cv_t<T>>::value' failed.\n> \n> Thread 5 \"cam\" received signal SIGABRT, Aborted.\n> [...]\n> #5  0x0000fffff4b99324 in libcamera::ControlValue::get<int, decltype(nullptr)> (this=<optimized out>) at ../include/libcamera/controls.h:188\n> #6  libcamera::ipa::soft::IPASoftSimple::processStats (this=0xfffff0009f40, frame=4, bufferId=<optimized out>, sensorControls=...) at ../src/ipa/simple/soft_simple.cpp:312\n> [...]\n> \n> This omits IPASoftSimple::processStats, which I don't think is optional,\n> it does (somewhat misleadingly) more than just stats processing.\n\nIs this assertion failure reliably reproducible with these changes?\nI cannot see it with an ipu6 laptop (running `cam -c1 -C32` repeatedly).\n\nAs far as I can tell `SwStatsCpu::finishFrame()` will emit the same signal\nregardless, so I am not sure if the issue is here. I think it's that\n`IPASoftSimple::processStats()` expects `sensorControls` to have\n`V4L2_CID_{EXPOSURE,ANALOGUE_GAIN}` present, but at least one of them is not present.\n\nThere is even a \"sanity check\" in the function but it's too late...\nthat should be fixed at least.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \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 9d343e46..03e0d784 100644\n>> --- a/src/libcamera/software_isp/debayer_cpu.h\n>> +++ b/src/libcamera/software_isp/debayer_cpu.h\n>> @@ -133,8 +133,8 @@ private:\n>>   \tvoid setupInputMemcpy(const uint8_t *linePointers[]);\n>>   \tvoid shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src);\n>>   \tvoid memcpyNextLine(const uint8_t *linePointers[]);\n>> -\tvoid process2(const uint8_t *src, uint8_t *dst);\n>> -\tvoid process4(const uint8_t *src, uint8_t *dst);\n>> +\tvoid process2(uint32_t frame, const uint8_t *src, uint8_t *dst);\n>> +\tvoid process4(uint32_t frame, const uint8_t *src, uint8_t *dst);\n>>   \n>>   \t/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */\n>>   \tstatic constexpr unsigned int kMaxLineBuffers = 5;\n>> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n>> index da91f912..35ba0a46 100644\n>> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n>> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n>> @@ -89,6 +89,11 @@ namespace libcamera {\n>>    * \\brief Signals that the statistics are ready\n>>    */\n>>   \n>> +/**\n>> + * \\var SwStatsCpu::kStatPerNumFrames\n>> + * \\brief Run stats once every kStatPerNumFrames frames\n>> + */\n>> +\n>>   /**\n>>    * \\typedef SwStatsCpu::statsProcessFn\n>>    * \\brief Called when there is data to get statistics from\n>> diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h\n>> index 6ac3c4de..ea0e6d5a 100644\n>> --- a/src/libcamera/software_isp/swstats_cpu.h\n>> +++ b/src/libcamera/software_isp/swstats_cpu.h\n>> @@ -32,6 +32,9 @@ public:\n>>   \tSwStatsCpu();\n>>   \t~SwStatsCpu() = default;\n>>   \n>> +\t/* Run stats once every 4 frames */\n>> +\tstatic constexpr uint32_t kStatPerNumFrames = 4;\n>> +\n>>   \tbool isValid() const { return sharedStats_.fd().isValid(); }\n>>   \n>>   \tconst SharedFD &getStatsFD() { return sharedStats_.fd(); }\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 824C9BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 26 Sep 2025 14:21:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 416086B5F3;\n\tFri, 26 Sep 2025 16:21:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 13CE5613AB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Sep 2025 16:21:18 +0200 (CEST)","from [192.168.33.19] (185.221.140.70.nat.pool.zt.hu\n\t[185.221.140.70])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9C666E70;\n\tFri, 26 Sep 2025 16:19:52 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TTUtdbsH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758896392;\n\tbh=yvBshEC/VLj/JO5bqkjPS5+CxuWvGy/7xeWViDL2lk8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=TTUtdbsH2C8wtUiXwWgQguFMwyXaNrlhcamiWgTSWbHW70RF2jKsi13+A+S5qx8xw\n\tDuJ5L9EY9EQmLXZm9Mv3VeuZjTERJSyY56wwKPvroeJzhOJ0CU/4kEPE39dC0qSjAJ\n\tdnxy104scqNZS0164dNykp7PdOLbgeca32mzrhb8=","Message-ID":"<c70f2f85-1bab-4614-93b4-66d14e71aa9d@ideasonboard.com>","Date":"Fri, 26 Sep 2025 16:21:14 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once\n\tevery 4th frame","To":"Milan Zamazal <mzamazal@redhat.com>, Hans de Goede <hansg@kernel.org>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250925221708.7471-1-hansg@kernel.org>\n\t<20250925221708.7471-7-hansg@kernel.org>\n\t<853489mj87.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<853489mj87.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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>"}},{"id":36003,"web_url":"https://patchwork.libcamera.org/comment/36003/","msgid":"<85ms6hkpeu.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-09-26T19:02:49","subject":"Re: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once\n\tevery 4th frame","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes:\n\n> Hi\n>\n> 2025. 09. 26. 15:33 keltezéssel, Milan Zamazal írta:\n>> Hi Hans,\n>> thank you for the patch.\n>> Hans de Goede <hansg@kernel.org> writes:\n>> \n>>> Run sw-statistics once every 4th frame,\n>> I still miss any explanation why exactly every 4th frame.\n>> \n>>> instead of every frame. There are 2 reasons for this:\n>>>\n>>> 1. There really is no need to have statistics for every frame\n>> Well, depends on fps, I'd say.  Arguably, with slow fps rates,\n>> everything is and can be slow, but still.\n>> \n>>> and only doing this every 4th frame helps save some CPU time.\n>>>\n>>> 2. The generic nature of the simple pipeline-handler, so no information\n>>> about possible CSI receiver frame-delays. In combination with the software\n>>> ISP often being used with sensors without sensor info in the sensor-helper\n>>> code, so no reliable control-delay information means that the software ISP\n>>> is prone to AGC oscillation. Skipping statistics gathering also means\n>>> skipping running the AGC algorithm slowing it down, avoiding this\n>>> oscillation.\n>> The last sentence is not very clear to me.\n>> \n>>> Note ideally the AGC oscillation problem would be fixed by adding sensor\n>>> metadata support all through the stack so that the exact gain and exposure\n>>> used for a specific frame are reliably provided by the sensor metadata.\n>>>\n>>> Signed-off-by: Hans de Goede <hansg@kernel.org>\n>>> ---\n>>>   src/libcamera/software_isp/debayer_cpu.cpp | 25 +++++++++++++---------\n>>>   src/libcamera/software_isp/debayer_cpu.h   |  4 ++--\n>>>   src/libcamera/software_isp/swstats_cpu.cpp |  5 +++++\n>>>   src/libcamera/software_isp/swstats_cpu.h   |  3 +++\n>>>   4 files changed, 25 insertions(+), 12 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n>>> index bfa60888..9010333e 100644\n>>> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n>>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n>>> @@ -655,7 +655,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>>>   \tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n>>>   }\n>>>   -void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>> +void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst)\n>>>   {\n>>>   \tunsigned int yEnd = window_.y + window_.height;\n>>>   \t/* Holds [0] previous- [1] current- [2] next-line */\n>>> @@ -681,7 +681,8 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>>   \tfor (unsigned int y = window_.y; y < yEnd; y += 2) {\n>>>   \t\tshiftLinePointers(linePointers, src);\n>>>   \t\tmemcpyNextLine(linePointers);\n>>> -\t\tstats_->processLine0(y, linePointers);\n>>> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>>> +\t\t\tstats_->processLine0(y, linePointers);\n>>>   \t\t(this->*debayer0_)(dst, linePointers);\n>>>   \t\tsrc += inputConfig_.stride;\n>>>   \t\tdst += outputConfig_.stride;\n>>> @@ -696,7 +697,8 @@ void DebayerCpu::process2(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(yEnd, linePointers);\n>>> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>>> +\t\t\tstats_->processLine0(yEnd, linePointers);\n>>>   \t\t(this->*debayer0_)(dst, linePointers);\n>>>   \t\tsrc += inputConfig_.stride;\n>>>   \t\tdst += outputConfig_.stride;\n>>> @@ -710,7 +712,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>>   \t}\n>>>   }\n>>>   -void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>>> +void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)\n>>>   {\n>>>   \tconst unsigned int yEnd = window_.y + window_.height;\n>>>   \t/*\n>>> @@ -733,7 +735,8 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>>>   \tfor (unsigned int y = window_.y; y < yEnd; y += 4) {\n>>>   \t\tshiftLinePointers(linePointers, src);\n>>>   \t\tmemcpyNextLine(linePointers);\n>>> -\t\tstats_->processLine0(y, linePointers);\n>>> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>>> +\t\t\tstats_->processLine0(y, linePointers);\n>>>   \t\t(this->*debayer0_)(dst, linePointers);\n>>>   \t\tsrc += inputConfig_.stride;\n>>>   \t\tdst += outputConfig_.stride;\n>>> @@ -746,7 +749,8 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>>>     \t\tshiftLinePointers(linePointers, src);\n>>>   \t\tmemcpyNextLine(linePointers);\n>>> -\t\tstats_->processLine2(y, linePointers);\n>>> +\t\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>>> +\t\t\tstats_->processLine2(y, linePointers);\n>>>   \t\t(this->*debayer2_)(dst, linePointers);\n>>>   \t\tsrc += inputConfig_.stride;\n>>>   \t\tdst += outputConfig_.stride;\n>>> @@ -821,12 +825,13 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output\n>>>   \t\treturn;\n>>>   \t}\n>>>   -\tstats_->startFrame();\n>>> +\tif (frame % SwStatsCpu::kStatPerNumFrames == 0)\n>>> +\t\tstats_->startFrame();\n>>>     \tif (inputConfig_.patternSize.height == 2)\n>>> -\t\tprocess2(in.planes()[0].data(), out.planes()[0].data());\n>>> +\t\tprocess2(frame, in.planes()[0].data(), out.planes()[0].data());\n>>>   \telse\n>>> -\t\tprocess4(in.planes()[0].data(), out.planes()[0].data());\n>>> +\t\tprocess4(frame, in.planes()[0].data(), out.planes()[0].data());\n>>>     \tmetadata.planes()[0].bytesused = out.planes()[0].size();\n>>>   @@ -851,7 +856,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, true);\n>>> +\tstats_->finishFrame(frame, 0, frame % SwStatsCpu::kStatPerNumFrames == 0);\n>> This leads to a crash in my environment, after 4 frames:\n>> cam: ../include/libcamera/controls.h:188: T libcamera::ControlValue::get() const [with T = int; typename\n>> std::enable_if<((! libcamera::details::is_span<U>::value) && (!\n>> std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1>\n>> >::type>::value)), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `type_ ==\n>> details::control_type<std::remove_cv_t<T>>::value' failed.\n>> Thread 5 \"cam\" received signal SIGABRT, Aborted.\n>> [...]\n>> #5 0x0000fffff4b99324 in libcamera::ControlValue::get<int, decltype(nullptr)> (this=<optimized out>) at\n>> ../include/libcamera/controls.h:188\n>> #6 libcamera::ipa::soft::IPASoftSimple::processStats (this=0xfffff0009f40, frame=4, bufferId=<optimized\n>> out>, sensorControls=...) at ../src/ipa/simple/soft_simple.cpp:312\n>> [...]\n>> This omits IPASoftSimple::processStats, which I don't think is optional,\n>> it does (somewhat misleadingly) more than just stats processing.\n>\n> Is this assertion failure reliably reproducible with these changes?\n\nYes.\n\n> I cannot see it with an ipu6 laptop (running `cam -c1 -C32` repeatedly).\n\nI think it depends on kStatPerNumFrames, the sensor delay (imx219 in my\ncase) and whatever happens in the DelayControls ring.  The problem is\napparently that delayedCtrls_ is not updated on each frame, in which\ncase libcamera::DelayedControls::get() may hit an uninitialized value\nand crash on the value type check due to the type being ControlTypeNone.\n\n> As far as I can tell `SwStatsCpu::finishFrame()` will emit the same signal\n> regardless, so I am not sure if the issue is here. I think it's that\n> `IPASoftSimple::processStats()` expects `sensorControls` to have\n> `V4L2_CID_{EXPOSURE,ANALOGUE_GAIN}` present, but at least one of them is not present.\n\nAt least in my case, the keys are present but not the values.\n\n> There is even a \"sanity check\" in the function but it's too late...\n> that should be fixed at least.\n\nYes (although it wouldn't help here).\n\nBut regarding this series: Do we still need delayedCtrls_ with these\npatches?\n\n> Regards,\n> Barnabás Pőcze\n>\n>\n>> \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 9d343e46..03e0d784 100644\n>>> --- a/src/libcamera/software_isp/debayer_cpu.h\n>>> +++ b/src/libcamera/software_isp/debayer_cpu.h\n>>> @@ -133,8 +133,8 @@ private:\n>>>   \tvoid setupInputMemcpy(const uint8_t *linePointers[]);\n>>>   \tvoid shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src);\n>>>   \tvoid memcpyNextLine(const uint8_t *linePointers[]);\n>>> -\tvoid process2(const uint8_t *src, uint8_t *dst);\n>>> -\tvoid process4(const uint8_t *src, uint8_t *dst);\n>>> +\tvoid process2(uint32_t frame, const uint8_t *src, uint8_t *dst);\n>>> +\tvoid process4(uint32_t frame, const uint8_t *src, uint8_t *dst);\n>>>     \t/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */\n>>>   \tstatic constexpr unsigned int kMaxLineBuffers = 5;\n>>> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n>>> index da91f912..35ba0a46 100644\n>>> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n>>> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n>>> @@ -89,6 +89,11 @@ namespace libcamera {\n>>>    * \\brief Signals that the statistics are ready\n>>>    */\n>>>   +/**\n>>> + * \\var SwStatsCpu::kStatPerNumFrames\n>>> + * \\brief Run stats once every kStatPerNumFrames frames\n>>> + */\n>>> +\n>>>   /**\n>>>    * \\typedef SwStatsCpu::statsProcessFn\n>>>    * \\brief Called when there is data to get statistics from\n>>> diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h\n>>> index 6ac3c4de..ea0e6d5a 100644\n>>> --- a/src/libcamera/software_isp/swstats_cpu.h\n>>> +++ b/src/libcamera/software_isp/swstats_cpu.h\n>>> @@ -32,6 +32,9 @@ public:\n>>>   \tSwStatsCpu();\n>>>   \t~SwStatsCpu() = default;\n>>>   +\t/* Run stats once every 4 frames */\n>>> +\tstatic constexpr uint32_t kStatPerNumFrames = 4;\n>>> +\n>>>   \tbool isValid() const { return sharedStats_.fd().isValid(); }\n>>>     \tconst SharedFD &getStatsFD() { return sharedStats_.fd(); }\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 A3A08BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 26 Sep 2025 19:02:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AFEBC6B5F3;\n\tFri, 26 Sep 2025 21:02:58 +0200 (CEST)","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 ED5E6613AB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Sep 2025 21:02:56 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-32-DQmBLso8N-6K7E5A8-ZsQA-1; Fri, 26 Sep 2025 15:02:53 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-46e38bd6680so7506315e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Sep 2025 12:02:53 -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\t5b1f17b1804b1-46e2a996e2fsm140617075e9.2.2025.09.26.12.02.49\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 26 Sep 2025 12:02:50 -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=\"ZrVecjH6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1758913375;\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\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=SNaYv2YsdJuCfK1nl42JI5znOPi1HcRamP1lezPVv90=;\n\tb=ZrVecjH6YNExiLxLrtTL3TfuFgLiPiV6i+2WbTFOEe+t+Ivefogl+VS7TkO8yXGpkva09G\n\t0mEt4pGSpKX+sEJf8I0BRw2nb40dzfJAcpFgnOybOcdJalLJre3d8PDPgbHzuNDaDqbxJm\n\t7kTVljw/sngH8p1w1iV3R8LDYT707bI=","X-MC-Unique":"DQmBLso8N-6K7E5A8-ZsQA-1","X-Mimecast-MFC-AGG-ID":"DQmBLso8N-6K7E5A8-ZsQA_1758913372","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1758913371; x=1759518171;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=zcU1VwfAaz+9ToA0HUMW6q8GyIqAL/cspEhzx9L9yV8=;\n\tb=m+1+RfFAmmZklxs5VvQDevbhrd1N/k4IZlziXJelryx8UaB8d/q7fboSq2LJjRXBU1\n\t19qhyec2HjjUBD0l81MQ/jB7M/zyHTIDhu9P6857P3UQnsVUoTR2D776IumyE5C3Ydah\n\t9udl2dUJ7rqc8xb1wRHqUMux3ecGN/B1bttSVoLRTpgQ5qo8xX+sSiNOYsgwXDiTWUTO\n\tpdWOwxyV4v4yYT0OlGG7aHaytDKpVyR4Z+kwZn0j3WRAulsN9RdR4TQIPe38pIFY4NMI\n\tlZ/BQL3rbPhwX+iRUhtWwlKCRSYyBlD0yEztTFQUJPpk09MF9GmITpFH9kdmC6lu+wnS\n\t1c2w==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWxJ5RAdwV/r4Rvr0W28mJ1xTywpnCmU1aadjpe/Vj5HNNL/P9wmN0b37iZg/jH4H2tfTBgWF+d3iSqXXHHewk=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YyGhsxwUhiuxJO2zrBd+5Cn3+Sx1W6D//BZRdsK1T6Y+ZVV8eMU\n\tEZihkueMSIAVP4IHnCipyCdwoVRoxkyF5Ui+Vu1i2m6ioBXdsSTP16bk4X8ks5DMWv4FKPI6ir/\n\te+Ppc68QJl9AkBMxlJEBTAqG7QiT3nXJPqwDd9K9soMON68ETYbZXSms+vCA/3x3BbdZX++nYPF\n\t6KjgFN1vylOYETS4QZB+mR6LRk8VMzlQxOt6dt+nAy50MqlgzZsnpEZtSth4w=","X-Gm-Gg":"ASbGnctMuxHKySVFQssjMemLjndPWqCNDcP2ImpEy1x9UfTRKG1+uztMXvJDuf7wjAE\n\t1yQvqfABz9/BeUOb+m5YEkbiTAjdxqRQ5PweHXo3gHQWRdv4JmE2iJRCb3lbXdeNXjz3QHsSPPW\n\tNvhNtSPx3V0nI9lrmoh6n2pHqti4ms3viuGY44b1J8olwxmGnLiJEqVkitWvj78K6EawYprLhOY\n\tFsi2U0e8KucQG22WpkkgASR77f31hxKwQ7kHOfFl2xgyK0/OVc8/fm1dLHaiTGpUPolrhmIrajM\n\t4QNSkfWIwYmBB+1k4Ph+Ylg9WiO51lMIBQ0DJDocxaSkKMHtpFN4H83VYVhqa6i4Vg5vQdqqaI+\n\t0ouOclQqBGOuUFAMOqQ==","X-Received":["by 2002:a05:600c:1c24:b0:46e:3709:d88a with SMTP id\n\t5b1f17b1804b1-46e3709dcd4mr78647525e9.33.1758913371397; \n\tFri, 26 Sep 2025 12:02:51 -0700 (PDT)","by 2002:a05:600c:1c24:b0:46e:3709:d88a with SMTP id\n\t5b1f17b1804b1-46e3709dcd4mr78647085e9.33.1758913370809; \n\tFri, 26 Sep 2025 12:02:50 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IG7xEuU6Qd435YH3iVJN8CU1teBogjoErRzhY9I5j+GVPUP+DX7Xy3NfwsJwGxmGuP1CD4bkA==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Hans de Goede <hansg@kernel.org>,  libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once\n\tevery 4th frame","In-Reply-To":"<c70f2f85-1bab-4614-93b4-66d14e71aa9d@ideasonboard.com> (\n\t=?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Fri,\n\t26 Sep 2025  16:21:14 +0200\")","References":"<20250925221708.7471-1-hansg@kernel.org>\n\t<20250925221708.7471-7-hansg@kernel.org>\n\t<853489mj87.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<c70f2f85-1bab-4614-93b4-66d14e71aa9d@ideasonboard.com>","Date":"Fri, 26 Sep 2025 21:02:49 +0200","Message-ID":"<85ms6hkpeu.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":"lx76puacuO71IBXYphHudaXf7fLbyp-p-O4XrWeYZIg_1758913372","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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>"}},{"id":36006,"web_url":"https://patchwork.libcamera.org/comment/36006/","msgid":"<25fd224d-b2f5-4e31-a423-351d7843302c@kernel.org>","date":"2025-09-27T10:44:19","subject":"Re: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once\n\tevery 4th frame","submitter":{"id":239,"url":"https://patchwork.libcamera.org/api/people/239/","name":"Hans de Goede","email":"hansg@kernel.org"},"content":"Hi,\n\nOn 26-Sep-25 9:02 PM, Milan Zamazal wrote:\n> Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes:\n> \n>> Hi\n>>\n>> 2025. 09. 26. 15:33 keltezéssel, Milan Zamazal írta:\n>>> Hi Hans,\n>>> thank you for the patch.\n\n...\n\n>>>>   @@ -851,7 +856,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, true);\n>>>> +\tstats_->finishFrame(frame, 0, frame % SwStatsCpu::kStatPerNumFrames == 0);\n>>> This leads to a crash in my environment, after 4 frames:\n>>> cam: ../include/libcamera/controls.h:188: T libcamera::ControlValue::get() const [with T = int; typename\n>>> std::enable_if<((! libcamera::details::is_span<U>::value) && (!\n>>> std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1>\n>>>> ::type>::value)), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `type_ ==\n>>> details::control_type<std::remove_cv_t<T>>::value' failed.\n>>> Thread 5 \"cam\" received signal SIGABRT, Aborted.\n>>> [...]\n>>> #5 0x0000fffff4b99324 in libcamera::ControlValue::get<int, decltype(nullptr)> (this=<optimized out>) at\n>>> ../include/libcamera/controls.h:188\n>>> #6 libcamera::ipa::soft::IPASoftSimple::processStats (this=0xfffff0009f40, frame=4, bufferId=<optimized\n>>> out>, sensorControls=...) at ../src/ipa/simple/soft_simple.cpp:312\n>>> [...]\n>>> This omits IPASoftSimple::processStats, which I don't think is optional,\n>>> it does (somewhat misleadingly) more than just stats processing.\n>>\n>> Is this assertion failure reliably reproducible with these changes?\n> \n> Yes.\n> \n>> I cannot see it with an ipu6 laptop (running `cam -c1 -C32` repeatedly).\n> \n> I think it depends on kStatPerNumFrames, the sensor delay (imx219 in my\n> case) and whatever happens in the DelayControls ring.  The problem is\n> apparently that delayedCtrls_ is not updated on each frame, in which\n> case libcamera::DelayedControls::get() may hit an uninitialized value\n> and crash on the value type check due to the type being ControlTypeNone.\n> \n>> As far as I can tell `SwStatsCpu::finishFrame()` will emit the same signal\n>> regardless, so I am not sure if the issue is here. I think it's that\n>> `IPASoftSimple::processStats()` expects `sensorControls` to have\n>> `V4L2_CID_{EXPOSURE,ANALOGUE_GAIN}` present, but at least one of them is not present.\n> \n> At least in my case, the keys are present but not the values.\n> \n>> There is even a \"sanity check\" in the function but it's too late...\n>> that should be fixed at least.\n> \n> Yes (although it wouldn't help here).\n> \n> But regarding this series: Do we still need delayedCtrls_ with these\n> patches?\n\nWe want to keep using delayedCtrls_ to make the software ISP IPA work\nthe same as other IPAs. This will also be important with Kieran's\nupcoming work to share common AGC code between IPAs which will likely\nalso expect delayed ctrls.\n\nRegards,\n\nHans","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 A3DB2BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 27 Sep 2025 10:44:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 810B56B5F3;\n\tSat, 27 Sep 2025 12:44:24 +0200 (CEST)","from tor.source.kernel.org (tor.source.kernel.org\n\t[IPv6:2600:3c04:e001:324:0:1991:8:25])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 508386B5A2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Sep 2025 12:44:23 +0200 (CEST)","from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby tor.source.kernel.org (Postfix) with ESMTP id 6B28D60236;\n\tSat, 27 Sep 2025 10:44:22 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 3E403C4CEE7;\n\tSat, 27 Sep 2025 10:44:21 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=kernel.org header.i=@kernel.org\n\theader.b=\"T8/iV1v0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1758969861;\n\tbh=EqzNo5NzFfQ4n1wbTlenOAVdFBD+SdgdjihaJJlIXb8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=T8/iV1v033hkcQBCozjOHjApYSHpLEjeR5ZNsmWH+Xjaow6ntpc6leefSXcxWj5iv\n\tvH8J3t2OPBO8qofB7molvn65uKeGxO8HUlmMyjfra22YGZlF9gDbOhZB3XI2wC9xx6\n\tL4Tj9BoH0hNB4bm4kR8p388JIKSLF5D/R1Yus68/iBqu1aNX8BWnnrtjOzK+x87ije\n\tpziIeOFtkyM4N5jKQmdW2hBdbVyM0g1GHuN0uDMca14rgM39oh++g/k4hJAaOZiQk5\n\t4V1MWlOmv9zDflS6ZeeT/xj8TFpFfGcad2HgL9vf2LL7Cr7ipWXQuvRmEPCP05bLJz\n\tLLQP3gHt3+6uQ==","Message-ID":"<25fd224d-b2f5-4e31-a423-351d7843302c@kernel.org>","Date":"Sat, 27 Sep 2025 12:44:19 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once\n\tevery 4th frame","To":"Milan Zamazal <mzamazal@redhat.com>, =?utf-8?b?QmFybmFiw6FzIFDFkWN6?=\n\t=?utf-8?q?e?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250925221708.7471-1-hansg@kernel.org>\n\t<20250925221708.7471-7-hansg@kernel.org>\n\t<853489mj87.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<c70f2f85-1bab-4614-93b4-66d14e71aa9d@ideasonboard.com>\n\t<85ms6hkpeu.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","From":"Hans de Goede <hansg@kernel.org>","Content-Language":"en-US, nl","In-Reply-To":"<85ms6hkpeu.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","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>"}}]