Patch Detail
Show a patch.
GET /api/1.1/patches/18020/?format=api
{ "id": 18020, "url": "https://patchwork.libcamera.org/api/1.1/patches/18020/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18020/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20221216122939.256534-7-paul.elder@ideasonboard.com>", "date": "2022-12-16T12:29:27", "name": "[libcamera-devel,v9,06/18] libcamera: pipeline: simple: Don't rely on bufferCount", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "babf3c1777b413692f83d42cb7608d65c269559e", "submitter": { "id": 17, "url": "https://patchwork.libcamera.org/api/1.1/people/17/?format=api", "name": "Paul Elder", "email": "paul.elder@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18020/mbox/", "series": [ { "id": 3675, "url": "https://patchwork.libcamera.org/api/1.1/series/3675/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3675", "date": "2022-12-16T12:29:21", "name": "lc-compliance: Add test to queue more requests than hardware depth", "version": 9, "mbox": "https://patchwork.libcamera.org/series/3675/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18020/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18020/checks/", "tags": {}, "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 0062CC31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 16 Dec 2022 12:30:14 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B4DBE633A2;\n\tFri, 16 Dec 2022 13:30:14 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 903FE633A8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 16 Dec 2022 13:30:12 +0100 (CET)", "from pyrite.tail37cf.ts.net (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 137D5110E;\n\tFri, 16 Dec 2022 13:30:10 +0100 (CET)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1671193814;\n\tbh=+AxFQm3WbHzlPk12pkAGGEBjxtMNKSxUJxxbSLm0+5M=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=sZYYBw/m1jnDBM9kHz72r2/t3Uoo7F0mLOF3VXQpclqzZ6FqXuOJdIIoUK0xlkGf5\n\tC5ZWm1frT8gfZ5qJJNSe5XpCVkwv/SXiNmQcJ3iyARNnKpqdAW/yJ9fN0iyACpbd33\n\tSM1C06msnVkeT40LxskJsdB6OI96UJj7KVZakdPV/Ep/dGfrP6UWl0YSIzwXeUn8gR\n\tAHR00g8yxUGmAvSvH8CKeLv37IkDfiBDgHycC/4oWzWl0l1CvNAWb6UYnSGPmf2Zf1\n\tQ8OXLx7slySicLqheGN7l/ybUbcTFhbTsrOn4u3O5DuOt4sYkD8kCg3fH44nN+DBXg\n\t+gpb5h+RXyxVA==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1671193812;\n\tbh=+AxFQm3WbHzlPk12pkAGGEBjxtMNKSxUJxxbSLm0+5M=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=i4WfC2AoLOlZNVsnLlptzYp4rQMrnF2XN/IoYboC+tsVn/6Ba0jNOePkoOIN3s/EB\n\ttXixz3ez/ZFEjj4bcPTQMVSlKFH7tQUHkgCwtvPbPkcOWQIud8EM9WMr9hP/2kJZh9\n\twWarMcPIs2490KTSHxn0ZC+DnYsfiDxYkFEEXznI=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"i4WfC2Ao\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 16 Dec 2022 21:29:27 +0900", "Message-Id": "<20221216122939.256534-7-paul.elder@ideasonboard.com>", "X-Mailer": "git-send-email 2.35.1", "In-Reply-To": "<20221216122939.256534-1-paul.elder@ideasonboard.com>", "References": "<20221216122939.256534-1-paul.elder@ideasonboard.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v9 06/18] libcamera: pipeline: simple:\n\tDon't rely on bufferCount", "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>", "From": "Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>", "Reply-To": "Paul Elder <paul.elder@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "From: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n\nCurrently the simple pipeline handler relies on bufferCount to decide on\nthe number of buffers to allocate internally when a converter is in use\nand for the number of V4L2 buffer slots to reserve. Instead, the number\nof internal buffers should be the minimum required by the pipeline to\nkeep the requests flowing, in order to avoid wasting memory, while the\nnumber of V4L2 buffer slots should be greater than the expected number\nof requests queued by the application, in order to avoid thrashing\ndmabuf mappings, which would degrade performance.\n\nStop relying on bufferCount for these numbers and instead set them to\nappropriate, and independent, constants.\n\nSigned-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\n\n---\nChanges in v9:\n- rebased\n\nChanges in v8:\n- New\n---\n include/libcamera/internal/converter.h | 3 ++-\n .../internal/converter/converter_v4l2_m2m.h | 6 +++--\n .../converter/converter_v4l2_m2m.cpp | 12 +++++-----\n src/libcamera/pipeline/simple/simple.cpp | 22 ++++++++++++++-----\n 4 files changed, 30 insertions(+), 13 deletions(-)", "diff": "diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\nindex 834ec5ab..32490fe0 100644\n--- a/include/libcamera/internal/converter.h\n+++ b/include/libcamera/internal/converter.h\n@@ -49,7 +49,8 @@ public:\n \tvirtual int exportBuffers(unsigned int output, unsigned int count,\n \t\t\t\t std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;\n \n-\tvirtual int start() = 0;\n+\tvirtual int start(unsigned int internalBufferCount,\n+\t\t\t unsigned int bufferSlotCount) = 0;\n \tvirtual void stop() = 0;\n \n \tvirtual int queueBuffers(FrameBuffer *input,\ndiff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h\nindex 815916d0..1f471071 100644\n--- a/include/libcamera/internal/converter/converter_v4l2_m2m.h\n+++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h\n@@ -50,7 +50,8 @@ public:\n \tint exportBuffers(unsigned int ouput, unsigned int count,\n \t\t\t std::vector<std::unique_ptr<FrameBuffer>> *buffers);\n \n-\tint start();\n+\tint start(unsigned int internalBufferCount,\n+\t\t unsigned int bufferSlotCount);\n \tvoid stop();\n \n \tint queueBuffers(FrameBuffer *input,\n@@ -69,7 +70,8 @@ private:\n \t\tint exportBuffers(unsigned int count,\n \t\t\t\t std::vector<std::unique_ptr<FrameBuffer>> *buffers);\n \n-\t\tint start();\n+\t\tint start(unsigned int internalBufferCount,\n+\t\t\t unsigned int bufferSlotCount);\n \t\tvoid stop();\n \n \t\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\ndiff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp\nindex 2a4d1d99..9d25f25a 100644\n--- a/src/libcamera/converter/converter_v4l2_m2m.cpp\n+++ b/src/libcamera/converter/converter_v4l2_m2m.cpp\n@@ -107,13 +107,14 @@ int V4L2M2MConverter::Stream::exportBuffers(unsigned int count,\n \treturn m2m_->capture()->exportBuffers(count, buffers);\n }\n \n-int V4L2M2MConverter::Stream::start()\n+int V4L2M2MConverter::Stream::start(unsigned int internalBufferCount,\n+\t\t\t\t unsigned int bufferSlotCount)\n {\n-\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n+\tint ret = m2m_->output()->importBuffers(internalBufferCount);\n \tif (ret < 0)\n \t\treturn ret;\n \n-\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n+\tret = m2m_->capture()->importBuffers(bufferSlotCount);\n \tif (ret < 0) {\n \t\tstop();\n \t\treturn ret;\n@@ -373,12 +374,13 @@ int V4L2M2MConverter::exportBuffers(unsigned int output, unsigned int count,\n /**\n * \\copydoc libcamera::Converter::start\n */\n-int V4L2M2MConverter::start()\n+int V4L2M2MConverter::start(unsigned int internalBufferCount,\n+\t\t\t unsigned int bufferSlotCount)\n {\n \tint ret;\n \n \tfor (Stream &stream : streams_) {\n-\t\tret = stream.start();\n+\t\tret = stream.start(internalBufferCount, bufferSlotCount);\n \t\tif (ret < 0) {\n \t\t\tstop();\n \t\t\treturn ret;\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 6b7c6d5c..196e5252 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -339,6 +339,7 @@ protected:\n \n private:\n \tstatic constexpr unsigned int kNumInternalBuffers = 3;\n+\tstatic constexpr unsigned int kSimpleBufferSlotCount = 16;\n \n \tstruct EntityData {\n \t\tstd::unique_ptr<V4L2VideoDevice> video;\n@@ -1232,17 +1233,27 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n \t\treturn -EBUSY;\n \t}\n \n+\t/*\n+\t * Number of internal buffers that will be used to move video capture\n+\t * device frames to the converter.\n+\t *\n+\t * \\todo Make this depend on the driver in use instead of being\n+\t * hardcoded. In order to not drop frames, the realtime requirements for\n+\t * each device should be checked, so it may not be as simple as just\n+\t * using the minimum number of buffers required by the driver.\n+\t */\n+\tstatic constexpr unsigned int internalBufferCount = 3;\n+\n \tif (data->useConverter_) {\n \t\t/*\n \t\t * When using the converter allocate a fixed number of internal\n \t\t * buffers.\n \t\t */\n-\t\tret = video->allocateBuffers(kNumInternalBuffers,\n+\t\tret = video->allocateBuffers(internalBufferCount,\n \t\t\t\t\t &data->converterBuffers_);\n \t} else {\n-\t\t/* Otherwise, prepare for using buffers from the only stream. */\n-\t\tStream *stream = &data->streams_[0];\n-\t\tret = video->importBuffers(stream->configuration().bufferCount);\n+\t\t/* Otherwise, prepare for using buffers. */\n+\t\tret = video->importBuffers(kSimpleBufferSlotCount);\n \t}\n \tif (ret < 0) {\n \t\treleasePipeline(data);\n@@ -1258,7 +1269,8 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n \t}\n \n \tif (data->useConverter_) {\n-\t\tret = data->converter_->start();\n+\t\tret = data->converter_->start(internalBufferCount,\n+\t\t\t\t\t kSimpleBufferSlotCount);\n \t\tif (ret < 0) {\n \t\t\tstop(camera);\n \t\t\treturn ret;\n", "prefixes": [ "libcamera-devel", "v9", "06/18" ] }