[libcamera-devel] [PATCH v3 02/10] android: camera_device: Transform descriptors_ map to queue

Umang Jain umang.jain at ideasonboard.com
Mon Sep 20 19:37:44 CEST 2021


The descriptors_ map hold Camera3RequestDescriptor(s) which are
per-capture request placed by the framework to libcamera HAL.
CameraDevice::requestComplete() looks for the descriptor for which the
camera request has been completed and removes it from the map.
However, this approach has its limitations going forwards.

In subsequent commits, the post-processing operation which happens
in requestComplete() synchronously, is going to be run asynchronously.
Therefore, instead of a map for descriptors, a queue makes more sense
going forwards and the framework expects capture results to be received
in the same order as they were queued. When the async processing is
completed, the descriptor queue is inspected to send back the capture
results and then de-queued. This helps to maintain the order of sending
back the capture results whilst preventing unnecessary complexity of
using a map.

Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
---
 src/android/camera_device.cpp | 89 ++++++++++++++++++-----------------
 src/android/camera_device.h   |  5 +-
 2 files changed, 48 insertions(+), 46 deletions(-)

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index f461e14c..0562c225 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -926,7 +926,9 @@ 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);
+	std::unique_ptr<Camera3RequestDescriptor> descriptor =
+		std::make_unique<Camera3RequestDescriptor>(camera_.get(),
+							   camera3Request);
 
 	/*
 	 * \todo The Android request model is incremental, settings passed in
@@ -937,12 +939,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	if (camera3Request->settings)
 		lastSettings_ = camera3Request->settings;
 	else
-		descriptor.settings_ = lastSettings_;
+		descriptor->settings_ = lastSettings_;
 
-	LOG(HAL, Debug) << "Queueing request " << descriptor.request_->cookie()
-			<< " with " << descriptor.buffers_.size() << " streams";
-	for (unsigned int i = 0; i < descriptor.buffers_.size(); ++i) {
-		const camera3_stream_buffer_t &camera3Buffer = descriptor.buffers_[i];
+	LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie()
+			<< " with " << descriptor->buffers_.size() << " streams";
+	for (unsigned int i = 0; i < descriptor->buffers_.size(); ++i) {
+		const camera3_stream_buffer_t &camera3Buffer = descriptor->buffers_[i];
 		camera3_stream *camera3Stream = camera3Buffer.stream;
 		CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv);
 
@@ -977,7 +979,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 			buffer = createFrameBuffer(*camera3Buffer.buffer,
 						   cameraStream->configuration().pixelFormat,
 						   cameraStream->configuration().size);
-			descriptor.frameBuffers_.emplace_back(buffer);
+			descriptor->frameBuffers_.emplace_back(buffer);
 			LOG(HAL, Debug) << ss.str() << " (direct)";
 			break;
 
@@ -999,7 +1001,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 			return -ENOMEM;
 		}
 
-		descriptor.request_->addBuffer(cameraStream->stream(), buffer,
+		descriptor->request_->addBuffer(cameraStream->stream(), buffer,
 						camera3Buffer.acquire_fence);
 	}
 
@@ -1007,7 +1009,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	 * Translate controls from Android to libcamera and queue the request
 	 * to the CameraWorker thread.
 	 */
-	int ret = processControls(&descriptor);
+	int ret = processControls(descriptor.get());
 	if (ret)
 		return ret;
 
@@ -1035,11 +1037,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		state_ = State::Running;
 	}
 
-	worker_.queueRequest(descriptor.request_.get());
+	worker_.queueRequest(descriptor->request_.get());
 
 	{
 		MutexLocker descriptorsLock(descriptorsMutex_);
-		descriptors_[descriptor.request_->cookie()] = std::move(descriptor);
+		descriptors_.push_back(std::move(descriptor));
 	}
 
 	return 0;
@@ -1047,26 +1049,22 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 
 void CameraDevice::requestComplete(Request *request)
 {
-	decltype(descriptors_)::node_type node;
-	{
-		MutexLocker descriptorsLock(descriptorsMutex_);
-		auto it = descriptors_.find(request->cookie());
-		if (it == descriptors_.end()) {
-			/*
-			 * \todo Clarify if the Camera has to be closed on
-			 * ERROR_DEVICE and possibly demote the Fatal to simple
-			 * Error.
-			 */
-			notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);
-			LOG(HAL, Fatal)
-				<< "Unknown request: " << request->cookie();
+	if (descriptors_.empty())
+		return;
 
-			return;
-		}
+	Camera3RequestDescriptor *descriptor = descriptors_.front().get();
+	if (descriptor->request_->cookie() != request->cookie()) {
+		/*
+		 * \todo Clarify if the Camera has to be closed on
+		 * ERROR_DEVICE and possibly demote the Fatal to simple
+		 * Error.
+		 */
+		notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);
+		LOG(HAL, Fatal)
+			<< "Unknown request: " << request->cookie();
 
-		node = descriptors_.extract(it);
+		return;
 	}
-	Camera3RequestDescriptor &descriptor = node.mapped();
 
 	/*
 	 * Prepare the capture result for the Android camera stack.
@@ -1075,14 +1073,14 @@ void CameraDevice::requestComplete(Request *request)
 	 * post-processing/compression fails.
 	 */
 	camera3_capture_result_t captureResult = {};
-	captureResult.frame_number = descriptor.frameNumber_;
-	captureResult.num_output_buffers = descriptor.buffers_.size();
-	for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
+	captureResult.frame_number = descriptor->frameNumber_;
+	captureResult.num_output_buffers = descriptor->buffers_.size();
+	for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
 		buffer.acquire_fence = -1;
 		buffer.release_fence = -1;
 		buffer.status = CAMERA3_BUFFER_STATUS_OK;
 	}
-	captureResult.output_buffers = descriptor.buffers_.data();
+	captureResult.output_buffers = descriptor->buffers_.data();
 	captureResult.partial_result = 1;
 
 	/*
@@ -1094,14 +1092,15 @@ void CameraDevice::requestComplete(Request *request)
 				<< " not successfully completed: "
 				<< request->status();
 
-		notifyError(descriptor.frameNumber_, nullptr,
+		notifyError(descriptor->frameNumber_, nullptr,
 			    CAMERA3_MSG_ERROR_REQUEST);
 
 		captureResult.partial_result = 0;
-		for (camera3_stream_buffer_t &buffer : descriptor.buffers_)
+		for (camera3_stream_buffer_t &buffer : descriptor->buffers_)
 			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
 		callbacks_->process_capture_result(callbacks_, &captureResult);
 
+		descriptors_.pop_front();
 		return;
 	}
 
@@ -1113,10 +1112,10 @@ void CameraDevice::requestComplete(Request *request)
 	 */
 	uint64_t sensorTimestamp = static_cast<uint64_t>(request->metadata()
 							 .get(controls::SensorTimestamp));
-	notifyShutter(descriptor.frameNumber_, sensorTimestamp);
+	notifyShutter(descriptor->frameNumber_, sensorTimestamp);
 
 	LOG(HAL, Debug) << "Request " << request->cookie() << " completed with "
-			<< descriptor.buffers_.size() << " streams";
+			<< descriptor->buffers_.size() << " streams";
 
 	/*
 	 * Generate the metadata associated with the captured buffers.
@@ -1126,14 +1125,14 @@ void CameraDevice::requestComplete(Request *request)
 	 */
 	std::unique_ptr<CameraMetadata> resultMetadata = getResultMetadata(descriptor);
 	if (!resultMetadata) {
-		notifyError(descriptor.frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
+		notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
 
 		/* The camera framework expects an empty metadata pack on error. */
 		resultMetadata = std::make_unique<CameraMetadata>(0, 0);
 	}
 
 	/* Handle any JPEG compression. */
-	for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
+	for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
 		CameraStream *cameraStream =
 			static_cast<CameraStream *>(buffer.stream->priv);
 
@@ -1144,13 +1143,13 @@ void CameraDevice::requestComplete(Request *request)
 		if (!src) {
 			LOG(HAL, Error) << "Failed to find a source stream buffer";
 			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
-			notifyError(descriptor.frameNumber_, buffer.stream,
+			notifyError(descriptor->frameNumber_, buffer.stream,
 				    CAMERA3_MSG_ERROR_BUFFER);
 			continue;
 		}
 
 		int ret = cameraStream->process(*src, *buffer.buffer,
-						descriptor.settings_,
+						descriptor->settings_,
 						resultMetadata.get());
 		/*
 		 * Return the FrameBuffer to the CameraStream now that we're
@@ -1161,13 +1160,15 @@ void CameraDevice::requestComplete(Request *request)
 
 		if (ret) {
 			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
-			notifyError(descriptor.frameNumber_, buffer.stream,
+			notifyError(descriptor->frameNumber_, buffer.stream,
 				    CAMERA3_MSG_ERROR_BUFFER);
 		}
 	}
 
 	captureResult.result = resultMetadata->get();
 	callbacks_->process_capture_result(callbacks_, &captureResult);
+
+	descriptors_.pop_front();
 }
 
 std::string CameraDevice::logPrefix() const
@@ -1203,10 +1204,10 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,
  * Produce a set of fixed result metadata.
  */
 std::unique_ptr<CameraMetadata>
-CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) const
+CameraDevice::getResultMetadata(const Camera3RequestDescriptor *descriptor) const
 {
-	const ControlList &metadata = descriptor.request_->metadata();
-	const CameraMetadata &settings = descriptor.settings_;
+	const ControlList &metadata = descriptor->request_->metadata();
+	const CameraMetadata &settings = descriptor->settings_;
 	camera_metadata_ro_entry_t entry;
 	bool found;
 
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index a5576927..9c895b25 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -7,6 +7,7 @@
 #ifndef __ANDROID_CAMERA_DEVICE_H__
 #define __ANDROID_CAMERA_DEVICE_H__
 
+#include <deque>
 #include <map>
 #include <memory>
 #include <mutex>
@@ -103,7 +104,7 @@ private:
 			 camera3_error_msg_code code);
 	int processControls(Camera3RequestDescriptor *descriptor);
 	std::unique_ptr<CameraMetadata> getResultMetadata(
-		const Camera3RequestDescriptor &descriptor) const;
+		const Camera3RequestDescriptor *descriptor) const;
 
 	unsigned int id_;
 	camera3_device_t camera3Device_;
@@ -123,7 +124,7 @@ private:
 	std::vector<CameraStream> streams_;
 
 	libcamera::Mutex descriptorsMutex_; /* Protects descriptors_. */
-	std::map<uint64_t, Camera3RequestDescriptor> descriptors_;
+	std::deque<std::unique_ptr<Camera3RequestDescriptor>> descriptors_;
 
 	std::string maker_;
 	std::string model_;
-- 
2.31.1



More information about the libcamera-devel mailing list