| Message ID | 20190115140749.8297-3-jacopo@jmondi.org |
|---|---|
| State | Rejected |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, On 15/01/2019 14:07, Jacopo Mondi wrote: > The Camera class destructor is defined as private, but it needs to be > accessed by classes that create Camera instances, such as pipeline > handlers. > This seems reasonable to me Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/camera.h | 2 +- > src/libcamera/pipeline/vimc.cpp | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 9a7579d..d751d2d 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -15,13 +15,13 @@ class Camera > { > public: > Camera(const std::string &name); > + virtual ~Camera() { }; > > const std::string &name() const; > void get(); > void put(); > > private: > - virtual ~Camera() { }; > int ref_; > std::string name_; > }; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 720d9c2..00c544c 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -41,6 +41,8 @@ PipeHandlerVimc::~PipeHandlerVimc() > > if (dev_) > dev_->release(); > + > + delete camera_; > } > > unsigned int PipeHandlerVimc::count() >
On Tuesday, 15 January 2019 16:57:54 EET Kieran Bingham wrote: > Hi Jacopo, > > On 15/01/2019 14:07, Jacopo Mondi wrote: > > The Camera class destructor is defined as private, but it needs to be > > accessed by classes that create Camera instances, such as pipeline > > handlers. > > This seems reasonable to me Not to me. Do you guys read the documentation ? :-) This isn't how cameras are destroyed. > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > > > include/libcamera/camera.h | 2 +- > > src/libcamera/pipeline/vimc.cpp | 2 ++ > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > index 9a7579d..d751d2d 100644 > > --- a/include/libcamera/camera.h > > +++ b/include/libcamera/camera.h > > @@ -15,13 +15,13 @@ class Camera > > { > > public: > > Camera(const std::string &name); > > + virtual ~Camera() { }; > > > > const std::string &name() const; > > void get(); > > void put(); > > > > private: > > - virtual ~Camera() { }; > > int ref_; > > std::string name_; > > }; > > diff --git a/src/libcamera/pipeline/vimc.cpp > > b/src/libcamera/pipeline/vimc.cpp index 720d9c2..00c544c 100644 > > --- a/src/libcamera/pipeline/vimc.cpp > > +++ b/src/libcamera/pipeline/vimc.cpp > > @@ -41,6 +41,8 @@ PipeHandlerVimc::~PipeHandlerVimc() > > > > if (dev_) > > dev_->release(); > > + > > + delete camera_; > > } > > > > unsigned int PipeHandlerVimc::count()
Hi Laurent, On Tue, Jan 15, 2019 at 05:43:08PM +0200, Laurent Pinchart wrote: > On Tuesday, 15 January 2019 16:57:54 EET Kieran Bingham wrote: > > Hi Jacopo, > > > > On 15/01/2019 14:07, Jacopo Mondi wrote: > > > The Camera class destructor is defined as private, but it needs to be > > > accessed by classes that create Camera instances, such as pipeline > > > handlers. > > > > This seems reasonable to me > > Not to me. Do you guys read the documentation ? :-) This isn't how cameras are > destroyed. Ah, ups void Camera::put() { if (--ref_ == 0) delete this; } Yes, this calls for shared_ptr<> > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > > > > include/libcamera/camera.h | 2 +- > > > src/libcamera/pipeline/vimc.cpp | 2 ++ > > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > > index 9a7579d..d751d2d 100644 > > > --- a/include/libcamera/camera.h > > > +++ b/include/libcamera/camera.h > > > @@ -15,13 +15,13 @@ class Camera > > > { > > > public: > > > Camera(const std::string &name); > > > + virtual ~Camera() { }; > > > > > > const std::string &name() const; > > > void get(); > > > void put(); > > > > > > private: > > > - virtual ~Camera() { }; > > > int ref_; > > > std::string name_; > > > }; > > > diff --git a/src/libcamera/pipeline/vimc.cpp > > > b/src/libcamera/pipeline/vimc.cpp index 720d9c2..00c544c 100644 > > > --- a/src/libcamera/pipeline/vimc.cpp > > > +++ b/src/libcamera/pipeline/vimc.cpp > > > @@ -41,6 +41,8 @@ PipeHandlerVimc::~PipeHandlerVimc() > > > > > > if (dev_) > > > dev_->release(); > > > + > > > + delete camera_; > > > } > > > > > > unsigned int PipeHandlerVimc::count() > > -- > Regards, > > Laurent Pinchart > > >
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 9a7579d..d751d2d 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -15,13 +15,13 @@ class Camera { public: Camera(const std::string &name); + virtual ~Camera() { }; const std::string &name() const; void get(); void put(); private: - virtual ~Camera() { }; int ref_; std::string name_; }; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 720d9c2..00c544c 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -41,6 +41,8 @@ PipeHandlerVimc::~PipeHandlerVimc() if (dev_) dev_->release(); + + delete camera_; } unsigned int PipeHandlerVimc::count()
The Camera class destructor is defined as private, but it needs to be accessed by classes that create Camera instances, such as pipeline handlers. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/camera.h | 2 +- src/libcamera/pipeline/vimc.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)