[libcamera-devel,11/11] android: camera_stream: Define explicit move constructor and destructors
diff mbox series

Message ID 20211018132923.476242-12-umang.jain@ideasonboard.com
State Superseded
Delegated to: Umang Jain
Headers show
Series
  • android: Overhaul request handling
Related show

Commit Message

Umang Jain Oct. 18, 2021, 1:29 p.m. UTC 
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

There's no need for the move constructor and the destructor to be
inline. Define them explicitly, with default implementations. This
allows usage of the CameraStream class without a complete definition of
the PostProcessor class.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/android/camera_stream.cpp | 4 ++++
 src/android/camera_stream.h   | 2 ++
 2 files changed, 6 insertions(+)

Comments

Jacopo Mondi Oct. 18, 2021, 4:40 p.m. UTC | #1 
Hi Laurent,

On Mon, Oct 18, 2021 at 06:59:23PM +0530, Umang Jain wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> There's no need for the move constructor and the destructor to be
> inline. Define them explicitly, with default implementations. This

Were they inline ? Or rather implicitly defiend ?

> allows usage of the CameraStream class without a complete definition of
> the PostProcessor class.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

And saves a bit of space too probably!

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

> ---
>  src/android/camera_stream.cpp | 4 ++++
>  src/android/camera_stream.h   | 2 ++
>  2 files changed, 6 insertions(+)
>
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 8e6ccb83..f44a2717 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -56,6 +56,10 @@ CameraStream::CameraStream(CameraDevice *const cameraDevice,
>  {
>  }
>
> +CameraStream::CameraStream(CameraStream &&other) = default;
> +
> +CameraStream::~CameraStream() = default;
> +
>  const StreamConfiguration &CameraStream::configuration() const
>  {
>  	return config_->at(index_);
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 85064268..f242336e 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -112,6 +112,8 @@ public:
>  	CameraStream(CameraDevice *const cameraDevice,
>  		     libcamera::CameraConfiguration *config, Type type,
>  		     camera3_stream_t *camera3Stream, unsigned int index);
> +	CameraStream(CameraStream &&other);
> +	~CameraStream();
>
>  	Type type() const { return type_; }
>  	camera3_stream_t *camera3Stream() const { return camera3Stream_; }
> --
> 2.31.0
>
Laurent Pinchart Oct. 18, 2021, 5:36 p.m. UTC | #2 
Hi Jacopo,

On Mon, Oct 18, 2021 at 06:40:10PM +0200, Jacopo Mondi wrote:
> On Mon, Oct 18, 2021 at 06:59:23PM +0530, Umang Jain wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > There's no need for the move constructor and the destructor to be
> > inline. Define them explicitly, with default implementations. This
> 
> Were they inline ? Or rather implicitly defiend ?

Both,

"If no user-declared constructors of any kind are provided for a class
type (struct, class, or union), the compiler will always declare a
default constructor as an inline public member of its class."

(https://en.cppreference.com/w/cpp/language/default_constructor)

> > allows usage of the CameraStream class without a complete definition of
> > the PostProcessor class.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> And saves a bit of space too probably!
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > ---
> >  src/android/camera_stream.cpp | 4 ++++
> >  src/android/camera_stream.h   | 2 ++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> > index 8e6ccb83..f44a2717 100644
> > --- a/src/android/camera_stream.cpp
> > +++ b/src/android/camera_stream.cpp
> > @@ -56,6 +56,10 @@ CameraStream::CameraStream(CameraDevice *const cameraDevice,
> >  {
> >  }
> >
> > +CameraStream::CameraStream(CameraStream &&other) = default;
> > +
> > +CameraStream::~CameraStream() = default;
> > +
> >  const StreamConfiguration &CameraStream::configuration() const
> >  {
> >  	return config_->at(index_);
> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> > index 85064268..f242336e 100644
> > --- a/src/android/camera_stream.h
> > +++ b/src/android/camera_stream.h
> > @@ -112,6 +112,8 @@ public:
> >  	CameraStream(CameraDevice *const cameraDevice,
> >  		     libcamera::CameraConfiguration *config, Type type,
> >  		     camera3_stream_t *camera3Stream, unsigned int index);
> > +	CameraStream(CameraStream &&other);
> > +	~CameraStream();
> >
> >  	Type type() const { return type_; }
> >  	camera3_stream_t *camera3Stream() const { return camera3Stream_; }
Umang Jain Oct. 19, 2021, 10:56 a.m. UTC | #3 
Hi Laurent,

On 10/18/21 6:59 PM, Umang Jain wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> There's no need for the move constructor and the destructor to be
> inline. Define them explicitly, with default implementations. This
> allows usage of the CameraStream class without a complete definition of
> the PostProcessor class.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Reviewed-by: Umang Jain<umang.jain@ideasonboard.com>

> ---
>   src/android/camera_stream.cpp | 4 ++++
>   src/android/camera_stream.h   | 2 ++
>   2 files changed, 6 insertions(+)
>
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 8e6ccb83..f44a2717 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -56,6 +56,10 @@ CameraStream::CameraStream(CameraDevice *const cameraDevice,
>   {
>   }
>   
> +CameraStream::CameraStream(CameraStream &&other) = default;
> +
> +CameraStream::~CameraStream() = default;
> +
>   const StreamConfiguration &CameraStream::configuration() const
>   {
>   	return config_->at(index_);
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 85064268..f242336e 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -112,6 +112,8 @@ public:
>   	CameraStream(CameraDevice *const cameraDevice,
>   		     libcamera::CameraConfiguration *config, Type type,
>   		     camera3_stream_t *camera3Stream, unsigned int index);
> +	CameraStream(CameraStream &&other);
> +	~CameraStream();
>   
>   	Type type() const { return type_; }
>   	camera3_stream_t *camera3Stream() const { return camera3Stream_; }

Patch
diff mbox series 

diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 8e6ccb83..f44a2717 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -56,6 +56,10 @@  CameraStream::CameraStream(CameraDevice *const cameraDevice,
 {
 }
 
+CameraStream::CameraStream(CameraStream &&other) = default;
+
+CameraStream::~CameraStream() = default;
+
 const StreamConfiguration &CameraStream::configuration() const
 {
 	return config_->at(index_);
diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
index 85064268..f242336e 100644
--- a/src/android/camera_stream.h
+++ b/src/android/camera_stream.h
@@ -112,6 +112,8 @@  public:
 	CameraStream(CameraDevice *const cameraDevice,
 		     libcamera::CameraConfiguration *config, Type type,
 		     camera3_stream_t *camera3Stream, unsigned int index);
+	CameraStream(CameraStream &&other);
+	~CameraStream();
 
 	Type type() const { return type_; }
 	camera3_stream_t *camera3Stream() const { return camera3Stream_; }