From 64bcbd0e2c2ac78fae7e329cec86bc4806cd5c45 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Tue, 26 Oct 2021 12:51:44 +0530 Subject: android: camera_device: Refactor descriptor status and sendCaptureResults() Currently, we use Camera3RequestDescriptor::Status to determine: - When the descriptor has been completely processed by HAL - Whether any errors were encountered, during its processing Both of these are essential to know whether the descriptor is eligible to call process_capture_results() through sendCaptureResults(). When a status(Success/Error) is set on the descriptor, it is ready to be sent back via sendCaptureResults(). However, this might lead to undesired results especially when sendCaptureResults() runs in a different thread (for e.g. stream's post-processor async completion slot). This patch decouples the descriptor status (Success/Error) from the descriptor's completion status (pending or complete). The advantage of this is we can set the completion status when the descriptor has been processed fully by the layer and we can set the error status on the descriptor wherever an error is encountered, throughout the lifetime of the descriptor in the HAL layer. While at it, introduce a wrapper completeDescriptor() around sendCaptureResults(). completeDescriptor() as the name suggests will mark the descriptor as complete, so it is ready to be sent back. The locking mechanism is moved from sendCaptureResults() to this wrapper since the intention is to use completeDescriptor() in place of existing sendCaptureResults() calls. Also make sure the sequence of abortRequest() call happens in the same order at all places i.e. after its added to the descriptors_ queue. Fix one of the abortRequest() call accordingly. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) (limited to 'src/android/camera_device.cpp') diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 806b4090..bf9a2e69 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -982,13 +982,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques MutexLocker stateLock(stateMutex_); if (state_ == State::Flushing) { - abortRequest(descriptor.get()); + Camera3RequestDescriptor *rawDescriptor = descriptor.get(); { MutexLocker descriptorsLock(descriptorsMutex_); descriptors_.push(std::move(descriptor)); } - - sendCaptureResults(); + abortRequest(rawDescriptor); + completeDescriptor(rawDescriptor); return 0; } @@ -1079,7 +1079,7 @@ void CameraDevice::requestComplete(Request *request) << request->status(); abortRequest(descriptor); - sendCaptureResults(); + completeDescriptor(descriptor); return; } @@ -1129,6 +1129,7 @@ void CameraDevice::requestComplete(Request *request) buffer.status = Camera3RequestDescriptor::Status::Error; notifyError(descriptor->frameNumber_, stream->camera3Stream(), CAMERA3_MSG_ERROR_BUFFER); + descriptor->status_ = Camera3RequestDescriptor::Status::Error; continue; } @@ -1145,27 +1146,27 @@ void CameraDevice::requestComplete(Request *request) buffer.status = Camera3RequestDescriptor::Status::Error; notifyError(descriptor->frameNumber_, stream->camera3Stream(), CAMERA3_MSG_ERROR_BUFFER); + descriptor->status_ = Camera3RequestDescriptor::Status::Error; } } - descriptor->status_ = Camera3RequestDescriptor::Status::Success; + completeDescriptor(descriptor); +} + +void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor) +{ + MutexLocker lock(descriptorsMutex_); + descriptor->complete_ = true; + sendCaptureResults(); } void CameraDevice::sendCaptureResults() { - MutexLocker lock(descriptorsMutex_); while (!descriptors_.empty() && !descriptors_.front()->isPending()) { auto descriptor = std::move(descriptors_.front()); descriptors_.pop(); - /* - * \todo Releasing and re-acquiring the critical section for - * every request completion (grain-locking) might have an - * impact on performance which should be measured. - */ - lock.unlock(); - camera3_capture_result_t captureResult = {}; captureResult.frame_number = descriptor->frameNumber_; @@ -1201,8 +1202,6 @@ void CameraDevice::sendCaptureResults() captureResult.partial_result = 1; callbacks_->process_capture_result(callbacks_, &captureResult); - - lock.lock(); } } -- cgit v1.2.1