[libcamera-devel] [PATCH 1/2] android: wait on fences in CameraStream::process()
Hirokazu Honda
hiroh at chromium.org
Mon Sep 27 07:19:29 CEST 2021
Hi Jacopo, thank you for the patch.
On Sat, Sep 25, 2021 at 2:01 AM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Acquisition fences for streams generated by post-processing are not
> correctly handled and are currently ignored by the camera HAL.
>
If I understand correctly, the problem happens with the stream whose
type() is CameraStream::Type::Mapped?
The acquire_fence is added to descriptor_.request_, so they are waited
and closed CameraWorker.
Am I wrong?
Then, it seems to be wrong to wait acquire_fence in process() because
it waits and closes the fence for Type::Internal.
-Hiro
> Add a waitFence() function to the CameraStream class to be executed before
> starting the post-processing in CameraStream::process().
>
> The CameraStream::waitFence() implementation is copied from the
> CameraWorker::Worker::waitFence() one, and both should be replaced by a
> libcamera core mechanism.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/android/camera_device.cpp | 2 +-
> src/android/camera_stream.cpp | 47 ++++++++++++++++++++++++++++++++++-
> src/android/camera_stream.h | 4 ++-
> 3 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 21844e5114a9..db35947afc2f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1179,7 +1179,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 e30c7ee4fcb3..c723da2c6407 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -7,7 +7,10 @@
>
> #include "camera_stream.h"
>
> +#include <errno.h>
> #include <sys/mman.h>
> +#include <sys/poll.h>
> +#include <unistd.h>
>
> #include <libcamera/formats.h>
>
> @@ -109,11 +112,52 @@ 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 &camera3Buffer,
> const CameraMetadata &requestMetadata,
> CameraMetadata *resultMetadata)
> {
> + /* Handle waiting on fences on the destination buffer. */
> + int fence = camera3Buffer.acquire_fence;
> + if (fence != -1) {
> + int ret = waitFence(fence);
> + ::close(fence);
> + if (ret < 0) {
> + LOG(HAL, Error) << "Failed waiting for fence: "
> + << fence << ": " << strerror(-ret);
> + return ret;
> + }
> + }
> +
> if (!postProcessor_)
> return 0;
>
> @@ -122,6 +166,7 @@ int CameraStream::process(const FrameBuffer &source,
> * separate thread.
> */
> const StreamConfiguration &output = configuration();
> + buffer_handle_t &camera3Dest = *camera3Buffer.buffer;
> CameraBuffer dest(camera3Dest, formats::MJPEG, output.size,
> PROT_READ | PROT_WRITE);
> if (!dest.isValid()) {
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 2dab6c3a37d1..1566e5e4009c 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -119,7 +119,7 @@ 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();
> @@ -132,6 +132,8 @@ private:
> camera3_stream_t *camera3Stream_;
> const unsigned int index_;
>
> + int waitFence(int fence);
> +
> std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
> std::vector<libcamera::FrameBuffer *> buffers_;
> /*
> --
> 2.32.0
>
More information about the libcamera-devel
mailing list