[v6,2/3] libcamera: simple: Track requests in conversionQueue_

21824 diff mbox series

Simple pipeline retrieves the requests to complete from the
conversionQueue_.  This patch stores the requests in conversionQueue_
explicitly.  This explicit tracking is supposed to be preferred to
implicit retrieval and it simplifies the completion code a bit here and
in the followup patch that adds request cleanup on stop.

The change as implemented assumes that all the buffers in each of the
conversionQueue_ elements point to the same request, the one specified.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 26 ++++++++++++------------
 1 file changed, 13 insertions(+), 13 deletions(-)

On Wed, Nov 06, 2024 at 09:17:20PM +0100, Milan Zamazal wrote:
> Simple pipeline retrieves the requests to complete from the
> conversionQueue_.  This patch stores the requests in conversionQueue_
> explicitly.  This explicit tracking is supposed to be preferred to
> implicit retrieval and it simplifies the completion code a bit here and
> in the followup patch that adds request cleanup on stop.
> 
> The change as implemented assumes that all the buffers in each of the
> conversionQueue_ elements point to the same request, the one specified.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Tested-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 26 ++++++++++++------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 67f583b8a..13c0a1891 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -282,7 +282,11 @@ public:
>  	std::unique_ptr<DelayedControls> delayedCtrls_;
>  
>  	std::vector<std::unique_ptr<FrameBuffer>> conversionBuffers_;
> -	std::queue<std::map<const Stream *, FrameBuffer *>> conversionQueue_;
> +	struct RequestOutputs {
> +		Request *request;
> +		std::map<const Stream *, FrameBuffer *> outputs;
> +	};
> +	std::queue<RequestOutputs> conversionQueue_;
>  	bool useConversion_;
>  
>  	std::unique_ptr<Converter> converter_;
> @@ -808,16 +812,12 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>  		if (conversionQueue_.empty())
>  			return;
>  
> -		Request *request = nullptr;
> -		for (auto &item : conversionQueue_.front()) {
> -			FrameBuffer *outputBuffer = item.second;
> -			request = outputBuffer->request();
> -			pipe->completeBuffer(request, outputBuffer);
> -		}
> +		const RequestOutputs &outputs = conversionQueue_.front();
> +		for (auto &[stream, buf] : outputs.outputs)
> +			pipe->completeBuffer(outputs.request, buf);
> +		pipe->completeRequest(outputs.request);
>  		conversionQueue_.pop();
>  
> -		if (request)
> -			pipe->completeRequest(request);
>  		return;
>  	}
>  
> @@ -833,7 +833,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>  
>  	if (useConversion_ && !conversionQueue_.empty()) {
>  		const std::map<const Stream *, FrameBuffer *> &outputs =
> -			conversionQueue_.front();
> +			conversionQueue_.front().outputs;
>  		if (!outputs.empty()) {
>  			FrameBuffer *outputBuffer = outputs.begin()->second;
>  			if (outputBuffer)
> @@ -857,7 +857,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>  		}
>  
>  		if (converter_)
> -			converter_->queueBuffers(buffer, conversionQueue_.front());
> +			converter_->queueBuffers(buffer, conversionQueue_.front().outputs);
>  		else
>  			/*
>  			 * request->sequence() cannot be retrieved from `buffer' inside
> @@ -865,7 +865,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>  			 * already here.
>  			 */
>  			swIsp_->queueBuffers(request->sequence(), buffer,
> -					     conversionQueue_.front());
> +					     conversionQueue_.front().outputs);
>  
>  		conversionQueue_.pop();
>  		return;
> @@ -1429,7 +1429,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
>  	}
>  
>  	if (data->useConversion_) {
> -		data->conversionQueue_.push(std::move(buffers));
> +		data->conversionQueue_.push({ request, std::move(buffers) });
>  		if (data->swIsp_)
>  			data->swIsp_->queueRequest(request->sequence(), request->controls());
>  	}
> -- 
> 2.44.1
>