[libcamera-devel,3/4] libcamera: camera_manager: Inherit from Extensible
diff mbox series

Message ID 20200921031057.3564-4-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Implement d-pointer design pattern
Related show

Commit Message

Laurent Pinchart Sept. 21, 2020, 3:10 a.m. UTC 
Use the d-pointer infrastructure offered by the Extensible class to
replace the custom implementation.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/camera_manager.h |  7 ++--
 src/libcamera/camera_manager.cpp   | 55 +++++++++++++++++++-----------
 2 files changed, 38 insertions(+), 24 deletions(-)

Comments

Niklas Söderlund Sept. 21, 2020, 9:24 a.m. UTC | #1 
Hi Laurent,

Thanks for your work.

On 2020-09-21 06:10:56 +0300, Laurent Pinchart wrote:
> Use the d-pointer infrastructure offered by the Extensible class to
> replace the custom implementation.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/camera_manager.h |  7 ++--
>  src/libcamera/camera_manager.cpp   | 55 +++++++++++++++++++-----------
>  2 files changed, 38 insertions(+), 24 deletions(-)
> 
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index 9eb2b6f5a5f5..6d5341c76412 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -12,6 +12,7 @@
>  #include <sys/types.h>
>  #include <vector>
>  
> +#include <libcamera/extensible.h>
>  #include <libcamera/object.h>
>  #include <libcamera/signal.h>
>  
> @@ -20,8 +21,9 @@ namespace libcamera {
>  class Camera;
>  class EventDispatcher;
>  
> -class CameraManager : public Object
> +class CameraManager : public Object, public Extensible
>  {
> +	LIBCAMERA_DECLARE_PRIVATE(CameraManager)
>  public:
>  	CameraManager();
>  	CameraManager(const CameraManager &) = delete;
> @@ -50,9 +52,6 @@ public:
>  private:
>  	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 780a66a7ac10..77b49709cd96 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -30,8 +30,10 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(Camera)
>  
> -class CameraManager::Private : public Thread
> +class CameraManager::Private : public Extensible::Private, public Thread
>  {
> +	LIBCAMERA_DECLARE_PUBLIC(CameraManager)
> +
>  public:
>  	Private(CameraManager *cm);
>  
> @@ -58,8 +60,6 @@ private:
>  	void createPipelineHandlers();
>  	void cleanup();
>  
> -	CameraManager *cm_;
> -
>  	std::condition_variable cv_;
>  	bool initialized_;
>  	int status_;
> @@ -70,7 +70,7 @@ private:
>  };
>  
>  CameraManager::Private::Private(CameraManager *cm)
> -	: cm_(cm), initialized_(false)
> +	: Extensible::Private(cm), initialized_(false)
>  {
>  }
>  
> @@ -131,6 +131,8 @@ int CameraManager::Private::init()
>  
>  void CameraManager::Private::createPipelineHandlers()
>  {
> +	LIBCAMERA_O_PTR(CameraManager);
> +
>  	/*
>  	 * \todo Try to read handlers and order from configuration
>  	 * file and only fallback on all handlers if there is no
> @@ -145,7 +147,7 @@ void CameraManager::Private::createPipelineHandlers()
>  		 * all pipelines it can provide.
>  		 */
>  		while (1) {
> -			std::shared_ptr<PipelineHandler> pipe = factory->create(cm_);
> +			std::shared_ptr<PipelineHandler> pipe = factory->create(o);
>  			if (!pipe->match(enumerator_.get()))
>  				break;
>  
> @@ -256,7 +258,7 @@ void CameraManager::Private::removeCamera(Camera *camera)
>  CameraManager *CameraManager::self_ = nullptr;
>  
>  CameraManager::CameraManager()
> -	: p_(new CameraManager::Private(this))
> +	: Extensible(new CameraManager::Private(this))
>  {
>  	if (self_)
>  		LOG(Camera, Fatal)
> @@ -284,9 +286,11 @@ CameraManager::~CameraManager()
>   */
>  int CameraManager::start()
>  {
> +	LIBCAMERA_D_PTR(CameraManager);
> +
>  	LOG(Camera, Info) << "libcamera " << version_;
>  
> -	int ret = p_->start();
> +	int ret = d->start();
>  	if (ret)
>  		LOG(Camera, Error) << "Failed to start camera manager: "
>  				   << strerror(-ret);
> @@ -306,8 +310,9 @@ int CameraManager::start()
>   */
>  void CameraManager::stop()
>  {
> -	p_->exit();
> -	p_->wait();
> +	LIBCAMERA_D_PTR(CameraManager);
> +	d->exit();
> +	d->wait();
>  }
>  
>  /**
> @@ -323,9 +328,11 @@ void CameraManager::stop()
>   */
>  std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
>  {
> -	MutexLocker locker(p_->mutex_);
> +	LIBCAMERA_D_PTR(CameraManager);
>  
> -	return p_->cameras_;
> +	MutexLocker locker(d->mutex_);
> +
> +	return d->cameras_;
>  }
>  
>  /**
> @@ -341,9 +348,11 @@ std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
>   */
>  std::shared_ptr<Camera> CameraManager::get(const std::string &id)
>  {
> -	MutexLocker locker(p_->mutex_);
> +	LIBCAMERA_D_PTR(CameraManager);
>  
> -	for (std::shared_ptr<Camera> camera : p_->cameras_) {
> +	MutexLocker locker(d->mutex_);
> +
> +	for (std::shared_ptr<Camera> camera : d->cameras_) {
>  		if (camera->id() == id)
>  			return camera;
>  	}
> @@ -369,10 +378,12 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &id)
>   */
>  std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
>  {
> -	MutexLocker locker(p_->mutex_);
> +	LIBCAMERA_D_PTR(CameraManager);
>  
> -	auto iter = p_->camerasByDevnum_.find(devnum);
> -	if (iter == p_->camerasByDevnum_.end())
> +	MutexLocker locker(d->mutex_);
> +
> +	auto iter = d->camerasByDevnum_.find(devnum);
> +	if (iter == d->camerasByDevnum_.end())
>  		return nullptr;
>  
>  	return iter->second.lock();
> @@ -423,9 +434,11 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
>  void CameraManager::addCamera(std::shared_ptr<Camera> camera,
>  			      const std::vector<dev_t> &devnums)
>  {
> -	ASSERT(Thread::current() == p_.get());
> +	LIBCAMERA_D_PTR(CameraManager);
>  
> -	p_->addCamera(camera, devnums);
> +	ASSERT(Thread::current() == d);
> +
> +	d->addCamera(camera, devnums);
>  	cameraAdded.emit(camera);
>  }
>  
> @@ -441,9 +454,11 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera,
>   */
>  void CameraManager::removeCamera(std::shared_ptr<Camera> camera)
>  {
> -	ASSERT(Thread::current() == p_.get());
> +	LIBCAMERA_D_PTR(CameraManager);
>  
> -	p_->removeCamera(camera.get());
> +	ASSERT(Thread::current() == d);
> +
> +	d->removeCamera(camera.get());
>  	cameraRemoved.emit(camera);
>  }
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
index 9eb2b6f5a5f5..6d5341c76412 100644
--- a/include/libcamera/camera_manager.h
+++ b/include/libcamera/camera_manager.h
@@ -12,6 +12,7 @@ 
 #include <sys/types.h>
 #include <vector>
 
+#include <libcamera/extensible.h>
 #include <libcamera/object.h>
 #include <libcamera/signal.h>
 
@@ -20,8 +21,9 @@  namespace libcamera {
 class Camera;
 class EventDispatcher;
 
-class CameraManager : public Object
+class CameraManager : public Object, public Extensible
 {
+	LIBCAMERA_DECLARE_PRIVATE(CameraManager)
 public:
 	CameraManager();
 	CameraManager(const CameraManager &) = delete;
@@ -50,9 +52,6 @@  public:
 private:
 	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 780a66a7ac10..77b49709cd96 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -30,8 +30,10 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(Camera)
 
-class CameraManager::Private : public Thread
+class CameraManager::Private : public Extensible::Private, public Thread
 {
+	LIBCAMERA_DECLARE_PUBLIC(CameraManager)
+
 public:
 	Private(CameraManager *cm);
 
@@ -58,8 +60,6 @@  private:
 	void createPipelineHandlers();
 	void cleanup();
 
-	CameraManager *cm_;
-
 	std::condition_variable cv_;
 	bool initialized_;
 	int status_;
@@ -70,7 +70,7 @@  private:
 };
 
 CameraManager::Private::Private(CameraManager *cm)
-	: cm_(cm), initialized_(false)
+	: Extensible::Private(cm), initialized_(false)
 {
 }
 
@@ -131,6 +131,8 @@  int CameraManager::Private::init()
 
 void CameraManager::Private::createPipelineHandlers()
 {
+	LIBCAMERA_O_PTR(CameraManager);
+
 	/*
 	 * \todo Try to read handlers and order from configuration
 	 * file and only fallback on all handlers if there is no
@@ -145,7 +147,7 @@  void CameraManager::Private::createPipelineHandlers()
 		 * all pipelines it can provide.
 		 */
 		while (1) {
-			std::shared_ptr<PipelineHandler> pipe = factory->create(cm_);
+			std::shared_ptr<PipelineHandler> pipe = factory->create(o);
 			if (!pipe->match(enumerator_.get()))
 				break;
 
@@ -256,7 +258,7 @@  void CameraManager::Private::removeCamera(Camera *camera)
 CameraManager *CameraManager::self_ = nullptr;
 
 CameraManager::CameraManager()
-	: p_(new CameraManager::Private(this))
+	: Extensible(new CameraManager::Private(this))
 {
 	if (self_)
 		LOG(Camera, Fatal)
@@ -284,9 +286,11 @@  CameraManager::~CameraManager()
  */
 int CameraManager::start()
 {
+	LIBCAMERA_D_PTR(CameraManager);
+
 	LOG(Camera, Info) << "libcamera " << version_;
 
-	int ret = p_->start();
+	int ret = d->start();
 	if (ret)
 		LOG(Camera, Error) << "Failed to start camera manager: "
 				   << strerror(-ret);
@@ -306,8 +310,9 @@  int CameraManager::start()
  */
 void CameraManager::stop()
 {
-	p_->exit();
-	p_->wait();
+	LIBCAMERA_D_PTR(CameraManager);
+	d->exit();
+	d->wait();
 }
 
 /**
@@ -323,9 +328,11 @@  void CameraManager::stop()
  */
 std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
 {
-	MutexLocker locker(p_->mutex_);
+	LIBCAMERA_D_PTR(CameraManager);
 
-	return p_->cameras_;
+	MutexLocker locker(d->mutex_);
+
+	return d->cameras_;
 }
 
 /**
@@ -341,9 +348,11 @@  std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
  */
 std::shared_ptr<Camera> CameraManager::get(const std::string &id)
 {
-	MutexLocker locker(p_->mutex_);
+	LIBCAMERA_D_PTR(CameraManager);
 
-	for (std::shared_ptr<Camera> camera : p_->cameras_) {
+	MutexLocker locker(d->mutex_);
+
+	for (std::shared_ptr<Camera> camera : d->cameras_) {
 		if (camera->id() == id)
 			return camera;
 	}
@@ -369,10 +378,12 @@  std::shared_ptr<Camera> CameraManager::get(const std::string &id)
  */
 std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
 {
-	MutexLocker locker(p_->mutex_);
+	LIBCAMERA_D_PTR(CameraManager);
 
-	auto iter = p_->camerasByDevnum_.find(devnum);
-	if (iter == p_->camerasByDevnum_.end())
+	MutexLocker locker(d->mutex_);
+
+	auto iter = d->camerasByDevnum_.find(devnum);
+	if (iter == d->camerasByDevnum_.end())
 		return nullptr;
 
 	return iter->second.lock();
@@ -423,9 +434,11 @@  std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
 void CameraManager::addCamera(std::shared_ptr<Camera> camera,
 			      const std::vector<dev_t> &devnums)
 {
-	ASSERT(Thread::current() == p_.get());
+	LIBCAMERA_D_PTR(CameraManager);
 
-	p_->addCamera(camera, devnums);
+	ASSERT(Thread::current() == d);
+
+	d->addCamera(camera, devnums);
 	cameraAdded.emit(camera);
 }
 
@@ -441,9 +454,11 @@  void CameraManager::addCamera(std::shared_ptr<Camera> camera,
  */
 void CameraManager::removeCamera(std::shared_ptr<Camera> camera)
 {
-	ASSERT(Thread::current() == p_.get());
+	LIBCAMERA_D_PTR(CameraManager);
 
-	p_->removeCamera(camera.get());
+	ASSERT(Thread::current() == d);
+
+	d->removeCamera(camera.get());
 	cameraRemoved.emit(camera);
 }