[libcamera-devel,3/4] libcamera: device_enumerator: Remove move() on search() return
diff mbox series

Message ID 20190212222021.28517-4-jacopo@jmondi.org
State Accepted
Headers show
Series
  • libcamera: mixed updated
Related show

Commit Message

Jacopo Mondi Feb. 12, 2019, 10:20 p.m. UTC 
Remove the std::move() call on the shared_ptr<MediaDevice *> returned by
the search() method and remove the std::move() call on temporary return
value in pipeline handlers that use the method.

Thanks to copy elision, the regular constructor of the newly created
object is called, avoiding un-necessary copies.

Furthermore, the use of std::move() in the return and assignment
statements prevents the compiler from performing copy elision, forcing
it to generate two sequences of un-necessary calls to the class'
move constructor and destructor.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/device_enumerator.cpp   | 2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp  | 4 ++--
 src/libcamera/pipeline/uvcvideo.cpp   | 2 +-
 src/libcamera/pipeline/vimc.cpp       | 2 +-
 test/v4l2_device/v4l2_device_test.cpp | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

Comments

Laurent Pinchart Feb. 12, 2019, 10:39 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Tue, Feb 12, 2019 at 11:20:20PM +0100, Jacopo Mondi wrote:
> Remove the std::move() call on the shared_ptr<MediaDevice *> returned by
> the search() method and remove the std::move() call on temporary return
> value in pipeline handlers that use the method.
> 
> Thanks to copy elision, the regular constructor of the newly created
> object is called, avoiding un-necessary copies.
> 
> Furthermore, the use of std::move() in the return and assignment
> statements prevents the compiler from performing copy elision, forcing
> it to generate two sequences of un-necessary calls to the class'
> move constructor and destructor.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/device_enumerator.cpp   | 2 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp  | 4 ++--
>  src/libcamera/pipeline/uvcvideo.cpp   | 2 +-
>  src/libcamera/pipeline/vimc.cpp       | 2 +-
>  test/v4l2_device/v4l2_device_test.cpp | 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index 03923b3bb457..7db40812906b 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -308,7 +308,7 @@ std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)
>  			LOG(DeviceEnumerator, Debug)
>  				<< "Successful match for media device \""
>  				<< media->driver() << "\"";
> -			return std::move(media);
> +			return media;
>  		}
>  	}
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 34b03995ae31..f11e9cc61283 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -279,11 +279,11 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
>  	imgu_dm.add("ipu3-imgu 1 viewfinder");
>  	imgu_dm.add("ipu3-imgu 1 3a stat");
>  
> -	cio2_ = std::move(enumerator->search(cio2_dm));
> +	cio2_ = enumerator->search(cio2_dm);
>  	if (!cio2_)
>  		return false;
>  
> -	imgu_ = std::move(enumerator->search(imgu_dm));
> +	imgu_ = enumerator->search(imgu_dm);
>  	if (!imgu_)
>  		return false;
>  
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index fc31c52c0ecd..bf0d2fee3851 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -139,7 +139,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>  {
>  	DeviceMatch dm("uvcvideo");
>  
> -	media_ = std::move(enumerator->search(dm));
> +	media_ = enumerator->search(dm);
>  
>  	if (!media_)
>  		return false;
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 46840a4f4104..3aed24c3158a 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -148,7 +148,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  	dm.add("RGB/YUV Input");
>  	dm.add("Scaler");
>  
> -	media_ = std::move(enumerator->search(dm));
> +	media_ = enumerator->search(dm);
>  	if (!media_)
>  		return false;
>  
> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp
> index 18d014caf4c8..76f2e55d8cb4 100644
> --- a/test/v4l2_device/v4l2_device_test.cpp
> +++ b/test/v4l2_device/v4l2_device_test.cpp
> @@ -40,7 +40,7 @@ int V4L2DeviceTest::init()
>  	}
>  
>  	DeviceMatch dm("uvcvideo");
> -	media_ = std::move(enumerator_->search(dm));
> +	media_ = enumerator_->search(dm);
>  	if (!media_)
>  		return TestSkip;
>
Niklas Söderlund Feb. 13, 2019, 11:01 a.m. UTC | #2 
Hi Jacopo,

Thanks for your patch.

On 2019-02-12 23:20:20 +0100, Jacopo Mondi wrote:
> Remove the std::move() call on the shared_ptr<MediaDevice *> returned by
> the search() method and remove the std::move() call on temporary return
> value in pipeline handlers that use the method.
> 
> Thanks to copy elision, the regular constructor of the newly created
> object is called, avoiding un-necessary copies.
> 
> Furthermore, the use of std::move() in the return and assignment
> statements prevents the compiler from performing copy elision, forcing
> it to generate two sequences of un-necessary calls to the class'
> move constructor and destructor.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  src/libcamera/device_enumerator.cpp   | 2 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp  | 4 ++--
>  src/libcamera/pipeline/uvcvideo.cpp   | 2 +-
>  src/libcamera/pipeline/vimc.cpp       | 2 +-
>  test/v4l2_device/v4l2_device_test.cpp | 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index 03923b3bb457..7db40812906b 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -308,7 +308,7 @@ std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)
>  			LOG(DeviceEnumerator, Debug)
>  				<< "Successful match for media device \""
>  				<< media->driver() << "\"";
> -			return std::move(media);
> +			return media;
>  		}
>  	}
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 34b03995ae31..f11e9cc61283 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -279,11 +279,11 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
>  	imgu_dm.add("ipu3-imgu 1 viewfinder");
>  	imgu_dm.add("ipu3-imgu 1 3a stat");
>  
> -	cio2_ = std::move(enumerator->search(cio2_dm));
> +	cio2_ = enumerator->search(cio2_dm);
>  	if (!cio2_)
>  		return false;
>  
> -	imgu_ = std::move(enumerator->search(imgu_dm));
> +	imgu_ = enumerator->search(imgu_dm);
>  	if (!imgu_)
>  		return false;
>  
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index fc31c52c0ecd..bf0d2fee3851 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -139,7 +139,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>  {
>  	DeviceMatch dm("uvcvideo");
>  
> -	media_ = std::move(enumerator->search(dm));
> +	media_ = enumerator->search(dm);
>  
>  	if (!media_)
>  		return false;
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 46840a4f4104..3aed24c3158a 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -148,7 +148,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  	dm.add("RGB/YUV Input");
>  	dm.add("Scaler");
>  
> -	media_ = std::move(enumerator->search(dm));
> +	media_ = enumerator->search(dm);
>  	if (!media_)
>  		return false;
>  
> diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp
> index 18d014caf4c8..76f2e55d8cb4 100644
> --- a/test/v4l2_device/v4l2_device_test.cpp
> +++ b/test/v4l2_device/v4l2_device_test.cpp
> @@ -40,7 +40,7 @@ int V4L2DeviceTest::init()
>  	}
>  
>  	DeviceMatch dm("uvcvideo");
> -	media_ = std::move(enumerator_->search(dm));
> +	media_ = enumerator_->search(dm);
>  	if (!media_)
>  		return TestSkip;
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
index 03923b3bb457..7db40812906b 100644
--- a/src/libcamera/device_enumerator.cpp
+++ b/src/libcamera/device_enumerator.cpp
@@ -308,7 +308,7 @@  std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)
 			LOG(DeviceEnumerator, Debug)
 				<< "Successful match for media device \""
 				<< media->driver() << "\"";
-			return std::move(media);
+			return media;
 		}
 	}
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 34b03995ae31..f11e9cc61283 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -279,11 +279,11 @@  bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
 	imgu_dm.add("ipu3-imgu 1 viewfinder");
 	imgu_dm.add("ipu3-imgu 1 3a stat");
 
-	cio2_ = std::move(enumerator->search(cio2_dm));
+	cio2_ = enumerator->search(cio2_dm);
 	if (!cio2_)
 		return false;
 
-	imgu_ = std::move(enumerator->search(imgu_dm));
+	imgu_ = enumerator->search(imgu_dm);
 	if (!imgu_)
 		return false;
 
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index fc31c52c0ecd..bf0d2fee3851 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -139,7 +139,7 @@  bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
 {
 	DeviceMatch dm("uvcvideo");
 
-	media_ = std::move(enumerator->search(dm));
+	media_ = enumerator->search(dm);
 
 	if (!media_)
 		return false;
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 46840a4f4104..3aed24c3158a 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -148,7 +148,7 @@  bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
 	dm.add("RGB/YUV Input");
 	dm.add("Scaler");
 
-	media_ = std::move(enumerator->search(dm));
+	media_ = enumerator->search(dm);
 	if (!media_)
 		return false;
 
diff --git a/test/v4l2_device/v4l2_device_test.cpp b/test/v4l2_device/v4l2_device_test.cpp
index 18d014caf4c8..76f2e55d8cb4 100644
--- a/test/v4l2_device/v4l2_device_test.cpp
+++ b/test/v4l2_device/v4l2_device_test.cpp
@@ -40,7 +40,7 @@  int V4L2DeviceTest::init()
 	}
 
 	DeviceMatch dm("uvcvideo");
-	media_ = std::move(enumerator_->search(dm));
+	media_ = enumerator_->search(dm);
 	if (!media_)
 		return TestSkip;