Patch Detail
Show a patch.
GET /api/1.1/patches/22677/?format=api
{ "id": 22677, "url": "https://patchwork.libcamera.org/api/1.1/patches/22677/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22677/", "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": "<20250130115001.1129305-17-pobrn@protonmail.com>", "date": "2025-01-30T11:51:22", "name": "[RFC,v3,16/21] apps: lc-compliance: Merge `CaptureBalanced` and `CaptureUnbalanced`", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "d03be774963847d3964d051d34372a4ce1bc7007", "submitter": { "id": 133, "url": "https://patchwork.libcamera.org/api/1.1/people/133/?format=api", "name": "Pőcze Barnabás", "email": "pobrn@protonmail.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22677/mbox/", "series": [ { "id": 4978, "url": "https://patchwork.libcamera.org/api/1.1/series/4978/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4978", "date": "2025-01-30T11:50:08", "name": "apps: lc-compliance: Multi-stream tests", "version": 3, "mbox": "https://patchwork.libcamera.org/series/4978/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22677/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22677/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 61A66BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 30 Jan 2025 11:51:47 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 16C8168584;\n\tThu, 30 Jan 2025 12:51:47 +0100 (CET)", "from mail-10628.protonmail.ch (mail-10628.protonmail.ch\n\t[79.135.106.28])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 07B126856B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 30 Jan 2025 12:51:44 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"Rm+E1JGB\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1738237903; x=1738497103;\n\tbh=CfpOwcxNQ4PgpJJcgE7e19c1DM78S0upTgtixZ04Wvo=;\n\th=Date:To:From:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=Rm+E1JGBwaDFdVcHQnOXeP4GdowyPfgO0QKl7YIQAjLC+uLZl2UiJuOw+uWqB55Yw\n\tKGpeDhoSSqbbTpIAo29onR//jj5F+4RLdWbfBmA18wbZYs76cdzb2sjW56LuF5Wznr\n\tIBw1vKuuFCDWjXE2wXHrpKm9nUuB+wXKgPDBzemYB7XSrh7yfi0TVa4vyAq98v85e8\n\txlbPROaHAW/N+GjdnelsviZjxOKJXV+VfoPjXKUISeISpE/WAxmpBanvvR0QmJVxvY\n\tYrf61wPQ4rWH8/KZ8LI//GvAKfR6zc33BpkyzuPzvKOK0b3iujBBntW7NH0Zr+tcIb\n\tqmuPKHjx6qQCw==", "Date": "Thu, 30 Jan 2025 11:51:22 +0000", "To": "libcamera-devel@lists.libcamera.org", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>", "Subject": "[RFC PATCH v3 16/21] apps: lc-compliance: Merge `CaptureBalanced`\n\tand `CaptureUnbalanced`", "Message-ID": "<20250130115001.1129305-17-pobrn@protonmail.com>", "In-Reply-To": "<20250130115001.1129305-1-pobrn@protonmail.com>", "References": "<20250130115001.1129305-1-pobrn@protonmail.com>", "Feedback-ID": "20568564:user:proton", "X-Pm-Message-ID": "2b914d415b2f656034ef799a88158ce03b3303b0", "MIME-Version": "1.0", "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>" }, "content": "The above two classes have very similar implementations, in fact, the\nonly essential difference is how many requests are queued. `CaptureBalanced`\nqueues a predetermined number of requests, while `CaptureUnbalanced`\nqueues requests without limit.\n\nThis can be addressed by introducing a \"capture\" and a \"queue\" limit\ninto the `Capture` class, which determine at most how many requests\ncan be queued, and how many request completions are expected before\nstopping.\n\nSigned-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n---\n src/apps/lc-compliance/helpers/capture.cpp | 142 ++++++------------\n src/apps/lc-compliance/helpers/capture.h | 50 ++----\n src/apps/lc-compliance/tests/capture_test.cpp | 12 +-\n 3 files changed, 71 insertions(+), 133 deletions(-)", "diff": "diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\nindex 43db15d2d..940646f6c 100644\n--- a/src/apps/lc-compliance/helpers/capture.cpp\n+++ b/src/apps/lc-compliance/helpers/capture.cpp\n@@ -7,12 +7,14 @@\n \n #include \"capture.h\"\n \n+#include <assert.h>\n+\n #include <gtest/gtest.h>\n \n using namespace libcamera;\n \n Capture::Capture(std::shared_ptr<Camera> camera)\n-\t: loop_(nullptr), camera_(std::move(camera)),\n+\t: camera_(std::move(camera)),\n \t allocator_(camera_)\n {\n }\n@@ -40,153 +42,109 @@ void Capture::configure(StreamRole role)\n \t}\n }\n \n-void Capture::start()\n+void Capture::run(unsigned int captureLimit, std::optional<unsigned int> queueLimit)\n {\n-\tStream *stream = config_->at(0).stream();\n-\tint count = allocator_.allocate(stream);\n+\tassert(!queueLimit || captureLimit <= *queueLimit);\n \n-\tASSERT_GE(count, 0) << \"Failed to allocate buffers\";\n-\tEXPECT_EQ(count, config_->at(0).bufferCount) << \"Allocated less buffers than expected\";\n+\tcaptureLimit_ = captureLimit;\n+\tqueueLimit_ = queueLimit;\n \n-\tcamera_->requestCompleted.connect(this, &Capture::requestComplete);\n+\tcaptureCount_ = queueCount_ = 0;\n \n-\tASSERT_EQ(camera_->start(), 0) << \"Failed to start camera\";\n-}\n+\tEventLoop loop;\n+\tloop_ = &loop;\n \n-void Capture::stop()\n-{\n-\tif (!config_ || !allocator_.allocated())\n-\t\treturn;\n-\n-\tcamera_->stop();\n+\tstart();\n+\tprepareRequests();\n \n-\tcamera_->requestCompleted.disconnect(this);\n+\tfor (const auto &request : requests_)\n+\t\tqueueRequest(request.get());\n \n-\tStream *stream = config_->at(0).stream();\n-\trequests_.clear();\n-\tallocator_.free(stream);\n-}\n+\tEXPECT_EQ(loop_->exec(), 0);\n \n-/* CaptureBalanced */\n+\tstop();\n \n-CaptureBalanced::CaptureBalanced(std::shared_ptr<Camera> camera)\n-\t: Capture(std::move(camera))\n-{\n+\tEXPECT_LE(captureLimit_, captureCount_);\n+\tEXPECT_LE(captureCount_, queueCount_);\n+\tEXPECT_TRUE(!queueLimit_ || queueCount_ <= *queueLimit_);\n }\n \n-void CaptureBalanced::capture(unsigned int numRequests)\n+void Capture::prepareRequests()\n {\n-\tstart();\n+\tassert(config_);\n+\tassert(requests_.empty());\n \n \tStream *stream = config_->at(0).stream();\n \tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_.buffers(stream);\n \n \t/* No point in testing less requests then the camera depth. */\n-\tif (buffers.size() > numRequests) {\n+\tif (queueLimit_ && *queueLimit_ < buffers.size()) {\n \t\tGTEST_SKIP() << \"Camera needs \" << buffers.size()\n-\t\t\t << \" requests, can't test only \" << numRequests;\n+\t\t\t << \" requests, can't test only \" << *queueLimit_;\n \t}\n \n-\tqueueCount_ = 0;\n-\tcaptureCount_ = 0;\n-\tcaptureLimit_ = numRequests;\n-\n-\t/* Queue the recommended number of requests. */\n \tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n \t\tstd::unique_ptr<Request> request = camera_->createRequest();\n \t\tASSERT_TRUE(request) << \"Can't create request\";\n \n \t\tASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << \"Can't set buffer for request\";\n \n-\t\tASSERT_EQ(queueRequest(request.get()), 0) << \"Failed to queue request\";\n-\n \t\trequests_.push_back(std::move(request));\n \t}\n-\n-\t/* Run capture session. */\n-\tloop_ = new EventLoop();\n-\tloop_->exec();\n-\tstop();\n-\tdelete loop_;\n-\n-\tASSERT_EQ(captureCount_, captureLimit_);\n }\n \n-int CaptureBalanced::queueRequest(Request *request)\n+int Capture::queueRequest(libcamera::Request *request)\n {\n-\tqueueCount_++;\n-\tif (queueCount_ > captureLimit_)\n+\tif (queueLimit_ && queueCount_ >= *queueLimit_)\n \t\treturn 0;\n \n-\treturn camera_->queueRequest(request);\n+\tint ret = camera_->queueRequest(request);\n+\tif (ret < 0)\n+\t\treturn ret;\n+\n+\tqueueCount_ += 1;\n+\treturn 0;\n }\n \n-void CaptureBalanced::requestComplete(Request *request)\n+void Capture::requestComplete(Request *request)\n {\n-\tEXPECT_EQ(request->status(), Request::Status::RequestComplete)\n-\t\t<< \"Request didn't complete successfully\";\n-\n \tcaptureCount_++;\n \tif (captureCount_ >= captureLimit_) {\n \t\tloop_->exit(0);\n \t\treturn;\n \t}\n \n+\tEXPECT_EQ(request->status(), Request::Status::RequestComplete)\n+\t\t<< \"Request didn't complete successfully\";\n+\n \trequest->reuse(Request::ReuseBuffers);\n \tif (queueRequest(request))\n \t\tloop_->exit(-EINVAL);\n }\n \n-/* CaptureUnbalanced */\n-\n-CaptureUnbalanced::CaptureUnbalanced(std::shared_ptr<Camera> camera)\n-\t: Capture(std::move(camera))\n-{\n-}\n-\n-void CaptureUnbalanced::capture(unsigned int numRequests)\n+void Capture::start()\n {\n-\tstart();\n-\n \tStream *stream = config_->at(0).stream();\n-\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_.buffers(stream);\n-\n-\tcaptureCount_ = 0;\n-\tcaptureLimit_ = numRequests;\n-\n-\t/* Queue the recommended number of requests. */\n-\tfor (const std::unique_ptr<FrameBuffer> &buffer : buffers) {\n-\t\tstd::unique_ptr<Request> request = camera_->createRequest();\n-\t\tASSERT_TRUE(request) << \"Can't create request\";\n-\n-\t\tASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << \"Can't set buffer for request\";\n-\n-\t\tASSERT_EQ(camera_->queueRequest(request.get()), 0) << \"Failed to queue request\";\n+\tint count = allocator_.allocate(stream);\n \n-\t\trequests_.push_back(std::move(request));\n-\t}\n+\tASSERT_GE(count, 0) << \"Failed to allocate buffers\";\n+\tEXPECT_EQ(count, config_->at(0).bufferCount) << \"Allocated less buffers than expected\";\n \n-\t/* Run capture session. */\n-\tloop_ = new EventLoop();\n-\tint status = loop_->exec();\n-\tstop();\n-\tdelete loop_;\n+\tcamera_->requestCompleted.connect(this, &Capture::requestComplete);\n \n-\tASSERT_EQ(status, 0);\n+\tASSERT_EQ(camera_->start(), 0) << \"Failed to start camera\";\n }\n \n-void CaptureUnbalanced::requestComplete(Request *request)\n+void Capture::stop()\n {\n-\tcaptureCount_++;\n-\tif (captureCount_ >= captureLimit_) {\n-\t\tloop_->exit(0);\n+\tif (!config_ || !allocator_.allocated())\n \t\treturn;\n-\t}\n \n-\tEXPECT_EQ(request->status(), Request::Status::RequestComplete)\n-\t\t<< \"Request didn't complete successfully\";\n+\tcamera_->stop();\n \n-\trequest->reuse(Request::ReuseBuffers);\n-\tif (camera_->queueRequest(request))\n-\t\tloop_->exit(-EINVAL);\n+\tcamera_->requestCompleted.disconnect(this);\n+\n+\tStream *stream = config_->at(0).stream();\n+\trequests_.clear();\n+\tallocator_.free(stream);\n }\ndiff --git a/src/apps/lc-compliance/helpers/capture.h b/src/apps/lc-compliance/helpers/capture.h\nindex a4cc3a99e..ede395e2a 100644\n--- a/src/apps/lc-compliance/helpers/capture.h\n+++ b/src/apps/lc-compliance/helpers/capture.h\n@@ -8,6 +8,7 @@\n #pragma once\n \n #include <memory>\n+#include <optional>\n \n #include <libcamera/libcamera.h>\n \n@@ -16,51 +17,30 @@\n class Capture\n {\n public:\n+\tCapture(std::shared_ptr<libcamera::Camera> camera);\n+\t~Capture();\n+\n \tvoid configure(libcamera::StreamRole role);\n+\tvoid run(unsigned int captureLimit, std::optional<unsigned int> queueLimit = {});\n \n-protected:\n-\tCapture(std::shared_ptr<libcamera::Camera> camera);\n-\tvirtual ~Capture();\n+private:\n+\tLIBCAMERA_DISABLE_COPY_AND_MOVE(Capture)\n \n \tvoid start();\n \tvoid stop();\n \n-\tvirtual void requestComplete(libcamera::Request *request) = 0;\n-\n-\tEventLoop *loop_;\n+\tvoid prepareRequests();\n+\tint queueRequest(libcamera::Request *request);\n+\tvoid requestComplete(libcamera::Request *request);\n \n \tstd::shared_ptr<libcamera::Camera> camera_;\n \tlibcamera::FrameBufferAllocator allocator_;\n \tstd::unique_ptr<libcamera::CameraConfiguration> config_;\n \tstd::vector<std::unique_ptr<libcamera::Request>> requests_;\n-};\n-\n-class CaptureBalanced : public Capture\n-{\n-public:\n-\tCaptureBalanced(std::shared_ptr<libcamera::Camera> camera);\n-\n-\tvoid capture(unsigned int numRequests);\n-\n-private:\n-\tint queueRequest(libcamera::Request *request);\n-\tvoid requestComplete(libcamera::Request *request) override;\n-\n-\tunsigned int queueCount_;\n-\tunsigned int captureCount_;\n-\tunsigned int captureLimit_;\n-};\n-\n-class CaptureUnbalanced : public Capture\n-{\n-public:\n-\tCaptureUnbalanced(std::shared_ptr<libcamera::Camera> camera);\n-\n-\tvoid capture(unsigned int numRequests);\n-\n-private:\n-\tvoid requestComplete(libcamera::Request *request) override;\n \n-\tunsigned int captureCount_;\n-\tunsigned int captureLimit_;\n+\tEventLoop *loop_ = nullptr;\n+\tunsigned int captureLimit_ = 0;\n+\tstd::optional<unsigned int> queueLimit_;\n+\tunsigned int captureCount_ = 0;\n+\tunsigned int queueCount_ = 0;\n };\ndiff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp\nindex 97465a612..93bed48f0 100644\n--- a/src/apps/lc-compliance/tests/capture_test.cpp\n+++ b/src/apps/lc-compliance/tests/capture_test.cpp\n@@ -87,11 +87,11 @@ TEST_P(SingleStream, Capture)\n {\n \tauto [role, numRequests] = GetParam();\n \n-\tCaptureBalanced capture(camera_);\n+\tCapture capture(camera_);\n \n \tcapture.configure(role);\n \n-\tcapture.capture(numRequests);\n+\tcapture.run(numRequests, numRequests);\n }\n \n /*\n@@ -106,12 +106,12 @@ TEST_P(SingleStream, CaptureStartStop)\n \tauto [role, numRequests] = GetParam();\n \tunsigned int numRepeats = 3;\n \n-\tCaptureBalanced capture(camera_);\n+\tCapture capture(camera_);\n \n \tcapture.configure(role);\n \n \tfor (unsigned int starts = 0; starts < numRepeats; starts++)\n-\t\tcapture.capture(numRequests);\n+\t\tcapture.run(numRequests, numRequests);\n }\n \n /*\n@@ -125,11 +125,11 @@ TEST_P(SingleStream, UnbalancedStop)\n {\n \tauto [role, numRequests] = GetParam();\n \n-\tCaptureUnbalanced capture(camera_);\n+\tCapture capture(camera_);\n \n \tcapture.configure(role);\n \n-\tcapture.capture(numRequests);\n+\tcapture.run(numRequests);\n }\n \n INSTANTIATE_TEST_SUITE_P(CaptureTests,\n", "prefixes": [ "RFC", "v3", "16/21" ] }