[{"id":19852,"web_url":"https://patchwork.libcamera.org/comment/19852/","msgid":"","date":"2021-09-27T05:19:29","subject":"Re: [libcamera-devel] [PATCH 1/2] android: wait on fences in\n\tCameraStream::process()","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, thank you for the patch.\n\nOn Sat, Sep 25, 2021 at 2:01 AM Jacopo Mondi wrote:\n>\n> Acquisition fences for streams generated by post-processing are not\n> correctly handled and are currently ignored by the camera HAL.\n>\n\nIf I understand correctly, the problem happens with the stream whose\ntype() is CameraStream::Type::Mapped?\nThe acquire_fence is added to descriptor_.request_, so they are waited\nand closed CameraWorker.\nAm I wrong?\nThen, it seems to be wrong to wait acquire_fence in process() because\nit waits and closes the fence for Type::Internal.\n\n-Hiro\n\n\n> Add a waitFence() function to the CameraStream class to be executed before\n> starting the post-processing in CameraStream::process().\n>\n> The CameraStream::waitFence() implementation is copied from the\n> CameraWorker::Worker::waitFence() one, and both should be replaced by a\n> libcamera core mechanism.\n>\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 2 +-\n> src/android/camera_stream.cpp | 47 ++++++++++++++++++++++++++++++++++-\n> src/android/camera_stream.h | 4 ++-\n> 3 files changed, 50 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 21844e5114a9..db35947afc2f 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1179,7 +1179,7 @@ void CameraDevice::requestComplete(Request *request)\n> continue;\n> }\n>\n> - int ret = cameraStream->process(*src, *buffer.buffer,\n> + int ret = cameraStream->process(*src, buffer,\n> descriptor.settings_,\n> resultMetadata.get());\n> /*\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index e30c7ee4fcb3..c723da2c6407 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -7,7 +7,10 @@\n>\n> #include \"camera_stream.h\"\n>\n> +#include \n> #include \n> +#include \n> +#include \n>\n> #include \n>\n> @@ -109,11 +112,52 @@ int CameraStream::configure()\n> return 0;\n> }\n>\n> +int CameraStream::waitFence(int fence)\n> +{\n> + /*\n> + * \\todo The implementation here is copied from camera_worker.cpp\n> + * and both should be removed once libcamera is instrumented to handle\n> + * fences waiting in the core.\n> + *\n> + * \\todo Better characterize the timeout. Currently equal to the one\n> + * used by the Rockchip Camera HAL on ChromeOS.\n> + */\n> + constexpr unsigned int timeoutMs = 300;\n> + struct pollfd fds = { fence, POLLIN, 0 };\n> +\n> + do {\n> + int ret = poll(&fds, 1, timeoutMs);\n> + if (ret == 0)\n> + return -ETIME;\n> +\n> + if (ret > 0) {\n> + if (fds.revents & (POLLERR | POLLNVAL))\n> + return -EINVAL;\n> +\n> + return 0;\n> + }\n> + } while (errno == EINTR || errno == EAGAIN);\n> +\n> + return -errno;\n> +}\n> +\n> int CameraStream::process(const FrameBuffer &source,\n> - buffer_handle_t camera3Dest,\n> + camera3_stream_buffer_t &camera3Buffer,\n> const CameraMetadata &requestMetadata,\n> CameraMetadata *resultMetadata)\n> {\n> + /* Handle waiting on fences on the destination buffer. */\n> + int fence = camera3Buffer.acquire_fence;\n> + if (fence != -1) {\n> + int ret = waitFence(fence);\n> + ::close(fence);\n> + if (ret < 0) {\n> + LOG(HAL, Error) << \"Failed waiting for fence: \"\n> + << fence << \": \" << strerror(-ret);\n> + return ret;\n> + }\n> + }\n> +\n> if (!postProcessor_)\n> return 0;\n>\n> @@ -122,6 +166,7 @@ int CameraStream::process(const FrameBuffer &source,\n> * separate thread.\n> */\n> const StreamConfiguration &output = configuration();\n> + buffer_handle_t &camera3Dest = *camera3Buffer.buffer;\n> CameraBuffer dest(camera3Dest, formats::MJPEG, output.size,\n> PROT_READ | PROT_WRITE);\n> if (!dest.isValid()) {\n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index 2dab6c3a37d1..1566e5e4009c 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -119,7 +119,7 @@ public:\n>\n> int configure();\n> int process(const libcamera::FrameBuffer &source,\n> - buffer_handle_t camera3Dest,\n> + camera3_stream_buffer_t &camera3Buffer,\n> const CameraMetadata &requestMetadata,\n> CameraMetadata *resultMetadata);\n> libcamera::FrameBuffer *getBuffer();\n> @@ -132,6 +132,8 @@ private:\n> camera3_stream_t *camera3Stream_;\n> const unsigned int index_;\n>\n> + int waitFence(int fence);\n> +\n> std::unique_ptr allocator_;\n> std::vector buffers_;\n> /*\n> --\n> 2.32.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A923BBF01C\n\tfor ;\n\tMon, 27 Sep 2021 05:19:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E44D16918E;\n\tMon, 27 Sep 2021 07:19:42 +0200 (CEST)","from mail-ed1-x530.google.com (mail-ed1-x530.google.com\n\t[IPv6:2a00:1450:4864:20::530])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B064D684C8\n\tfor ;\n\tMon, 27 Sep 2021 07:19:40 +0200 (CEST)","by mail-ed1-x530.google.com with SMTP id y35so12996265ede.3\n\tfor ;\n\tSun, 26 Sep 2021 22:19:40 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"SBWu18nM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=ybZe0S6tf0ronxXf6rrUqfentHOR4oKJ/LapV2ScBxI=;\n\tb=SBWu18nMpb5YdDJcRzaQTek+Th8pUZZKYso700P82Yt6D+PN/wcN3NVaOTrkW3GS3A\n\tNfKlh2aSmaClW1q/5pgFf6HiT4YHJSFJqWYyS2VEd5DnGUq7/Sc2cXBIWOahckkLyQVP\n\tC1ykrjKun6vmliMg0gD0wXu1GNQG/TFMIZRFo=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ybZe0S6tf0ronxXf6rrUqfentHOR4oKJ/LapV2ScBxI=;\n\tb=7m222R0zrjHEfAEPoscSgnoMZ1U4iHor1g5m5cug3IKL94mQZhPo1okeP74DWOLdiv\n\tffYxre7HQTCPcbY/4znGy4cytZJE+R8cpqhWU9qPUaULa+HgtD5uIHTmm9AIeFE/JsG2\n\tH61Hz/9inmtSwJpyJwkWIQQ38M7XZzb3dYpolTPQ6AMRUiTbdn90qKY7Ial5sgXLNK6W\n\thfF4Zj4QCzcLLpQOL/evhaCOYD0ELtWcQsOCmgCka2cnUlauWK6ljTAcIUs6MbrLMp7Q\n\ttOakWYzl8PbuXT/FU9BZxTceu7U4MRTD3FTj2sbCdDEb3Zx8QdWbd/QpiuchlZDBWgaw\n\tli9Q==","X-Gm-Message-State":"AOAM533D1h+XzbHCVbNCwvcyeJkCBo1W/DdZarVYVyE012H1/NCut0WL\n\tJjs5hpKFKvWiAhfljCyVX7h0zumSwMBBuFeYCrcCMyWF54k=","X-Google-Smtp-Source":"ABdhPJygjm6b5MmqJrG+40ozet/FOgcJ4mplOwOLBisBXiOIg5JgUdW7/kHFMtp5ioulN/D0KfJWiZbR8qVjnRHIpeU=","X-Received":"by 2002:aa7:c401:: with SMTP id j1mr3402574edq.354.1632719980241;\n\tSun, 26 Sep 2021 22:19:40 -0700 (PDT)","MIME-Version":"1.0","References":"<20210924170234.152783-1-jacopo@jmondi.org>\n\t<20210924170234.152783-2-jacopo@jmondi.org>","In-Reply-To":"<20210924170234.152783-2-jacopo@jmondi.org>","From":"Hirokazu Honda ","Date":"Mon, 27 Sep 2021 14:19:29 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: wait on fences in\n\tCameraStream::process()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19870,"web_url":"https://patchwork.libcamera.org/comment/19870/","msgid":"<20210927102819.vaqqvwto4caecvuu@uno.localdomain>","date":"2021-09-27T10:28:19","subject":"Re: [libcamera-devel] [PATCH 1/2] android: wait on fences in\n\tCameraStream::process()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Mon, Sep 27, 2021 at 02:19:29PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo, thank you for the patch.\n>\n> On Sat, Sep 25, 2021 at 2:01 AM Jacopo Mondi wrote:\n> >\n> > Acquisition fences for streams generated by post-processing are not\n> > correctly handled and are currently ignored by the camera HAL.\n> >\n>\n> If I understand correctly, the problem happens with the stream whose\n> type() is CameraStream::Type::Mapped?\n> The acquire_fence is added to descriptor_.request_, so they are waited\n> and closed CameraWorker.\n> Am I wrong?\n\nAcquire fences for streams of type ::Mapped are not added to the\ndescriptor_ and are not handled by the CameraWorker\n\n\tfor (unsigned int i = 0; i < descriptor.buffers_.size(); ++i) {\n\n ....\n\n\t\tswitch (cameraStream->type()) {\n\t\tcase CameraStream::Type::Mapped:\n\t\t\t/*\n\t\t\t * Mapped streams don't need buffers added to the\n\t\t\t * Request.\n\t\t\t */\n\t\t\tLOG(HAL, Debug) << ss.str() << \" (mapped)\";\n\t\t\tcontinue; <------------------- THIS\n\n\t\tcase CameraStream::Type::Direct:\n ...\n\t\t}\n\n ...\n\n\n\t\tdescriptor.request_->addBuffer(cameraStream->stream(), buffer,\n\t\t\t\t\t\tcamera3Buffer.acquire_fence);\n\t}\n\n\n\n> Then, it seems to be wrong to wait acquire_fence in process() because\n> it waits and closes the fence for Type::Internal.\n\nIf I got your comment right, we have a problem for Type::Internal, as\ntheir acquisition fence are waited on by the CameraWorker, but the\npost-processing phase waits on them too.\n\nIt doesn't seems to create issues as I've tested it with multiple CTS\nruns but it indeed it's not correct. I wonder why it is not\nproblematic, giving poll() a closed file descriptor should trigger the\nPULLHUP event flag. As we don't check for that flag, we might miss it\nand happly proceed maybe.\n\nSo yes, we should make sure ::Internal are waited on once only, and I\nwould do so at post-processing time before actually writing data to\nthe destination buffer. I think to do so, it woul be enough to pass -1\nas acquire fence to descriptor.request_->addBuffer() for ::Internal\nstreams.\n\nDid I get your comment right ?\n\n>\n> -Hiro\n>\n>\n> > Add a waitFence() function to the CameraStream class to be executed before\n> > starting the post-processing in CameraStream::process().\n> >\n> > The CameraStream::waitFence() implementation is copied from the\n> > CameraWorker::Worker::waitFence() one, and both should be replaced by a\n> > libcamera core mechanism.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 2 +-\n> > src/android/camera_stream.cpp | 47 ++++++++++++++++++++++++++++++++++-\n> > src/android/camera_stream.h | 4 ++-\n> > 3 files changed, 50 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 21844e5114a9..db35947afc2f 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1179,7 +1179,7 @@ void CameraDevice::requestComplete(Request *request)\n> > continue;\n> > }\n> >\n> > - int ret = cameraStream->process(*src, *buffer.buffer,\n> > + int ret = cameraStream->process(*src, buffer,\n> > descriptor.settings_,\n> > resultMetadata.get());\n> > /*\n> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > index e30c7ee4fcb3..c723da2c6407 100644\n> > --- a/src/android/camera_stream.cpp\n> > +++ b/src/android/camera_stream.cpp\n> > @@ -7,7 +7,10 @@\n> >\n> > #include \"camera_stream.h\"\n> >\n> > +#include \n> > #include \n> > +#include \n> > +#include \n> >\n> > #include \n> >\n> > @@ -109,11 +112,52 @@ int CameraStream::configure()\n> > return 0;\n> > }\n> >\n> > +int CameraStream::waitFence(int fence)\n> > +{\n> > + /*\n> > + * \\todo The implementation here is copied from camera_worker.cpp\n> > + * and both should be removed once libcamera is instrumented to handle\n> > + * fences waiting in the core.\n> > + *\n> > + * \\todo Better characterize the timeout. Currently equal to the one\n> > + * used by the Rockchip Camera HAL on ChromeOS.\n> > + */\n> > + constexpr unsigned int timeoutMs = 300;\n> > + struct pollfd fds = { fence, POLLIN, 0 };\n> > +\n> > + do {\n> > + int ret = poll(&fds, 1, timeoutMs);\n> > + if (ret == 0)\n> > + return -ETIME;\n> > +\n> > + if (ret > 0) {\n> > + if (fds.revents & (POLLERR | POLLNVAL))\n> > + return -EINVAL;\n> > +\n> > + return 0;\n> > + }\n> > + } while (errno == EINTR || errno == EAGAIN);\n> > +\n> > + return -errno;\n> > +}\n> > +\n> > int CameraStream::process(const FrameBuffer &source,\n> > - buffer_handle_t camera3Dest,\n> > + camera3_stream_buffer_t &camera3Buffer,\n> > const CameraMetadata &requestMetadata,\n> > CameraMetadata *resultMetadata)\n> > {\n> > + /* Handle waiting on fences on the destination buffer. */\n> > + int fence = camera3Buffer.acquire_fence;\n> > + if (fence != -1) {\n> > + int ret = waitFence(fence);\n> > + ::close(fence);\n> > + if (ret < 0) {\n> > + LOG(HAL, Error) << \"Failed waiting for fence: \"\n> > + << fence << \": \" << strerror(-ret);\n> > + return ret;\n> > + }\n> > + }\n> > +\n> > if (!postProcessor_)\n> > return 0;\n> >\n> > @@ -122,6 +166,7 @@ int CameraStream::process(const FrameBuffer &source,\n> > * separate thread.\n> > */\n> > const StreamConfiguration &output = configuration();\n> > + buffer_handle_t &camera3Dest = *camera3Buffer.buffer;\n> > CameraBuffer dest(camera3Dest, formats::MJPEG, output.size,\n> > PROT_READ | PROT_WRITE);\n> > if (!dest.isValid()) {\n> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > index 2dab6c3a37d1..1566e5e4009c 100644\n> > --- a/src/android/camera_stream.h\n> > +++ b/src/android/camera_stream.h\n> > @@ -119,7 +119,7 @@ public:\n> >\n> > int configure();\n> > int process(const libcamera::FrameBuffer &source,\n> > - buffer_handle_t camera3Dest,\n> > + camera3_stream_buffer_t &camera3Buffer,\n> > const CameraMetadata &requestMetadata,\n> > CameraMetadata *resultMetadata);\n> > libcamera::FrameBuffer *getBuffer();\n> > @@ -132,6 +132,8 @@ private:\n> > camera3_stream_t *camera3Stream_;\n> > const unsigned int index_;\n> >\n> > + int waitFence(int fence);\n> > +\n> > std::unique_ptr allocator_;\n> > std::vector buffers_;\n> > /*\n> > --\n> > 2.32.0\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3A5E1BDC71\n\tfor ;\n\tMon, 27 Sep 2021 10:27:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4D4456918B;\n\tMon, 27 Sep 2021 12:27:34 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A7626012C\n\tfor ;\n\tMon, 27 Sep 2021 12:27:32 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id B60BBE0005;\n\tMon, 27 Sep 2021 10:27:31 +0000 (UTC)"],"Date":"Mon, 27 Sep 2021 12:28:19 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210927102819.vaqqvwto4caecvuu@uno.localdomain>","References":"<20210924170234.152783-1-jacopo@jmondi.org>\n\t<20210924170234.152783-2-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: wait on fences in\n\tCameraStream::process()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19871,"web_url":"https://patchwork.libcamera.org/comment/19871/","msgid":"","date":"2021-09-27T11:04:33","subject":"Re: [libcamera-devel] [PATCH 1/2] android: wait on fences in\n\tCameraStream::process()","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo,\n\nOn Mon, Sep 27, 2021 at 7:27 PM Jacopo Mondi wrote:\n>\n> Hi Hiro,\n>\n> On Mon, Sep 27, 2021 at 02:19:29PM +0900, Hirokazu Honda wrote:\n> > Hi Jacopo, thank you for the patch.\n> >\n> > On Sat, Sep 25, 2021 at 2:01 AM Jacopo Mondi wrote:\n> > >\n> > > Acquisition fences for streams generated by post-processing are not\n> > > correctly handled and are currently ignored by the camera HAL.\n> > >\n> >\n> > If I understand correctly, the problem happens with the stream whose\n> > type() is CameraStream::Type::Mapped?\n> > The acquire_fence is added to descriptor_.request_, so they are waited\n> > and closed CameraWorker.\n> > Am I wrong?\n>\n> Acquire fences for streams of type ::Mapped are not added to the\n> descriptor_ and are not handled by the CameraWorker\n>\n> for (unsigned int i = 0; i < descriptor.buffers_.size(); ++i) {\n>\n> ....\n>\n> switch (cameraStream->type()) {\n> case CameraStream::Type::Mapped:\n> /*\n> * Mapped streams don't need buffers added to the\n> * Request.\n> */\n> LOG(HAL, Debug) << ss.str() << \" (mapped)\";\n> continue; <------------------- THIS\n>\n> case CameraStream::Type::Direct:\n> ...\n> }\n>\n> ...\n>\n>\n> descriptor.request_->addBuffer(cameraStream->stream(), buffer,\n> camera3Buffer.acquire_fence);\n> }\n>\n>\n>\n> > Then, it seems to be wrong to wait acquire_fence in process() because\n> > it waits and closes the fence for Type::Internal.\n>\n> If I got your comment right, we have a problem for Type::Internal, as\n> their acquisition fence are waited on by the CameraWorker, but the\n> post-processing phase waits on them too.\n>\n\nThanks. Sorry I wrongly wrote Mapped in the first sentence thinking of Internal.\n\n> It doesn't seems to create issues as I've tested it with multiple CTS\n> runs but it indeed it's not correct. I wonder why it is not\n> problematic, giving poll() a closed file descriptor should trigger the\n> PULLHUP event flag. As we don't check for that flag, we might miss it\n> and happly proceed maybe.\n>\n> So yes, we should make sure ::Internal are waited on once only, and I\n> would do so at post-processing time before actually writing data to\n> the destination buffer. I think to do so, it woul be enough to pass -1\n> as acquire fence to descriptor.request_->addBuffer() for ::Internal\n> streams.\n>\n> Did I get your comment right ?\n>\n\nIt sounds good to me.\n\n-Hiro\n\n> >\n> > -Hiro\n> >\n> >\n> > > Add a waitFence() function to the CameraStream class to be executed before\n> > > starting the post-processing in CameraStream::process().\n> > >\n> > > The CameraStream::waitFence() implementation is copied from the\n> > > CameraWorker::Worker::waitFence() one, and both should be replaced by a\n> > > libcamera core mechanism.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/android/camera_device.cpp | 2 +-\n> > > src/android/camera_stream.cpp | 47 ++++++++++++++++++++++++++++++++++-\n> > > src/android/camera_stream.h | 4 ++-\n> > > 3 files changed, 50 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index 21844e5114a9..db35947afc2f 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -1179,7 +1179,7 @@ void CameraDevice::requestComplete(Request *request)\n> > > continue;\n> > > }\n> > >\n> > > - int ret = cameraStream->process(*src, *buffer.buffer,\n> > > + int ret = cameraStream->process(*src, buffer,\n> > > descriptor.settings_,\n> > > resultMetadata.get());\n> > > /*\n> > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > > index e30c7ee4fcb3..c723da2c6407 100644\n> > > --- a/src/android/camera_stream.cpp\n> > > +++ b/src/android/camera_stream.cpp\n> > > @@ -7,7 +7,10 @@\n> > >\n> > > #include \"camera_stream.h\"\n> > >\n> > > +#include \n> > > #include \n> > > +#include \n> > > +#include \n> > >\n> > > #include \n> > >\n> > > @@ -109,11 +112,52 @@ int CameraStream::configure()\n> > > return 0;\n> > > }\n> > >\n> > > +int CameraStream::waitFence(int fence)\n> > > +{\n> > > + /*\n> > > + * \\todo The implementation here is copied from camera_worker.cpp\n> > > + * and both should be removed once libcamera is instrumented to handle\n> > > + * fences waiting in the core.\n> > > + *\n> > > + * \\todo Better characterize the timeout. Currently equal to the one\n> > > + * used by the Rockchip Camera HAL on ChromeOS.\n> > > + */\n> > > + constexpr unsigned int timeoutMs = 300;\n> > > + struct pollfd fds = { fence, POLLIN, 0 };\n> > > +\n> > > + do {\n> > > + int ret = poll(&fds, 1, timeoutMs);\n> > > + if (ret == 0)\n> > > + return -ETIME;\n> > > +\n> > > + if (ret > 0) {\n> > > + if (fds.revents & (POLLERR | POLLNVAL))\n> > > + return -EINVAL;\n> > > +\n> > > + return 0;\n> > > + }\n> > > + } while (errno == EINTR || errno == EAGAIN);\n> > > +\n> > > + return -errno;\n> > > +}\n> > > +\n> > > int CameraStream::process(const FrameBuffer &source,\n> > > - buffer_handle_t camera3Dest,\n> > > + camera3_stream_buffer_t &camera3Buffer,\n> > > const CameraMetadata &requestMetadata,\n> > > CameraMetadata *resultMetadata)\n> > > {\n> > > + /* Handle waiting on fences on the destination buffer. */\n> > > + int fence = camera3Buffer.acquire_fence;\n> > > + if (fence != -1) {\n> > > + int ret = waitFence(fence);\n> > > + ::close(fence);\n> > > + if (ret < 0) {\n> > > + LOG(HAL, Error) << \"Failed waiting for fence: \"\n> > > + << fence << \": \" << strerror(-ret);\n> > > + return ret;\n> > > + }\n> > > + }\n> > > +\n> > > if (!postProcessor_)\n> > > return 0;\n> > >\n> > > @@ -122,6 +166,7 @@ int CameraStream::process(const FrameBuffer &source,\n> > > * separate thread.\n> > > */\n> > > const StreamConfiguration &output = configuration();\n> > > + buffer_handle_t &camera3Dest = *camera3Buffer.buffer;\n> > > CameraBuffer dest(camera3Dest, formats::MJPEG, output.size,\n> > > PROT_READ | PROT_WRITE);\n> > > if (!dest.isValid()) {\n> > > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > > index 2dab6c3a37d1..1566e5e4009c 100644\n> > > --- a/src/android/camera_stream.h\n> > > +++ b/src/android/camera_stream.h\n> > > @@ -119,7 +119,7 @@ public:\n> > >\n> > > int configure();\n> > > int process(const libcamera::FrameBuffer &source,\n> > > - buffer_handle_t camera3Dest,\n> > > + camera3_stream_buffer_t &camera3Buffer,\n> > > const CameraMetadata &requestMetadata,\n> > > CameraMetadata *resultMetadata);\n> > > libcamera::FrameBuffer *getBuffer();\n> > > @@ -132,6 +132,8 @@ private:\n> > > camera3_stream_t *camera3Stream_;\n> > > const unsigned int index_;\n> > >\n> > > + int waitFence(int fence);\n> > > +\n> > > std::unique_ptr allocator_;\n> > > std::vector buffers_;\n> > > /*\n> > > --\n> > > 2.32.0\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 68C2BBDC71\n\tfor ;\n\tMon, 27 Sep 2021 11:04:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF2826918B;\n\tMon, 27 Sep 2021 13:04:46 +0200 (CEST)","from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com\n\t[IPv6:2a00:1450:4864:20::52a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ADC3D6012C\n\tfor ;\n\tMon, 27 Sep 2021 13:04:44 +0200 (CEST)","by mail-ed1-x52a.google.com with SMTP id b26so19232416edt.0\n\tfor ;\n\tMon, 27 Sep 2021 04:04:44 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"RzuDnnGK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=0qFnQWwjWUatgv4C00AQ/2mCaL22OMJnBpzQRpj3ty8=;\n\tb=RzuDnnGKhH/Qx3mODpR5KguUy+2T3CEPIZh8lVXOhENUEs7q6wgwXPnIQsyPEohBKn\n\tVX0eBk1mH3cI+L6XLGccTzQ00HhCb/yRKfZpDEmT9EapyWOL/m06ZwPTLoKOhod+72bc\n\t9N36CTNUXZqFErguJvprmRtGvK/zH4vDDC3VI=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=0qFnQWwjWUatgv4C00AQ/2mCaL22OMJnBpzQRpj3ty8=;\n\tb=GKYCkdQS4FMTftcZ8ios+Zj15KfvdAnDCxaO4aOnybkR9yhhT2KchhlXSOXfcTRDeW\n\t6b0lG3rY4R2o9+srl+G0iRkIZAumCov7FcQ5aewaDWZ4SO4KsxNvTF2jTrO+lVIVIDqp\n\t7Tf83Kvo7Xqj1UkZ4zAP6MwEfLurULVMoN6Tpbwdz25FxStslqH3x6QSdxKZ1YLZh6gG\n\tCdYhG+lFXehTZMBQWHemwl3ShSZclSXi/jOVjHLlxOW7Pwp8k3oRaXZGOCx+1iqBMn1V\n\t8AHWl3Cv7D0c4RBV6yFTg6wWm6gvYRGUuq2eMHGIyQsIXELcjKdNKFhe5hhzbJW4ELGG\n\t7h6Q==","X-Gm-Message-State":"AOAM530D3ieTTjGsEwymLB9AlhC01U9i5i5UIMme3d1L+QH8hSG81012\n\tHbPVu9EvxtloQsTTKVoyzR2wweH7HdJ8oGg2OCWVwgsu27c=","X-Google-Smtp-Source":"ABdhPJxQjWblvc3ZX9IhAA29IJrXuoYM9LzLvOGqsvfPxUS6WzlhY5SC206DUVd5V1kgryI6rPu3QmkxKjQtindIj74=","X-Received":"by 2002:a17:906:32c9:: with SMTP id\n\tk9mr26858826ejk.218.1632740684246; \n\tMon, 27 Sep 2021 04:04:44 -0700 (PDT)","MIME-Version":"1.0","References":"<20210924170234.152783-1-jacopo@jmondi.org>\n\t<20210924170234.152783-2-jacopo@jmondi.org>\n\t\n\t<20210927102819.vaqqvwto4caecvuu@uno.localdomain>","In-Reply-To":"<20210927102819.vaqqvwto4caecvuu@uno.localdomain>","From":"Hirokazu Honda ","Date":"Mon, 27 Sep 2021 20:04:33 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: wait on fences in\n\tCameraStream::process()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]