From 74c0e8cbf1d53a757f60367b13236a807fe65322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 19 Dec 2024 16:58:42 +0100 Subject: [PATCH] apps: lc-compliance: Merge `CaptureBalanced` and `CaptureUnbalanced` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The above two classes have very similar implementations, 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 --- src/apps/lc-compliance/helpers/capture.cpp | 206 +++++++----------- src/apps/lc-compliance/helpers/capture.h | 53 ++--- src/apps/lc-compliance/tests/capture_test.cpp | 12 +- 3 files changed, 102 insertions(+), 169 deletions(-) diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp index 43db15d2d..f2c6d58ce 100644 --- a/src/apps/lc-compliance/helpers/capture.cpp +++ b/src/apps/lc-compliance/helpers/capture.cpp @@ -7,13 +7,14 @@ #include "capture.h" +#include + #include using namespace libcamera; Capture::Capture(std::shared_ptr camera) - : loop_(nullptr), camera_(std::move(camera)), - allocator_(camera_) + : camera_(std::move(camera)), allocator_(camera_) { } @@ -40,14 +41,91 @@ void Capture::configure(StreamRole role) } } +void Capture::run(unsigned int captureLimit, std::optional queueLimit) +{ + assert(!queueLimit || captureLimit <= *queueLimit); + + captureLimit_ = captureLimit; + queueLimit_ = queueLimit; + + captureCount_ = queueCount_ = 0; + + EventLoop loop; + loop_ = &loop; + + start(); + + for (const auto &request : requests_) + queueRequest(request.get()); + + EXPECT_EQ(loop_->exec(), 0); + + stop(); + + EXPECT_LE(captureLimit_, captureCount_); + EXPECT_LE(captureCount_, queueCount_); + EXPECT_TRUE(!queueLimit_ || queueCount_ <= *queueLimit_); +} + +int Capture::queueRequest(libcamera::Request *request) +{ + if (queueLimit_ && queueCount_ >= *queueLimit_) + return 0; + + int ret = camera_->queueRequest(request); + if (ret < 0) + return ret; + + queueCount_ += 1; + return 0; +} + +void Capture::requestComplete(Request *request) +{ + 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); +} + void Capture::start() { + assert(config_); + assert(!config_->empty()); + assert(!allocator_.allocated()); + assert(requests_.empty()); + Stream *stream = config_->at(0).stream(); int count = allocator_.allocate(stream); ASSERT_GE(count, 0) << "Failed to allocate buffers"; EXPECT_EQ(count, config_->at(0).bufferCount) << "Allocated less buffers than expected"; + const std::vector> &buffers = allocator_.buffers(stream); + + /* No point in testing less requests then the camera depth. */ + if (queueLimit_ && *queueLimit_ < buffers.size()) { + GTEST_SKIP() << "Camera needs " << buffers.size() + << " requests, can't test only " << *queueLimit_; + } + + 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"; + + requests_.push_back(std::move(request)); + } + camera_->requestCompleted.connect(this, &Capture::requestComplete); ASSERT_EQ(camera_->start(), 0) << "Failed to start camera"; @@ -66,127 +144,3 @@ void Capture::stop() requests_.clear(); allocator_.free(stream); } - -/* CaptureBalanced */ - -CaptureBalanced::CaptureBalanced(std::shared_ptr camera) - : Capture(std::move(camera)) -{ -} - -void CaptureBalanced::capture(unsigned int numRequests) -{ - start(); - - 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) { - GTEST_SKIP() << "Camera needs " << buffers.size() - << " requests, can't test only " << numRequests; - } - - 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) -{ - queueCount_++; - if (queueCount_ > captureLimit_) - return 0; - - return camera_->queueRequest(request); -} - -void CaptureBalanced::requestComplete(Request *request) -{ - EXPECT_EQ(request->status(), Request::Status::RequestComplete) - << "Request didn't complete successfully"; - - captureCount_++; - if (captureCount_ >= captureLimit_) { - loop_->exit(0); - return; - } - - 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) -{ - 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"; - - requests_.push_back(std::move(request)); - } - - /* Run capture session. */ - loop_ = new EventLoop(); - int status = loop_->exec(); - stop(); - delete loop_; - - ASSERT_EQ(status, 0); -} - -void CaptureUnbalanced::requestComplete(Request *request) -{ - 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 (camera_->queueRequest(request)) - loop_->exit(-EINVAL); -} diff --git a/src/apps/lc-compliance/helpers/capture.h b/src/apps/lc-compliance/helpers/capture.h index a4cc3a99e..0e7b848fb 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,29 @@ class Capture { public: - void configure(libcamera::StreamRole role); - -protected: Capture(std::shared_ptr camera); - virtual ~Capture(); + ~Capture(); + + void configure(libcamera::StreamRole role); + void run(unsigned int captureLimit, std::optional queueLimit = {}); + +private: + LIBCAMERA_DISABLE_COPY_AND_MOVE(Capture) void start(); void stop(); - virtual void requestComplete(libcamera::Request *request) = 0; - - EventLoop *loop_; + 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,