{"id":22677,"url":"https://patchwork.libcamera.org/api/patches/22677/?format=json","web_url":"https://patchwork.libcamera.org/patch/22677/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","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/people/133/?format=json","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/series/4978/?format=json","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":"","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 ;\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 ;\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?= ","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":"","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\" "},"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 \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 \n+\n #include \n \n using namespace libcamera;\n \n Capture::Capture(std::shared_ptr 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 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)\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> &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 &buffer : buffers) {\n \t\tstd::unique_ptr 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)\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> &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 &buffer : buffers) {\n-\t\tstd::unique_ptr 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 \n+#include \n \n #include \n \n@@ -16,51 +17,30 @@\n class Capture\n {\n public:\n+\tCapture(std::shared_ptr camera);\n+\t~Capture();\n+\n \tvoid configure(libcamera::StreamRole role);\n+\tvoid run(unsigned int captureLimit, std::optional queueLimit = {});\n \n-protected:\n-\tCapture(std::shared_ptr 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 camera_;\n \tlibcamera::FrameBufferAllocator allocator_;\n \tstd::unique_ptr config_;\n \tstd::vector> requests_;\n-};\n-\n-class CaptureBalanced : public Capture\n-{\n-public:\n-\tCaptureBalanced(std::shared_ptr 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 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 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"]}