[libcamera-devel] [PATCH 4/4] libcamera: camera: Handle camera objects through shared pointers

[Laurent Pinchart]

The Camera class is explicitly reference-counted to manage the lifetime
of camera objects. Replace this open-coded implementation with usage of
the std::shared_ptr<> class.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 include/libcamera/camera.h         | 13 +++++----
 include/libcamera/camera_manager.h |  8 +++---
 src/libcamera/camera.cpp           | 46 +++++++++++++++++-------------
 src/libcamera/camera_manager.cpp   | 20 ++++++-------
 src/libcamera/pipeline/vimc.cpp    |  2 +-
 test/list-cameras.cpp              |  2 +-
 6 files changed, 50 insertions(+), 41 deletions(-)

diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index 9a7579d61fa3..ef0a794e3a82 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -7,6 +7,7 @@
 #ifndef __LIBCAMERA_CAMERA_H__
 #define __LIBCAMERA_CAMERA_H__
 
+#include <memory>
 #include <string>
 
 namespace libcamera {
@@ -14,15 +15,17 @@ namespace libcamera {
 class Camera
 {
 public:
-	Camera(const std::string &name);
+	static std::shared_ptr<Camera> create(const std::string &name);
+
+	Camera(const Camera &) = delete;
+	void operator=(const Camera &) = delete;
 
 	const std::string &name() const;
-	void get();
-	void put();
 
 private:
-	virtual ~Camera() { };
-	int ref_;
+	Camera(const std::string &name);
+	~Camera();
+
 	std::string name_;
 };
 
diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
index 4b941fd9183b..738b13f4cfb1 100644
--- a/include/libcamera/camera_manager.h
+++ b/include/libcamera/camera_manager.h
@@ -24,10 +24,10 @@ public:
 	int start();
 	void stop();
 
-	const std::vector<Camera *> &cameras() const { return cameras_; }
-	Camera *get(const std::string &name);
+	const std::vector<std::shared_ptr<Camera>> &cameras() const { return cameras_; }
+	std::shared_ptr<Camera> get(const std::string &name);
 
-	void addCamera(Camera *camera);
+	void addCamera(std::shared_ptr<Camera> &camera);
 
 	static CameraManager *instance();
 
@@ -42,7 +42,7 @@ private:
 
 	std::unique_ptr<DeviceEnumerator> enumerator_;
 	std::vector<PipelineHandler *> pipes_;
-	std::vector<Camera *> cameras_;
+	std::vector<std::shared_ptr<Camera>> cameras_;
 
 	std::unique_ptr<EventDispatcher> dispatcher_;
 };
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 6da2b20137d4..acf912bee95c 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -41,19 +41,36 @@ namespace libcamera {
  * streams from a single image source. It provides the main interface to
  * configuring and controlling the device, and capturing image streams. It is
  * the central object exposed by libcamera.
+ *
+ * To support the central nature of Camera objects, libcamera manages the
+ * lifetime of camera instances with std::shared_ptr<>. Instances shall be
+ * created with the create() function which returns a shared pointer. The
+ * Camera constructors and destructor are private, to prevent instances from
+ * being constructed and destroyed manually.
  */
 
 /**
- * \brief Construct a named camera device
+ * \brief Create a camera instance
+ * \param[in] name The name of the camera device
  *
- * \param[in] name The name to set on the camera device
+ * The caller is responsible for guaranteeing unicity of the camera name.
  *
- * The caller is responsible for guaranteeing unicity of the camera
- * device name.
+ * \return A shared pointer to the newly created camera object
  */
-Camera::Camera(const std::string &name)
-	: ref_(1), name_(name)
+std::shared_ptr<Camera> Camera::create(const std::string &name)
 {
+	struct Allocator : std::allocator<Camera> {
+		void construct(void *p, const std::string &name)
+		{
+			::new(p) Camera(name);
+		}
+		void destroy(Camera *p)
+		{
+			p->~Camera();
+		}
+	};
+
+	return std::allocate_shared<Camera>(Allocator(), name);
 }
 
 /**
@@ -66,24 +83,13 @@ const std::string &Camera::name() const
 	return name_;
 }
 
-/**
- * \brief Acquire a reference to the camera
- */
-void Camera::get()
+Camera::Camera(const std::string &name)
+	: name_(name)
 {
-	ref_++;
 }
 
-/**
- * \brief Release a reference to the camera
- *
- * When the last reference is released the camera device is deleted. Callers
- * shall not access the camera device after calling this function.
- */
-void Camera::put()
+Camera::~Camera()
 {
-	if (--ref_ == 0)
-		delete this;
 }
 
 } /* namespace libcamera */
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 852e5ed70fe3..d12bd42001ae 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -39,9 +39,11 @@ namespace libcamera {
  * This will enumerate all the cameras present in the system, which can then be
  * listed with list() and retrieved with get().
  *
- * Cameras are reference-counted, and shall be returned to the camera manager
- * with Camera::put() after being used. Once all cameras have been returned to
- * the manager, it can be stopped with stop().
+ * Cameras are shared through std::shared_ptr<>, ensuring that a camera will
+ * stay valid until the last reference is released without requiring any special
+ * action from the application. Once the application has released all the
+ * references it held to cameras, the camera manager can be stopped with
+ * stop().
  *
  * \todo Add ability to add and remove media devices based on hot-(un)plug
  * events coming from the device enumerator.
@@ -147,15 +149,13 @@ void CameraManager::stop()
  * \param[in] name Name of camera to get
  *
  * Before calling this function the caller is responsible for ensuring that
- * the camera manger is running. A camera fetched this way shall be
- * released by the user with the put() method of the Camera object once
- * it is done using the camera.
+ * the camera manger is running.
  *
- * \return Pointer to Camera object or nullptr if camera not found
+ * \return Shared pointer to Camera object or nullptr if camera not found
  */
-Camera *CameraManager::get(const std::string &name)
+std::shared_ptr<Camera> CameraManager::get(const std::string &name)
 {
-	for (Camera *camera : cameras_) {
+	for (std::shared_ptr<Camera> camera : cameras_) {
 		if (camera->name() == name)
 			return camera;
 	}
@@ -171,7 +171,7 @@ Camera *CameraManager::get(const std::string &name)
  * handle with the camera manager. Registered cameras are immediately made
  * available to the system.
  */
-void CameraManager::addCamera(Camera *camera)
+void CameraManager::addCamera(std::shared_ptr<Camera> &camera)
 {
 	cameras_.push_back(camera);
 }
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 8742e0bae9a8..306a5b85cd60 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -64,7 +64,7 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator
 	 * will be chosen depends on how the Camera
 	 * object is modeled.
 	 */
-	Camera *camera = new Camera("Dummy VIMC Camera");
+	std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera");
 	manager->addCamera(camera);
 
 	return true;
diff --git a/test/list-cameras.cpp b/test/list-cameras.cpp
index fdbbda0957b2..070cbf2be977 100644
--- a/test/list-cameras.cpp
+++ b/test/list-cameras.cpp
@@ -30,7 +30,7 @@ protected:
 	{
 		unsigned int count = 0;
 
-		for (Camera *camera : cm->cameras()) {
+		for (const std::shared_ptr<Camera> &camera : cm->cameras()) {
 			cout << "- " << camera->name() << endl;
 			count++;
 		}
-- 
Regards,

Laurent Pinchart