[libcamera-devel,RFC,05/12] libcamera: buffer: Remove request tracking

Message ID 20191028022525.796995-6-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Rework buffer API
Related show

Commit Message

Niklas Söderlund Oct. 28, 2019, 2:25 a.m. UTC
It's no longer needed to track which request the application provided
buffer belongs to, drop the helpers for this.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/buffer.h | 4 ----
 src/libcamera/buffer.cpp   | 3 +--
 src/libcamera/request.cpp  | 3 ---
 3 files changed, 1 insertion(+), 9 deletions(-)

Comments

Laurent Pinchart Nov. 18, 2019, 7:18 p.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Mon, Oct 28, 2019 at 03:25:18AM +0100, Niklas Söderlund wrote:
> It's no longer needed to track which request the application provided
> buffer belongs to, drop the helpers for this.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Same concern as for 05/12, and

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

for the same reason, still with lots of doubts.

> ---
>  include/libcamera/buffer.h | 4 ----
>  src/libcamera/buffer.cpp   | 3 +--
>  src/libcamera/request.cpp  | 3 ---
>  3 files changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index c349b995e1eb35ee..54c757ef7db8b5f6 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -83,7 +83,6 @@ public:
>  	unsigned int sequence() const { return sequence_; }
>  
>  	Status status() const { return status_; }
> -	Request *request() const { return request_; }
>  	Stream *stream() const { return stream_; }
>  
>  private:
> @@ -94,8 +93,6 @@ private:
>  
>  	void cancel();
>  
> -	void setRequest(Request *request) { request_ = request; }
> -
>  	unsigned int index_;
>  	std::array<int, 3> dmabuf_;
>  	BufferMemory *mem_;
> @@ -105,7 +102,6 @@ private:
>  	unsigned int sequence_;
>  
>  	Status status_;
> -	Request *request_;
>  	Stream *stream_;
>  };
>  
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index 4407201bd81c368d..10b16a862393b536 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -270,8 +270,7 @@ void BufferPool::destroyBuffers()
>   */
>  Buffer::Buffer(unsigned int index, const Buffer *metadata)
>  	: index_(index), dmabuf_({ -1, -1, -1 }),
> -	  status_(Buffer::BufferSuccess), request_(nullptr),
> -	  stream_(nullptr)
> +	  status_(Buffer::BufferSuccess), stream_(nullptr)
>  {
>  	if (metadata) {
>  		bytesused_ = metadata->bytesused_;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index c14ed1a4d3ce55d0..a9468ed4b0512a7f 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -220,7 +220,6 @@ int Request::prepare()
>  
>  	for (auto const &pair : bufferMap_) {
>  		Buffer *buffer = pair.second;
> -		buffer->setRequest(this);
>  		pending_.insert(buffer);
>  	}
>  
> @@ -258,8 +257,6 @@ bool Request::completeBuffer(Buffer *buffer)
>  	int ret = pending_.erase(buffer);
>  	ASSERT(ret == 1);
>  
> -	buffer->setRequest(nullptr);
> -
>  	if (buffer->status() == Buffer::BufferCancelled)
>  		cancelled_ = true;
>

Patch

diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
index c349b995e1eb35ee..54c757ef7db8b5f6 100644
--- a/include/libcamera/buffer.h
+++ b/include/libcamera/buffer.h
@@ -83,7 +83,6 @@  public:
 	unsigned int sequence() const { return sequence_; }
 
 	Status status() const { return status_; }
-	Request *request() const { return request_; }
 	Stream *stream() const { return stream_; }
 
 private:
@@ -94,8 +93,6 @@  private:
 
 	void cancel();
 
-	void setRequest(Request *request) { request_ = request; }
-
 	unsigned int index_;
 	std::array<int, 3> dmabuf_;
 	BufferMemory *mem_;
@@ -105,7 +102,6 @@  private:
 	unsigned int sequence_;
 
 	Status status_;
-	Request *request_;
 	Stream *stream_;
 };
 
diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
index 4407201bd81c368d..10b16a862393b536 100644
--- a/src/libcamera/buffer.cpp
+++ b/src/libcamera/buffer.cpp
@@ -270,8 +270,7 @@  void BufferPool::destroyBuffers()
  */
 Buffer::Buffer(unsigned int index, const Buffer *metadata)
 	: index_(index), dmabuf_({ -1, -1, -1 }),
-	  status_(Buffer::BufferSuccess), request_(nullptr),
-	  stream_(nullptr)
+	  status_(Buffer::BufferSuccess), stream_(nullptr)
 {
 	if (metadata) {
 		bytesused_ = metadata->bytesused_;
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index c14ed1a4d3ce55d0..a9468ed4b0512a7f 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -220,7 +220,6 @@  int Request::prepare()
 
 	for (auto const &pair : bufferMap_) {
 		Buffer *buffer = pair.second;
-		buffer->setRequest(this);
 		pending_.insert(buffer);
 	}
 
@@ -258,8 +257,6 @@  bool Request::completeBuffer(Buffer *buffer)
 	int ret = pending_.erase(buffer);
 	ASSERT(ret == 1);
 
-	buffer->setRequest(nullptr);
-
 	if (buffer->status() == Buffer::BufferCancelled)
 		cancelled_ = true;