From 33dd4fab9d39726be4fcbd300a27f2640be1cd6f Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Fri, 23 Jul 2021 02:41:16 +0300 Subject: libcamera: base: class: Don't pass Extensible pointer to Private constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- 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 +++--- 7 files changed, 21 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp index 833cf4ba..bbbb1410 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 *cameras); @@ -50,8 +50,7 @@ private: std::map *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 bb55b95e..437502fb 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 166be36e..2a4b77ea 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 d4f0ac64..d24043c2 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(&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 c8858e71..c126b492 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 &streams); ~Private(); @@ -368,11 +368,11 @@ private: std::atomic state_; }; -Camera::Private::Private(Camera *camera, PipelineHandler *pipe, +Camera::Private::Private(PipelineHandler *pipe, const std::string &id, const std::set &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 &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 1c79308a..384a574f 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, @@ -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 40bf64b0..48d14b31 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 &planes, unsigned int cookie) - : Extensible(new Private(this)), planes_(planes), cookie_(cookie) + : Extensible(new Private()), planes_(planes), cookie_(cookie) { } -- cgit v1.2.1