[libcamera-devel] [PATCH 04/11] libcamera: camera_manager: Make the class a singleton

[Laurent Pinchart]

Hi Jacopo,

On Monday, 7 January 2019 12:34:21 EET Jacopo Mondi wrote:
> On Sun, Jan 06, 2019 at 04:33:21AM +0200, Laurent Pinchart wrote:
> > There can only be a single camera manager instance in the application.
> > Creating it as a singleton helps avoiding mistakes. It also allows the
> > camera manager to be used as a storage of global data, such as the
> > future event dispatcher.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > 
> >  include/libcamera/camera_manager.h |  6 ++++--
> >  src/libcamera/camera_manager.cpp   | 15 +++++++++++++++
> >  test/list.cpp                      |  4 +---
> >  3 files changed, 20 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/libcamera/camera_manager.h
> > b/include/libcamera/camera_manager.h index 2768a5bd2384..56a3f32d8b6f
> > 100644
> > --- a/include/libcamera/camera_manager.h
> > +++ b/include/libcamera/camera_manager.h
> > @@ -19,15 +19,17 @@ class PipelineHandler;
> >  class CameraManager
> >  {
> >  public:
> > -	CameraManager();
> > -
> >  	int start();
> >  	void stop();
> >  	
> >  	std::vector<std::string> list() const;
> >  	Camera *get(const std::string &name);
> > 
> > +	static CameraManager *instance();
> > +
> >  private:
> > +	CameraManager();
> > +
> >  	DeviceEnumerator *enumerator_;
> >  	std::vector<PipelineHandler *> pipes_;
> >  
> >  };
> > diff --git a/src/libcamera/camera_manager.cpp
> > b/src/libcamera/camera_manager.cpp index 50a805fc665c..db2bc4b7424e
> > 100644
> > --- a/src/libcamera/camera_manager.cpp
> > +++ b/src/libcamera/camera_manager.cpp
> > @@ -161,4 +161,19 @@ Camera *CameraManager::get(const std::string &name)
> >  	return nullptr;
> >  }
> > 
> > +/**
> > + * \brief Retrieve the camera manager instance
> > + *
> > + * The CameraManager is a singleton and can't be constructed manually.
> > This + * function can instead be used to retrieve the single global
> > instance of the
> 
> nit: s/can/shall

Fixed.

> Apart from this minor thing, one other thing I wonder about is copy
> constructors, should we set them to "= delete" ?

Yes, and the operator= too. Fixed.

> With this clarified:
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thank you.

> > + * manager.
> > + *
> > + * \return The camera manager instance
> > + */
> > +CameraManager *CameraManager::instance()
> > +{
> > +	static CameraManager manager;
> > +	return &manager;
> > +}
> > +
> > 
> >  } /* namespace libcamera */
> > 
> > diff --git a/test/list.cpp b/test/list.cpp
> > index 39b8a41d1fef..49610c697c63 100644
> > --- a/test/list.cpp
> > +++ b/test/list.cpp
> > @@ -19,7 +19,7 @@ class ListTest : public Test
> >  protected:
> >  	int init()
> >  	{
> > -		cm = new CameraManager();
> > +		cm = CameraManager::instance();
> >  		if (!cm)
> >  			return -ENOMEM;
> > 
> > @@ -43,8 +43,6 @@ protected:
> >  	void cleanup()
> >  	{
> >  		cm->stop();
> > -
> > -		delete cm;
> >  	}
> >  
> >  private:

-- 
Regards,

Laurent Pinchart