[libcamera-devel,v4,4/4] android: camera_device: Send capture results by inspecting the queue
diff mbox series

Message ID 20210929133030.401961-5-umang.jain@ideasonboard.com
State Accepted
Delegated to: Umang Jain
Headers show
Series
  • Camera3RequestDescriptors std::map => std::queue
Related show

Commit Message

Umang Jain Sept. 29, 2021, 1:30 p.m. UTC 
There is a possibility that an out-of-order completion of capture
request happens by calling process_capture_result() directly on error
paths. The framework expects that errors should be notified as soon as
possible, but the request completion order should remain intact.
An existing instance of this is abortRequest(), which sends the capture
results on flushing state, without considering order-of-completion.

Since we have a queue of Camera3RequestDescriptor tracking each
capture request placed by framework to libcamera HAL, we should be only
sending back capture results from a single location, by inspecting
the queue. As per the patch, this now happens in
CameraDevice::sendCaptureResults().

Each descriptor is now equipped with its own status to denote whether
the capture request is complete and ready to send back to the framework
or needs to be waited upon. This ensures that the order of completion is
respected for the requests.

Since we are fixing out-of-order request completion in abortRequest(),
change the function to read from the Camera3RequestDescriptor directly,
instead of camera3_capture_request_t. The descriptor should have all the
information necessary to set the request buffers' state to error.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
---
 src/android/camera_device.cpp | 59 ++++++++++++++++++++++++-----------
 src/android/camera_device.h   | 14 ++++++++-
 2 files changed, 54 insertions(+), 19 deletions(-)

Comments

Jacopo Mondi Sept. 29, 2021, 6:50 p.m. UTC | #1 
Hi Umang,

On Wed, Sep 29, 2021 at 07:00:30PM +0530, Umang Jain wrote:
> There is a possibility that an out-of-order completion of capture
> request happens by calling process_capture_result() directly on error
> paths. The framework expects that errors should be notified as soon as
> possible, but the request completion order should remain intact.
> An existing instance of this is abortRequest(), which sends the capture
> results on flushing state, without considering order-of-completion.
>
> Since we have a queue of Camera3RequestDescriptor tracking each
> capture request placed by framework to libcamera HAL, we should be only
> sending back capture results from a single location, by inspecting
> the queue. As per the patch, this now happens in
> CameraDevice::sendCaptureResults().
>
> Each descriptor is now equipped with its own status to denote whether
> the capture request is complete and ready to send back to the framework

ready to be sent

> or needs to be waited upon. This ensures that the order of completion is
> respected for the requests.
>
> Since we are fixing out-of-order request completion in abortRequest(),
> change the function to read from the Camera3RequestDescriptor directly,
> instead of camera3_capture_request_t. The descriptor should have all the
> information necessary to set the request buffers' state to error.
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  src/android/camera_device.cpp | 59 ++++++++++++++++++++++++-----------
>  src/android/camera_device.h   | 14 ++++++++-
>  2 files changed, 54 insertions(+), 19 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 45350563..38c64bb7 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -860,25 +860,25 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>  	return 0;
>  }
>
> -void CameraDevice::abortRequest(camera3_capture_request_t *request) const
> +void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
>  {
> -	notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);
> +	notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);
>
> -	camera3_capture_result_t result = {};
> -	result.num_output_buffers = request->num_output_buffers;
> -	result.frame_number = request->frame_number;
> +	camera3_capture_result_t &result = descriptor->captureResult_;
> +	result.num_output_buffers = descriptor->buffers_.size();
> +	result.frame_number = descriptor->frameNumber_;
>  	result.partial_result = 0;
>
>  	std::vector<camera3_stream_buffer_t> resultBuffers(result.num_output_buffers);
>  	for (auto [i, buffer] : utils::enumerate(resultBuffers)) {
> -		buffer = request->output_buffers[i];
> -		buffer.release_fence = request->output_buffers[i].acquire_fence;
> +		buffer = descriptor->buffers_[i];
> +		buffer.release_fence = descriptor->buffers_[i].acquire_fence;
>  		buffer.acquire_fence = -1;
>  		buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
>  	}
>  	result.output_buffers = resultBuffers.data();
>
> -	callbacks_->process_capture_result(callbacks_, &result);
> +	descriptor->status_ = Camera3RequestDescriptor::Status::Error;
>  }
>
>  bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const
> @@ -1050,13 +1050,21 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		return ret;
>
>  	/*
> -	 * If flush is in progress abort the request. If the camera has been
> -	 * stopped we have to re-start it to be able to process the request.
> +	 * If flush is in progress set the request status to error and place it
> +	 * on the queue to be later completed. If the camera has been stopped we
> +	 * have to re-start it to be able to process the request.
>  	 */
>  	MutexLocker stateLock(stateMutex_);
>
>  	if (state_ == State::Flushing) {
> -		abortRequest(camera3Request);
> +		abortRequest(descriptor.get());
> +		{
> +			MutexLocker descriptorsLock(descriptorsMutex_);
> +			descriptors_.push(std::move(descriptor));
> +		}
> +
> +		sendCaptureResults();
> +
>  		return 0;
>  	}
>
> @@ -1116,7 +1124,7 @@ void CameraDevice::requestComplete(Request *request)
>  	 * The buffer status is set to OK and later changed to ERROR if
>  	 * post-processing/compression fails.
>  	 */
> -	camera3_capture_result_t captureResult = {};
> +	camera3_capture_result_t &captureResult = descriptor->captureResult_;
>  	captureResult.frame_number = descriptor->frameNumber_;
>  	captureResult.num_output_buffers = descriptor->buffers_.size();
>  	for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
> @@ -1166,10 +1174,9 @@ void CameraDevice::requestComplete(Request *request)
>  			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
>  		}
>
> -		callbacks_->process_capture_result(callbacks_, &captureResult);
> +		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> +		sendCaptureResults();
>
> -		MutexLocker descriptorsLock(descriptorsMutex_);
> -		descriptors_.pop();
>  		return;
>  	}
>
> @@ -1235,10 +1242,26 @@ void CameraDevice::requestComplete(Request *request)
>  	}
>
>  	captureResult.result = resultMetadata->get();
> -	callbacks_->process_capture_result(callbacks_, &captureResult);
> +	descriptor->status_ = Camera3RequestDescriptor::Status::Success;
> +	sendCaptureResults();
> +}
>
> -	MutexLocker descriptorsLock(descriptorsMutex_);
> -	descriptors_.pop();
> +void CameraDevice::sendCaptureResults()
> +{
> +	MutexLocker lock(descriptorsMutex_);
> +	while (!descriptors_.empty() && !descriptors_.front()->isPending()) {
> +		auto descriptor = std::move(descriptors_.front());
> +		descriptors_.pop();
> +
> +		/*
> +		 * \todo Measure performance impact of grain locking here against
> +		 * coarse locking.

I find this a bit hard to parse. Maybe

		 * \todo Releasing and re-acquiring the critical
                 * section for every request completion might have an
                 * impact on performaces which should be measured.

Up to you, really.

All minors might be optionally fixed when applying

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

> +		 */
> +		lock.unlock();
> +		callbacks_->process_capture_result(callbacks_,
> +						   &descriptor->captureResult_);
> +		lock.lock();
> +	}
>  }
>
>  std::string CameraDevice::logPrefix() const
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 85497921..b7d774fe 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -74,17 +74,28 @@ private:
>  	CameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> camera);
>
>  	struct Camera3RequestDescriptor {
> +		enum class Status {
> +			Pending,
> +			Success,
> +			Error,
> +		};
> +
>  		Camera3RequestDescriptor() = default;
>  		~Camera3RequestDescriptor() = default;
>  		Camera3RequestDescriptor(libcamera::Camera *camera,
>  					 const camera3_capture_request_t *camera3Request);
>  		Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;
>
> +		bool isPending() const { return status_ == Status::Pending; }
> +
>  		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_;
> +
> +		camera3_capture_result_t captureResult_ = {};
> +		Status status_ = Status::Pending;
>  	};
>
>  	enum class State {
> @@ -99,12 +110,13 @@ private:
>  	createFrameBuffer(const buffer_handle_t camera3buffer,
>  			  libcamera::PixelFormat pixelFormat,
>  			  const libcamera::Size &size);
> -	void abortRequest(camera3_capture_request_t *request) const;
> +	void abortRequest(Camera3RequestDescriptor *descriptor) const;
>  	bool isValidRequest(camera3_capture_request_t *request) const;
>  	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
>  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
>  			 camera3_error_msg_code code) const;
>  	int processControls(Camera3RequestDescriptor *descriptor);
> +	void sendCaptureResults();
>  	std::unique_ptr<CameraMetadata> getResultMetadata(
>  		const Camera3RequestDescriptor &descriptor) const;
>
> --
> 2.31.0
>
Umang Jain Sept. 30, 2021, 5:13 a.m. UTC | #2 
Hi Jacopo

On 9/30/21 12:20 AM, Jacopo Mondi wrote:
> Hi Umang,
>
> On Wed, Sep 29, 2021 at 07:00:30PM +0530, Umang Jain wrote:
>> There is a possibility that an out-of-order completion of capture
>> request happens by calling process_capture_result() directly on error
>> paths. The framework expects that errors should be notified as soon as
>> possible, but the request completion order should remain intact.
>> An existing instance of this is abortRequest(), which sends the capture
>> results on flushing state, without considering order-of-completion.
>>
>> Since we have a queue of Camera3RequestDescriptor tracking each
>> capture request placed by framework to libcamera HAL, we should be only
>> sending back capture results from a single location, by inspecting
>> the queue. As per the patch, this now happens in
>> CameraDevice::sendCaptureResults().
>>
>> Each descriptor is now equipped with its own status to denote whether
>> the capture request is complete and ready to send back to the framework
> ready to be sent
>
>> or needs to be waited upon. This ensures that the order of completion is
>> respected for the requests.
>>
>> Since we are fixing out-of-order request completion in abortRequest(),
>> change the function to read from the Camera3RequestDescriptor directly,
>> instead of camera3_capture_request_t. The descriptor should have all the
>> information necessary to set the request buffers' state to error.
>>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
>> ---
>>   src/android/camera_device.cpp | 59 ++++++++++++++++++++++++-----------
>>   src/android/camera_device.h   | 14 ++++++++-
>>   2 files changed, 54 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index 45350563..38c64bb7 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -860,25 +860,25 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>>   	return 0;
>>   }
>>
>> -void CameraDevice::abortRequest(camera3_capture_request_t *request) const
>> +void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
>>   {
>> -	notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);
>> +	notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);
>>
>> -	camera3_capture_result_t result = {};
>> -	result.num_output_buffers = request->num_output_buffers;
>> -	result.frame_number = request->frame_number;
>> +	camera3_capture_result_t &result = descriptor->captureResult_;
>> +	result.num_output_buffers = descriptor->buffers_.size();
>> +	result.frame_number = descriptor->frameNumber_;
>>   	result.partial_result = 0;
>>
>>   	std::vector<camera3_stream_buffer_t> resultBuffers(result.num_output_buffers);
>>   	for (auto [i, buffer] : utils::enumerate(resultBuffers)) {
>> -		buffer = request->output_buffers[i];
>> -		buffer.release_fence = request->output_buffers[i].acquire_fence;
>> +		buffer = descriptor->buffers_[i];
>> +		buffer.release_fence = descriptor->buffers_[i].acquire_fence;
>>   		buffer.acquire_fence = -1;
>>   		buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
>>   	}
>>   	result.output_buffers = resultBuffers.data();
>>
>> -	callbacks_->process_capture_result(callbacks_, &result);
>> +	descriptor->status_ = Camera3RequestDescriptor::Status::Error;
>>   }
>>
>>   bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const
>> @@ -1050,13 +1050,21 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>>   		return ret;
>>
>>   	/*
>> -	 * If flush is in progress abort the request. If the camera has been
>> -	 * stopped we have to re-start it to be able to process the request.
>> +	 * If flush is in progress set the request status to error and place it
>> +	 * on the queue to be later completed. If the camera has been stopped we
>> +	 * have to re-start it to be able to process the request.
>>   	 */
>>   	MutexLocker stateLock(stateMutex_);
>>
>>   	if (state_ == State::Flushing) {
>> -		abortRequest(camera3Request);
>> +		abortRequest(descriptor.get());
>> +		{
>> +			MutexLocker descriptorsLock(descriptorsMutex_);
>> +			descriptors_.push(std::move(descriptor));
>> +		}
>> +
>> +		sendCaptureResults();
>> +
>>   		return 0;
>>   	}
>>
>> @@ -1116,7 +1124,7 @@ void CameraDevice::requestComplete(Request *request)
>>   	 * The buffer status is set to OK and later changed to ERROR if
>>   	 * post-processing/compression fails.
>>   	 */
>> -	camera3_capture_result_t captureResult = {};
>> +	camera3_capture_result_t &captureResult = descriptor->captureResult_;
>>   	captureResult.frame_number = descriptor->frameNumber_;
>>   	captureResult.num_output_buffers = descriptor->buffers_.size();
>>   	for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
>> @@ -1166,10 +1174,9 @@ void CameraDevice::requestComplete(Request *request)
>>   			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
>>   		}
>>
>> -		callbacks_->process_capture_result(callbacks_, &captureResult);
>> +		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
>> +		sendCaptureResults();
>>
>> -		MutexLocker descriptorsLock(descriptorsMutex_);
>> -		descriptors_.pop();
>>   		return;
>>   	}
>>
>> @@ -1235,10 +1242,26 @@ void CameraDevice::requestComplete(Request *request)
>>   	}
>>
>>   	captureResult.result = resultMetadata->get();
>> -	callbacks_->process_capture_result(callbacks_, &captureResult);
>> +	descriptor->status_ = Camera3RequestDescriptor::Status::Success;
>> +	sendCaptureResults();
>> +}
>>
>> -	MutexLocker descriptorsLock(descriptorsMutex_);
>> -	descriptors_.pop();
>> +void CameraDevice::sendCaptureResults()
>> +{
>> +	MutexLocker lock(descriptorsMutex_);
>> +	while (!descriptors_.empty() && !descriptors_.front()->isPending()) {
>> +		auto descriptor = std::move(descriptors_.front());
>> +		descriptors_.pop();
>> +
>> +		/*
>> +		 * \todo Measure performance impact of grain locking here against
>> +		 * coarse locking.
> I find this a bit hard to parse. Maybe
>
> 		 * \todo Releasing and re-acquiring the critical
>                   * section for every request completion might have an
>                   * impact on performaces which should be measured.

s/performaces/performance/

I have updated to:

         * \todo Releasing and re-acquiring the critical section for
         * every request completion (grain-locking) might have an
         * impact on performance which should be measured.

>
> Up to you, really.
>
> All minors might be optionally fixed when applying
>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>


Thanks!

>
>> +		 */
>> +		lock.unlock();
>> +		callbacks_->process_capture_result(callbacks_,
>> +						   &descriptor->captureResult_);
>> +		lock.lock();
>> +	}
>>   }
>>
>>   std::string CameraDevice::logPrefix() const
>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
>> index 85497921..b7d774fe 100644
>> --- a/src/android/camera_device.h
>> +++ b/src/android/camera_device.h
>> @@ -74,17 +74,28 @@ private:
>>   	CameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> camera);
>>
>>   	struct Camera3RequestDescriptor {
>> +		enum class Status {
>> +			Pending,
>> +			Success,
>> +			Error,
>> +		};
>> +
>>   		Camera3RequestDescriptor() = default;
>>   		~Camera3RequestDescriptor() = default;
>>   		Camera3RequestDescriptor(libcamera::Camera *camera,
>>   					 const camera3_capture_request_t *camera3Request);
>>   		Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;
>>
>> +		bool isPending() const { return status_ == Status::Pending; }
>> +
>>   		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_;
>> +
>> +		camera3_capture_result_t captureResult_ = {};
>> +		Status status_ = Status::Pending;
>>   	};
>>
>>   	enum class State {
>> @@ -99,12 +110,13 @@ private:
>>   	createFrameBuffer(const buffer_handle_t camera3buffer,
>>   			  libcamera::PixelFormat pixelFormat,
>>   			  const libcamera::Size &size);
>> -	void abortRequest(camera3_capture_request_t *request) const;
>> +	void abortRequest(Camera3RequestDescriptor *descriptor) const;
>>   	bool isValidRequest(camera3_capture_request_t *request) const;
>>   	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
>>   	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
>>   			 camera3_error_msg_code code) const;
>>   	int processControls(Camera3RequestDescriptor *descriptor);
>> +	void sendCaptureResults();
>>   	std::unique_ptr<CameraMetadata> getResultMetadata(
>>   		const Camera3RequestDescriptor &descriptor) const;
>>
>> --
>> 2.31.0
>>

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 45350563..38c64bb7 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -860,25 +860,25 @@  int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
 	return 0;
 }
 
-void CameraDevice::abortRequest(camera3_capture_request_t *request) const
+void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
 {
-	notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);
+	notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);
 
-	camera3_capture_result_t result = {};
-	result.num_output_buffers = request->num_output_buffers;
-	result.frame_number = request->frame_number;
+	camera3_capture_result_t &result = descriptor->captureResult_;
+	result.num_output_buffers = descriptor->buffers_.size();
+	result.frame_number = descriptor->frameNumber_;
 	result.partial_result = 0;
 
 	std::vector<camera3_stream_buffer_t> resultBuffers(result.num_output_buffers);
 	for (auto [i, buffer] : utils::enumerate(resultBuffers)) {
-		buffer = request->output_buffers[i];
-		buffer.release_fence = request->output_buffers[i].acquire_fence;
+		buffer = descriptor->buffers_[i];
+		buffer.release_fence = descriptor->buffers_[i].acquire_fence;
 		buffer.acquire_fence = -1;
 		buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
 	}
 	result.output_buffers = resultBuffers.data();
 
-	callbacks_->process_capture_result(callbacks_, &result);
+	descriptor->status_ = Camera3RequestDescriptor::Status::Error;
 }
 
 bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const
@@ -1050,13 +1050,21 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		return ret;
 
 	/*
-	 * If flush is in progress abort the request. If the camera has been
-	 * stopped we have to re-start it to be able to process the request.
+	 * If flush is in progress set the request status to error and place it
+	 * on the queue to be later completed. If the camera has been stopped we
+	 * have to re-start it to be able to process the request.
 	 */
 	MutexLocker stateLock(stateMutex_);
 
 	if (state_ == State::Flushing) {
-		abortRequest(camera3Request);
+		abortRequest(descriptor.get());
+		{
+			MutexLocker descriptorsLock(descriptorsMutex_);
+			descriptors_.push(std::move(descriptor));
+		}
+
+		sendCaptureResults();
+
 		return 0;
 	}
 
@@ -1116,7 +1124,7 @@  void CameraDevice::requestComplete(Request *request)
 	 * The buffer status is set to OK and later changed to ERROR if
 	 * post-processing/compression fails.
 	 */
-	camera3_capture_result_t captureResult = {};
+	camera3_capture_result_t &captureResult = descriptor->captureResult_;
 	captureResult.frame_number = descriptor->frameNumber_;
 	captureResult.num_output_buffers = descriptor->buffers_.size();
 	for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
@@ -1166,10 +1174,9 @@  void CameraDevice::requestComplete(Request *request)
 			buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
 		}
 
-		callbacks_->process_capture_result(callbacks_, &captureResult);
+		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
+		sendCaptureResults();
 
-		MutexLocker descriptorsLock(descriptorsMutex_);
-		descriptors_.pop();
 		return;
 	}
 
@@ -1235,10 +1242,26 @@  void CameraDevice::requestComplete(Request *request)
 	}
 
 	captureResult.result = resultMetadata->get();
-	callbacks_->process_capture_result(callbacks_, &captureResult);
+	descriptor->status_ = Camera3RequestDescriptor::Status::Success;
+	sendCaptureResults();
+}
 
-	MutexLocker descriptorsLock(descriptorsMutex_);
-	descriptors_.pop();
+void CameraDevice::sendCaptureResults()
+{
+	MutexLocker lock(descriptorsMutex_);
+	while (!descriptors_.empty() && !descriptors_.front()->isPending()) {
+		auto descriptor = std::move(descriptors_.front());
+		descriptors_.pop();
+
+		/*
+		 * \todo Measure performance impact of grain locking here against
+		 * coarse locking.
+		 */
+		lock.unlock();
+		callbacks_->process_capture_result(callbacks_,
+						   &descriptor->captureResult_);
+		lock.lock();
+	}
 }
 
 std::string CameraDevice::logPrefix() const
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 85497921..b7d774fe 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -74,17 +74,28 @@  private:
 	CameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> camera);
 
 	struct Camera3RequestDescriptor {
+		enum class Status {
+			Pending,
+			Success,
+			Error,
+		};
+
 		Camera3RequestDescriptor() = default;
 		~Camera3RequestDescriptor() = default;
 		Camera3RequestDescriptor(libcamera::Camera *camera,
 					 const camera3_capture_request_t *camera3Request);
 		Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default;
 
+		bool isPending() const { return status_ == Status::Pending; }
+
 		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_;
+
+		camera3_capture_result_t captureResult_ = {};
+		Status status_ = Status::Pending;
 	};
 
 	enum class State {
@@ -99,12 +110,13 @@  private:
 	createFrameBuffer(const buffer_handle_t camera3buffer,
 			  libcamera::PixelFormat pixelFormat,
 			  const libcamera::Size &size);
-	void abortRequest(camera3_capture_request_t *request) const;
+	void abortRequest(Camera3RequestDescriptor *descriptor) const;
 	bool isValidRequest(camera3_capture_request_t *request) const;
 	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
 	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
 			 camera3_error_msg_code code) const;
 	int processControls(Camera3RequestDescriptor *descriptor);
+	void sendCaptureResults();
 	std::unique_ptr<CameraMetadata> getResultMetadata(
 		const Camera3RequestDescriptor &descriptor) const;