[libcamera-devel] [PATCH v2 02/13] libcamera: camera_manager: Move private data members to private implementation
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jan 22 21:57:12 CET 2020
Use the d-pointer idiom ([1], [2]) to hide the private data members from
the CameraManager class interface. This will ease maintaining ABI
compatibility, and prepares for the implementation of the CameraManager
class threading model.
[1] https://wiki.qt.io/D-Pointer
[2] https://en.cppreference.com/w/cpp/language/pimpl
libcamera: camera_manager: Run the camera manager in a thread:q
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
Documentation/Doxyfile.in | 1 +
include/libcamera/camera_manager.h | 13 +-
src/libcamera/camera_manager.cpp | 218 ++++++++++++++++++-----------
3 files changed, 143 insertions(+), 89 deletions(-)
diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
index 8e6fbdbb92b6..1f746393568a 100644
--- a/Documentation/Doxyfile.in
+++ b/Documentation/Doxyfile.in
@@ -877,6 +877,7 @@ EXCLUDE_SYMBOLS = libcamera::BoundMethodArgs \
libcamera::BoundMethodPackBase \
libcamera::BoundMethodStatic \
libcamera::SignalBase \
+ libcamera::*::Private \
std::*
# The EXAMPLE_PATH tag can be used to specify one or more files or directories
diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
index 094197668698..068afd58762f 100644
--- a/include/libcamera/camera_manager.h
+++ b/include/libcamera/camera_manager.h
@@ -7,7 +7,6 @@
#ifndef __LIBCAMERA_CAMERA_MANAGER_H__
#define __LIBCAMERA_CAMERA_MANAGER_H__
-#include <map>
#include <memory>
#include <string>
#include <sys/types.h>
@@ -18,9 +17,7 @@
namespace libcamera {
class Camera;
-class DeviceEnumerator;
class EventDispatcher;
-class PipelineHandler;
class CameraManager : public Object
{
@@ -33,7 +30,7 @@ public:
int start();
void stop();
- const std::vector<std::shared_ptr<Camera>> &cameras() const { return cameras_; }
+ const std::vector<std::shared_ptr<Camera>> &cameras() const;
std::shared_ptr<Camera> get(const std::string &name);
std::shared_ptr<Camera> get(dev_t devnum);
@@ -46,13 +43,11 @@ public:
EventDispatcher *eventDispatcher();
private:
- std::unique_ptr<DeviceEnumerator> enumerator_;
- std::vector<std::shared_ptr<PipelineHandler>> pipes_;
- std::vector<std::shared_ptr<Camera>> cameras_;
- std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
-
static const std::string version_;
static CameraManager *self_;
+
+ class Private;
+ std::unique_ptr<Private> p_;
};
} /* namespace libcamera */
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index a71caf8536bb..e0a07ec557d3 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -7,6 +7,8 @@
#include <libcamera/camera_manager.h>
+#include <map>
+
#include <libcamera/camera.h>
#include <libcamera/event_dispatcher.h>
@@ -26,6 +28,124 @@ namespace libcamera {
LOG_DEFINE_CATEGORY(Camera)
+class CameraManager::Private
+{
+public:
+ Private(CameraManager *cm);
+
+ int start();
+ void stop();
+
+ void addCamera(std::shared_ptr<Camera> &camera, dev_t devnum);
+ void removeCamera(Camera *camera);
+
+ std::vector<std::shared_ptr<Camera>> cameras_;
+ std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
+
+private:
+ CameraManager *cm_;
+
+ std::vector<std::shared_ptr<PipelineHandler>> pipes_;
+ std::unique_ptr<DeviceEnumerator> enumerator_;
+};
+
+CameraManager::Private::Private(CameraManager *cm)
+ : cm_(cm)
+{
+}
+
+int CameraManager::Private::start()
+{
+ enumerator_ = DeviceEnumerator::create();
+ if (!enumerator_ || enumerator_->enumerate())
+ return -ENODEV;
+
+ /*
+ * TODO: Try to read handlers and order from configuration
+ * file and only fallback on all handlers if there is no
+ * configuration file.
+ */
+ std::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();
+
+ for (PipelineHandlerFactory *factory : factories) {
+ /*
+ * Try each pipeline handler until it exhaust
+ * all pipelines it can provide.
+ */
+ while (1) {
+ std::shared_ptr<PipelineHandler> pipe = factory->create(cm_);
+ if (!pipe->match(enumerator_.get()))
+ break;
+
+ LOG(Camera, Debug)
+ << "Pipeline handler \"" << factory->name()
+ << "\" matched";
+ pipes_.push_back(std::move(pipe));
+ }
+ }
+
+ /* TODO: register hot-plug callback here */
+
+ return 0;
+}
+
+void CameraManager::Private::stop()
+{
+ /* TODO: unregister hot-plug callback here */
+
+ /*
+ * 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);
+}
+
+void CameraManager::Private::addCamera(std::shared_ptr<Camera> &camera,
+ dev_t devnum)
+{
+ for (std::shared_ptr<Camera> c : cameras_) {
+ if (c->name() == camera->name()) {
+ LOG(Camera, Warning)
+ << "Registering camera with duplicate name '"
+ << camera->name() << "'";
+ break;
+ }
+ }
+
+ cameras_.push_back(std::move(camera));
+
+ if (devnum) {
+ unsigned int index = cameras_.size() - 1;
+ camerasByDevnum_[devnum] = cameras_[index];
+ }
+}
+
+void CameraManager::Private::removeCamera(Camera *camera)
+{
+ auto iter = std::find_if(cameras_.begin(), cameras_.end(),
+ [camera](std::shared_ptr<Camera> &c) {
+ return c.get() == camera;
+ });
+ if (iter == cameras_.end())
+ return;
+
+ LOG(Camera, Debug)
+ << "Unregistering camera '" << camera->name() << "'";
+
+ auto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),
+ [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {
+ return p.second.lock().get() == camera;
+ });
+ if (iter_d != camerasByDevnum_.end())
+ camerasByDevnum_.erase(iter_d);
+
+ cameras_.erase(iter);
+}
+
/**
* \class CameraManager
* \brief Provide access and manage all cameras in the system
@@ -62,7 +182,7 @@ LOG_DEFINE_CATEGORY(Camera)
CameraManager *CameraManager::self_ = nullptr;
CameraManager::CameraManager()
- : enumerator_(nullptr)
+ : p_(new CameraManager::Private(this))
{
if (self_)
LOG(Camera, Fatal)
@@ -73,6 +193,8 @@ CameraManager::CameraManager()
CameraManager::~CameraManager()
{
+ stop();
+
self_ = nullptr;
}
@@ -88,42 +210,14 @@ CameraManager::~CameraManager()
*/
int CameraManager::start()
{
- if (enumerator_)
- return -EBUSY;
-
LOG(Camera, Info) << "libcamera " << version_;
- enumerator_ = DeviceEnumerator::create();
- if (!enumerator_ || enumerator_->enumerate())
- return -ENODEV;
-
- /*
- * TODO: Try to read handlers and order from configuration
- * file and only fallback on all handlers if there is no
- * configuration file.
- */
- std::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();
+ int ret = p_->start();
+ if (ret)
+ LOG(Camera, Error) << "Failed to start camera manager: "
+ << strerror(-ret);
- for (PipelineHandlerFactory *factory : factories) {
- /*
- * Try each pipeline handler until it exhaust
- * all pipelines it can provide.
- */
- while (1) {
- 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(std::move(pipe));
- }
- }
-
- /* TODO: register hot-plug callback here */
-
- return 0;
+ return ret;
}
/**
@@ -138,17 +232,7 @@ int CameraManager::start()
*/
void CameraManager::stop()
{
- /* TODO: unregister hot-plug callback here */
-
- /*
- * 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);
+ p_->stop();
}
/**
@@ -160,6 +244,10 @@ void CameraManager::stop()
*
* \return List of all available cameras
*/
+const std::vector<std::shared_ptr<Camera>> &CameraManager::cameras() const
+{
+ return p_->cameras_;
+}
/**
* \brief Get a camera based on name
@@ -172,7 +260,7 @@ void CameraManager::stop()
*/
std::shared_ptr<Camera> CameraManager::get(const std::string &name)
{
- for (std::shared_ptr<Camera> camera : cameras_) {
+ for (std::shared_ptr<Camera> camera : p_->cameras_) {
if (camera->name() == name)
return camera;
}
@@ -196,8 +284,8 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &name)
*/
std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
{
- auto iter = camerasByDevnum_.find(devnum);
- if (iter == camerasByDevnum_.end())
+ auto iter = p_->camerasByDevnum_.find(devnum);
+ if (iter == p_->camerasByDevnum_.end())
return nullptr;
return iter->second.lock();
@@ -217,21 +305,8 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
*/
void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
{
- for (std::shared_ptr<Camera> c : cameras_) {
- if (c->name() == camera->name()) {
- LOG(Camera, Warning)
- << "Registering camera with duplicate name '"
- << camera->name() << "'";
- break;
- }
- }
- cameras_.push_back(std::move(camera));
-
- if (devnum) {
- unsigned int index = cameras_.size() - 1;
- camerasByDevnum_[devnum] = cameras_[index];
- }
+ p_->addCamera(camera, devnum);
}
/**
@@ -244,24 +319,7 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera, dev_t devnum)
*/
void CameraManager::removeCamera(Camera *camera)
{
- auto iter = std::find_if(cameras_.begin(), cameras_.end(),
- [camera](std::shared_ptr<Camera> &c) {
- return c.get() == camera;
- });
- if (iter == cameras_.end())
- return;
-
- LOG(Camera, Debug)
- << "Unregistering camera '" << camera->name() << "'";
-
- auto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),
- [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {
- return p.second.lock().get() == camera;
- });
- if (iter_d != camerasByDevnum_.end())
- camerasByDevnum_.erase(iter_d);
-
- cameras_.erase(iter);
+ p_->removeCamera(camera);
}
/**
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list