[libcamera-devel,v3,02/11] libcamera: buffer: Break friendship with Request
diff mbox series

Message ID 20210325134231.1400051-5-kieran.bingham@ideasonboard.com
State Accepted
Delegated to: Kieran Bingham
Headers show
Series
  • [libcamera-devel,v3,01/11] utils: ipc: proxy: Track IPA with a state machine
Related show

Commit Message

Kieran Bingham March 25, 2021, 1:42 p.m. UTC 
The FrameBuffer class is only friends with Request so that the request
can be associated with the buffers.

FrameBuffer already has a helper to setRequest(), so let's use that
directly instead.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/buffer.h | 1 -
 src/libcamera/request.cpp  | 9 +++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Laurent Pinchart March 26, 2021, 2:03 a.m. UTC | #1 
Hi Kieran,

Thank you for the patch.

On Thu, Mar 25, 2021 at 01:42:22PM +0000, Kieran Bingham wrote:
> The FrameBuffer class is only friends with Request so that the request
> can be associated with the buffers.
> 
> FrameBuffer already has a helper to setRequest(), so let's use that
> directly instead.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Such an old friendship being broken, I'll shed a tear.

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

> ---
>  include/libcamera/buffer.h | 1 -
>  src/libcamera/request.cpp  | 9 +++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 302fe3d3e86b..620f8a66f6a2 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -56,7 +56,6 @@ public:
>  private:
>  	LIBCAMERA_DISABLE_COPY_AND_MOVE(FrameBuffer)
>  
> -	friend class Request; /* Needed to update request_. */
>  	friend class V4L2VideoDevice; /* Needed to update metadata_. */
>  
>  	std::vector<Plane> planes_;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 0071667e4a2c..3ad83f3b8418 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -118,8 +118,9 @@ void Request::reuse(ReuseFlag flags)
>  	pending_.clear();
>  	if (flags & ReuseBuffers) {
>  		for (auto pair : bufferMap_) {
> -			pair.second->request_ = this;
> -			pending_.insert(pair.second);
> +			FrameBuffer *buffer = pair.second;
> +			buffer->setRequest(this);
> +			pending_.insert(buffer);
>  		}
>  	} else {
>  		bufferMap_.clear();
> @@ -187,7 +188,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)
>  		return -EEXIST;
>  	}
>  
> -	buffer->request_ = this;
> +	buffer->setRequest(this);
>  	pending_.insert(buffer);
>  	bufferMap_[stream] = buffer;
>  
> @@ -294,7 +295,7 @@ bool Request::completeBuffer(FrameBuffer *buffer)
>  	int ret = pending_.erase(buffer);
>  	ASSERT(ret == 1);
>  
> -	buffer->request_ = nullptr;
> +	buffer->setRequest(nullptr);
>  
>  	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
>  		cancelled_ = true;
Paul Elder March 26, 2021, 2:51 a.m. UTC | #2 
Hi Kieran,

On Thu, Mar 25, 2021 at 01:42:22PM +0000, Kieran Bingham wrote:
> The FrameBuffer class is only friends with Request so that the request
> can be associated with the buffers.
> 
> FrameBuffer already has a helper to setRequest(), so let's use that
> directly instead.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

> ---
>  include/libcamera/buffer.h | 1 -
>  src/libcamera/request.cpp  | 9 +++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 302fe3d3e86b..620f8a66f6a2 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -56,7 +56,6 @@ public:
>  private:
>  	LIBCAMERA_DISABLE_COPY_AND_MOVE(FrameBuffer)
>  
> -	friend class Request; /* Needed to update request_. */
>  	friend class V4L2VideoDevice; /* Needed to update metadata_. */
>  
>  	std::vector<Plane> planes_;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 0071667e4a2c..3ad83f3b8418 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -118,8 +118,9 @@ void Request::reuse(ReuseFlag flags)
>  	pending_.clear();
>  	if (flags & ReuseBuffers) {
>  		for (auto pair : bufferMap_) {
> -			pair.second->request_ = this;
> -			pending_.insert(pair.second);
> +			FrameBuffer *buffer = pair.second;
> +			buffer->setRequest(this);
> +			pending_.insert(buffer);
>  		}
>  	} else {
>  		bufferMap_.clear();
> @@ -187,7 +188,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)
>  		return -EEXIST;
>  	}
>  
> -	buffer->request_ = this;
> +	buffer->setRequest(this);
>  	pending_.insert(buffer);
>  	bufferMap_[stream] = buffer;
>  
> @@ -294,7 +295,7 @@ bool Request::completeBuffer(FrameBuffer *buffer)
>  	int ret = pending_.erase(buffer);
>  	ASSERT(ret == 1);
>  
> -	buffer->request_ = nullptr;
> +	buffer->setRequest(nullptr);
>  
>  	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
>  		cancelled_ = true;
> -- 
> 2.25.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
index 302fe3d3e86b..620f8a66f6a2 100644
--- a/include/libcamera/buffer.h
+++ b/include/libcamera/buffer.h
@@ -56,7 +56,6 @@  public:
 private:
 	LIBCAMERA_DISABLE_COPY_AND_MOVE(FrameBuffer)
 
-	friend class Request; /* Needed to update request_. */
 	friend class V4L2VideoDevice; /* Needed to update metadata_. */
 
 	std::vector<Plane> planes_;
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 0071667e4a2c..3ad83f3b8418 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -118,8 +118,9 @@  void Request::reuse(ReuseFlag flags)
 	pending_.clear();
 	if (flags & ReuseBuffers) {
 		for (auto pair : bufferMap_) {
-			pair.second->request_ = this;
-			pending_.insert(pair.second);
+			FrameBuffer *buffer = pair.second;
+			buffer->setRequest(this);
+			pending_.insert(buffer);
 		}
 	} else {
 		bufferMap_.clear();
@@ -187,7 +188,7 @@  int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)
 		return -EEXIST;
 	}
 
-	buffer->request_ = this;
+	buffer->setRequest(this);
 	pending_.insert(buffer);
 	bufferMap_[stream] = buffer;
 
@@ -294,7 +295,7 @@  bool Request::completeBuffer(FrameBuffer *buffer)
 	int ret = pending_.erase(buffer);
 	ASSERT(ret == 1);
 
-	buffer->request_ = nullptr;
+	buffer->setRequest(nullptr);
 
 	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
 		cancelled_ = true;