[libcamera-devel] [PATCH 3/7] android: camera_device: Copy camera3 buffers in descriptor

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Jan 21, 2021 at 05:53:01PM +0100, Jacopo Mondi wrote:
> The camera3_stream_buffer_t instances part of a capture request contain
> information on the stream for which capture has been requested (size,
> format and fences) and an handle to the stream's memory buffers.

s/an handle/a handle/

> Those information are copied in the descriptor one at the time while

s/Those information are/This information is/
s/one/one piece/

> processing the camera3 streams to be re-used at request completion time.
> 
> Simplyify the code by copying the stream information in the descriptor
> at construction time.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

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

> ---
>  src/android/camera_device.cpp | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index f10b572749eb..578fb2271a9e 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -299,8 +299,11 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(
>  {
>  	frameNumber_ = camera3Request->frame_number;
>  
> +	/* Copy the camera3 request stream information for later access. */
>  	numBuffers_ = camera3Request->num_output_buffers;
>  	buffers_ = new camera3_stream_buffer_t[numBuffers_];
> +	for (unsigned int i = 0; i < numBuffers_; ++i)
> +		buffers_[i] = camera3Request->output_buffers[i];
>  
>  	/*
>  	 * FrameBuffer instances created by wrapping a camera3 provided dmabuf
> @@ -1672,13 +1675,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		running_ = true;
>  	}
>  
> -	/*
> -	 * Queue a request for the Camera with the provided dmabuf file
> -	 * descriptors.
> -	 */
> -	const camera3_stream_buffer_t *camera3Buffers =
> -					camera3Request->output_buffers;
> -
>  	/*
>  	 * Save the request descriptors for use at completion time.
>  	 * The descriptor and the associated memory reserved here are freed
> @@ -1690,16 +1686,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	LOG(HAL, Debug) << "Queueing Request to libcamera with "
>  			<< descriptor->numBuffers_ << " HAL streams";
>  	for (unsigned int i = 0; i < descriptor->numBuffers_; ++i) {
> -		camera3_stream *camera3Stream = camera3Buffers[i].stream;
> -		CameraStream *cameraStream =
> -			static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
> -
> -		/*
> -		 * Keep track of which stream the request belongs to and store
> -		 * the native buffer handles.
> -		 */
> -		descriptor->buffers_[i].stream = camera3Buffers[i].stream;
> -		descriptor->buffers_[i].buffer = camera3Buffers[i].buffer;
> +		const camera3_stream_buffer_t *camera3Buffer = &descriptor->buffers_[i];
> +		camera3_stream *camera3Stream = camera3Buffer->stream;
> +		CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv);
>  
>  		std::stringstream ss;
>  		ss << i << " - (" << camera3Stream->width << "x"
> @@ -1729,7 +1718,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  			 * associate it with the Camera3RequestDescriptor for
>  			 * lifetime management only.
>  			 */
> -			buffer = createFrameBuffer(*camera3Buffers[i].buffer);
> +			buffer = createFrameBuffer(*camera3Buffer->buffer);
>  			descriptor->frameBuffers_.emplace_back(buffer);
>  			LOG(HAL, Debug) << ss.str() << " (direct)";
>  			break;
> @@ -1754,7 +1743,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		}
>  
>  		descriptor->request_->addBuffer(cameraStream->stream(), buffer,
> -						camera3Buffers[i].acquire_fence);
> +						camera3Buffer->acquire_fence);
>  	}
>  
>  	/*

-- 
Regards,

Laurent Pinchart