[libcamera-devel] [PATCH 1/8] android: Rework request completion notification
Jacopo Mondi
jacopo at jmondi.org
Mon May 10 12:52:28 CEST 2021
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 at jmondi.org>
---
src/android/camera_device.cpp | 138 +++++++++++++++++++++-------------
src/android/camera_device.h | 3 +-
2 files changed, 86 insertions(+), 55 deletions(-)
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index f1f26c775611..d8b7e62ef896 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -2025,17 +2025,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;
}
@@ -2043,16 +2046,72 @@ 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_,
+ descriptor.buffers_[0].stream,
+ 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_, descriptor.buffers_[0].stream,
+ CAMERA3_MSG_ERROR_RESULT);
+
+ /* The camera framework expects an empy metadata pack on error. */
+ resultMetadata = std::make_unique<CameraMetadata>(0, 0);
+ }
+ captureResult.result = resultMetadata->get();
/* Handle any JPEG compression. */
for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
@@ -2065,56 +2124,27 @@ 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);
+ }
}
callbacks_->process_capture_result(callbacks_, &captureResult);
@@ -2136,23 +2166,23 @@ void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
callbacks_->notify(callbacks_, ¬ify);
}
-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_, ¬ify);
+
+ if (stream)
+ LOG(HAL, Error) << "Error occurred on frame " << frameNumber << " ("
+ << toPixelFormat(stream->format).toString() << ")";
+ else
+ LOG(HAL, Error) << "Fatal error occurred on device";
}
/*
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 23457e47767a..8d5da8bc59e1 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -101,7 +101,8 @@ private:
std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
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;
--
2.31.1
More information about the libcamera-devel
mailing list