[libcamera-devel] [PATCH 2/2] android: camera_device: Queue request to Worker

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Oct 09, 2020 at 03:39:56PM +0200, Jacopo Mondi wrote:
> Add a CameraWorker class member to the CameraDevice class and
> queue capture requests to it to delegate fence handling and capture
> requests queueing to the camera.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 19 +++++++++----------
>  src/android/camera_device.h   |  3 +++
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 0a94c1ae17ac..0e42e2e194d8 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -194,8 +194,8 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor()
>   */
>  
>  CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera)
> -	: id_(id), running_(false), camera_(camera), staticMetadata_(nullptr),
> -	  facing_(CAMERA_FACING_FRONT), orientation_(0)
> +	: id_(id), worker_(camera), running_(false), camera_(camera),
> +	  staticMetadata_(nullptr), facing_(CAMERA_FACING_FRONT), orientation_(0)
>  {
>  	camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
>  
> @@ -519,6 +519,7 @@ void CameraDevice::close()
>  {
>  	streams_.clear();
>  
> +	worker_.stop();
>  	camera_->stop();
>  	camera_->release();
>  
> @@ -1356,6 +1357,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  			return ret;
>  		}
>  
> +		worker_.start();
> +
>  		running_ = true;
>  	}
>  
> @@ -1377,6 +1380,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  
>  	Request *request =
>  		camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
> +	std::unique_ptr<CaptureRequest> captureRequest = std::make_unique<CaptureRequest>(request);

Line break ?

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

>  
>  	LOG(HAL, Debug) << "Queueing Request to libcamera with "
>  			<< descriptor->numBuffers << " HAL streams";
> @@ -1445,16 +1449,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  			return -ENOMEM;
>  		}
>  
> -		request->addBuffer(cameraStream->stream(), buffer);
> +		captureRequest->addBuffer(cameraStream->stream(), buffer,
> +					  camera3Buffers[i].acquire_fence);
>  	}
>  
> -	int ret = camera_->queueRequest(request);
> -	if (ret) {
> -		LOG(HAL, Error) << "Failed to queue request";
> -		delete request;
> -		delete descriptor;
> -		return ret;
> -	}
> +	worker_.queueRequest(std::move(captureRequest));
>  
>  	return 0;
>  }
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 777d1a35e801..b4b32f77a29a 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -25,6 +25,7 @@
>  #include "libcamera/internal/message.h"
>  
>  #include "camera_stream.h"
> +#include "camera_worker.h"
>  #include "jpeg/encoder.h"
>  
>  class CameraMetadata;
> @@ -108,6 +109,8 @@ private:
>  	unsigned int id_;
>  	camera3_device_t camera3Device_;
>  
> +	CameraWorker worker_;
> +
>  	bool running_;
>  	std::shared_ptr<libcamera::Camera> camera_;
>  	std::unique_ptr<libcamera::CameraConfiguration> config_;

-- 
Regards,

Laurent Pinchart