summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacopo Mondi <jacopo@jmondi.org>2021-05-04 16:02:33 +0200
committerJacopo Mondi <jacopo@jmondi.org>2021-06-14 12:26:27 +0200
commita86d281e52c772c4d40c6b9578c1eec8de8c5ca2 (patch)
tree7b6952592c362706bf113b720c65aa777aa264ab
parent15196e5b76f184d3d27900749de0bf7010226261 (diff)
android: Rework request completion notification
The current implementation of CameraDevice::requestComplete() which handles event notification and calls the framework capture result callback does not handle error notification precisely enough. In detail: - Error notification is an asynchronous callback that has to be notified to the framework as soon as an error condition is detected, and it independent from the process_capture_result() callback - Error notification requires the HAL to report the precise error cause, by specifying the correct CAMERA3_MSG_ERROR_* error code. The current implementation only notifies errors of type CAMERA3_MSG_ERROR_REQUEST at the end of the procedure, before the callback invocation. Rework the procedure to: - Notify CAMERA3_MSG_ERROR_DEVICE and perform library tear-down in case a Fatal error is detected - Notify CAMERA3_MSG_ERROR_REQUEST if the libcamera::Request::status is different than RequestCompleted and immediately call process_capture_result() with all buffers in error state. - Notify the shutter event as soon as possible - Notify CAMERA3_MSG_ERROR_RESULT in case the metadata cannot be generated correctly and call process_capture_result() with the right buffer state regardless of metadata availability. - Notify CAMERA3_MSG_ERROR_BUFFER for buffers whose post-processing failed While at it, return the CameraStream buffer by calling cameraStream->putBuffer() regardless of the post-processing result. No regression detected when running CTS in LIMITED mode. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
-rw-r--r--src/android/camera_device.cpp130
-rw-r--r--src/android/camera_device.h3
2 files changed, 78 insertions, 55 deletions
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index fe332ec3..d7f24101 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -2012,17 +2012,20 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
void CameraDevice::requestComplete(Request *request)
{
- camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK;
- std::unique_ptr<CameraMetadata> resultMetadata;
-
decltype(descriptors_)::node_type node;
{
std::scoped_lock<std::mutex> lock(mutex_);
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();
- status = CAMERA3_BUFFER_STATUS_ERROR;
+
return;
}
@@ -2030,16 +2033,69 @@ void CameraDevice::requestComplete(Request *request)
}
Camera3RequestDescriptor &descriptor = node.mapped();
+ /*
+ * Prepare the capture result for the Android camera stack.
+ *
+ * The buffer status is set to OK and later changed to ERROR if
+ * 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_) {
+ buffer.acquire_fence = -1;
+ buffer.release_fence = -1;
+ buffer.status = CAMERA3_BUFFER_STATUS_OK;
+ }
+ captureResult.output_buffers = descriptor.buffers_.data();
+ captureResult.partial_result = 1;
+
+ /*
+ * If the Request has failed, abort the request by notifying the error
+ * and complete the request with all buffers in error state.
+ */
if (request->status() != Request::RequestComplete) {
- LOG(HAL, Error) << "Request not successfully completed: "
+ LOG(HAL, Error) << "Request " << request->cookie()
+ << " not successfully completed: "
<< request->status();
- status = CAMERA3_BUFFER_STATUS_ERROR;
+
+ notifyError(descriptor.frameNumber_, nullptr,
+ CAMERA3_MSG_ERROR_REQUEST);
+
+ captureResult.partial_result = 0;
+ for (camera3_stream_buffer_t &buffer : descriptor.buffers_)
+ buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
+ callbacks_->process_capture_result(callbacks_, &captureResult);
+
+ return;
}
+ /*
+ * Notify shutter as soon as we have verified we have a valid request.
+ *
+ * \todo The shutter event notification should be sent to the framework
+ * as soon as possible, earlier than request completion time.
+ */
+ uint64_t sensorTimestamp = static_cast<uint64_t>(request->metadata()
+ .get(controls::SensorTimestamp));
+ notifyShutter(descriptor.frameNumber_, sensorTimestamp);
+
LOG(HAL, Debug) << "Request " << request->cookie() << " completed with "
<< descriptor.buffers_.size() << " streams";
- resultMetadata = getResultMetadata(descriptor);
+ /*
+ * Generate the metadata associated with the captured buffers.
+ *
+ * Notify if the metadata generation has failed, but continue processing
+ * buffers and return an empty metadata pack.
+ */
+ std::unique_ptr<CameraMetadata> resultMetadata = getResultMetadata(descriptor);
+ if (!resultMetadata) {
+ notifyError(descriptor.frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
+
+ /* The camera framework expects an empy metadata pack on error. */
+ resultMetadata = std::make_unique<CameraMetadata>(0, 0);
+ }
/* Handle any JPEG compression. */
for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
@@ -2052,58 +2108,30 @@ void CameraDevice::requestComplete(Request *request)
FrameBuffer *src = request->findBuffer(cameraStream->stream());
if (!src) {
LOG(HAL, Error) << "Failed to find a source stream buffer";
+ buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
+ notifyError(descriptor.frameNumber_, buffer.stream,
+ CAMERA3_MSG_ERROR_BUFFER);
continue;
}
- int ret = cameraStream->process(*src,
- *buffer.buffer,
+ int ret = cameraStream->process(*src, *buffer.buffer,
descriptor.settings_,
resultMetadata.get());
- if (ret) {
- status = CAMERA3_BUFFER_STATUS_ERROR;
- continue;
- }
-
/*
* Return the FrameBuffer to the CameraStream now that we're
* done processing it.
*/
if (cameraStream->type() == CameraStream::Type::Internal)
cameraStream->putBuffer(src);
- }
- /* Prepare to call back the Android camera stack. */
- 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_) {
- buffer.acquire_fence = -1;
- buffer.release_fence = -1;
- buffer.status = status;
- }
- captureResult.output_buffers = descriptor.buffers_.data();
-
- if (status == CAMERA3_BUFFER_STATUS_OK) {
- uint64_t timestamp =
- static_cast<uint64_t>(request->metadata()
- .get(controls::SensorTimestamp));
- notifyShutter(descriptor.frameNumber_, timestamp);
-
- captureResult.partial_result = 1;
- captureResult.result = resultMetadata->get();
- }
-
- if (status == CAMERA3_BUFFER_STATUS_ERROR || !captureResult.result) {
- /* \todo Improve error handling. In case we notify an error
- * because the metadata generation fails, a shutter event has
- * already been notified for this frame number before the error
- * is here signalled. Make sure the error path plays well with
- * the camera stack state machine.
- */
- notifyError(descriptor.frameNumber_,
- descriptor.buffers_[0].stream);
+ if (ret) {
+ buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
+ notifyError(descriptor.frameNumber_, buffer.stream,
+ CAMERA3_MSG_ERROR_BUFFER);
+ }
}
+ captureResult.result = resultMetadata->get();
callbacks_->process_capture_result(callbacks_, &captureResult);
}
@@ -2123,21 +2151,15 @@ void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
callbacks_->notify(callbacks_, &notify);
}
-void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
+void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,
+ camera3_error_msg_code code)
{
camera3_notify_msg_t notify = {};
- /*
- * \todo Report and identify the stream number or configuration to
- * clarify the stream that failed.
- */
- LOG(HAL, Error) << "Error occurred on frame " << frameNumber << " ("
- << toPixelFormat(stream->format).toString() << ")";
-
notify.type = CAMERA3_MSG_ERROR;
notify.message.error.error_stream = stream;
notify.message.error.frame_number = frameNumber;
- notify.message.error.error_code = CAMERA3_MSG_ERROR_REQUEST;
+ notify.message.error.error_code = code;
callbacks_->notify(callbacks_, &notify);
}
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 8190d929..decf1754 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -101,7 +101,8 @@ private:
libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
- void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
+ void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
+ camera3_error_msg_code code);
std::unique_ptr<CameraMetadata> requestTemplatePreview();
std::unique_ptr<CameraMetadata> requestTemplateVideo();
libcamera::PixelFormat toPixelFormat(int format) const;