7 files changed, 369 insertions, 67 deletions

diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
index 3f392cdc..815d1cae 100644
--- a/test/camera/buffer_import.cpp
+++ b/test/camera/buffer_import.cpp
@@ -12,15 +12,20 @@
 #include <numeric>
 #include <vector>
 
-#include "device_enumerator.h"
-#include "media_device.h"
-#include "v4l2_videodevice.h"
+#include <libcamera/base/event_dispatcher.h>
+#include <libcamera/base/thread.h>
+#include <libcamera/base/timer.h>
+
+#include "libcamera/internal/device_enumerator.h"
+#include "libcamera/internal/media_device.h"
+#include "libcamera/internal/v4l2_videodevice.h"
 
 #include "buffer_source.h"
 #include "camera_test.h"
 #include "test.h"
 
 using namespace libcamera;
+using namespace std::chrono_literals;
 
 namespace {
 
@@ -28,12 +33,13 @@ class BufferImportTest : public CameraTest, public Test
 {
 public:
 	BufferImportTest()
-		: CameraTest("VIMC Sensor B")
+		: CameraTest("platform/vimc.0 Sensor B")
 	{
 	}
 
 protected:
-	void bufferComplete(Request *request, FrameBuffer *buffer)
+	void bufferComplete([[maybe_unused]] Request *request,
+			    FrameBuffer *buffer)
 	{
 		if (buffer->metadata().status != FrameMetadata::FrameSuccess)
 			return;
@@ -46,17 +52,19 @@ protected:
 		if (request->status() != Request::RequestComplete)
 			return;
 
-		const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
+		const Request::BufferMap &buffers = request->buffers();
 
 		completeRequestsCount_++;
 
 		/* Create a new request. */
-		Stream *stream = buffers.begin()->first;
+		const Stream *stream = buffers.begin()->first;
 		FrameBuffer *buffer = buffers.begin()->second;
 
-		request = camera_->createRequest();
+		request->reuse();
 		request->addBuffer(stream, buffer);
 		camera_->queueRequest(request);
+
+		dispatcher_->interrupt();
 	}
 
 	int init() override
@@ -70,6 +78,8 @@ protected:
 			return TestFail;
 		}
 
+		dispatcher_ = Thread::current()->eventDispatcher();
+
 		return TestPass;
 	}
 
@@ -94,9 +104,8 @@ protected:
 		if (ret != TestPass)
 			return ret;
 
-		std::vector<Request *> requests;
 		for (const std::unique_ptr<FrameBuffer> &buffer : source.buffers()) {
-			Request *request = camera_->createRequest();
+			std::unique_ptr<Request> request = camera_->createRequest();
 			if (!request) {
 				std::cout << "Failed to create request" << std::endl;
 				return TestFail;
@@ -107,7 +116,7 @@ protected:
 				return TestFail;
 			}
 
-			requests.push_back(request);
+			requests_.push_back(std::move(request));
 		}
 
 		completeRequestsCount_ = 0;
@@ -121,24 +130,27 @@ protected:
 			return TestFail;
 		}
 
-		for (Request *request : requests) {
-			if (camera_->queueRequest(request)) {
+		for (std::unique_ptr<Request> &request : requests_) {
+			if (camera_->queueRequest(request.get())) {
 				std::cout << "Failed to queue request" << std::endl;
 				return TestFail;
 			}
 		}
 
-		EventDispatcher *dispatcher = cm_->eventDispatcher();
+		const unsigned int nFrames = cfg.bufferCount * 2;
 
 		Timer timer;
-		timer.start(1000);
-		while (timer.isRunning())
-			dispatcher->processEvents();
+		timer.start(500ms * nFrames);
+		while (timer.isRunning()) {
+			dispatcher_->processEvents();
+			if (completeRequestsCount_ > nFrames)
+				break;
+		}
 
-		if (completeRequestsCount_ <= cfg.bufferCount * 2) {
+		if (completeRequestsCount_ < nFrames) {
 			std::cout << "Failed to capture enough frames (got "
 				  << completeRequestsCount_ << " expected at least "
-				  << cfg.bufferCount * 2 << ")" << std::endl;
+				  << nFrames << ")" << std::endl;
 			return TestFail;
 		}
 
@@ -156,6 +168,10 @@ protected:
 	}
 
 private:
+	EventDispatcher *dispatcher_;
+
+	std::vector<std::unique_ptr<Request>> requests_;
+
 	unsigned int completeBuffersCount_;
 	unsigned int completeRequestsCount_;
 	std::unique_ptr<CameraConfiguration> config_;
@@ -163,4 +179,4 @@ private:
 
 } /* namespace */
 
-TEST_REGISTER(BufferImportTest);
+TEST_REGISTER(BufferImportTest)
diff --git a/test/camera/camera_reconfigure.cpp b/test/camera/camera_reconfigure.cpp
new file mode 100644
index 00000000..06c87730
--- /dev/null
+++ b/test/camera/camera_reconfigure.cpp
@@ -0,0 +1,264 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2021, Google Inc.
+ *
+ * Test:
+ * - Multiple reconfigurations of the Camera without stopping the CameraManager
+ * - Validate there are no file descriptor leaks when using IPC
+ */
+
+#include <dirent.h>
+#include <fstream>
+#include <iostream>
+
+#include <libcamera/base/event_dispatcher.h>
+#include <libcamera/base/file.h>
+#include <libcamera/base/thread.h>
+#include <libcamera/base/timer.h>
+
+#include <libcamera/framebuffer_allocator.h>
+
+#include "camera_test.h"
+#include "test.h"
+
+using namespace libcamera;
+using namespace std;
+using namespace std::chrono_literals;
+
+namespace {
+
+class CameraReconfigure : public CameraTest, public Test
+{
+public:
+	/* Initialize CameraTest with isolated IPA */
+	CameraReconfigure()
+		: CameraTest(kCamId_, true)
+	{
+	}
+
+private:
+	static constexpr const char *kCamId_ = "platform/vimc.0 Sensor B";
+	static constexpr const char *kIpaProxyName_ = "vimc_ipa_proxy";
+	static constexpr unsigned int kNumOfReconfigures_ = 10;
+
+	void requestComplete(Request *request)
+	{
+		if (request->status() != Request::RequestComplete)
+			return;
+
+		const Request::BufferMap &buffers = request->buffers();
+
+		const Stream *stream = buffers.begin()->first;
+		FrameBuffer *buffer = buffers.begin()->second;
+
+		/* Reuse the request and re-queue it with the same buffers. */
+		request->reuse();
+		request->addBuffer(stream, buffer);
+		camera_->queueRequest(request);
+	}
+
+	int startAndStop()
+	{
+		StreamConfiguration &cfg = config_->at(0);
+
+		if (camera_->acquire()) {
+			cerr << "Failed to acquire the camera" << endl;
+			return TestFail;
+		}
+
+		if (camera_->configure(config_.get())) {
+			cerr << "Failed to set default configuration" << endl;
+			return TestFail;
+		}
+
+		Stream *stream = cfg.stream();
+
+		/*
+		 * The configuration is consistent so we can re-use the
+		 * same buffer allocation for each run.
+		 */
+		if (!allocated_) {
+			int ret = allocator_->allocate(stream);
+			if (ret < 0) {
+				cerr << "Failed to allocate buffers" << endl;
+				return TestFail;
+			}
+			allocated_ = true;
+		}
+
+		for (const unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
+			unique_ptr<Request> request = camera_->createRequest();
+			if (!request) {
+				cerr << "Failed to create request" << endl;
+				return TestFail;
+			}
+
+			if (request->addBuffer(stream, buffer.get())) {
+				cerr << "Failed to associate buffer with request" << endl;
+				return TestFail;
+			}
+
+			requests_.push_back(std::move(request));
+		}
+
+		camera_->requestCompleted.connect(this, &CameraReconfigure::requestComplete);
+
+		if (camera_->start()) {
+			cerr << "Failed to start camera" << endl;
+			return TestFail;
+		}
+
+		for (unique_ptr<Request> &request : requests_) {
+			if (camera_->queueRequest(request.get())) {
+				cerr << "Failed to queue request" << endl;
+				return TestFail;
+			}
+		}
+
+		EventDispatcher *dispatcher = Thread::current()->eventDispatcher();
+
+		Timer timer;
+		timer.start(100ms);
+		while (timer.isRunning())
+			dispatcher->processEvents();
+
+		if (camera_->stop()) {
+			cerr << "Failed to stop camera" << endl;
+			return TestFail;
+		}
+
+		if (camera_->release()) {
+			cerr << "Failed to release camera" << endl;
+			return TestFail;
+		}
+
+		camera_->requestCompleted.disconnect(this);
+
+		requests_.clear();
+
+		return 0;
+	}
+
+	int fdsOpen(pid_t pid)
+	{
+		string proxyFdPath = "/proc/" + to_string(pid) + "/fd";
+		DIR *dir;
+		struct dirent *ptr;
+		unsigned int openFds = 0;
+
+		dir = opendir(proxyFdPath.c_str());
+		if (dir == nullptr) {
+			int err = errno;
+			cerr << "Error opening " << proxyFdPath << ": "
+			     << strerror(-err) << endl;
+			return 0;
+		}
+
+		while ((ptr = readdir(dir)) != nullptr) {
+			if ((strcmp(ptr->d_name, ".") == 0) ||
+			    (strcmp(ptr->d_name, "..") == 0))
+				continue;
+
+			openFds++;
+		}
+		closedir(dir);
+
+		return openFds;
+	}
+
+	pid_t findProxyPid()
+	{
+		string proxyPid;
+		string proxyName(kIpaProxyName_);
+		DIR *dir;
+		struct dirent *ptr;
+
+		dir = opendir("/proc");
+		while ((ptr = readdir(dir)) != nullptr) {
+			if (ptr->d_type != DT_DIR)
+				continue;
+
+			string pname("/proc/" + string(ptr->d_name) + "/comm");
+			if (File::exists(pname)) {
+				ifstream pfile(pname.c_str());
+				string comm;
+				getline(pfile, comm);
+				pfile.close();
+
+				proxyPid = comm == proxyName ? string(ptr->d_name) : "";
+			}
+
+			if (!proxyPid.empty())
+				break;
+		}
+		closedir(dir);
+
+		if (!proxyPid.empty())
+			return atoi(proxyPid.c_str());
+
+		return -1;
+	}
+
+	int init() override
+	{
+		if (status_ != TestPass)
+			return status_;
+
+		config_ = camera_->generateConfiguration({ StreamRole::StillCapture });
+		if (!config_ || config_->size() != 1) {
+			cerr << "Failed to generate default configuration" << endl;
+			return TestFail;
+		}
+
+		allocator_ = make_unique<FrameBufferAllocator>(camera_);
+		allocated_ = false;
+
+		return TestPass;
+	}
+
+	int run() override
+	{
+		unsigned int openFdsAtStart = 0;
+		unsigned int openFds = 0;
+
+		pid_t proxyPid = findProxyPid();
+		if (proxyPid < 0) {
+			cerr << "Cannot find " << kIpaProxyName_
+			     << " pid, exiting" << endl;
+			return TestFail;
+		}
+
+		openFdsAtStart = fdsOpen(proxyPid);
+		for (unsigned int i = 0; i < kNumOfReconfigures_; i++) {
+			startAndStop();
+			openFds = fdsOpen(proxyPid);
+			if (openFds == 0) {
+				cerr << "No open fds found whereas "
+				     << "open fds at start: " << openFdsAtStart
+				     << endl;
+				return TestFail;
+			}
+
+			if (openFds != openFdsAtStart) {
+				cerr << "Leaking fds for " << kIpaProxyName_
+				     << " - Open fds: " << openFds << " vs "
+				     << "Open fds at start: " << openFdsAtStart
+				     << endl;
+				return TestFail;
+			}
+		}
+
+		return TestPass;
+	}
+
+	bool allocated_;
+
+	vector<unique_ptr<Request>> requests_;
+
+	unique_ptr<CameraConfiguration> config_;
+	unique_ptr<FrameBufferAllocator> allocator_;
+};
+
+} /* namespace */
+
+TEST_REGISTER(CameraReconfigure)
diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
index f6b2f348..8766fb19 100644
--- a/test/camera/capture.cpp
+++ b/test/camera/capture.cpp
@@ -7,10 +7,18 @@
 
 #include <iostream>
 
+#include <libcamera/framebuffer_allocator.h>
+
+#include <libcamera/base/event_dispatcher.h>
+#include <libcamera/base/thread.h>
+#include <libcamera/base/timer.h>
+
 #include "camera_test.h"
 #include "test.h"
 
+using namespace libcamera;
 using namespace std;
+using namespace std::chrono_literals;
 
 namespace {
 
@@ -18,7 +26,7 @@ class Capture : public CameraTest, public Test
 {
 public:
 	Capture()
-		: CameraTest("VIMC Sensor B")
+		: CameraTest("platform/vimc.0 Sensor B")
 	{
 	}
 
@@ -26,7 +34,8 @@ protected:
 	unsigned int completeBuffersCount_;
 	unsigned int completeRequestsCount_;
 
-	void bufferComplete(Request *request, FrameBuffer *buffer)
+	void bufferComplete([[maybe_unused]] Request *request,
+			    FrameBuffer *buffer)
 	{
 		if (buffer->metadata().status != FrameMetadata::FrameSuccess)
 			return;
@@ -39,17 +48,19 @@ protected:
 		if (request->status() != Request::RequestComplete)
 			return;
 
-		const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
+		const Request::BufferMap &buffers = request->buffers();
 
 		completeRequestsCount_++;
 
 		/* Create a new request. */
-		Stream *stream = buffers.begin()->first;
+		const Stream *stream = buffers.begin()->first;
 		FrameBuffer *buffer = buffers.begin()->second;
 
-		request = camera_->createRequest();
+		request->reuse();
 		request->addBuffer(stream, buffer);
 		camera_->queueRequest(request);
+
+		dispatcher_->interrupt();
 	}
 
 	int init() override
@@ -64,6 +75,7 @@ protected:
 		}
 
 		allocator_ = new FrameBufferAllocator(camera_);
+		dispatcher_ = Thread::current()->eventDispatcher();
 
 		return TestPass;
 	}
@@ -93,20 +105,19 @@ protected:
 		if (ret < 0)
 			return TestFail;
 
-		std::vector<Request *> requests;
 		for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
-			Request *request = camera_->createRequest();
+			std::unique_ptr<Request> request = camera_->createRequest();
 			if (!request) {
 				cout << "Failed to create request" << endl;
 				return TestFail;
 			}
 
 			if (request->addBuffer(stream, buffer.get())) {
-				cout << "Failed to associating buffer with request" << endl;
+				cout << "Failed to associate buffer with request" << endl;
 				return TestFail;
 			}
 
-			requests.push_back(request);
+			requests_.push_back(std::move(request));
 		}
 
 		completeRequestsCount_ = 0;
@@ -120,26 +131,27 @@ protected:
 			return TestFail;
 		}
 
-		for (Request *request : requests) {
-			if (camera_->queueRequest(request)) {
+		for (std::unique_ptr<Request> &request : requests_) {
+			if (camera_->queueRequest(request.get())) {
 				cout << "Failed to queue request" << endl;
 				return TestFail;
 			}
 		}
 
-		EventDispatcher *dispatcher = cm_->eventDispatcher();
+		unsigned int nFrames = allocator_->buffers(stream).size() * 2;
 
 		Timer timer;
-		timer.start(1000);
-		while (timer.isRunning())
-			dispatcher->processEvents();
-
-		unsigned int nbuffers = allocator_->buffers(stream).size();
+		timer.start(500ms * nFrames);
+		while (timer.isRunning()) {
+			dispatcher_->processEvents();
+			if (completeRequestsCount_ > nFrames)
+				break;
+		}
 
-		if (completeRequestsCount_ <= nbuffers * 2) {
+		if (completeRequestsCount_ < nFrames) {
 			cout << "Failed to capture enough frames (got "
 			     << completeRequestsCount_ << " expected at least "
-			     << nbuffers * 2 << ")" << endl;
+			     << nFrames * 2 << ")" << endl;
 			return TestFail;
 		}
 
@@ -156,10 +168,14 @@ protected:
 		return TestPass;
 	}
 
+	EventDispatcher *dispatcher_;
+
+	std::vector<std::unique_ptr<Request>> requests_;
+
 	std::unique_ptr<CameraConfiguration> config_;
 	FrameBufferAllocator *allocator_;
 };
 
 } /* namespace */
 
-TEST_REGISTER(Capture);
+TEST_REGISTER(Capture)
diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
index 31c908d2..209eb6a5 100644
--- a/test/camera/configuration_default.cpp
+++ b/test/camera/configuration_default.cpp
@@ -10,6 +10,7 @@
 #include "camera_test.h"
 #include "test.h"
 
+using namespace libcamera;
 using namespace std;
 
 namespace {
@@ -18,7 +19,7 @@ class ConfigurationDefault : public CameraTest, public Test
 {
 public:
 	ConfigurationDefault()
-		: CameraTest("VIMC Sensor B")
+		: CameraTest("platform/vimc.0 Sensor B")
 	{
 	}
 
@@ -56,4 +57,4 @@ protected:
 
 } /* namespace */
 
-TEST_REGISTER(ConfigurationDefault);
+TEST_REGISTER(ConfigurationDefault)
diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
index b4b59681..4281a1c4 100644
--- a/test/camera/configuration_set.cpp
+++ b/test/camera/configuration_set.cpp
@@ -10,6 +10,7 @@
 #include "camera_test.h"
 #include "test.h"
 
+using namespace libcamera;
 using namespace std;
 
 namespace {
@@ -18,7 +19,7 @@ class ConfigurationSet : public CameraTest, public Test
 {
 public:
 	ConfigurationSet()
-		: CameraTest("VIMC Sensor B")
+		: CameraTest("platform/vimc.0 Sensor B")
 	{
 	}
 
@@ -102,4 +103,4 @@ protected:
 
 } /* namespace */
 
-TEST_REGISTER(ConfigurationSet);
+TEST_REGISTER(ConfigurationSet)
diff --git a/test/camera/meson.build b/test/camera/meson.build
index e2a6660a..4f9f8c8c 100644
--- a/test/camera/meson.build
+++ b/test/camera/meson.build
@@ -1,17 +1,20 @@
+# SPDX-License-Identifier: CC0-1.0
+
 # Tests are listed in order of complexity.
 # They are not alphabetically sorted.
 camera_tests = [
-    [ 'configuration_default',  'configuration_default.cpp' ],
-    [ 'configuration_set',      'configuration_set.cpp' ],
-    [ 'buffer_import',          'buffer_import.cpp' ],
-    [ 'statemachine',           'statemachine.cpp' ],
-    [ 'capture',                'capture.cpp' ],
+    {'name': 'configuration_default', 'sources': ['configuration_default.cpp']},
+    {'name': 'configuration_set', 'sources': ['configuration_set.cpp']},
+    {'name': 'buffer_import', 'sources': ['buffer_import.cpp']},
+    {'name': 'statemachine', 'sources': ['statemachine.cpp']},
+    {'name': 'capture', 'sources': ['capture.cpp']},
+    {'name': 'camera_reconfigure', 'sources': ['camera_reconfigure.cpp']},
 ]
 
-foreach t : camera_tests
-    exe = executable(t[0], t[1],
-                     dependencies : libcamera_dep,
+foreach test : camera_tests
+    exe = executable(test['name'], test['sources'],
+                     dependencies : libcamera_private,
                      link_with : test_libraries,
                      include_directories : test_includes_internal)
-    test(t[0], exe, suite : 'camera', is_parallel : false)
+    test(test['name'], exe, suite : 'camera', is_parallel : false)
 endforeach
diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
index 325b4674..9c2b0c6a 100644
--- a/test/camera/statemachine.cpp
+++ b/test/camera/statemachine.cpp
@@ -7,9 +7,12 @@
 
 #include <iostream>
 
+#include <libcamera/framebuffer_allocator.h>
+
 #include "camera_test.h"
 #include "test.h"
 
+using namespace libcamera;
 using namespace std;
 
 namespace {
@@ -18,7 +21,7 @@ class Statemachine : public CameraTest, public Test
 {
 public:
 	Statemachine()
-		: CameraTest("VIMC Sensor B")
+		: CameraTest("platform/vimc.0 Sensor B")
 	{
 	}
 
@@ -39,13 +42,13 @@ protected:
 		if (camera_->queueRequest(&request) != -EACCES)
 			return TestFail;
 
-		if (camera_->stop() != -EACCES)
-			return TestFail;
-
 		/* Test operations which should pass. */
 		if (camera_->release())
 			return TestFail;
 
+		if (camera_->stop())
+			return TestFail;
+
 		/* Test valid state transitions, end in Acquired state. */
 		if (camera_->acquire())
 			return TestFail;
@@ -69,7 +72,8 @@ protected:
 		if (camera_->queueRequest(&request) != -EACCES)
 			return TestFail;
 
-		if (camera_->stop() != -EACCES)
+		/* Test operations which should pass. */
+		if (camera_->stop())
 			return TestFail;
 
 		/* Test valid state transitions, end in Configured state. */
@@ -95,16 +99,13 @@ protected:
 		if (camera_->queueRequest(&request1) != -EACCES)
 			return TestFail;
 
-		if (camera_->stop() != -EACCES)
-			return TestFail;
-
 		/* Test operations which should pass. */
-		Request *request2 = camera_->createRequest();
+		std::unique_ptr<Request> request2 = camera_->createRequest();
 		if (!request2)
 			return TestFail;
 
-		/* Never handed to hardware so need to manually delete it. */
-		delete request2;
+		if (camera_->stop())
+			return TestFail;
 
 		/* Test valid state transitions, end in Running state. */
 		if (camera_->release())
@@ -144,7 +145,7 @@ protected:
 			return TestFail;
 
 		/* Test operations which should pass. */
-		Request *request = camera_->createRequest();
+		std::unique_ptr<Request> request = camera_->createRequest();
 		if (!request)
 			return TestFail;
 
@@ -152,7 +153,7 @@ protected:
 		if (request->addBuffer(stream, allocator_->buffers(stream)[0].get()))
 			return TestFail;
 
-		if (camera_->queueRequest(request))
+		if (camera_->queueRequest(request.get()))
 			return TestFail;
 
 		/* Test valid state transitions, end in Available state. */
@@ -212,4 +213,4 @@ protected:
 
 } /* namespace */
 
-TEST_REGISTER(Statemachine);
+TEST_REGISTER(Statemachine)