From b6467bd9c67f69fe5023cb524391b8f1fc5c44c4 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sat, 18 Jan 2020 02:10:18 +0200 Subject: libcamera: camera_manager: Move private data members to private implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham Reviewed-by: Niklas Söderlund --- 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 8e6fbdbb..1f746393 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 09419766..068afd58 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 #include #include #include @@ -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> &cameras() const { return cameras_; } + const std::vector> &cameras() const; std::shared_ptr get(const std::string &name); std::shared_ptr get(dev_t devnum); @@ -46,13 +43,11 @@ public: EventDispatcher *eventDispatcher(); private: - std::unique_ptr enumerator_; - std::vector> pipes_; - std::vector> cameras_; - std::map> camerasByDevnum_; - static const std::string version_; static CameraManager *self_; + + class Private; + std::unique_ptr p_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index a71caf85..e0a07ec5 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -7,6 +7,8 @@ #include +#include + #include #include @@ -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, dev_t devnum); + void removeCamera(Camera *camera); + + std::vector> cameras_; + std::map> camerasByDevnum_; + +private: + CameraManager *cm_; + + std::vector> pipes_; + std::unique_ptr 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 &factories = PipelineHandlerFactory::factories(); + + for (PipelineHandlerFactory *factory : factories) { + /* + * Try each pipeline handler until it exhaust + * all pipelines it can provide. + */ + while (1) { + std::shared_ptr 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, + dev_t devnum) +{ + for (std::shared_ptr 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 &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> &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 &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 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> &CameraManager::cameras() const +{ + return p_->cameras_; +} /** * \brief Get a camera based on name @@ -172,7 +260,7 @@ void CameraManager::stop() */ std::shared_ptr CameraManager::get(const std::string &name) { - for (std::shared_ptr camera : cameras_) { + for (std::shared_ptr camera : p_->cameras_) { if (camera->name() == name) return camera; } @@ -196,8 +284,8 @@ std::shared_ptr CameraManager::get(const std::string &name) */ std::shared_ptr 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 CameraManager::get(dev_t devnum) */ void CameraManager::addCamera(std::shared_ptr camera, dev_t devnum) { - for (std::shared_ptr 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, dev_t devnum) */ void CameraManager::removeCamera(Camera *camera) { - auto iter = std::find_if(cameras_.begin(), cameras_.end(), - [camera](std::shared_ptr &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> &p) { - return p.second.lock().get() == camera; - }); - if (iter_d != camerasByDevnum_.end()) - camerasByDevnum_.erase(iter_d); - - cameras_.erase(iter); + p_->removeCamera(camera); } /** -- cgit v1.2.1