[RFC,v1,22/27] v4l2: v4l2_camera: Avoid a level of indirection
diff mbox series

Message ID 20260618123844.656396-23-barnabas.pocze@ideasonboard.com
State Superseded
Headers show
Series
  • Misc. changes before request-buffer split
Related show

Commit Message

Barnabás Pőcze June 18, 2026, 12:38 p.m. UTC 
Convert the `completedBuffers_` container to store the items directly
and not via an `std::unique_ptr`.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/v4l2/v4l2_camera.cpp | 11 ++++-------
 src/v4l2/v4l2_camera.h   |  2 +-
 2 files changed, 5 insertions(+), 8 deletions(-)

Comments

Jacopo Mondi June 22, 2026, 1:50 p.m. UTC | #1 
Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

On Thu, Jun 18, 2026 at 02:38:39PM +0200, Barnabás Pőcze wrote:
> Convert the `completedBuffers_` container to store the items directly
> and not via an `std::unique_ptr`.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera.cpp | 11 ++++-------
>  src/v4l2/v4l2_camera.h   |  2 +-
>  2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 412ab4ea67..648578c22b 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -70,11 +70,10 @@ void V4L2Camera::unbind()
>
>  std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
>  {
> -	std::vector<Buffer> v;
> -
>  	MutexLocker lock(bufferLock_);
> -	for (std::unique_ptr<Buffer> &metadata : completedBuffers_)
> -		v.push_back(*metadata.get());
> +	std::vector v(std::move_iterator(completedBuffers_.begin()),
> +		      std::move_iterator(completedBuffers_.end()));
> +
>  	completedBuffers_.clear();
>
>  	return v;
> @@ -88,9 +87,7 @@ void V4L2Camera::requestComplete(Request *request)
>  	/* We only have one stream at the moment. */
>  	bufferLock_.lock();
>  	FrameBuffer *buffer = request->buffers().begin()->second;
> -	std::unique_ptr<Buffer> metadata =
> -		std::make_unique<Buffer>(request->cookie(), buffer->metadata());
> -	completedBuffers_.push_back(std::move(metadata));
> +	completedBuffers_.emplace_back(request->cookie(), buffer->metadata());
>  	bufferLock_.unlock();
>
>  	uint64_t data = 1;
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 3d09ae7d60..1528f9aad8 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -85,7 +85,7 @@ private:
>  	std::vector<std::unique_ptr<libcamera::Request>> requestPool_;
>
>  	std::deque<libcamera::Request *> pendingRequests_;
> -	std::deque<std::unique_ptr<Buffer>> completedBuffers_
> +	std::deque<Buffer> completedBuffers_
>  		LIBCAMERA_TSA_GUARDED_BY(bufferLock_);
>
>  	int efd_;
> --
> 2.54.0
>
Laurent Pinchart June 23, 2026, 10:03 p.m. UTC | #2 
On Thu, Jun 18, 2026 at 02:38:39PM +0200, Barnabás Pőcze wrote:
> Convert the `completedBuffers_` container to store the items directly
> and not via an `std::unique_ptr`.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera.cpp | 11 ++++-------
>  src/v4l2/v4l2_camera.h   |  2 +-
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 412ab4ea67..648578c22b 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -70,11 +70,10 @@ void V4L2Camera::unbind()
>  
>  std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
>  {
> -	std::vector<Buffer> v;
> -
>  	MutexLocker lock(bufferLock_);
> -	for (std::unique_ptr<Buffer> &metadata : completedBuffers_)
> -		v.push_back(*metadata.get());
> +	std::vector v(std::move_iterator(completedBuffers_.begin()),

std::vector<Buffer> is more readable for me.

> +		      std::move_iterator(completedBuffers_.end()));

Today I learnt about std::move_iterator. Thank you.

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

> +
>  	completedBuffers_.clear();
>  
>  	return v;
> @@ -88,9 +87,7 @@ void V4L2Camera::requestComplete(Request *request)
>  	/* We only have one stream at the moment. */
>  	bufferLock_.lock();
>  	FrameBuffer *buffer = request->buffers().begin()->second;
> -	std::unique_ptr<Buffer> metadata =
> -		std::make_unique<Buffer>(request->cookie(), buffer->metadata());
> -	completedBuffers_.push_back(std::move(metadata));
> +	completedBuffers_.emplace_back(request->cookie(), buffer->metadata());
>  	bufferLock_.unlock();
>  
>  	uint64_t data = 1;
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 3d09ae7d60..1528f9aad8 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -85,7 +85,7 @@ private:
>  	std::vector<std::unique_ptr<libcamera::Request>> requestPool_;
>  
>  	std::deque<libcamera::Request *> pendingRequests_;
> -	std::deque<std::unique_ptr<Buffer>> completedBuffers_
> +	std::deque<Buffer> completedBuffers_
>  		LIBCAMERA_TSA_GUARDED_BY(bufferLock_);
>  
>  	int efd_;
Paul Elder June 24, 2026, 6:21 a.m. UTC | #3 
Quoting Barnabás Pőcze (2026-06-18 21:38:39)
> Convert the `completedBuffers_` container to store the items directly
> and not via an `std::unique_ptr`.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/v4l2/v4l2_camera.cpp | 11 ++++-------
>  src/v4l2/v4l2_camera.h   |  2 +-
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 412ab4ea67..648578c22b 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -70,11 +70,10 @@ void V4L2Camera::unbind()
>  
>  std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
>  {
> -       std::vector<Buffer> v;
> -
>         MutexLocker lock(bufferLock_);
> -       for (std::unique_ptr<Buffer> &metadata : completedBuffers_)
> -               v.push_back(*metadata.get());
> +       std::vector v(std::move_iterator(completedBuffers_.begin()),
> +                     std::move_iterator(completedBuffers_.end()));
> +
>         completedBuffers_.clear();
>  
>         return v;
> @@ -88,9 +87,7 @@ void V4L2Camera::requestComplete(Request *request)
>         /* We only have one stream at the moment. */
>         bufferLock_.lock();
>         FrameBuffer *buffer = request->buffers().begin()->second;
> -       std::unique_ptr<Buffer> metadata =
> -               std::make_unique<Buffer>(request->cookie(), buffer->metadata());
> -       completedBuffers_.push_back(std::move(metadata));
> +       completedBuffers_.emplace_back(request->cookie(), buffer->metadata());
>         bufferLock_.unlock();
>  
>         uint64_t data = 1;
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 3d09ae7d60..1528f9aad8 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -85,7 +85,7 @@ private:
>         std::vector<std::unique_ptr<libcamera::Request>> requestPool_;
>  
>         std::deque<libcamera::Request *> pendingRequests_;
> -       std::deque<std::unique_ptr<Buffer>> completedBuffers_
> +       std::deque<Buffer> completedBuffers_
>                 LIBCAMERA_TSA_GUARDED_BY(bufferLock_);
>  
>         int efd_;
> -- 
> 2.54.0
>

Patch
diff mbox series 

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 412ab4ea67..648578c22b 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -70,11 +70,10 @@  void V4L2Camera::unbind()
 
 std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
 {
-	std::vector<Buffer> v;
-
 	MutexLocker lock(bufferLock_);
-	for (std::unique_ptr<Buffer> &metadata : completedBuffers_)
-		v.push_back(*metadata.get());
+	std::vector v(std::move_iterator(completedBuffers_.begin()),
+		      std::move_iterator(completedBuffers_.end()));
+
 	completedBuffers_.clear();
 
 	return v;
@@ -88,9 +87,7 @@  void V4L2Camera::requestComplete(Request *request)
 	/* We only have one stream at the moment. */
 	bufferLock_.lock();
 	FrameBuffer *buffer = request->buffers().begin()->second;
-	std::unique_ptr<Buffer> metadata =
-		std::make_unique<Buffer>(request->cookie(), buffer->metadata());
-	completedBuffers_.push_back(std::move(metadata));
+	completedBuffers_.emplace_back(request->cookie(), buffer->metadata());
 	bufferLock_.unlock();
 
 	uint64_t data = 1;
diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
index 3d09ae7d60..1528f9aad8 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -85,7 +85,7 @@  private:
 	std::vector<std::unique_ptr<libcamera::Request>> requestPool_;
 
 	std::deque<libcamera::Request *> pendingRequests_;
-	std::deque<std::unique_ptr<Buffer>> completedBuffers_
+	std::deque<Buffer> completedBuffers_
 		LIBCAMERA_TSA_GUARDED_BY(bufferLock_);
 
 	int efd_;