[libcamera-devel] [RFC PATCH 03/17] libcamera: base: class: Don't pass Extensible pointer to Private constructor

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Jul 23 06:00:22 CEST 2021


The Extensible and Extensible::Private classes contain pointers to each
other. These pointers are initialized in the respective class's
constructor, by passing a pointer to the other class to each
constructor. This particular construct reduces the flexibility of the
Extensible pattern, as the Private class instance has to be allocated
and constructed in the members initializer list of the Extensible
class's constructor. It is thus impossible to perform any operation on
the Private class between its construction and the construction of the
Extensible class, or to subclass the Private class without subclassing
the Extensible class.

To make the design pattern more flexible, don't pass the pointer to the
Extensible class to the Private class's constructor, but initialize the
pointer manually in the Extensible class's constructor. This requires a
const_cast as the o_ member of the Private class is const.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 include/libcamera/base/class.h           |  3 ++-
 include/libcamera/internal/framebuffer.h |  2 +-
 src/android/camera_hal_config.cpp        |  7 +++----
 src/android/mm/cros_camera_buffer.cpp    |  4 ++--
 src/android/mm/generic_camera_buffer.cpp |  3 +--
 src/libcamera/base/class.cpp             |  6 +++---
 src/libcamera/camera.cpp                 | 10 +++++-----
 src/libcamera/camera_manager.cpp         |  8 ++++----
 src/libcamera/framebuffer.cpp            |  6 +++---
 9 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h
index 9c7f0f2e6e27..c9d9cd949ca1 100644
--- a/include/libcamera/base/class.h
+++ b/include/libcamera/base/class.h
@@ -64,7 +64,7 @@ public:
 	class Private
 	{
 	public:
-		Private(Extensible *o);
+		Private();
 		virtual ~Private();
 
 #ifndef __DOXYGEN__
@@ -82,6 +82,7 @@ public:
 #endif
 
 	private:
+		friend class Extensible;
 		Extensible *const o_;
 	};
 
diff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h
index a11e895d9b88..8c187adf70c7 100644
--- a/include/libcamera/internal/framebuffer.h
+++ b/include/libcamera/internal/framebuffer.h
@@ -52,7 +52,7 @@ class FrameBuffer::Private : public Extensible::Private
 	LIBCAMERA_DECLARE_PUBLIC(FrameBuffer)
 
 public:
-	Private(FrameBuffer *buffer);
+	Private();
 
 	void setRequest(Request *request) { request_ = request; }
 
diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp
index 833cf4ba98a9..bbbb1410b52c 100644
--- a/src/android/camera_hal_config.cpp
+++ b/src/android/camera_hal_config.cpp
@@ -32,7 +32,7 @@ class CameraHalConfig::Private : public Extensible::Private
 	LIBCAMERA_DECLARE_PUBLIC(CameraHalConfig)
 
 public:
-	Private(CameraHalConfig *halConfig);
+	Private();
 
 	int parseConfigFile(FILE *fh, std::map<std::string, CameraConfigData> *cameras);
 
@@ -50,8 +50,7 @@ private:
 	std::map<std::string, CameraConfigData> *cameras_;
 };
 
-CameraHalConfig::Private::Private(CameraHalConfig *halConfig)
-	: Extensible::Private(halConfig)
+CameraHalConfig::Private::Private()
 {
 }
 
@@ -344,7 +343,7 @@ int CameraHalConfig::Private::parseConfigFile(FILE *fh,
 }
 
 CameraHalConfig::CameraHalConfig()
-	: Extensible(new Private(this)), exists_(false), valid_(false)
+	: Extensible(new Private()), exists_(false), valid_(false)
 {
 	parseConfigurationFile();
 }
diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
index bb55b95e3a39..437502fb8276 100644
--- a/src/android/mm/cros_camera_buffer.cpp
+++ b/src/android/mm/cros_camera_buffer.cpp
@@ -47,8 +47,8 @@ private:
 CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,
 			       buffer_handle_t camera3Buffer,
 			       [[maybe_unused]] int flags)
-	: Extensible::Private(cameraBuffer), handle_(camera3Buffer),
-	  numPlanes_(0), valid_(false), registered_(false)
+	: handle_(camera3Buffer), numPlanes_(0), valid_(false),
+	  registered_(false)
 {
 	bufferManager_ = cros::CameraBufferManager::GetInstance();
 
diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
index 166be36efd5b..2a4b77ea5236 100644
--- a/src/android/mm/generic_camera_buffer.cpp
+++ b/src/android/mm/generic_camera_buffer.cpp
@@ -34,9 +34,8 @@ public:
 	size_t jpegBufferSize(size_t maxJpegBufferSize) const;
 };
 
-CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,
+CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
 			       buffer_handle_t camera3Buffer, int flags)
-	: Extensible::Private(cameraBuffer)
 {
 	maps_.reserve(camera3Buffer->numFds);
 	error_ = 0;
diff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp
index d4f0ac64ad48..d24043c20e55 100644
--- a/src/libcamera/base/class.cpp
+++ b/src/libcamera/base/class.cpp
@@ -151,6 +151,7 @@ namespace libcamera {
 Extensible::Extensible(Extensible::Private *d)
 	: d_(d)
 {
+	*const_cast<Extensible **>(&d_->o_) = this;
 }
 
 /**
@@ -182,10 +183,9 @@ Extensible::Extensible(Extensible::Private *d)
 
 /**
  * \brief Construct an instance of an Extensible class private data
- * \param[in] o Pointer to the public class object
  */
-Extensible::Private::Private(Extensible *o)
-	: o_(o)
+Extensible::Private::Private()
+	: o_(nullptr)
 {
 }
 
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index c8858e71d105..c126b49290ce 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -344,7 +344,7 @@ public:
 		CameraRunning,
 	};
 
-	Private(Camera *camera, PipelineHandler *pipe, const std::string &id,
+	Private(PipelineHandler *pipe, const std::string &id,
 		const std::set<Stream *> &streams);
 	~Private();
 
@@ -368,11 +368,11 @@ private:
 	std::atomic<State> state_;
 };
 
-Camera::Private::Private(Camera *camera, PipelineHandler *pipe,
+Camera::Private::Private(PipelineHandler *pipe,
 			 const std::string &id,
 			 const std::set<Stream *> &streams)
-	: Extensible::Private(camera), pipe_(pipe->shared_from_this()), id_(id),
-	  streams_(streams), disconnected_(false), state_(CameraAvailable)
+	: pipe_(pipe->shared_from_this()), id_(id), streams_(streams),
+	  disconnected_(false), state_(CameraAvailable)
 {
 }
 
@@ -632,7 +632,7 @@ const std::string &Camera::id() const
 
 Camera::Camera(PipelineHandler *pipe, const std::string &id,
 	       const std::set<Stream *> &streams)
-	: Extensible(new Private(this, pipe, id, streams))
+	: Extensible(new Private(pipe, id, streams))
 {
 }
 
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 1c79308ad4b5..384a574f2baa 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -39,7 +39,7 @@ class CameraManager::Private : public Extensible::Private, public Thread
 	LIBCAMERA_DECLARE_PUBLIC(CameraManager)
 
 public:
-	Private(CameraManager *cm);
+	Private();
 
 	int start();
 	void addCamera(std::shared_ptr<Camera> camera,
@@ -74,8 +74,8 @@ private:
 	ProcessManager processManager_;
 };
 
-CameraManager::Private::Private(CameraManager *cm)
-	: Extensible::Private(cm), initialized_(false)
+CameraManager::Private::Private()
+	: initialized_(false)
 {
 }
 
@@ -258,7 +258,7 @@ void CameraManager::Private::removeCamera(Camera *camera)
 CameraManager *CameraManager::self_ = nullptr;
 
 CameraManager::CameraManager()
-	: Extensible(new CameraManager::Private(this))
+	: Extensible(new CameraManager::Private())
 {
 	if (self_)
 		LOG(Camera, Fatal)
diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
index 40bf64b0a4fe..48d14b31f68d 100644
--- a/src/libcamera/framebuffer.cpp
+++ b/src/libcamera/framebuffer.cpp
@@ -100,8 +100,8 @@ LOG_DEFINE_CATEGORY(Buffer)
  * \brief Array of per-plane metadata
  */
 
-FrameBuffer::Private::Private(FrameBuffer *buffer)
-	: Extensible::Private(buffer), request_(nullptr)
+FrameBuffer::Private::Private()
+	: request_(nullptr)
 {
 }
 
@@ -176,7 +176,7 @@ FrameBuffer::Private::Private(FrameBuffer *buffer)
  * \param[in] cookie Cookie
  */
 FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
-	: Extensible(new Private(this)), planes_(planes), cookie_(cookie)
+	: Extensible(new Private()), planes_(planes), cookie_(cookie)
 {
 }
 
-- 
Regards,

Laurent Pinchart



More information about the libcamera-devel mailing list