[libcamera-devel] [PATCH 7/8] libcamera: pipeline: ipu3: frames: Associate buffers with the reqeust

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

s/reqeust/request/ in the subject line

On Fri, Mar 12, 2021 at 06:11:30AM +0000, Kieran Bingham wrote:
> Ensure that the buffers are associated with the request even if they are

s/the request/a request/ (or their request) ?

> used internally to be able to correctly map back to the resources they
> are being used to fulfil.

Sounds a bit weird, but I get what you mean :-)

> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

I was a bit worried we could expand the usage of the request_ pointer in
the FrameBuffer class in the future to handle more operations
automatically, leading to issues if we associate internal buffers with
requests, but thinking about it, your proposal makes the most sense. We
even state that this is the expected use case in the
FrameBuffer::setRequest() documentation :-)

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

While at it, should we also apply the following ?

diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index 0ef3bc04659b..3cd777d1b742 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -278,10 +278,9 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)

 		buffer = availableBuffers_.front();
 		availableBuffers_.pop();
+		buffer->setRequest(request);
 	}

-	buffer->setRequest(request);
-
 	int ret = output_->queueBuffer(buffer);
 	if (ret)
 		return nullptr;

> ---
>  src/libcamera/pipeline/ipu3/frames.cpp | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
> index 7a7c5643df43..2a0590258d03 100644
> --- a/src/libcamera/pipeline/ipu3/frames.cpp
> +++ b/src/libcamera/pipeline/ipu3/frames.cpp
> @@ -56,6 +56,9 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)
>  	FrameBuffer *paramBuffer = availableParamBuffers_.front();
>  	FrameBuffer *statBuffer = availableStatBuffers_.front();
>  
> +	paramBuffer->setRequest(request);
> +	statBuffer->setRequest(request);
> +
>  	availableParamBuffers_.pop();
>  	availableStatBuffers_.pop();
>  

-- 
Regards,

Laurent Pinchart