[{"id":38680,"web_url":"https://patchwork.libcamera.org/comment/38680/","msgid":"<26847e57-32a7-49ed-bade-4785070feea0@ideasonboard.com>","date":"2026-05-01T08:56:10","subject":"Re: [RFC PATCH v2 04/14] libcamera: software_isp: Track unused\n\tparameters buffers","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 02. 16. 21:30 keltezéssel, Milan Zamazal írta:\n> As a preparation for introducing a ring of parameters buffers, this\n> patch introduces tracking of parameters buffers available for use.\n> \n> SofwareIsp::availableParams_ is a queue of ids of the buffers available\n> for use. When a fresh parameter buffer is needed, its id is retrieved\n> from there and once the buffer is no longer needed, it's put back there,\n> in a method invoked by a signal. This is similar to what the hardware\n> pipelines do.\n> \n> If a parameters buffer is requested but there is no free available, it\n> is an erroneous situation. To recover from it, we have currently no\n> better mechanism than to wait for an available buffer. Simply returning\n> from the processing would mean the finishing signals are not called,\n> resulting in e.g. the input buffer not being returned. This is\n> different from the hardware pipelines, which are structured somewhat\n> differently.\n> \n> We use buffers' file descriptors as buffer ids. The parameters buffers\n\nThat is not going to work if the IPA is isolated. Even though this IPA\nis expected not to be isolated, but:\n * it would be nice to not break it regardless,\n * some distributions meddle with the signatures (e.g. opensuse) so\n everything runs isolated there.\n\n\n> will be shared with the IPA and debayering and we need their common\n> identification everywhere. The buffer file descriptors are shared\n> unchanged so they can be used for the purpose, avoiding a need to pass a\n> special id->buffer mapping to the IPA and debayering on initialization.\n> \n> Note that the parameters buffer id is still actually unused and the only\n> buffer currently used is still copied when passed to debayering. This\n> patch is just preparation for followup patches that will introduce\n> multiple buffers.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> .../internal/software_isp/software_isp.h | 2 ++\n> src/libcamera/software_isp/software_isp.cpp | 18 +++++++++++++++---\n> 2 files changed, 17 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> index dc6b6f7c1..a8b7b1eb1 100644\n> --- a/include/libcamera/internal/software_isp/software_isp.h\n> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -98,6 +99,7 @@ private:\n> \tThread ispWorkerThread_;\n> \tSharedMemObject sharedParams_;\n> \tDebayerParams debayerParams_;\n> +\tstd::queue availableParams_;\n\nIf this does not require FIFO operation, then I'd use a simple vector + {push,pop}_back().\n\n\n> \tbool allocateParamsBuffers();\n> \tDmaBufAllocator dmaHeap_;\n> \tbool ccmEnabled_;\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index dc0a8eb84..adf2a03ab 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -180,6 +180,10 @@ bool SoftwareIsp::allocateParamsBuffers()\n> \t\treturn false;\n> \t}\n> \n> +\tASSERT(sharedParams_.fd().get() >= 0);\n> +\tconst uint32_t bufferId = sharedParams_.fd().get();\n> +\tavailableParams_.push(bufferId);\n> +\n> \treturn true;\n> }\n> \n> @@ -404,8 +408,15 @@ void SoftwareIsp::stop()\n> */\n> void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output)\n> {\n> -\t/* \\todo Provide a real value */\n> -\tconstexpr uint32_t paramsBufferId = 0;\n> +\tif (availableParams_.empty()) {\n> +\t\tLOG(SoftwareIsp, Error) << \"Parameters buffer underrun\";\n> +\t\t/* Well, busy loop, but this situation shouldn't normally happen. */\n> +\t\twhile (availableParams_.empty())\n> +\t\t\t;\n\nThis is problematic because it blocks the current thread, so the signals are not processed,\nhence the queue will stay empty. And the loop itself also violates https://en.cppreference.com/cpp/language/multithread#Progress_guarantee\nsince this is a loop with no observable behaviour.\n\n`Thread::current()->eventDispatcher()->processEvents()` could be used to\nenter the event loop recursively, but I think that's not a good approach.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> +\t}\n> +\n> +\tconst uint32_t paramsBufferId = availableParams_.front();\n> +\tavailableParams_.pop();\n> \tipa_->computeParams(frame, paramsBufferId);\n> \tdebayer_->invokeMethod(&Debayer::process,\n> \t\t\t ConnectionTypeQueued, frame, paramsBufferId, input, output, debayerParams_);\n> @@ -416,8 +427,9 @@ void SoftwareIsp::saveIspParams([[maybe_unused]] uint32_t paramsBufferId)\n> \tdebayerParams_ = *sharedParams_;\n> }\n> \n> -void SoftwareIsp::releaseIspParams([[maybe_unused]] uint32_t paramsBufferId)\n> +void SoftwareIsp::releaseIspParams(uint32_t paramsBufferId)\n> {\n> +\tavailableParams_.push(paramsBufferId);\n> }\n> \n> void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)","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 5011CBE173\n\tfor ;\n\tFri, 1 May 2026 08:56:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 692BE62FE8;\n\tFri, 1 May 2026 10:56:15 +0200 (CEST)","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 E1ECE62E9D\n\tfor ;\n\tFri, 1 May 2026 10:56:13 +0200 (CEST)","from [192.168.33.74] (185.221.140.120.nat.pool.zt.hu\n\t[185.221.140.120])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A25647CE;\n\tFri, 1 May 2026 10:54:28 +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=\"OBueYaQs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1777625668;\n\tbh=ZYyjKF1OLlyL6l4gImUftuWzTp8LQHTc71nnzjuRKZA=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=OBueYaQsHNjgkU1ZMhBqXyOpI6ozXUePmUI/LqhLfnf5jH+U7EDDDPaC8CmkN8/7j\n\t5rmQoMkXtWtM7IatvDFpxTjFjeaYAGzrJdqedPcZv+wa6d94c1QriNd6LgRRQR79Ir\n\tJlfGjNBvNy9swxRuFCnjRWwjvn60JdYpRZf4EbRQ=","Message-ID":"<26847e57-32a7-49ed-bade-4785070feea0@ideasonboard.com>","Date":"Fri, 1 May 2026 10:56:10 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 04/14] libcamera: software_isp: Track unused\n\tparameters buffers","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","References":"<20260216203034.27558-1-mzamazal@redhat.com>\n\t<20260216203034.27558-5-mzamazal@redhat.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260216203034.27558-5-mzamazal@redhat.com>","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":"","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":38907,"web_url":"https://patchwork.libcamera.org/comment/38907/","msgid":"<85h5o84te8.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-05-15T14:52:31","subject":"Re: [RFC PATCH v2 04/14] libcamera: software_isp: Track unused\n\tparameters buffers","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Barnabás,\n\nBarnabás Pőcze writes:\n\n> 2026. 02. 16. 21:30 keltezéssel, Milan Zamazal írta:\n>> As a preparation for introducing a ring of parameters buffers, this\n>> patch introduces tracking of parameters buffers available for use.\n>> SofwareIsp::availableParams_ is a queue of ids of the buffers available\n>> for use. When a fresh parameter buffer is needed, its id is retrieved\n>> from there and once the buffer is no longer needed, it's put back there,\n>> in a method invoked by a signal. This is similar to what the hardware\n>> pipelines do.\n>> If a parameters buffer is requested but there is no free available, it\n>> is an erroneous situation. To recover from it, we have currently no\n>> better mechanism than to wait for an available buffer. Simply returning\n>> from the processing would mean the finishing signals are not called,\n>> resulting in e.g. the input buffer not being returned. This is\n>> different from the hardware pipelines, which are structured somewhat\n>> differently.\n>> We use buffers' file descriptors as buffer ids. The parameters buffers\n>\n> That is not going to work if the IPA is isolated. Even though this IPA\n> is expected not to be isolated, but:\n> * it would be nice to not break it regardless,\n> * some distributions meddle with the signatures (e.g. opensuse) so\n> everything runs isolated there.\n\nI guess it's the reason why hardware pipelines use somewhat more\ncomplicated mapping. OK, then my desire for some simplification here\ndoesn't work, I'll change it.\n\n>> will be shared with the IPA and debayering and we need their common\n>> identification everywhere. The buffer file descriptors are shared\n>> unchanged so they can be used for the purpose, avoiding a need to pass a\n>> special id->buffer mapping to the IPA and debayering on initialization.\n>> Note that the parameters buffer id is still actually unused and the only\n>> buffer currently used is still copied when passed to debayering. This\n>> patch is just preparation for followup patches that will introduce\n>> multiple buffers.\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> .../internal/software_isp/software_isp.h | 2 ++\n>> src/libcamera/software_isp/software_isp.cpp | 18 +++++++++++++++---\n>> 2 files changed, 17 insertions(+), 3 deletions(-)\n>> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n>> index dc6b6f7c1..a8b7b1eb1 100644\n>> --- a/include/libcamera/internal/software_isp/software_isp.h\n>> +++ b/include/libcamera/internal/software_isp/software_isp.h\n>> @@ -11,6 +11,7 @@\n>> #include \n>> #include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> #include \n>> @@ -98,6 +99,7 @@ private:\n>> \tThread ispWorkerThread_;\n>> \tSharedMemObject sharedParams_;\n>> \tDebayerParams debayerParams_;\n>> +\tstd::queue availableParams_;\n>\n> If this does not require FIFO operation, then I'd use a simple vector + {push,pop}_back().\n\nOK.\n\n>> \tbool allocateParamsBuffers();\n>> \tDmaBufAllocator dmaHeap_;\n>> \tbool ccmEnabled_;\n>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>> index dc0a8eb84..adf2a03ab 100644\n>> --- a/src/libcamera/software_isp/software_isp.cpp\n>> +++ b/src/libcamera/software_isp/software_isp.cpp\n>> @@ -180,6 +180,10 @@ bool SoftwareIsp::allocateParamsBuffers()\n>> \t\treturn false;\n>> \t}\n>> +\tASSERT(sharedParams_.fd().get() >= 0);\n>> +\tconst uint32_t bufferId = sharedParams_.fd().get();\n>> +\tavailableParams_.push(bufferId);\n>> +\n>> \treturn true;\n>> }\n>> @@ -404,8 +408,15 @@ void SoftwareIsp::stop()\n>> */\n>> void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output)\n>> {\n>> -\t/* \\todo Provide a real value */\n>> -\tconstexpr uint32_t paramsBufferId = 0;\n>> +\tif (availableParams_.empty()) {\n>> +\t\tLOG(SoftwareIsp, Error) << \"Parameters buffer underrun\";\n>> +\t\t/* Well, busy loop, but this situation shouldn't normally happen. */\n>> +\t\twhile (availableParams_.empty())\n>> +\t\t\t;\n>\n> This is problematic because it blocks the current thread, so the signals are not processed,\n> hence the queue will stay empty. And the loop itself also violates https://en.cppreference.com/cpp/language/multithread#Progress_guarantee\n> since this is a loop with no observable behaviour.\n>\n> `Thread::current()->eventDispatcher()->processEvents()` could be used to\n> enter the event loop recursively, but I think that's not a good approach.\n\nA possible solution could be to return an error here and cancel the\nrequest in the callers. Not very nice to the user but it should work\nflawlessly (if cancelling the request and handling the buffers is\nimplemented correctly) and it shouldn't occur frequently.\n\n>\n> Regards,\n> Barnabás Pőcze\n>\n>\n>> +\t}\n>> +\n>> +\tconst uint32_t paramsBufferId = availableParams_.front();\n>> +\tavailableParams_.pop();\n>> \tipa_->computeParams(frame, paramsBufferId);\n>> \tdebayer_->invokeMethod(&Debayer::process,\n>> \t\t\t ConnectionTypeQueued, frame, paramsBufferId, input, output, debayerParams_);\n>> @@ -416,8 +427,9 @@ void SoftwareIsp::saveIspParams([[maybe_unused]] uint32_t paramsBufferId)\n>> \tdebayerParams_ = *sharedParams_;\n>> }\n>> -void SoftwareIsp::releaseIspParams([[maybe_unused]] uint32_t paramsBufferId)\n>> +void SoftwareIsp::releaseIspParams(uint32_t paramsBufferId)\n>> {\n>> +\tavailableParams_.push(paramsBufferId);\n>> }\n>> void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)","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 17CF6BDCBC\n\tfor ;\n\tFri, 15 May 2026 14:52:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD66F63020;\n\tFri, 15 May 2026 16:52:39 +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 4356062FB1\n\tfor ;\n\tFri, 15 May 2026 16:52:38 +0200 (CEST)","from mail-wr1-f69.google.com (mail-wr1-f69.google.com\n\t[209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-169-SpfZ7HMSNmiCe1-cV8gvdQ-1; Fri, 15 May 2026 10:52:35 -0400","by mail-wr1-f69.google.com with SMTP id\n\tffacd0b85a97d-45d95cba6f1so1327971f8f.0\n\tfor ;\n\tFri, 15 May 2026 07:52:35 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-4.net.vodafone.cz. [77.48.47.4])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-45da15a6449sm14774623f8f.37.2026.05.15.07.52.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 15 May 2026 07:52:32 -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=\"NlSm5Umi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1778856757;\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=LvYFe7wQMr8wkiET0twh9mEQty/A11Fn01fwZ0mNzuI=;\n\tb=NlSm5Umi9FsiTeEwYtGPbYjMOaonfEsEz9aPxpxZ05X4V6Cbfq0M7BSKSBdTOCik0XTKX4\n\t9bddpDBeEVo2kb3oDqGJ7Bonnk9BrtWAnlnPRxW7cZPhFpGpe3G9CLs0Qf++Vpp2MnDGti\n\tzJ0a810t+a2SdijgKRdkpLxpbH3QKsk=","X-MC-Unique":"SpfZ7HMSNmiCe1-cV8gvdQ-1","X-Mimecast-MFC-AGG-ID":"SpfZ7HMSNmiCe1-cV8gvdQ_1778856754","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1778856754; x=1779461554;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=4CcjT4ynG9/KPyH5GaOikay9E9zd77+V7xoZ5tHez+Q=;\n\tb=aqhmo0867DUsjA6yR3sZ6ZgS9ku7QOX/n7QJdxpLVCjkDSut6UyW1TExM17U3xkyvK\n\tftpG083e6j2qeEMztJqH5JGfWF6tYV0UvAYtgeW/WJjHqLirJzHQeehYI/vTIGsDkuq7\n\t5nTWq2Y7XAV9mwJdl/77ylb73BU1yO2qjaqJQN0TmqCMc9poIUKOmFfB/TdWA57SlnqW\n\tF31WuvbsvdfJvB3nQQfMflH1IBcpW7ukBcAL17YkDhe5g2868VHshWtv+Uxw8hIDuJ/w\n\tLJyb6kNCBlqQ8dtLt+JWVWxiLU/J0RFqYXBGgUx+Fgi4X3sH/DDcj2d9nO31r8wqHpb2\n\tFFvw==","X-Gm-Message-State":"AOJu0Yz9nf6JRfIkfvKaa2dGLBgNe9122PTLV7P/cJZho0Ph2Qyzo/jO\n\tB1uhfNAWCPhruwXQCwukrKcl9kDKqzaHJR0hiNwrc4I2UIShlLRf1AxLfvdIjBdV/vBQg9Mlot8\n\tje5+DCtJ8SxrYSsOTrVydemP35JzFJJpxufySZKbt/PJL3A8o/OcOrSAyk8qv6tALt/SIyFSr/F\n\tb9IF5HjszPYQDSxv23g/QcL8SkctakIdReWEf+9VkRKk8kK6+uLZONnT65ias=","X-Gm-Gg":"Acq92OEvC92v5Y0H2aNHLHZB0/uaH8MDhk9vkRJlSHWfeB9lbzMudtiTzzClepsc3Ky\n\tXVr4KbDwMpbDWAXbv3OvFAf9eSP6hxB+RyfOil3jqsVU0M0zhiXmzQFItq91LXhQSkPnwfUz2YS\n\t5ZjddBikdeB9d7ymFQSN4ViSrzCrm8rQ8kh3V6p01kHY1v9iGRAsa+UicUCv1xRn5NgpYE8/QSA\n\tZGvVSoKNVlFBWTU4KjDQRbQhbNLEbxbe5NFVLf0ceihJVbhOBLH65HKNWm2Ux8Ih/dmMui7rBFG\n\tfCu8aXuqcrMKM95spZ/EHLL2mPEFgCymtT/gAnm1kRWkjW/A/0j1iCxukpivWN+C/ogTp76mRA2\n\th+YrZlHZyF6JqOsaXjo0To2jLY9wihBj6nyUhcIHkUq4EK95R9GsNupUwI+/+SFXx/KQkbfLWBz\n\tw0vK9KSPM1kQ==","X-Received":["by 2002:a05:6000:2313:b0:448:7049:a6c9 with SMTP id\n\tffacd0b85a97d-45e5c5b387bmr5698857f8f.5.1778856753655; \n\tFri, 15 May 2026 07:52:33 -0700 (PDT)","by 2002:a05:6000:2313:b0:448:7049:a6c9 with SMTP id\n\tffacd0b85a97d-45e5c5b387bmr5698811f8f.5.1778856753134; \n\tFri, 15 May 2026 07:52:33 -0700 (PDT)"],"From":"Milan Zamazal ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 04/14] libcamera: software_isp: Track unused\n\tparameters buffers","In-Reply-To":"<26847e57-32a7-49ed-bade-4785070feea0@ideasonboard.com> (\n\t=?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Fri,\n\t1 May 2026 10:56:10 +0200\")","References":"<20260216203034.27558-1-mzamazal@redhat.com>\n\t<20260216203034.27558-5-mzamazal@redhat.com>\n\t<26847e57-32a7-49ed-bade-4785070feea0@ideasonboard.com>","Date":"Fri, 15 May 2026 16:52:31 +0200","Message-ID":"<85h5o84te8.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":"twd3QvNUszgGv_8kxPU7xNBtBKymDxNvc_1s4fOXrOk_1778856754","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":"","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":38913,"web_url":"https://patchwork.libcamera.org/comment/38913/","msgid":"","date":"2026-05-18T12:49:44","subject":"Re: [RFC PATCH v2 04/14] libcamera: software_isp: Track unused\n\tparameters buffers","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2026. 05. 15. 16:52 keltezéssel, Milan Zamazal írta:\n> Hi Barnabás,\n> \n> Barnabás Pőcze writes:\n> \n>> 2026. 02. 16. 21:30 keltezéssel, Milan Zamazal írta:\n>>> As a preparation for introducing a ring of parameters buffers, this\n>>> patch introduces tracking of parameters buffers available for use.\n>>> SofwareIsp::availableParams_ is a queue of ids of the buffers available\n>>> for use. When a fresh parameter buffer is needed, its id is retrieved\n>>> from there and once the buffer is no longer needed, it's put back there,\n>>> in a method invoked by a signal. This is similar to what the hardware\n>>> pipelines do.\n>>> If a parameters buffer is requested but there is no free available, it\n>>> is an erroneous situation. To recover from it, we have currently no\n>>> better mechanism than to wait for an available buffer. Simply returning\n>>> from the processing would mean the finishing signals are not called,\n>>> resulting in e.g. the input buffer not being returned. This is\n>>> different from the hardware pipelines, which are structured somewhat\n>>> differently.\n>>> We use buffers' file descriptors as buffer ids. The parameters buffers\n>>\n>> That is not going to work if the IPA is isolated. Even though this IPA\n>> is expected not to be isolated, but:\n>> * it would be nice to not break it regardless,\n>> * some distributions meddle with the signatures (e.g. opensuse) so\n>> everything runs isolated there.\n> \n> I guess it's the reason why hardware pipelines use somewhat more\n> complicated mapping. OK, then my desire for some simplification here\n> doesn't work, I'll change it.\n> \n>>> will be shared with the IPA and debayering and we need their common\n>>> identification everywhere. The buffer file descriptors are shared\n>>> unchanged so they can be used for the purpose, avoiding a need to pass a\n>>> special id->buffer mapping to the IPA and debayering on initialization.\n>>> Note that the parameters buffer id is still actually unused and the only\n>>> buffer currently used is still copied when passed to debayering. This\n>>> patch is just preparation for followup patches that will introduce\n>>> multiple buffers.\n>>> Signed-off-by: Milan Zamazal \n>>> ---\n>>> .../internal/software_isp/software_isp.h | 2 ++\n>>> src/libcamera/software_isp/software_isp.cpp | 18 +++++++++++++++---\n>>> 2 files changed, 17 insertions(+), 3 deletions(-)\n> [...]\n>>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>>> index dc0a8eb84..adf2a03ab 100644\n>>> --- a/src/libcamera/software_isp/software_isp.cpp\n>>> +++ b/src/libcamera/software_isp/software_isp.cpp\n>>> @@ -180,6 +180,10 @@ bool SoftwareIsp::allocateParamsBuffers()\n>>> \t\treturn false;\n>>> \t}\n>>> +\tASSERT(sharedParams_.fd().get() >= 0);\n>>> +\tconst uint32_t bufferId = sharedParams_.fd().get();\n>>> +\tavailableParams_.push(bufferId);\n>>> +\n>>> \treturn true;\n>>> }\n>>> @@ -404,8 +408,15 @@ void SoftwareIsp::stop()\n>>> */\n>>> void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output)\n>>> {\n>>> -\t/* \\todo Provide a real value */\n>>> -\tconstexpr uint32_t paramsBufferId = 0;\n>>> +\tif (availableParams_.empty()) {\n>>> +\t\tLOG(SoftwareIsp, Error) << \"Parameters buffer underrun\";\n>>> +\t\t/* Well, busy loop, but this situation shouldn't normally happen. */\n>>> +\t\twhile (availableParams_.empty())\n>>> +\t\t\t;\n>>\n>> This is problematic because it blocks the current thread, so the signals are not processed,\n>> hence the queue will stay empty. And the loop itself also violates https://en.cppreference.com/cpp/language/multithread#Progress_guarantee\n>> since this is a loop with no observable behaviour.\n>>\n>> `Thread::current()->eventDispatcher()->processEvents()` could be used to\n>> enter the event loop recursively, but I think that's not a good approach.\n> \n> A possible solution could be to return an error here and cancel the\n> request in the callers. Not very nice to the user but it should work\n> flawlessly (if cancelling the request and handling the buffers is\n> implemented correctly) and it shouldn't occur frequently.\n> [...]\n\nIf you look at e.g. `PipelineHandlerRkISP1::queueRequestDevice()` it returns\nan error if param/stat buffers are not available, which will then cancel\nthe request in `PipelineHandler::doQueueRequest()`. Seems sensible to me.\n\n\nRegards,\nBarnabás Pőcze","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 6252ABDCBD\n\tfor ;\n\tMon, 18 May 2026 12:49:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECCC263024;\n\tMon, 18 May 2026 14:49:49 +0200 (CEST)","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 7B7E062FB1\n\tfor ;\n\tMon, 18 May 2026 14:49:48 +0200 (CEST)","from [IPV6:2a01:cb1d:8f2:800:9eb6:570e:4fb2:add3] (unknown\n\t[IPv6:2a01:cb1d:8f2:800:9eb6:570e:4fb2:add3])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2E88578E;\n\tMon, 18 May 2026 14:49:36 +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=\"Q88t0ih8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1779108576;\n\tbh=q8fa5zg7pRsjVWyCsPsxt8p569Y08NfyouEDeLT9zKE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Q88t0ih8IjkuM6r7qAFi35u8Eb7JDNyefoaOpp5s/64JxwkzngRwexbS5qWFPiO4T\n\t3GTp06pXbpFmt1fVU4Ljz5MV9G/MZ7wLXaT8yfIFFWhgqVaWQdhJYaxnW9BoFzAHt7\n\twJjuvFC3K7ga0i+jcnioaB45nGe1YaGpV+dm794k=","Message-ID":"","Date":"Mon, 18 May 2026 14:49:44 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 04/14] libcamera: software_isp: Track unused\n\tparameters buffers","To":"Milan Zamazal , =?utf-8?b?QmFybmFiw6FzIFDFkWN6?=\n\t=?utf-8?q?e?= ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260216203034.27558-1-mzamazal@redhat.com>\n\t<20260216203034.27558-5-mzamazal@redhat.com>\n\t<26847e57-32a7-49ed-bade-4785070feea0@ideasonboard.com>\n\t\n\t<85h5o84te8.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<85h5o84te8.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":"","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\" "}}]