libcamera: camera_manager: Move private data members to private implementation

Use the d-pointer idiom ([1], [2]) to hide the private data members from
the CameraManager class interface. This will ease maintaining ABI
compatibility, and prepares for the implementation of the CameraManager
class threading model.

[1] https://wiki.qt.io/D-Pointer
[2] https://en.cppreference.com/w/cpp/language/pimpl

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Documentation/Doxyfile.in

@@ -877,6 +877,7 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
                          libcamera::BoundMethodPackBase \
                          libcamera::BoundMethodStatic \
                          libcamera::SignalBase \
+                         libcamera::*::Private \
                          std::*
 
 # The EXAMPLE_PATH tag can be used to specify one or more files or directories

include/libcamera/camera_manager.h

@@ -7,7 +7,6 @@
 #ifndef __LIBCAMERA_CAMERA_MANAGER_H__
 #define __LIBCAMERA_CAMERA_MANAGER_H__
 
-#include <map>
 #include <memory>
 #include <string>
 #include <sys/types.h>
@@ -18,9 +17,7 @@
 namespace libcamera {
 
 class Camera;
-class DeviceEnumerator;
 class EventDispatcher;
-class PipelineHandler;
 
 class CameraManager : public Object
 {
@@ -33,7 +30,7 @@ public:
 	int start();
 	void stop();
 
-	const std::vector<std::shared_ptr<Camera>> &cameras() const { return cameras_; }
+	const std::vector<std::shared_ptr<Camera>> &cameras() const;
 	std::shared_ptr<Camera> get(const std::string &name);
 	std::shared_ptr<Camera> get(dev_t devnum);
 
@@ -46,13 +43,11 @@ public:
 	EventDispatcher *eventDispatcher();
 
 private:
-	std::unique_ptr<DeviceEnumerator> enumerator_;
-	std::vector<std::shared_ptr<PipelineHandler>> pipes_;
-	std::vector<std::shared_ptr<Camera>> cameras_;
-	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
-
 	static const std::string version_;
 	static CameraManager *self_;
+
+	class Private;
+	std::unique_ptr<Private> p_;
 };
 
 } /* namespace libcamera */

src/libcamera/camera_manager.cpp

@@ -7,6 +7,8 @@
 
 #include <libcamera/camera_manager.h>
 
+#include <map>
+
 #include <libcamera/camera.h>
 #include <libcamera/event_dispatcher.h>
 
@@ -26,6 +28,124 @@ namespace libcamera {
 
 LOG_DEFINE_CATEGORY(Camera)
 
+class CameraManager::Private
+{
+public:
+	Private(CameraManager *cm);
+
+	int start();
+	void stop();
+
+	void addCamera(std::shared_ptr<Camera> &camera, dev_t devnum);
+	void removeCamera(Camera *camera);
+
+	std::vector<std::shared_ptr<Camera>> cameras_;
+	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
+
+private:
+	CameraManager *cm_;
+
+	std::vector<std::shared_ptr<PipelineHandler>> pipes_;
+	std::unique_ptr<DeviceEnumerator> enumerator_;
+};
+
+CameraManager::Private::Private(CameraManager *cm)
+	: cm_(cm)
+{
+}
+
+int CameraManager::Private::start()
+{
+	enumerator_ = DeviceEnumerator::create();
+	if (!enumerator_ || enumerator_->enumerate())
+		return -ENODEV;
+
+	/*
+	 * TODO: Try to read handlers and order from configuration
+	 * file and only fallback on all handlers if there is no
+	 * configuration file.
+	 */
+	std::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();
+
+	for (PipelineHandlerFactory *factory : factories) {
+		/*
+		 * Try each pipeline handler until it exhaust
+		 * all pipelines it can provide.
+		 */
+		while (1) {
+			std::shared_ptr<PipelineHandler> pipe = factory->create(cm_);
+			if (!pipe->match(enumerator_.get()))
+				break;
+
+			LOG(Camera, Debug)
+				<< "Pipeline handler \"" << factory->name()
+				<< "\" matched";
+			pipes_.push_back(std::move(pipe));
+		}
+	}
+
+	/* TODO: register hot-plug callback here */
+
+	return 0;
+}
+
+void CameraManager::Private::stop()
+{
+	/* TODO: unregister hot-plug callback here */
+
+	/*
+	 * Release all references to cameras and pipeline handlers to ensure
+	 * they all get destroyed before the device enumerator deletes the
+	 * media devices.
+	 */
+	pipes_.clear();
+	cameras_.clear();
+
+	enumerator_.reset(nullptr);
+}
+
+void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,
+				       dev_t devnum)
+{
+	for (std::shared_ptr<Camera> c : cameras_) {
+		if (c->name() == camera->name()) {
+			LOG(Camera, Warning)
+				<< "Registering camera with duplicate name '"
+				<< camera->name() << "'";
+			break;
+		}
+	}
+
+	cameras_.push_back(std::move(camera));
+
+	if (devnum) {
+		unsigned int index = cameras_.size() - 1;
+		camerasByDevnum_[devnum] = cameras_[index];
+	}
+}
+
+void CameraManager::Private::removeCamera(Camera *camera)
+{
+	auto iter = std::find_if(cameras_.begin(), cameras_.end(),
+				 [camera](std::shared_ptr<Camera> &c) {
+					 return c.get() == camera;
+				 });
+	if (iter == cameras_.end())
+		return;
+
+	LOG(Camera, Debug)
+		<< "Unregistering camera '" << camera->name() << "'";
+
+	auto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),
+				   [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {
+					   return p.second.lock().get() == camera;
+				   });
+	if (iter_d != camerasByDevnum_.end())
+		camerasByDevnum_.erase(iter_d);
+
+	cameras_.erase(iter);
+}
+
 /**
  * \class CameraManager
  * \brief Provide access and manage all cameras in the system
@@ -62,7 +182,7 @@ LOG_DEFINE_CATEGORY(Camera)
 CameraManager *CameraManager::self_ = nullptr;
 
 CameraManager::CameraManager()
-	: enumerator_(nullptr)
+	: p_(new CameraManager::Private(this))
 {
 	if (self_)
 		LOG(Camera, Fatal)
@@ -73,6 +193,8 @@ CameraManager::CameraManager()
 
 CameraManager::~CameraManager()
 {
+	stop();
+
 	self_ = nullptr;
 }
 
@@ -88,42 +210,14 @@ CameraManager::~CameraManager()
  */
 int CameraManager::start()
 {
-	if (enumerator_)
-		return -EBUSY;
-
 	LOG(Camera, Info) << "libcamera " << version_;
 
-	enumerator_ = DeviceEnumerator::create();
+	int ret = p_->start();
-	if (!enumerator_ || enumerator_->enumerate())
+	if (ret)
-		return -ENODEV;
+		LOG(Camera, Error) << "Failed to start camera manager: "
+				   << strerror(-ret);
 
-	/*
+	return ret;
-	 * TODO: Try to read handlers and order from configuration
-	 * file and only fallback on all handlers if there is no
-	 * configuration file.
-	 */
-	std::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();
-
-	for (PipelineHandlerFactory *factory : factories) {
-		/*
-		 * Try each pipeline handler until it exhaust
-		 * all pipelines it can provide.
-		 */
-		while (1) {
-			std::shared_ptr<PipelineHandler> pipe = factory->create(this);
-			if (!pipe->match(enumerator_.get()))
-				break;
-
-			LOG(Camera, Debug)
-				<< "Pipeline handler \"" << factory->name()
-				<< "\" matched";
-			pipes_.push_back(std::move(pipe));
-		}
-	}
-
-	/* TODO: register hot-plug callback here */
-
-	return 0;
 }
 
 /**
@@ -138,17 +232,7 @@ int CameraManager::start()
  */
 void CameraManager::stop()
 {
-	/* TODO: unregister hot-plug callback here */
+	p_->stop();
-
-	/*
-	 * Release all references to cameras and pipeline handlers to ensure
-	 * they all get destroyed before the device enumerator deletes the
-	 * media devices.
-	 */
-	pipes_.clear();
-	cameras_.clear();
-
-	enumerator_.reset(nullptr);
 }
 
 /**
@@ -160,6 +244,10 @@ void CameraManager::stop()
  *
  * \return List of all available cameras
  */
+const std::vector<std::shared_ptr<Camera>> &CameraManager::cameras() const
+{
+	return p_->cameras_;
+}
 
 /**
  * \brief Get a camera based on name
@@ -172,7 +260,7 @@ void CameraManager::stop()
  */
 std::shared_ptr<Camera> CameraManager::get(const std::string &name)
 {
-	for (std::shared_ptr<Camera> camera : cameras_) {
+	for (std::shared_ptr<Camera> camera : p_->cameras_) {
 		if (camera->name() == name)
 			return camera;
 	}
@@ -196,8 +284,8 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &name)
  */
 std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
 {
-	auto iter = camerasByDevnum_.find(devnum);
+	auto iter = p_->camerasByDevnum_.find(devnum);
-	if (iter == camerasByDevnum_.end())
+	if (iter == p_->camerasByDevnum_.end())
 		return nullptr;
 
 	return iter->second.lock();
@@ -217,21 +305,8 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
  */
 void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
 {
-	for (std::shared_ptr<Camera> c : cameras_) {
-		if (c->name() == camera->name()) {
-			LOG(Camera, Warning)
-				<< "Registering camera with duplicate name '"
-				<< camera->name() << "'";
-			break;
-		}
-	}
 
-	cameras_.push_back(std::move(camera));
+	p_->addCamera(camera, devnum);
-
-	if (devnum) {
-		unsigned int index = cameras_.size() - 1;
-		camerasByDevnum_[devnum] = cameras_[index];
-	}
 }
 
 /**
@@ -244,24 +319,7 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
  */
 void CameraManager::removeCamera(Camera *camera)
 {
-	auto iter = std::find_if(cameras_.begin(), cameras_.end(),
+	p_->removeCamera(camera);
-				 [camera](std::shared_ptr<Camera> &c) {
-					 return c.get() == camera;
-				 });
-	if (iter == cameras_.end())
-		return;
-
-	LOG(Camera, Debug)
-		<< "Unregistering camera '" << camera->name() << "'";
-
-	auto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),
-				   [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {
-					   return p.second.lock().get() == camera;
-				   });
-	if (iter_d != camerasByDevnum_.end())
-		camerasByDevnum_.erase(iter_d);
-
-	cameras_.erase(iter);
 }
 
 /**