[libcamera-devel,v3,1/3] android: CameraDevice: Separate CaptureRequest from Camera3RequestDescriptor
diff mbox series

Message ID 20210426083830.2965095-2-hiroh@chromium.org
State Superseded
Headers show
Series
  • Support HAL3 API flush
Related show

Commit Message

Hirokazu Honda April 26, 2021, 8:38 a.m. UTC
Camera3RequestDescriptors can be destroyed in stop(). Since
Camera3RequestDescriptor owns CaptureRequest, the CaptureRequest
should not be used anymore. But requestComplete() cannot check if
the associated descriptor with a request is valid without calling
CaptureRequest::cookie().
This resolves the problem by separating CaptureRequest lifetime
from Camera3RequestDescriptor. Thus CaptureRequest always is
destroyed in requestComplete(). This guarantees that a request is
valid until requestComplete() is called.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 src/android/camera_device.cpp | 27 +++++++++++++++------------
 src/android/camera_device.h   |  9 +++++----
 2 files changed, 20 insertions(+), 16 deletions(-)

Patch
diff mbox series

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index a71aee2f..b3def04b 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -361,7 +361,8 @@  bool validateCropRotate(const camera3_stream_configuration_t &streamList)
  */
 
 CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(
-	Camera *camera, const camera3_capture_request_t *camera3Request)
+	const camera3_capture_request_t *camera3Request, CaptureRequest *request)
+	: request_(request)
 {
 	frameNumber_ = camera3Request->frame_number;
 
@@ -379,12 +380,6 @@  CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(
 
 	/* Clone the controls associated with the camera3 request. */
 	settings_ = CameraMetadata(camera3Request->settings);
-
-	/*
-	 * Create the CaptureRequest, stored as a unique_ptr<> to tie its
-	 * lifetime to the descriptor.
-	 */
-	request_ = std::make_unique<CaptureRequest>(camera);
 }
 
 /*
@@ -1930,7 +1925,8 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	 * The descriptor and the associated memory reserved here are freed
 	 * at request complete time.
 	 */
-	Camera3RequestDescriptor descriptor(camera_.get(), camera3Request);
+	auto request = std::make_unique<CaptureRequest>(camera_.get());
+	Camera3RequestDescriptor descriptor(camera3Request, request.get());
 
 	/*
 	 * \todo The Android request model is incremental, settings passed in
@@ -2013,11 +2009,12 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	if (ret)
 		return ret;
 
-	worker_.queueRequest(descriptor.request_.get());
+	worker_.queueRequest(descriptor.request_);
 
 	{
 		std::scoped_lock<std::mutex> lock(mutex_);
 		descriptors_[descriptor.request_->cookie()] = std::move(descriptor);
+		requests_[request->cookie()] = std::move(request);
 	}
 
 	return 0;
@@ -2030,15 +2027,21 @@  void CameraDevice::requestComplete(Request *request)
 	std::unique_ptr<CameraMetadata> resultMetadata;
 
 	decltype(descriptors_)::node_type node;
+	std::unique_ptr<CaptureRequest> captureRequest;
 	{
 		std::scoped_lock<std::mutex> lock(mutex_);
-		auto it = descriptors_.find(request->cookie());
-		if (it == descriptors_.end()) {
+		auto requestIt = requests_.find(request->cookie());
+		if (requestIt == requests_.end()) {
 			LOG(HAL, Fatal)
 				<< "Unknown request: " << request->cookie();
-			status = CAMERA3_BUFFER_STATUS_ERROR;
 			return;
 		}
+		captureRequest = std::move(requestIt->second);
+		requests_.erase(requestIt);
+
+		auto it = descriptors_.find(request->cookie());
+		if (it == descriptors_.end())
+			return;
 
 		node = descriptors_.extract(it);
 	}
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index c63e8e21..95d77890 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -72,15 +72,15 @@  private:
 	struct Camera3RequestDescriptor {
 		Camera3RequestDescriptor() = default;
 		~Camera3RequestDescriptor() = default;
-		Camera3RequestDescriptor(libcamera::Camera *camera,
-					 const camera3_capture_request_t *camera3Request);
+		Camera3RequestDescriptor(const camera3_capture_request_t *camera3Request,
+					 CaptureRequest *request);
 		Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;
 
 		uint32_t frameNumber_ = 0;
 		std::vector<camera3_stream_buffer_t> buffers_;
 		std::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_;
 		CameraMetadata settings_;
-		std::unique_ptr<CaptureRequest> request_;
+		CaptureRequest *request_ = nullptr;
 	};
 
 	struct Camera3StreamConfiguration {
@@ -127,8 +127,9 @@  private:
 	std::map<int, libcamera::PixelFormat> formatsMap_;
 	std::vector<CameraStream> streams_;
 
-	std::mutex mutex_; /* Protect descriptors_ */
+	std::mutex mutex_; /* Protect descriptors_ and requests_. */
 	std::map<uint64_t, Camera3RequestDescriptor> descriptors_;
+	std::map<uint64_t, std::unique_ptr<CaptureRequest>> requests_;
 
 	std::string maker_;
 	std::string model_;