5 files changed, 96 insertions, 182 deletions

diff --git a/src/apps/lc-compliance/environment.h b/src/apps/lc-compliance/environment.h
index 543e5372..834c722e 100644
--- a/src/apps/lc-compliance/environment.h
+++ b/src/apps/lc-compliance/environment.h
@@ -23,5 +23,5 @@ private:
 	Environment() = default;
 
 	std::string cameraId_;
-	libcamera::CameraManager *cm_;
+	libcamera::CameraManager *cm_ = nullptr;
 };
diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp
index 90c1530b..f2c6d58c 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 <assert.h>
+
 #include <gtest/gtest.h>
 
 using namespace libcamera;
 
 Capture::Capture(std::shared_ptr<Camera> camera)
-	: loop_(nullptr), camera_(camera),
-	  allocator_(std::make_unique<FrameBufferAllocator>(camera))
+	: camera_(std::move(camera)), allocator_(camera_)
 {
 }
 
@@ -26,10 +27,8 @@ void Capture::configure(StreamRole role)
 {
 	config_ = camera_->generateConfiguration({ role });
 
-	if (!config_) {
-		std::cout << "Role not supported by camera" << std::endl;
-		GTEST_SKIP();
-	}
+	if (!config_)
+		GTEST_SKIP() << "Role not supported by camera";
 
 	if (config_->validate() != CameraConfiguration::Valid) {
 		config_.reset();
@@ -42,155 +41,106 @@ void Capture::configure(StreamRole role)
 	}
 }
 
-void Capture::start()
+void Capture::run(unsigned int captureLimit, std::optional<unsigned int> queueLimit)
 {
-	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";
-
-	camera_->requestCompleted.connect(this, &Capture::requestComplete);
-
-	ASSERT_EQ(camera_->start(), 0) << "Failed to start camera";
-}
+	assert(!queueLimit || captureLimit <= *queueLimit);
 
-void Capture::stop()
-{
-	if (!config_ || !allocator_->allocated())
-		return;
+	captureLimit_ = captureLimit;
+	queueLimit_ = queueLimit;
 
-	camera_->stop();
+	captureCount_ = queueCount_ = 0;
 
-	camera_->requestCompleted.disconnect(this);
+	EventLoop loop;
+	loop_ = &loop;
 
-	Stream *stream = config_->at(0).stream();
-	requests_.clear();
-	allocator_->free(stream);
-}
-
-/* CaptureBalanced */
-
-CaptureBalanced::CaptureBalanced(std::shared_ptr<Camera> camera)
-	: Capture(camera)
-{
-}
-
-void CaptureBalanced::capture(unsigned int numRequests)
-{
 	start();
 
-	Stream *stream = config_->at(0).stream();
-	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
-
-	/* No point in testing less requests then the camera depth. */
-	if (buffers.size() > numRequests) {
-		std::cout << "Camera needs " + std::to_string(buffers.size())
-			+ " requests, can't test only "
-			+ std::to_string(numRequests) << std::endl;
-		GTEST_SKIP();
-	}
+	for (const auto &request : requests_)
+		queueRequest(request.get());
 
-	queueCount_ = 0;
-	captureCount_ = 0;
-	captureLimit_ = numRequests;
+	EXPECT_EQ(loop_->exec(), 0);
 
-	/* Queue the recommended number of requests. */
-	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
-		std::unique_ptr<Request> 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_);
+	EXPECT_LE(captureLimit_, captureCount_);
+	EXPECT_LE(captureCount_, queueCount_);
+	EXPECT_TRUE(!queueLimit_ || queueCount_ <= *queueLimit_);
 }
 
-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);
+	int ret = camera_->queueRequest(request);
+	if (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> camera)
-	: Capture(camera)
-{
-}
-
-void CaptureUnbalanced::capture(unsigned int numRequests)
+void Capture::start()
 {
-	start();
+	assert(config_);
+	assert(!config_->empty());
+	assert(!allocator_.allocated());
+	assert(requests_.empty());
 
 	Stream *stream = config_->at(0).stream();
-	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
+	int count = allocator_.allocate(stream);
 
-	captureCount_ = 0;
-	captureLimit_ = numRequests;
+	ASSERT_GE(count, 0) << "Failed to allocate buffers";
+	EXPECT_EQ(count, config_->at(0).bufferCount) << "Allocated less buffers than expected";
+
+	const std::vector<std::unique_ptr<FrameBuffer>> &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_;
+	}
 
-	/* Queue the recommended number of requests. */
 	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
 		std::unique_ptr<Request> 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_;
+	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 19b6927c..0e7b848f 100644
--- a/src/apps/lc-compliance/helpers/capture.h
+++ b/src/apps/lc-compliance/helpers/capture.h
@@ -8,6 +8,7 @@
 #pragma once
 
 #include <memory>
+#include <optional>
 
 #include <libcamera/libcamera.h>
 
@@ -16,51 +17,29 @@
 class Capture
 {
 public:
+	Capture(std::shared_ptr<libcamera::Camera> camera);
+	~Capture();
+
 	void configure(libcamera::StreamRole role);
+	void run(unsigned int captureLimit, std::optional<unsigned int> queueLimit = {});
 
-protected:
-	Capture(std::shared_ptr<libcamera::Camera> camera);
-	virtual ~Capture();
+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<libcamera::Camera> camera_;
-	std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
+	libcamera::FrameBufferAllocator allocator_;
 	std::unique_ptr<libcamera::CameraConfiguration> config_;
 	std::vector<std::unique_ptr<libcamera::Request>> requests_;
-};
-
-class CaptureBalanced : public Capture
-{
-public:
-	CaptureBalanced(std::shared_ptr<libcamera::Camera> 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<libcamera::Camera> 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<unsigned int> queueLimit_;
+	unsigned int captureCount_ = 0;
+	unsigned int queueCount_ = 0;
 };
diff --git a/src/apps/lc-compliance/main.cpp b/src/apps/lc-compliance/main.cpp
index 3f1d2a61..e9f0ffbb 100644
--- a/src/apps/lc-compliance/main.cpp
+++ b/src/apps/lc-compliance/main.cpp
@@ -45,13 +45,11 @@ class ThrowListener : public testing::EmptyTestEventListener
 static void listCameras(CameraManager *cm)
 {
 	for (const std::shared_ptr<Camera> &cam : cm->cameras())
-		std::cout << "- " << cam.get()->id() << std::endl;
+		std::cout << "- " << cam->id() << std::endl;
 }
 
 static int initCamera(CameraManager *cm, OptionsParser::Options options)
 {
-	std::shared_ptr<Camera> camera;
-
 	int ret = cm->start();
 	if (ret) {
 		std::cout << "Failed to start camera manager: "
@@ -66,7 +64,7 @@ static int initCamera(CameraManager *cm, OptionsParser::Options options)
 	}
 
 	const std::string &cameraId = options[OptCamera];
-	camera = cm->get(cameraId);
+	std::shared_ptr<Camera> camera = cm->get(cameraId);
 	if (!camera) {
 		std::cout << "Camera " << cameraId << " not found, available cameras:" << std::endl;
 		listCameras(cm);
@@ -82,45 +80,27 @@ static int initCamera(CameraManager *cm, OptionsParser::Options options)
 
 static int initGtestParameters(char *arg0, OptionsParser::Options options)
 {
-	const std::map<std::string, std::string> gtestFlags = { { "list", "--gtest_list_tests" },
-								{ "filter", "--gtest_filter" } };
-
-	int argc = 0;
+	std::vector<const char *> argv;
 	std::string filterParam;
 
-	/*
-	 * +2 to have space for both the 0th argument that is needed but not
-	 * used and the null at the end.
-	 */
-	char **argv = new char *[(gtestFlags.size() + 2)];
-	if (!argv)
-		return -ENOMEM;
-
-	argv[0] = arg0;
-	argc++;
+	argv.push_back(arg0);
 
-	if (options.isSet(OptList)) {
-		argv[argc] = const_cast<char *>(gtestFlags.at("list").c_str());
-		argc++;
-	}
+	if (options.isSet(OptList))
+		argv.push_back("--gtest_list_tests");
 
 	if (options.isSet(OptFilter)) {
 		/*
 		 * The filter flag needs to be passed as a single parameter, in
 		 * the format --gtest_filter=filterStr
 		 */
-		filterParam = gtestFlags.at("filter") + "=" +
-			      static_cast<const std::string &>(options[OptFilter]);
-
-		argv[argc] = const_cast<char *>(filterParam.c_str());
-		argc++;
+		filterParam = "--gtest_filter=" + options[OptFilter].toString();
+		argv.push_back(filterParam.c_str());
 	}
 
-	argv[argc] = nullptr;
-
-	::testing::InitGoogleTest(&argc, argv);
+	argv.push_back(nullptr);
 
-	delete[] argv;
+	int argc = argv.size();
+	::testing::InitGoogleTest(&argc, const_cast<char **>(argv.data()));
 
 	return 0;
 }
diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
index ad3a1da2..93bed48f 100644
--- a/src/apps/lc-compliance/tests/capture_test.cpp
+++ b/src/apps/lc-compliance/tests/capture_test.cpp
@@ -14,10 +14,13 @@
 
 #include "environment.h"
 
+namespace {
+
 using namespace libcamera;
 
-const std::vector<int> NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
-const std::vector<StreamRole> ROLES = {
+const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
+
+const StreamRole ROLES[] = {
 	StreamRole::Raw,
 	StreamRole::StillCapture,
 	StreamRole::VideoRecording,
@@ -84,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);
 }
 
 /*
@@ -103,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);
 }
 
 /*
@@ -122,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,
@@ -134,3 +137,5 @@ INSTANTIATE_TEST_SUITE_P(CaptureTests,
 			 testing::Combine(testing::ValuesIn(ROLES),
 					  testing::ValuesIn(NUMREQUESTS)),
 			 SingleStream::nameParameters);
+
+} /* namespace */