[libcamera-devel] [PATCH v3 01/14] libcamera: base: extensible: Pass private pointer as unique_ptr<>
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Aug 12 09:21:05 CEST 2021
Hi Laurent,
On 12/08/2021 00:26, Laurent Pinchart wrote:
> The Extensible constructor takes a pointer to a Private instance, whose
> lifetime it then manages. Make this explicit in the API by passing the
> pointer as a std::unique_ptr<Private>.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/base/class.h | 2 +-
> src/android/camera_buffer.h | 2 +-
> src/android/camera_hal_config.cpp | 2 +-
> src/libcamera/base/class.cpp | 11 +++++++++--
> src/libcamera/camera.cpp | 2 +-
> src/libcamera/camera_manager.cpp | 2 +-
> src/libcamera/framebuffer.cpp | 3 ++-
> 7 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h
> index a5946836d2b9..9a82d61472c1 100644
> --- a/include/libcamera/base/class.h
> +++ b/include/libcamera/base/class.h
> @@ -87,7 +87,7 @@ public:
> Extensible *const o_;
> };
>
> - Extensible(Private *d);
> + Extensible(std::unique_ptr<Private> d);
>
> protected:
> template<typename T>
> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> index 21373fa25bf8..c4e3a9e7b912 100644
> --- a/src/android/camera_buffer.h
> +++ b/src/android/camera_buffer.h
> @@ -32,7 +32,7 @@ public:
>
> #define PUBLIC_CAMERA_BUFFER_IMPLEMENTATION \
> CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) \
> - : Extensible(new Private(this, camera3Buffer, flags)) \
> + : Extensible(std::make_unique<Private>(this, camera3Buffer, flags)) \
> { \
> } \
> CameraBuffer::~CameraBuffer() \
> diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp
> index 14549083a97a..aa90dac78075 100644
> --- a/src/android/camera_hal_config.cpp
> +++ b/src/android/camera_hal_config.cpp
> @@ -341,7 +341,7 @@ int CameraHalConfig::Private::parseConfigFile(FILE *fh,
> }
>
> CameraHalConfig::CameraHalConfig()
> - : Extensible(new Private()), exists_(false), valid_(false)
> + : Extensible(std::make_unique<Private>()), exists_(false), valid_(false)
> {
> parseConfigurationFile();
> }
> diff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp
> index d24043c20e55..9c2d9f218bd4 100644
> --- a/src/libcamera/base/class.cpp
> +++ b/src/libcamera/base/class.cpp
> @@ -147,9 +147,12 @@ namespace libcamera {
> /**
> * \brief Construct an instance of an Extensible class
> * \param[in] d Pointer to the private data instance
> + *
> + * The private data lifetime is managed by the Extensible class, which destroys
> + * it when the Extensible instance is destroyed.
> */
> -Extensible::Extensible(Extensible::Private *d)
> - : d_(d)
> +Extensible::Extensible(std::unique_ptr<Extensible::Private> d)
> + : d_(std::move(d))
> {
> *const_cast<Extensible **>(&d_->o_) = this;
> }
> @@ -163,6 +166,10 @@ Extensible::Extensible(Extensible::Private *d)
> * overriden _d() functions that return the correct pointer type to the
> * corresponding derived Private class.
> *
> + * The lifetime of the private data is tied to the Extensible class. The caller
> + * shall not retain any reference to the returned pointer for longer than it
> + * holds a reference to the Extensible instance.
> + *
> * \return A pointer to the private data instance
> */
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 6281e92057e4..8e99e2a96eaf 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -596,7 +596,7 @@ const std::string &Camera::id() const
>
> Camera::Camera(PipelineHandler *pipe, const std::string &id,
> const std::set<Stream *> &streams)
> - : Extensible(new Private(pipe, id, streams))
> + : Extensible(std::make_unique<Private>(pipe, id, streams))
> {
> }
>
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 893aa6067171..fe80a46f5d20 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -258,7 +258,7 @@ void CameraManager::Private::removeCamera(Camera *camera)
> CameraManager *CameraManager::self_ = nullptr;
>
> CameraManager::CameraManager()
> - : Extensible(new CameraManager::Private())
> + : Extensible(std::make_unique<CameraManager::Private>())
> {
> if (self_)
> LOG(Camera, Fatal)
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 42c73134152b..b401c50efd53 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -181,7 +181,8 @@ FrameBuffer::Private::Private()
> * \param[in] cookie Cookie
> */
> FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
> - : Extensible(new Private()), planes_(planes), cookie_(cookie)
> + : Extensible(std::make_unique<Private>()), planes_(planes),
> + cookie_(cookie)
> {
> }
>
>
More information about the libcamera-devel
mailing list