[libcamera-devel] [PATCH 3/4] libcamera: camera_manager: Inherit from Extensible

[Niklas Söderlund]

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 at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at 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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund