From patchwork Tue Jan 14 18:22:52 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22574 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id DD056C3304 for ; Tue, 14 Jan 2025 18:22:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 94F946855A; Tue, 14 Jan 2025 19:22:57 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="RZPN5acL"; dkim-atps=neutral Received: from mail-10631.protonmail.ch (mail-10631.protonmail.ch [79.135.106.31]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E99368545 for ; Tue, 14 Jan 2025 19:22:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1736878975; x=1737138175; bh=E+py5E6exHA7vmIVT5/mIvHYdzZID/idMKiZtx4c04c=; h=Date:To:From:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=RZPN5acLeBv1ObHP2AwTudZE17Isx+KEXE2cXopsbKX3NqAb6bOc7GHNSNJBZWFzR 9pf3xJ+LZXOy2iECt5axmMi3Wt+s/fCUS3yMvw50ObwVrF45l4x0H3qzpLGHU81ReO bKezO9K0CZFnWBUF8kgJxBw4m7Xc+dS2/5C0TvGS2MDozUTXO6sgUNJLlcM5sixnd6 a7S/WdUA0ozIZI3gkRTDGEqn8N6z003EqaXWnYWdazh+SgeA37bafJqqf95v+iL/BI qJ+shldLf6etSBBMvGTjLCXgt3TN3+5oW9hgYU2HA3ObOM9F+qd95pY4WgHSl0GdMU bmtra5hq9eJFg== Date: Tue, 14 Jan 2025 18:22:52 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH v2 13/16] apps: lc-compliance: Merge `CaptureBalanced` and `CaptureUnbalanced` Message-ID: <20250114182143.1773762-14-pobrn@protonmail.com> In-Reply-To: <20250114182143.1773762-1-pobrn@protonmail.com> References: <20250114182143.1773762-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: fa7a49b0d387bee46118ff57d8b9e4fbfba14841 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The above two classes implement very similar, in fact, the only essential difference is how many requests are queued. `CaptureBalanced` queues a predetermined number of requests, while `CaptureUnbalanced` queues requests without limit. This can be addressed by introducing a "capture" and a "queue" limit into the `Capture` class, which determine at most how many requests can be queued, and how many request completions are expected before stopping. Signed-off-by: Barnabás Pőcze Reviewed-by: Jacopo Mondi Reviewed-by: Paul Elder --- src/apps/lc-compliance/helpers/capture.cpp | 141 +++++++----------- src/apps/lc-compliance/helpers/capture.h | 50 ++----- src/apps/lc-compliance/tests/capture_test.cpp | 12 +- 3 files changed, 71 insertions(+), 132 deletions(-) diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp index 43db15d2d..77e87c9e4 100644 --- a/src/apps/lc-compliance/helpers/capture.cpp +++ b/src/apps/lc-compliance/helpers/capture.cpp @@ -7,12 +7,14 @@ #include "capture.h" +#include + #include using namespace libcamera; Capture::Capture(std::shared_ptr camera) - : loop_(nullptr), camera_(std::move(camera)), + : camera_(std::move(camera)), allocator_(camera_) { } @@ -40,153 +42,110 @@ void Capture::configure(StreamRole role) } } -void Capture::start() +void Capture::run(unsigned int captureLimit, std::optional queueLimit) { - Stream *stream = config_->at(0).stream(); - int count = allocator_.allocate(stream); + assert(!queueLimit || captureLimit <= *queueLimit); - ASSERT_GE(count, 0) << "Failed to allocate buffers"; - EXPECT_EQ(count, config_->at(0).bufferCount) << "Allocated less buffers than expected"; + captureLimit_ = captureLimit; + queueLimit_ = queueLimit; - camera_->requestCompleted.connect(this, &Capture::requestComplete); + captureCount_ = queueCount_ = 0; - ASSERT_EQ(camera_->start(), 0) << "Failed to start camera"; -} + EventLoop loop; + loop_ = &loop; -void Capture::stop() -{ - if (!config_ || !allocator_.allocated()) - return; + start(); + prepareRequests(queueLimit_); - camera_->stop(); + for (const auto &request : requests_) + queueRequest(request.get()); - camera_->requestCompleted.disconnect(this); + EXPECT_EQ(loop_->exec(), 0); - Stream *stream = config_->at(0).stream(); - requests_.clear(); - allocator_.free(stream); -} + stop(); -/* CaptureBalanced */ + loop_ = nullptr; -CaptureBalanced::CaptureBalanced(std::shared_ptr camera) - : Capture(std::move(camera)) -{ + EXPECT_LE(captureLimit_, captureCount_); + EXPECT_LE(captureCount_, queueCount_); + EXPECT_TRUE(!queueLimit_ || queueCount_ <= *queueLimit_); } -void CaptureBalanced::capture(unsigned int numRequests) +void Capture::prepareRequests(std::optional queueLimit) { - start(); + assert(config_); + assert(requests_.empty()); Stream *stream = config_->at(0).stream(); const std::vector> &buffers = allocator_.buffers(stream); /* No point in testing less requests then the camera depth. */ - if (buffers.size() > numRequests) { + if (queueLimit && *queueLimit < buffers.size()) { GTEST_SKIP() << "Camera needs " << buffers.size() - << " requests, can't test only " << numRequests; + << " requests, can't test only " << *queueLimit; } - queueCount_ = 0; - captureCount_ = 0; - captureLimit_ = numRequests; - - /* Queue the recommended number of requests. */ for (const std::unique_ptr &buffer : buffers) { std::unique_ptr request = camera_->createRequest(); ASSERT_TRUE(request) << "Can't create request"; ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request"; - ASSERT_EQ(queueRequest(request.get()), 0) << "Failed to queue request"; - requests_.push_back(std::move(request)); } - - /* Run capture session. */ - loop_ = new EventLoop(); - loop_->exec(); - stop(); - delete loop_; - - ASSERT_EQ(captureCount_, captureLimit_); } -int CaptureBalanced::queueRequest(Request *request) +int Capture::queueRequest(libcamera::Request *request) { - queueCount_++; - if (queueCount_ > captureLimit_) + if (queueLimit_ && queueCount_ >= *queueLimit_) return 0; - return camera_->queueRequest(request); + if (int ret = camera_->queueRequest(request); ret < 0) + return ret; + + queueCount_ += 1; + return 0; } -void CaptureBalanced::requestComplete(Request *request) +void Capture::requestComplete(Request *request) { - EXPECT_EQ(request->status(), Request::Status::RequestComplete) - << "Request didn't complete successfully"; - captureCount_++; if (captureCount_ >= captureLimit_) { loop_->exit(0); return; } + EXPECT_EQ(request->status(), Request::Status::RequestComplete) + << "Request didn't complete successfully"; + request->reuse(Request::ReuseBuffers); if (queueRequest(request)) loop_->exit(-EINVAL); } -/* CaptureUnbalanced */ - -CaptureUnbalanced::CaptureUnbalanced(std::shared_ptr camera) - : Capture(std::move(camera)) -{ -} - -void CaptureUnbalanced::capture(unsigned int numRequests) +void Capture::start() { - start(); - Stream *stream = config_->at(0).stream(); - const std::vector> &buffers = allocator_.buffers(stream); - - captureCount_ = 0; - captureLimit_ = numRequests; - - /* Queue the recommended number of requests. */ - for (const std::unique_ptr &buffer : buffers) { - std::unique_ptr request = camera_->createRequest(); - ASSERT_TRUE(request) << "Can't create request"; - - ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request"; - - ASSERT_EQ(camera_->queueRequest(request.get()), 0) << "Failed to queue request"; + int count = allocator_.allocate(stream); - requests_.push_back(std::move(request)); - } + ASSERT_GE(count, 0) << "Failed to allocate buffers"; + EXPECT_EQ(count, config_->at(0).bufferCount) << "Allocated less buffers than expected"; - /* Run capture session. */ - loop_ = new EventLoop(); - int status = loop_->exec(); - stop(); - delete loop_; + camera_->requestCompleted.connect(this, &Capture::requestComplete); - ASSERT_EQ(status, 0); + ASSERT_EQ(camera_->start(), 0) << "Failed to start camera"; } -void CaptureUnbalanced::requestComplete(Request *request) +void Capture::stop() { - captureCount_++; - if (captureCount_ >= captureLimit_) { - loop_->exit(0); + if (!config_ || !allocator_.allocated()) return; - } - EXPECT_EQ(request->status(), Request::Status::RequestComplete) - << "Request didn't complete successfully"; + camera_->stop(); - request->reuse(Request::ReuseBuffers); - if (camera_->queueRequest(request)) - loop_->exit(-EINVAL); + camera_->requestCompleted.disconnect(this); + + Stream *stream = config_->at(0).stream(); + requests_.clear(); + allocator_.free(stream); } diff --git a/src/apps/lc-compliance/helpers/capture.h b/src/apps/lc-compliance/helpers/capture.h index a4cc3a99e..173421fd2 100644 --- a/src/apps/lc-compliance/helpers/capture.h +++ b/src/apps/lc-compliance/helpers/capture.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include @@ -16,51 +17,30 @@ class Capture { public: + Capture(std::shared_ptr camera); + ~Capture(); + void configure(libcamera::StreamRole role); + void run(unsigned int captureLimit, std::optional queueLimit = {}); -protected: - Capture(std::shared_ptr camera); - virtual ~Capture(); +private: + LIBCAMERA_DISABLE_COPY_AND_MOVE(Capture) void start(); void stop(); - virtual void requestComplete(libcamera::Request *request) = 0; - - EventLoop *loop_; + void prepareRequests(std::optional queueLimit = {}); + int queueRequest(libcamera::Request *request); + void requestComplete(libcamera::Request *request); std::shared_ptr camera_; libcamera::FrameBufferAllocator allocator_; std::unique_ptr config_; std::vector> requests_; -}; - -class CaptureBalanced : public Capture -{ -public: - CaptureBalanced(std::shared_ptr camera); - - void capture(unsigned int numRequests); - -private: - int queueRequest(libcamera::Request *request); - void requestComplete(libcamera::Request *request) override; - - unsigned int queueCount_; - unsigned int captureCount_; - unsigned int captureLimit_; -}; - -class CaptureUnbalanced : public Capture -{ -public: - CaptureUnbalanced(std::shared_ptr camera); - - void capture(unsigned int numRequests); - -private: - void requestComplete(libcamera::Request *request) override; - unsigned int captureCount_; - unsigned int captureLimit_; + EventLoop *loop_ = nullptr; + unsigned int captureLimit_ = 0; + std::optional queueLimit_; + unsigned int captureCount_ = 0; + unsigned int queueCount_ = 0; }; diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp index 97465a612..93bed48f0 100644 --- a/src/apps/lc-compliance/tests/capture_test.cpp +++ b/src/apps/lc-compliance/tests/capture_test.cpp @@ -87,11 +87,11 @@ TEST_P(SingleStream, Capture) { auto [role, numRequests] = GetParam(); - CaptureBalanced capture(camera_); + Capture capture(camera_); capture.configure(role); - capture.capture(numRequests); + capture.run(numRequests, numRequests); } /* @@ -106,12 +106,12 @@ TEST_P(SingleStream, CaptureStartStop) auto [role, numRequests] = GetParam(); unsigned int numRepeats = 3; - CaptureBalanced capture(camera_); + Capture capture(camera_); capture.configure(role); for (unsigned int starts = 0; starts < numRepeats; starts++) - capture.capture(numRequests); + capture.run(numRequests, numRequests); } /* @@ -125,11 +125,11 @@ TEST_P(SingleStream, UnbalancedStop) { auto [role, numRequests] = GetParam(); - CaptureUnbalanced capture(camera_); + Capture capture(camera_); capture.configure(role); - capture.capture(numRequests); + capture.run(numRequests); } INSTANTIATE_TEST_SUITE_P(CaptureTests,