summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacopo Mondi <jacopo@jmondi.org>2021-09-24 17:59:20 +0200
committerJacopo Mondi <jacopo@jmondi.org>2021-09-29 20:29:07 +0200
commit2eca16b674ff9e849d7681fbc670325de87ad0a2 (patch)
treee007d0860173186c024fec060a8f31c4f80ec321
parentc8c1d07cea2d8ef63905bf4f0b5266a5e6fd749f (diff)
android: Wait on fences in CameraStream::process()
Acquire fences for streams of type Mapped generated by post-processing are not correctly handled and are currently ignored by the camera HAL. Fix this by adding CameraStream::waitFence(), executed before starting the post-processing in CameraStream::process(). The change applies to all streams generated by post-processing (Mapped and Internal) but currently acquire fences of Internal streams are handled by the camera worker. Postpone that to post-processing time by passing -1 to the Worker for Internal streams. Also correct the release_fence handling for failed captures, as the framework requires the release fences to be set to the acquire fence value if the acquire fence has not been waited on. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Tested-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
-rw-r--r--src/android/camera_device.cpp39
-rw-r--r--src/android/camera_stream.cpp50
-rw-r--r--src/android/camera_stream.h4
3 files changed, 85 insertions, 8 deletions
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index ab381168..5e2fad1e 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -983,9 +983,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
/*
* Inspect the camera stream type, create buffers opportunely
- * and add them to the Request if required.
+ * and add them to the Request if required. Only acquire fences
+ * for streams of type Direct are handled by the CameraWorker,
+ * while fences for streams of type Internal and Mapped are
+ * handled at post-processing time.
*/
FrameBuffer *buffer = nullptr;
+ int acquireFence = -1;
switch (cameraStream->type()) {
case CameraStream::Type::Mapped:
/*
@@ -1008,6 +1012,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
cameraStream->configuration().size));
buffer = descriptor.frameBuffers_.back().get();
+ acquireFence = camera3Buffer.acquire_fence;
LOG(HAL, Debug) << ss.str() << " (direct)";
break;
@@ -1030,7 +1035,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
}
descriptor.request_->addBuffer(cameraStream->stream(), buffer,
- camera3Buffer.acquire_fence);
+ acquireFence);
}
/*
@@ -1110,7 +1115,22 @@ void CameraDevice::requestComplete(Request *request)
captureResult.frame_number = descriptor.frameNumber_;
captureResult.num_output_buffers = descriptor.buffers_.size();
for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
- buffer.acquire_fence = -1;
+ CameraStream *cameraStream =
+ static_cast<CameraStream *>(buffer.stream->priv);
+
+ /*
+ * Streams of type Direct have been queued to the
+ * libcamera::Camera and their acquire fences have
+ * already been waited on by the CameraWorker.
+ *
+ * Acquire fences of streams of type Internal and Mapped
+ * will be handled during post-processing.
+ *
+ * \todo Instrument the CameraWorker to set the acquire
+ * fence to -1 once it has handled it and remove this check.
+ */
+ if (cameraStream->type() == CameraStream::Type::Direct)
+ buffer.acquire_fence = -1;
buffer.release_fence = -1;
buffer.status = CAMERA3_BUFFER_STATUS_OK;
}
@@ -1130,8 +1150,17 @@ void CameraDevice::requestComplete(Request *request)
CAMERA3_MSG_ERROR_REQUEST);
captureResult.partial_result = 0;
- for (camera3_stream_buffer_t &buffer : descriptor.buffers_)
+ for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
+ /*
+ * Signal to the framework it has to handle fences that
+ * have not been waited on by setting the release fence
+ * to the acquire fence value.
+ */
+ buffer.release_fence = buffer.acquire_fence;
+ buffer.acquire_fence = -1;
buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
+ }
+
callbacks_->process_capture_result(callbacks_, &captureResult);
return;
@@ -1181,7 +1210,7 @@ void CameraDevice::requestComplete(Request *request)
continue;
}
- int ret = cameraStream->process(*src, *buffer.buffer,
+ int ret = cameraStream->process(*src, buffer,
descriptor.settings_,
resultMetadata.get());
/*
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index e30c7ee4..e80f842d 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -7,7 +7,11 @@
#include "camera_stream.h"
+#include <errno.h>
+#include <string.h>
#include <sys/mman.h>
+#include <sys/poll.h>
+#include <unistd.h>
#include <libcamera/formats.h>
@@ -109,11 +113,53 @@ int CameraStream::configure()
return 0;
}
+int CameraStream::waitFence(int fence)
+{
+ /*
+ * \todo The implementation here is copied from camera_worker.cpp
+ * and both should be removed once libcamera is instrumented to handle
+ * fences waiting in the core.
+ *
+ * \todo Better characterize the timeout. Currently equal to the one
+ * used by the Rockchip Camera HAL on ChromeOS.
+ */
+ constexpr unsigned int timeoutMs = 300;
+ struct pollfd fds = { fence, POLLIN, 0 };
+
+ do {
+ int ret = poll(&fds, 1, timeoutMs);
+ if (ret == 0)
+ return -ETIME;
+
+ if (ret > 0) {
+ if (fds.revents & (POLLERR | POLLNVAL))
+ return -EINVAL;
+
+ return 0;
+ }
+ } while (errno == EINTR || errno == EAGAIN);
+
+ return -errno;
+}
+
int CameraStream::process(const FrameBuffer &source,
- buffer_handle_t camera3Dest,
+ camera3_stream_buffer_t &camera3Dest,
const CameraMetadata &requestMetadata,
CameraMetadata *resultMetadata)
{
+ /* Handle waiting on fences on the destination buffer. */
+ int fence = camera3Dest.acquire_fence;
+ if (fence != -1) {
+ int ret = waitFence(fence);
+ ::close(fence);
+ camera3Dest.acquire_fence = -1;
+ if (ret < 0) {
+ LOG(HAL, Error) << "Failed waiting for fence: "
+ << fence << ": " << strerror(-ret);
+ return ret;
+ }
+ }
+
if (!postProcessor_)
return 0;
@@ -122,7 +168,7 @@ int CameraStream::process(const FrameBuffer &source,
* separate thread.
*/
const StreamConfiguration &output = configuration();
- CameraBuffer dest(camera3Dest, formats::MJPEG, output.size,
+ CameraBuffer dest(*camera3Dest.buffer, formats::MJPEG, output.size,
PROT_READ | PROT_WRITE);
if (!dest.isValid()) {
LOG(HAL, Error) << "Failed to map android blob buffer";
diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
index 2dab6c3a..03ecfa94 100644
--- a/src/android/camera_stream.h
+++ b/src/android/camera_stream.h
@@ -119,13 +119,15 @@ public:
int configure();
int process(const libcamera::FrameBuffer &source,
- buffer_handle_t camera3Dest,
+ camera3_stream_buffer_t &camera3Buffer,
const CameraMetadata &requestMetadata,
CameraMetadata *resultMetadata);
libcamera::FrameBuffer *getBuffer();
void putBuffer(libcamera::FrameBuffer *buffer);
private:
+ int waitFence(int fence);
+
CameraDevice *const cameraDevice_;
const libcamera::CameraConfiguration *config_;
const Type type_;