libcamera: camera_manager: Register cameras with the camera manager

Cameras are listed through a double indirection, first iterating over
all available pipeline handlers, and then listing the cameras they each
support. To simplify the API make the pipeline handlers register the
cameras with the camera manager directly, which lets the camera manager
easily expose the list of all available cameras.

The PipelineHandler API gets simplified as the handlers don't need to
expose the list of cameras they have created.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

6 files changed, 48 insertions, 71 deletions

diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
index b82a8ce9..4b941fd9 100644
--- a/include/libcamera/camera_manager.h
+++ b/include/libcamera/camera_manager.h
@@ -24,9 +24,11 @@ public:
 	int start();
 	void stop();
 
-	std::vector<std::string> list() const;
+	const std::vector<Camera *> &cameras() const { return cameras_; }
 	Camera *get(const std::string &name);
 
+	void addCamera(Camera *camera);
+
 	static CameraManager *instance();
 
 	void setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);
@@ -40,6 +42,7 @@ private:
 
 	std::unique_ptr<DeviceEnumerator> enumerator_;
 	std::vector<PipelineHandler *> pipes_;
+	std::vector<Camera *> cameras_;
 
 	std::unique_ptr<EventDispatcher> dispatcher_;
 };
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 1430bb0d..3fa9f23c 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -94,7 +94,7 @@ int CameraManager::start()
 		 */
 		while (1) {
 			PipelineHandler *pipe = factory->create();
-			if (!pipe->match(enumerator_.get())) {
+			if (!pipe->match(this, enumerator_.get())) {
 				delete pipe;
 				break;
 			}
@@ -133,26 +133,14 @@ void CameraManager::stop()
 }
 
 /**
- * \brief List all detected cameras
+ * \fn CameraManager::cameras()
+ * \brief Retrieve all available cameras
  *
  * Before calling this function the caller is responsible for ensuring that
  * the camera manger is running.
  *
- * \return List of names for all detected cameras
+ * \return List of all available cameras
  */
-std::vector<std::string> CameraManager::list() const
-{
-	std::vector<std::string> list;
-
-	for (PipelineHandler *pipe : pipes_) {
-		for (unsigned int i = 0; i < pipe->count(); i++) {
-			Camera *cam = pipe->camera(i);
-			list.push_back(cam->name());
-		}
-	}
-
-	return list;
-}
 
 /**
  * \brief Get a camera based on name
@@ -168,20 +156,36 @@ std::vector<std::string> CameraManager::list() const
  */
 Camera *CameraManager::get(const std::string &name)
 {
-	for (PipelineHandler *pipe : pipes_) {
-		for (unsigned int i = 0; i < pipe->count(); i++) {
-			Camera *cam = pipe->camera(i);
-			if (cam->name() == name) {
-				cam->get();
-				return cam;
-			}
-		}
+	for (Camera *camera : cameras_) {
+		if (camera->name() == name)
+			return camera;
 	}
 
 	return nullptr;
 }
 
 /**
+ * \brief Add a camera to the camera manager
+ * \param[in] camera The camera to be added
+ *
+ * This function is called by pipeline handlers to register the cameras they
+ * handle with the camera manager. Registered cameras are immediately made
+ * available to the system.
+ */
+void CameraManager::addCamera(Camera *camera)
+{
+	for (Camera *c : cameras_) {
+		if (c->name() == camera->name()) {
+			LOG(Warning) << "Registering camera with duplicate name '"
+				     << camera->name() << "'";
+			break;
+		}
+	}
+
+	cameras_.push_back(camera);
+}
+
+/**
  * \brief Retrieve the camera manager instance
  *
  * The CameraManager is a singleton and can't be constructed manually. This
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index e976aaa1..f05f201f 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -15,6 +15,7 @@
 
 namespace libcamera {
 
+class CameraManager;
 class DeviceEnumerator;
 
 class PipelineHandler
@@ -22,10 +23,7 @@ class PipelineHandler
 public:
 	virtual ~PipelineHandler() { };
 
-	virtual bool match(DeviceEnumerator *enumerator) = 0;
-
-	virtual unsigned int count() = 0;
-	virtual Camera *camera(unsigned int id) = 0;
+	virtual bool match(CameraManager *manager, DeviceEnumerator *enumerator) = 0;
 };
 
 class PipelineHandlerFactory
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 720d9c20..8742e0ba 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -6,6 +6,7 @@
  */
 
 #include <libcamera/camera.h>
+#include <libcamera/camera_manager.h>
 
 #include "device_enumerator.h"
 #include "media_device.h"
@@ -19,44 +20,24 @@ public:
 	PipeHandlerVimc();
 	~PipeHandlerVimc();
 
-	bool match(DeviceEnumerator *enumerator);
-
-	unsigned int count();
-	Camera *camera(unsigned int id) final;
+	bool match(CameraManager *manager, DeviceEnumerator *enumerator);
 
 private:
 	MediaDevice *dev_;
-	Camera *camera_;
 };
 
 PipeHandlerVimc::PipeHandlerVimc()
-	: dev_(nullptr), camera_(nullptr)
+	: dev_(nullptr)
 {
 }
 
 PipeHandlerVimc::~PipeHandlerVimc()
 {
-	if (camera_)
-		camera_->put();
-
 	if (dev_)
 		dev_->release();
 }
 
-unsigned int PipeHandlerVimc::count()
-{
-	return 1;
-}
-
-Camera *PipeHandlerVimc::camera(unsigned int id)
-{
-	if (id != 0)
-		return nullptr;
-
-	return camera_;
-}
-
-bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
+bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator)
 {
 	DeviceMatch dm("vimc");
 
@@ -83,7 +64,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
 	 * will be chosen depends on how the Camera
 	 * object is modeled.
 	 */
-	camera_ = new Camera("Dummy VIMC Camera");
+	Camera *camera = new Camera("Dummy VIMC Camera");
+	manager->addCamera(camera);
 
 	return true;
 }
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index c19ab94f..f2e08a6a 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -35,13 +35,15 @@ namespace libcamera {
 /**
  * \fn PipelineHandler::match(DeviceEnumerator *enumerator)
  * \brief Match media devices and create camera instances
+ * \param manager The camera manager
  * \param enumerator The enumerator providing all media devices found in the
  * system
  *
  * This function is the main entry point of the pipeline handler. It is called
- * by the camera manager with the \a enumerator passed as an argument. It
- * shall acquire from the \a enumerator all the media devices it needs for a
- * single pipeline and create one or multiple Camera instances.
+ * by the camera manager with the \a manager and \a enumerator passed as
+ * arguments. It shall acquire from the \a enumerator all the media devices it
+ * needs for a single pipeline, create one or multiple Camera instances and
+ * register them with the \a manager.
  *
  * If all media devices needed by the pipeline handler are found, they must all
  * be acquired by a call to MediaDevice::acquire(). This function shall then
@@ -63,19 +65,6 @@ namespace libcamera {
  */
 
 /**
- * \fn PipelineHandler::count()
- * \brief Retrieve the number of cameras handled by this pipeline handler
- * \return the number of cameras that were created by the match() function
- */
-
-/**
- * \fn PipelineHandler::camera(unsigned int id)
- * \brief Retrieve one of the cameras handled by this pipeline handler
- * \param[in] id the camera index
- * \return a pointer to the Camera identified by \a id
- */
-
-/**
  * \class PipelineHandlerFactory
  * \brief Registration of PipelineHandler classes and creation of instances
  *
diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp
index e2026c99..fdbbda09 100644
--- a/test/list-cameras.cpp
+++ b/test/list-cameras.cpp
@@ -7,6 +7,7 @@
 
 #include <iostream>
 
+#include <libcamera/camera.h>
 #include <libcamera/camera_manager.h>
 
 #include "test.h"
@@ -29,8 +30,8 @@ protected:
 	{
 		unsigned int count = 0;
 
-		for (auto name : cm->list()) {
-			cout << "- " << name << endl;
+		for (Camera *camera : cm->cameras()) {
+			cout << "- " << camera->name() << endl;
 			count++;
 		}