[libcamera-devel] [PATCH 03/10] libcamera: camera: Associate cameras with their pipeline handler
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jan 24 11:16:44 CET 2019
From: Niklas Söderlund <niklas.soderlund at ragnatech.se>
The PipelineHandler which creates a Camera is responsible for serving
any operation requested by the user. In order forward the public API
calls, the camera needs to store a reference to its pipeline handler.
Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
Changes since v1:
- Create pipeline handlers is shared pointers, make them inherit from
std::enable_shared_from_this<> and stored them in shared pointers.
---
include/libcamera/camera.h | 8 ++++++--
include/libcamera/camera_manager.h | 2 +-
src/libcamera/camera.cpp | 16 ++++++++++------
src/libcamera/camera_manager.cpp | 17 +++++++++--------
src/libcamera/include/pipeline_handler.h | 9 +++++----
src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
src/libcamera/pipeline/uvcvideo.cpp | 2 +-
src/libcamera/pipeline/vimc.cpp | 9 +--------
src/libcamera/pipeline_handler.cpp | 10 ++++++++++
9 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index 2ea1a6883311..efafb9e28c56 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -12,10 +12,13 @@
namespace libcamera {
+class PipelineHandler;
+
class Camera final
{
public:
- static std::shared_ptr<Camera> create(const std::string &name);
+ static std::shared_ptr<Camera> create(PipelineHandler *pipe,
+ const std::string &name);
Camera(const Camera &) = delete;
void operator=(const Camera &) = delete;
@@ -23,9 +26,10 @@ public:
const std::string &name() const;
private:
- explicit Camera(const std::string &name);
+ Camera(PipelineHandler *pipe, const std::string &name);
~Camera();
+ std::shared_ptr<PipelineHandler> pipe_;
std::string name_;
};
diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
index 9ade29d76692..45e72df0ef65 100644
--- a/include/libcamera/camera_manager.h
+++ b/include/libcamera/camera_manager.h
@@ -41,7 +41,7 @@ private:
~CameraManager();
std::unique_ptr<DeviceEnumerator> enumerator_;
- std::vector<PipelineHandler *> pipes_;
+ std::vector<std::shared_ptr<PipelineHandler>> pipes_;
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 acf912bee95c..3a531c7e4d8f 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -8,6 +8,7 @@
#include <libcamera/camera.h>
#include "log.h"
+#include "pipeline_handler.h"
/**
* \file camera.h
@@ -52,17 +53,20 @@ namespace libcamera {
/**
* \brief Create a camera instance
* \param[in] name The name of the camera device
+ * \param[in] pipe The pipeline handler responsible for the camera device
*
* The caller is responsible for guaranteeing unicity of the camera name.
*
* \return A shared pointer to the newly created camera object
*/
-std::shared_ptr<Camera> Camera::create(const std::string &name)
+std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
+ const std::string &name)
{
struct Allocator : std::allocator<Camera> {
- void construct(void *p, const std::string &name)
+ void construct(void *p, PipelineHandler *pipe,
+ const std::string &name)
{
- ::new(p) Camera(name);
+ ::new(p) Camera(pipe, name);
}
void destroy(Camera *p)
{
@@ -70,7 +74,7 @@ std::shared_ptr<Camera> Camera::create(const std::string &name)
}
};
- return std::allocate_shared<Camera>(Allocator(), name);
+ return std::allocate_shared<Camera>(Allocator(), pipe, name);
}
/**
@@ -83,8 +87,8 @@ const std::string &Camera::name() const
return name_;
}
-Camera::Camera(const std::string &name)
- : name_(name)
+Camera::Camera(PipelineHandler *pipe, const std::string &name)
+ : pipe_(pipe->shared_from_this()), name_(name)
{
}
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 14410d4dcda7..3eccf20c4ce9 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -98,16 +98,14 @@ int CameraManager::start()
* all pipelines it can provide.
*/
while (1) {
- PipelineHandler *pipe = factory->create(this);
- if (!pipe->match(enumerator_.get())) {
- delete pipe;
+ 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(pipe);
+ pipes_.push_back(std::move(pipe));
}
}
@@ -130,10 +128,13 @@ void CameraManager::stop()
{
/* TODO: unregister hot-plug callback here */
- for (PipelineHandler *pipe : pipes_)
- delete pipe;
-
+ /*
+ * 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);
}
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index 1da6dc758ca6..e1d6369eb0c4 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -8,6 +8,7 @@
#define __LIBCAMERA_PIPELINE_HANDLER_H__
#include <map>
+#include <memory>
#include <string>
#include <vector>
@@ -16,7 +17,7 @@ namespace libcamera {
class CameraManager;
class DeviceEnumerator;
-class PipelineHandler
+class PipelineHandler : public std::enable_shared_from_this<PipelineHandler>
{
public:
PipelineHandler(CameraManager *manager);
@@ -34,7 +35,7 @@ public:
PipelineHandlerFactory(const char *name);
virtual ~PipelineHandlerFactory() { };
- virtual PipelineHandler *create(CameraManager *manager) = 0;
+ virtual std::shared_ptr<PipelineHandler> create(CameraManager *manager) = 0;
const std::string &name() const { return name_; }
@@ -50,9 +51,9 @@ class handler##Factory final : public PipelineHandlerFactory \
{ \
public: \
handler##Factory() : PipelineHandlerFactory(#handler) {} \
- PipelineHandler *create(CameraManager *manager) \
+ std::shared_ptr<PipelineHandler> create(CameraManager *manager) \
{ \
- return new handler(manager); \
+ return std::make_shared<handler>(manager); \
} \
}; \
static handler##Factory global_##handler##Factory;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 589b3078f301..13ff7da4c99e 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -171,7 +171,7 @@ void PipelineHandlerIPU3::registerCameras()
continue;
std::string cameraName = sensor->name() + " " + std::to_string(id);
- std::shared_ptr<Camera> camera = Camera::create(cameraName);
+ std::shared_ptr<Camera> camera = Camera::create(this, cameraName);
manager_->addCamera(std::move(camera));
LOG(IPU3, Info)
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index 92b23da73901..3ebc074093ab 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
dev_->acquire();
- std::shared_ptr<Camera> camera = Camera::create(dev_->model());
+ std::shared_ptr<Camera> camera = Camera::create(this, dev_->model());
manager_->addCamera(std::move(camera));
return true;
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index f12d007cd956..68bfe9b12ab6 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -57,14 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
dev_->acquire();
- /*
- * NOTE: A more complete Camera implementation could
- * be passed the MediaDevice(s) it controls here or
- * a reference to the PipelineHandler. Which method
- * will be chosen depends on how the Camera
- * object is modeled.
- */
- std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera");
+ std::shared_ptr<Camera> camera = Camera::create(this, "Dummy VIMC Camera");
manager_->addCamera(std::move(camera));
return true;
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 45788487b5c0..3850ea8fadb5 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -32,11 +32,21 @@ LOG_DEFINE_CATEGORY(Pipeline)
*
* The PipelineHandler matches the media devices provided by a DeviceEnumerator
* with the pipelines it supports and creates corresponding Camera devices.
+ *
+ * Pipeline handler instances are reference-counted through std::shared_ptr<>.
+ * They implement std::enable_shared_from_this<> in order to create new
+ * std::shared_ptr<> in code paths originating from member functions of the
+ * PipelineHandler class where only the 'this' pointer is available.
*/
/**
* \brief Construct a PipelineHandler instance
* \param[in] manager The camera manager
+ *
+ * In order to honour the std::enable_shared_from_this<> contract,
+ * PipelineHandler instances shall never be constructed manually, but always
+ * through the PipelineHandlerFactory::create() method implemented by the
+ * respective factories.
*/
PipelineHandler::PipelineHandler(CameraManager *manager)
: manager_(manager)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list