[v3,7/7] android: Remove Camera3RequestDescriptor::streamsProcessMutex_
diff mbox series

Message ID 20241204164137.3938891-8-chenghaoyang@chromium.org
State New
Headers show
Series
  • Refactor Android HAL before supporting partial result
Related show

Commit Message

Cheng-Hao Yang Dec. 4, 2024, 4:36 p.m. UTC 
This mutex was needed when CameraStream's worker thread posts a result
back to CameraDevice. We can simplify it by calling CameraDevice's
function on libcamera::Camera's owner thread. With this delegation,
`Camera3RequestDescriptor::pendingStreamsToProcess_` will be firstly
setup in the application's thread in processCaptureRequest(), and the
rest accesses will be done in libcamera::CameraManager's thread.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 src/android/camera_device.cpp | 33 +++++++++++++++++----------------
 src/android/camera_device.h   |  6 ++++--
 src/android/camera_request.h  |  4 +---
 src/android/camera_stream.cpp |  4 ++--
 4 files changed, 24 insertions(+), 23 deletions(-)

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 18e974f56..0e71a9f0c 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -990,8 +990,6 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		FrameBuffer *frameBuffer = nullptr;
 		UniqueFD acquireFence;
 
-		MutexLocker lock(descriptor->streamsProcessMutex_);
-
 		switch (cameraStream->type()) {
 		case CameraStream::Type::Mapped:
 			/* Mapped streams will be handled in the next loop. */
@@ -1067,7 +1065,6 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 				<< cameraStream->configuration().pixelFormat << "]"
 				<< " (mapped)";
 
-		MutexLocker lock(descriptor->streamsProcessMutex_);
 		descriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });
 
 		/*
@@ -1242,9 +1239,6 @@  void CameraDevice::requestComplete(Request *request)
 		descriptor->resultMetadata_ = std::make_unique<CameraMetadata>(0, 0);
 	}
 
-	/* Handle post-processing. */
-	MutexLocker locker(descriptor->streamsProcessMutex_);
-
 	/*
 	 * Queue all the post-processing streams request at once. The completion
 	 * slot streamProcessingComplete() can only execute when we are out
@@ -1272,10 +1266,8 @@  void CameraDevice::requestComplete(Request *request)
 		}
 	}
 
-	if (descriptor->pendingStreamsToProcess_.empty()) {
-		locker.unlock();
+	if (descriptor->pendingStreamsToProcess_.empty())
 		completeDescriptor(descriptor);
-	}
 }
 
 /**
@@ -1382,6 +1374,19 @@  void CameraDevice::setBufferStatus(StreamBuffer &streamBuffer,
 	}
 }
 
+void CameraDevice::streamProcessingCompleteDelegate(StreamBuffer *streamBuffer,
+						    StreamBuffer::Status status)
+{
+	/*
+	 * Delegate the callback to the camera manager thread to simplify race condition.
+	 */
+	auto *method = new BoundMethodMember{
+		this, camera_.get(), &CameraDevice::streamProcessingComplete, ConnectionTypeQueued
+	};
+
+	method->activate(streamBuffer, status);
+}
+
 /**
  * \brief Handle post-processing completion of a stream in a capture request
  * \param[in] streamBuffer The StreamBuffer for which processing is complete
@@ -1402,13 +1407,9 @@  void CameraDevice::streamProcessingComplete(StreamBuffer *streamBuffer,
 
 	Camera3RequestDescriptor *request = streamBuffer->request;
 
-	{
-		MutexLocker locker(request->streamsProcessMutex_);
-
-		request->pendingStreamsToProcess_.erase(streamBuffer->stream);
-		if (!request->pendingStreamsToProcess_.empty())
-			return;
-	}
+	request->pendingStreamsToProcess_.erase(streamBuffer->stream);
+	if (!request->pendingStreamsToProcess_.empty())
+		return;
 
 	completeDescriptor(streamBuffer->request);
 }
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 699aa8f17..69d163d76 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -65,8 +65,8 @@  public:
 	int configureStreams(camera3_stream_configuration_t *stream_list);
 	int processCaptureRequest(camera3_capture_request_t *request);
 	void requestComplete(libcamera::Request *request);
-	void streamProcessingComplete(StreamBuffer *bufferStream,
-				      StreamBuffer::Status status);
+	void streamProcessingCompleteDelegate(StreamBuffer *bufferStream,
+					      StreamBuffer::Status status);
 
 protected:
 	std::string logPrefix() const override;
@@ -96,6 +96,8 @@  private:
 	int processControls(Camera3RequestDescriptor *descriptor);
 	void completeDescriptor(Camera3RequestDescriptor *descriptor)
 		LIBCAMERA_TSA_EXCLUDES(descriptorsMutex_);
+	void streamProcessingComplete(StreamBuffer *bufferStream,
+				      StreamBuffer::Status status);
 	void sendCaptureResults() LIBCAMERA_TSA_REQUIRES(descriptorsMutex_);
 	void sendCaptureResult(Camera3RequestDescriptor *request) const;
 	void setBufferStatus(StreamBuffer &buffer,
diff --git a/src/android/camera_request.h b/src/android/camera_request.h
index 6b2a00795..bd75d4595 100644
--- a/src/android/camera_request.h
+++ b/src/android/camera_request.h
@@ -66,9 +66,7 @@  public:
 	};
 
 	/* Keeps track of streams requiring post-processing. */
-	std::map<CameraStream *, StreamBuffer *> pendingStreamsToProcess_
-		LIBCAMERA_TSA_GUARDED_BY(streamsProcessMutex_);
-	libcamera::Mutex streamsProcessMutex_;
+	std::map<CameraStream *, StreamBuffer *> pendingStreamsToProcess_;
 
 	Camera3RequestDescriptor(libcamera::Camera *camera,
 				 const camera3_capture_request_t *camera3Request);
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 53f292d4b..7837fd7aa 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -121,8 +121,8 @@  int CameraStream::configure()
 				else
 					bufferStatus = StreamBuffer::Status::Error;
 
-				cameraDevice_->streamProcessingComplete(streamBuffer,
-									bufferStatus);
+				cameraDevice_->streamProcessingCompleteDelegate(streamBuffer,
+										bufferStatus);
 			});
 
 		worker_->start();