[RFC,v1,09/27] libcamera: pipeline: ipu3: Remove `setRequest()` calls
diff mbox series

Message ID 20260618123844.656396-10-barnabas.pocze@ideasonboard.com
State Accepted
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 
No component queries the associated request of any buffer, so
setting it is unnecessary.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/cio2.cpp   | 3 +--
 src/libcamera/pipeline/ipu3/cio2.h     | 3 +--
 src/libcamera/pipeline/ipu3/frames.cpp | 3 ---
 src/libcamera/pipeline/ipu3/ipu3.cpp   | 2 +-
 4 files changed, 3 insertions(+), 8 deletions(-)

Comments

Jacopo Mondi June 22, 2026, 12:52 p.m. UTC | #1 
On Thu, Jun 18, 2026 at 02:38:26PM +0200, Barnabás Pőcze wrote:
> No component queries the associated request of any buffer, so
> setting it is unnecessary.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp   | 3 +--
>  src/libcamera/pipeline/ipu3/cio2.h     | 3 +--
>  src/libcamera/pipeline/ipu3/frames.cpp | 3 ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp   | 2 +-
>  4 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index b6fe84fe70..7481b2686d 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -378,7 +378,7 @@ int CIO2Device::stop()
>  	return ret;
>  }
>
> -FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
> +FrameBuffer *CIO2Device::queueBuffer(FrameBuffer *rawBuffer)
>  {
>  	FrameBuffer *buffer = rawBuffer;
>
> @@ -391,7 +391,6 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
>
>  		buffer = availableBuffers_.front();
>  		availableBuffers_.pop();
> -		buffer->_d()->setRequest(request);
>  	}
>
>  	int ret = output_->queueBuffer(buffer);
> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> index d844cb7ae8..91651a1640 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.h
> +++ b/src/libcamera/pipeline/ipu3/cio2.h
> @@ -22,7 +22,6 @@ class CameraSensor;
>  class FrameBuffer;
>  class MediaDevice;
>  class PixelFormat;
> -class Request;
>  class Size;
>  class SizeRange;
>  struct StreamConfiguration;
> @@ -56,7 +55,7 @@ public:
>  	CameraSensor *sensor() { return sensor_.get(); }
>  	const CameraSensor *sensor() const { return sensor_.get(); }
>
> -	FrameBuffer *queueBuffer(Request *request, FrameBuffer *rawBuffer);
> +	FrameBuffer *queueBuffer(FrameBuffer *rawBuffer);
>  	void tryReturnBuffer(FrameBuffer *buffer);
>  	Signal<FrameBuffer *> &bufferReady() { return output_->bufferReady; }
>  	Signal<uint32_t> &frameStart() { return csi2_->frameStart; }
> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
> index 4232e976a9..67ec922e49 100644
> --- a/src/libcamera/pipeline/ipu3/frames.cpp
> +++ b/src/libcamera/pipeline/ipu3/frames.cpp
> @@ -58,9 +58,6 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)
>  	FrameBuffer *paramBuffer = availableParamBuffers_.front();
>  	FrameBuffer *statBuffer = availableStatBuffers_.front();
>
> -	paramBuffer->_d()->setRequest(request);
> -	statBuffer->_d()->setRequest(request);
> -
>  	availableParamBuffers_.pop();
>  	availableStatBuffers_.pop();
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 560fccd8c0..af1b79d708 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -810,7 +810,7 @@ void IPU3CameraData::queuePendingRequests()
>  		 * otherwise.
>  		 */
>  		FrameBuffer *reqRawBuffer = request->findBuffer(&rawStream_);
> -		FrameBuffer *rawBuffer = cio2_.queueBuffer(request, reqRawBuffer);
> +		FrameBuffer *rawBuffer = cio2_.queueBuffer(reqRawBuffer);
>  		/*
>  		 * \todo If queueBuffer fails in queuing a buffer to the device,
>  		 * report the request as error by cancelling the request and
> --
> 2.54.0
>
Laurent Pinchart June 22, 2026, 10:55 p.m. UTC | #2 
On Thu, Jun 18, 2026 at 02:38:26PM +0200, Barnabás Pőcze wrote:
> No component queries the associated request of any buffer, so
> setting it is unnecessary.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

I'm a bit worried that we would introduce usage of the request pointer
later, and miss that it won't be set when using internal buffers, but
that's probably easy to notice as the associated code path is the
default case when not capturing a raw stream.

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

> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp   | 3 +--
>  src/libcamera/pipeline/ipu3/cio2.h     | 3 +--
>  src/libcamera/pipeline/ipu3/frames.cpp | 3 ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp   | 2 +-
>  4 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index b6fe84fe70..7481b2686d 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -378,7 +378,7 @@ int CIO2Device::stop()
>  	return ret;
>  }
>  
> -FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
> +FrameBuffer *CIO2Device::queueBuffer(FrameBuffer *rawBuffer)
>  {
>  	FrameBuffer *buffer = rawBuffer;
>  
> @@ -391,7 +391,6 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
>  
>  		buffer = availableBuffers_.front();
>  		availableBuffers_.pop();
> -		buffer->_d()->setRequest(request);
>  	}
>  
>  	int ret = output_->queueBuffer(buffer);
> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> index d844cb7ae8..91651a1640 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.h
> +++ b/src/libcamera/pipeline/ipu3/cio2.h
> @@ -22,7 +22,6 @@ class CameraSensor;
>  class FrameBuffer;
>  class MediaDevice;
>  class PixelFormat;
> -class Request;
>  class Size;
>  class SizeRange;
>  struct StreamConfiguration;
> @@ -56,7 +55,7 @@ public:
>  	CameraSensor *sensor() { return sensor_.get(); }
>  	const CameraSensor *sensor() const { return sensor_.get(); }
>  
> -	FrameBuffer *queueBuffer(Request *request, FrameBuffer *rawBuffer);
> +	FrameBuffer *queueBuffer(FrameBuffer *rawBuffer);
>  	void tryReturnBuffer(FrameBuffer *buffer);
>  	Signal<FrameBuffer *> &bufferReady() { return output_->bufferReady; }
>  	Signal<uint32_t> &frameStart() { return csi2_->frameStart; }
> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
> index 4232e976a9..67ec922e49 100644
> --- a/src/libcamera/pipeline/ipu3/frames.cpp
> +++ b/src/libcamera/pipeline/ipu3/frames.cpp
> @@ -58,9 +58,6 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)
>  	FrameBuffer *paramBuffer = availableParamBuffers_.front();
>  	FrameBuffer *statBuffer = availableStatBuffers_.front();
>  
> -	paramBuffer->_d()->setRequest(request);
> -	statBuffer->_d()->setRequest(request);
> -
>  	availableParamBuffers_.pop();
>  	availableStatBuffers_.pop();
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 560fccd8c0..af1b79d708 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -810,7 +810,7 @@ void IPU3CameraData::queuePendingRequests()
>  		 * otherwise.
>  		 */
>  		FrameBuffer *reqRawBuffer = request->findBuffer(&rawStream_);
> -		FrameBuffer *rawBuffer = cio2_.queueBuffer(request, reqRawBuffer);
> +		FrameBuffer *rawBuffer = cio2_.queueBuffer(reqRawBuffer);
>  		/*
>  		 * \todo If queueBuffer fails in queuing a buffer to the device,
>  		 * report the request as error by cancelling the request and

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index b6fe84fe70..7481b2686d 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -378,7 +378,7 @@  int CIO2Device::stop()
 	return ret;
 }
 
-FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
+FrameBuffer *CIO2Device::queueBuffer(FrameBuffer *rawBuffer)
 {
 	FrameBuffer *buffer = rawBuffer;
 
@@ -391,7 +391,6 @@  FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
 
 		buffer = availableBuffers_.front();
 		availableBuffers_.pop();
-		buffer->_d()->setRequest(request);
 	}
 
 	int ret = output_->queueBuffer(buffer);
diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
index d844cb7ae8..91651a1640 100644
--- a/src/libcamera/pipeline/ipu3/cio2.h
+++ b/src/libcamera/pipeline/ipu3/cio2.h
@@ -22,7 +22,6 @@  class CameraSensor;
 class FrameBuffer;
 class MediaDevice;
 class PixelFormat;
-class Request;
 class Size;
 class SizeRange;
 struct StreamConfiguration;
@@ -56,7 +55,7 @@  public:
 	CameraSensor *sensor() { return sensor_.get(); }
 	const CameraSensor *sensor() const { return sensor_.get(); }
 
-	FrameBuffer *queueBuffer(Request *request, FrameBuffer *rawBuffer);
+	FrameBuffer *queueBuffer(FrameBuffer *rawBuffer);
 	void tryReturnBuffer(FrameBuffer *buffer);
 	Signal<FrameBuffer *> &bufferReady() { return output_->bufferReady; }
 	Signal<uint32_t> &frameStart() { return csi2_->frameStart; }
diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
index 4232e976a9..67ec922e49 100644
--- a/src/libcamera/pipeline/ipu3/frames.cpp
+++ b/src/libcamera/pipeline/ipu3/frames.cpp
@@ -58,9 +58,6 @@  IPU3Frames::Info *IPU3Frames::create(Request *request)
 	FrameBuffer *paramBuffer = availableParamBuffers_.front();
 	FrameBuffer *statBuffer = availableStatBuffers_.front();
 
-	paramBuffer->_d()->setRequest(request);
-	statBuffer->_d()->setRequest(request);
-
 	availableParamBuffers_.pop();
 	availableStatBuffers_.pop();
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 560fccd8c0..af1b79d708 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -810,7 +810,7 @@  void IPU3CameraData::queuePendingRequests()
 		 * otherwise.
 		 */
 		FrameBuffer *reqRawBuffer = request->findBuffer(&rawStream_);
-		FrameBuffer *rawBuffer = cio2_.queueBuffer(request, reqRawBuffer);
+		FrameBuffer *rawBuffer = cio2_.queueBuffer(reqRawBuffer);
 		/*
 		 * \todo If queueBuffer fails in queuing a buffer to the device,
 		 * report the request as error by cancelling the request and