[{"id":36189,"web_url":"https://patchwork.libcamera.org/comment/36189/","msgid":"<176010029479.756374.2472312101017554668@ping.linuxembedded.co.uk>","date":"2025-10-10T12:44:54","subject":"Re: [PATCH v4 1/3] pipeline: simple: Allow buffer counts from 1 to\n\t32","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Robert Mader (2025-10-10 10:22:24)\n> While a default value of 4 buffers appears to be a good default that is\n> used by other pipelines as well, allowing both higher and lower values\n> can be desirable, notably for:\n> 1. Video encoding, e.g. encoding multiple buffers in parallel.\n> 2. Clients requesting a single buffer - e.g. in multi-stream scenarios.\n> \n> Thus allow buffer counts between 1 and 32 buffers - following the default\n> maximum from vb2 core - while keeping the default to the previous 4.\n> \n> While on it mark the config as adjusted when appropriate.\n> \n> Signed-off-by: Robert Mader \n> Reviewed-by: Milan Zamazal \n> \n> ---\n> \n> Changes in v4:\n> 1. Limit the number of queued requests to 4\n> \n> Changes in v3:\n> 1. Adopeted code cleanup suggestion - no change in behavior.\n> 2. Split out change of kNumInternalBuffers.\n> 3. Minor commit message changes.\n> \n> Changes since v1 with title \"pipeline: simple: Allow buffer counts from 1 to 16 for swISP\"\n> 1: Cover all cases, not just the swISP one.\n> 2: Increase maximum to 32 to match vb2 core.\n> 3: Change constant naming to better match similar ones.\n> 4: Bump kNumInternalBuffers to 4.\n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 18 ++++++++++++++++--\n> 1 file changed, 16 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index c816cffc9..2dcba04ec 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -378,6 +378,9 @@ public:\n> const Transform &combinedTransform() const { return combinedTransform_; }\n> \n> private:\n> + static constexpr unsigned int kNumBuffersDefault = 4;\n> + static constexpr unsigned int kNumBuffersMax = 32;\n> +\n> /*\n> * The SimpleCameraData instance is guaranteed to be valid as long as\n> * the corresponding Camera instance is valid. In order to borrow a\n> @@ -1239,7 +1242,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> cfg.size != pipeConfig_->captureSize)\n> needConversion_ = true;\n> \n> - /* Set the stride, frameSize and bufferCount. */\n> + /* Set the stride and frameSize. */\n> if (needConversion_) {\n> std::tie(cfg.stride, cfg.frameSize) =\n> data_->converter_\n> @@ -1262,7 +1265,18 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> cfg.frameSize = format.planes[0].size;\n> }\n> \n> - cfg.bufferCount = 4;\n> + const auto bufferCount = cfg.bufferCount;\n> + if (bufferCount <= 0)\n\nbufferCount is an unsigned int.\n\n> + cfg.bufferCount = kNumBuffersDefault;\n> + else if (bufferCount > kNumBuffersMax)\n> + cfg.bufferCount = kNumBuffersMax;\n\n\n\nDo we really expect to support 1,2,3 ?\n\nOr should we do : \ncfg.bufferCount = std::clamp(cfg.bufferCount, kNumBuffersDefault, kNumBuffersMax);\n\nHrm, I think we could so:\n\ncfg.bufferCount = std::clamp(cfg.bufferCount, 1, kNumBuffersMax);\n\nBut that doesn't handle the case you have with <= 0 = default.... so\nmaybe it's\n\n\tunsigned int bufferCount = cfg.bufferCount;\n\tif (!bufferCount)\n\t\tcfg.bufferCount = kNumBuffersDefault;\n\t\n\tcfg.bufferCount = std::min(cfg.bufferCount, kNumBuffersMax);\n\nThat's not specifically any much better or readable than your existing\npatch so for your patch:\n\n\nReviewed-by: Kieran Bingham \n\n\n> +\n> + if (cfg.bufferCount != bufferCount) {\n> + LOG(SimplePipeline, Debug)\n> + << \"Adjusting bufferCount from \" << bufferCount\n> + << \" to \" << cfg.bufferCount;\n> + status = Adjusted;\n> + }\n> }\n> \n> return status;\n> -- \n> 2.51.0\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 C0351C3264\n\tfor ;\n\tFri, 10 Oct 2025 12:45:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C77006B615;\n\tFri, 10 Oct 2025 14:44:58 +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 69FB46B5C4\n\tfor ;\n\tFri, 10 Oct 2025 14:44:57 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F1907EFE;\n\tFri, 10 Oct 2025 14:43:21 +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=\"XLBMiA09\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760100202;\n\tbh=2dzLPJwl9oRT7iavC7AsGi9l0h/XKWfVOSPXW4VdqPg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=XLBMiA09EMaWHvA3mgXthMbDp5DoZQMzv5wy30poy6vScS8mjsk8eZ6e4lOOtKUub\n\tiuSDGhvpu3R8A6Hc3WQT6qhuJa1mmxZl23y8Zmjl+uXgZOZ68bLF9+fcV6dCysbd6C\n\tGvDLeiCqdRhWWMtJhlQADJdrmr5pQBTUc4r3sWhA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251010092226.41228-1-robert.mader@collabora.com>","References":"<20251010092226.41228-1-robert.mader@collabora.com>","Subject":"Re: [PATCH v4 1/3] pipeline: simple: Allow buffer counts from 1 to\n\t32","From":"Kieran Bingham ","Cc":"Robert Mader ,\n\tMilan Zamazal ","To":"Robert Mader ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 10 Oct 2025 13:44:54 +0100","Message-ID":"<176010029479.756374.2472312101017554668@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":36190,"web_url":"https://patchwork.libcamera.org/comment/36190/","msgid":"<0f00e9b8-f549-4e51-a491-13eb570fa0c7@ideasonboard.com>","date":"2025-10-10T13:09:23","subject":"Re: [PATCH v4 1/3] pipeline: simple: Allow buffer counts from 1 to\n\t32","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. 10. 10. 14:44 keltezéssel, Kieran Bingham írta:\n> Quoting Robert Mader (2025-10-10 10:22:24)\n>> While a default value of 4 buffers appears to be a good default that is\n>> used by other pipelines as well, allowing both higher and lower values\n>> can be desirable, notably for:\n>> 1. Video encoding, e.g. encoding multiple buffers in parallel.\n>> 2. Clients requesting a single buffer - e.g. in multi-stream scenarios.\n>>\n>> Thus allow buffer counts between 1 and 32 buffers - following the default\n>> maximum from vb2 core - while keeping the default to the previous 4.\n>>\n>> While on it mark the config as adjusted when appropriate.\n>>\n>> Signed-off-by: Robert Mader \n>> Reviewed-by: Milan Zamazal \n>>\n>> ---\n> [...]\n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 18 ++++++++++++++++--\n>> 1 file changed, 16 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index c816cffc9..2dcba04ec 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -378,6 +378,9 @@ public:\n>> const Transform &combinedTransform() const { return combinedTransform_; }\n>> \n>> private:\n>> + static constexpr unsigned int kNumBuffersDefault = 4;\n>> + static constexpr unsigned int kNumBuffersMax = 32;\n>> +\n> [...]\n>> - cfg.bufferCount = 4;\n>> + const auto bufferCount = cfg.bufferCount;\n>> + if (bufferCount <= 0)\n> \n> bufferCount is an unsigned int.\n> \n>> + cfg.bufferCount = kNumBuffersDefault;\n>> + else if (bufferCount > kNumBuffersMax)\n>> + cfg.bufferCount = kNumBuffersMax;\n> \n> \n> \n> Do we really expect to support 1,2,3 ?\n\nSeems to work on this one ipu6 laptop with software isp. And the default\nis 4, so someone has to set it deliberately if they want 1, 2, or 3.\nI think it is probably fine to allow it. Or maybe there are other\ndownsides that I cannot see.\n\n\nRegards,\nBarnabás Pőcze\n\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 133CABF415\n\tfor ;\n\tFri, 10 Oct 2025 13:09:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5CE946B60E;\n\tFri, 10 Oct 2025 15:09:28 +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 A09B26B599\n\tfor ;\n\tFri, 10 Oct 2025 15:09:27 +0200 (CEST)","from [192.168.33.28] (185.182.214.121.nat.pool.zt.hu\n\t[185.182.214.121])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 389D7664;\n\tFri, 10 Oct 2025 15:07:51 +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=\"lpe8VC7/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760101671;\n\tbh=EP1iyN41zG+dh5UogUSeiEhcNWLc3UNwjfbQ3CK3M1w=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=lpe8VC7/7RKsrDemP7PwEVVqxhRMxeDQ7Bxsy/TVKvEI4AKnWM/Ib/WAPNgmu++Yq\n\tFfMbE9md16hNx/umejQWbFDyaZjfU0xnBTo99QmoqeKEW3VmUTU4OVD53tQMnG9GG9\n\t8Anko2m01VEniOqVvEgOxnJPmcnGu+dI0bb6rtoA=","Message-ID":"<0f00e9b8-f549-4e51-a491-13eb570fa0c7@ideasonboard.com>","Date":"Fri, 10 Oct 2025 15:09:23 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 1/3] pipeline: simple: Allow buffer counts from 1 to\n\t32","To":"Kieran Bingham ,\n\tRobert Mader ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ","References":"<20251010092226.41228-1-robert.mader@collabora.com>\n\t<176010029479.756374.2472312101017554668@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<176010029479.756374.2472312101017554668@ping.linuxembedded.co.uk>","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":36192,"web_url":"https://patchwork.libcamera.org/comment/36192/","msgid":"<9a8dbbe1-6127-4538-a5db-f975e6155899@collabora.com>","date":"2025-10-10T15:16:21","subject":"Re: [PATCH v4 1/3] pipeline: simple: Allow buffer counts from 1 to\n\t32","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"On 10/10/25 15:09, Barnabás Pőcze wrote:\n>>\n>>> +                       cfg.bufferCount = kNumBuffersDefault;\n>>> +               else if (bufferCount > kNumBuffersMax)\n>>> +                       cfg.bufferCount = kNumBuffersMax;\n>>\n>>\n>>\n>> Do we really expect to support 1,2,3 ?\n>\n> Seems to work on this one ipu6 laptop with software isp. And the default\n> is 4, so someone has to set it deliberately if they want 1, 2, or 3.\n> I think it is probably fine to allow it. Or maybe there are other\n> downsides that I cannot see.\n\nFor the record, the main intention to reduce overhead for clients \nwanting to capture just a single frame - let's say in multi-stream or \nlow-power environments. My hope is that such applications are rare \nenough that, in case something breaks, we'll be likely to get bug \nreports from whoever tries it.","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 A0EB2BE080\n\tfor ;\n\tFri, 10 Oct 2025 15:16:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 184126B5C4;\n\tFri, 10 Oct 2025 17:16:32 +0200 (CEST)","from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com\n\t[136.143.188.112])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 631516B599\n\tfor ;\n\tFri, 10 Oct 2025 17:16:29 +0200 (CEST)","by mx.zohomail.com with SMTPS id 176010938450672.89263729297454;\n\tFri, 10 Oct 2025 08:16:24 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"HrZ/ZmMj\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1760109385; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=BiFiaAb/fbJtNMv5H2cnEZtk/nm+0L6NvpJ8XO/BkbPtfe2aJHtcllfHuvMykMlGNAtz7FGUNmY5lV8DquJlGbEdYTp0rV3og0mKhwcpI7r6Qcze960W5J/TcU0pSxVzd7WRbuXsXtrwfNAyKsvaHd6UwAA6gD8/cNMWAA2pCt4=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1760109385;\n\th=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To;\n\tbh=PVnGwSjdaxZPEKSv/0SKjreXxk8o4m2pPa98a/q6ZFs=; \n\tb=f5xkhyqpekGASBx9QkopSPb+hpdamZ2fxBPFQ2PfNhjCht837/OffH+QeL4ZiiRbV2okRiEIT+ObbPOqHcmmoLMb4vi72Vo4aaSlEisQqbslGVegPmRkEfRI2hHatLoM+SrcGUG8aW++5Mlu0lENG00kixZzoEcnvFwy9sq96Zg=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass header.i=collabora.com;\n\tspf=pass smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1760109385;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:Cc:Cc:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To;\n\tbh=PVnGwSjdaxZPEKSv/0SKjreXxk8o4m2pPa98a/q6ZFs=;\n\tb=HrZ/ZmMjRs0yqS1S/ITdLexfMfql6zi9eQY6sg8+Qo9IbywGsAcPpQ085qGffRMR\n\t/kZMyL/IKZWyQqYPOue6YHloARgVJRQ8BFDVH2+8/lthlYWp4o08xMmyY56BPYWPKf5\n\t9pVlzg2A2sAk0gpJ/8mXwVSpoE4A2cf3vtr79B2Y=","Message-ID":"<9a8dbbe1-6127-4538-a5db-f975e6155899@collabora.com>","Date":"Fri, 10 Oct 2025 17:16:21 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 1/3] pipeline: simple: Allow buffer counts from 1 to\n\t32","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tKieran Bingham , \n\tlibcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ","References":"<20251010092226.41228-1-robert.mader@collabora.com>\n\t<176010029479.756374.2472312101017554668@ping.linuxembedded.co.uk>\n\t<0f00e9b8-f549-4e51-a491-13eb570fa0c7@ideasonboard.com>","Content-Language":"en-US, de-DE","From":"Robert Mader ","In-Reply-To":"<0f00e9b8-f549-4e51-a491-13eb570fa0c7@ideasonboard.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\" "}}]