[{"id":35768,"web_url":"https://patchwork.libcamera.org/comment/35768/","msgid":"<20250910164501.GG20904@pendragon.ideasonboard.com>","date":"2025-09-10T16:45:01","subject":"Re: [PATCH v2] libcamera: request: addBuffer(): Do not destroy fence\n\ton failure","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Sep 10, 2025 at 05:18:57PM +0200, Barnabás Pőcze wrote:\n> Take the unique pointer to the `Fence` object by rvalue reference\n> so that it is not destroyed if the function returns an error code\n> and does not take ownership of the unique pointer.\n> \n> Signed-off-by: Barnabás Pőcze \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> changes in v2:\n> * modify function documentation\n> \n> v1: https://patchwork.libcamera.org/patch/22907/\n> ---\n> include/libcamera/request.h | 2 +-\n> src/libcamera/request.cpp | 6 ++++--\n> 2 files changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index e214a9d13..0c5939f7b 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -53,7 +53,7 @@ public:\n> \tControlList &metadata() { return *metadata_; }\n> \tconst BufferMap &buffers() const { return bufferMap_; }\n> \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n> -\t\t std::unique_ptr fence = nullptr);\n> +\t\t std::unique_ptr &&fence = {});\n> \tFrameBuffer *findBuffer(const Stream *stream) const;\n> \n> \tuint32_t sequence() const;\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 7f1e11e8f..26bba8f28 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -452,7 +452,9 @@ void Request::reuse(ReuseFlag flags)\n> *\n> * When a valid Fence is provided to this function, \\a fence is moved to \\a\n> * buffer and this Request will only be queued to the device once the\n> - * fences of all its buffers have been correctly signalled.\n> + * fences of all its buffers have been correctly signalled. Ownership of the\n> + * fence will only be taken in case of success, otherwise the fence will\n> + * be left unmodified.\n> *\n> * If the \\a fence associated with \\a buffer isn't signalled, the request will\n> * fail after a timeout. The buffer will still contain the fence, which\n> @@ -468,7 +470,7 @@ void Request::reuse(ReuseFlag flags)\n> * \\retval -EINVAL The buffer does not reference a valid Stream\n> */\n> int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> -\t\t std::unique_ptr fence)\n> +\t\t std::unique_ptr &&fence)\n> {\n> \tif (!stream) {\n> \t\tLOG(Request, Error) << \"Invalid stream reference\";","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 A27B5C324E\n\tfor ;\n\tWed, 10 Sep 2025 16:45:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A26906936E;\n\tWed, 10 Sep 2025 18:45:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8564469357\n\tfor ;\n\tWed, 10 Sep 2025 18:45:25 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 59D83558;\n\tWed, 10 Sep 2025 18:44:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iwUFixRX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1757522650;\n\tbh=54kLJ2zi8a9GSBcKuKArsNDpd7c49uDj1PaFTGopjWU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iwUFixRXlDsCXWX00ruWkXI3LFe+SUX0ywl4oYPEmE/pw8zbSryD760ApKpLTNUDY\n\tcwhKz8GJFS/4uOK8howXei0Ig86hyIPGFdrVcve7DButswVxw1Cpt2CYebSpDhgN8o\n\trnSCbAhSUxmBp8XalaJOk+mudJzQIXiol0otbP50=","Date":"Wed, 10 Sep 2025 19:45:01 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: request: addBuffer(): Do not destroy fence\n\ton failure","Message-ID":"<20250910164501.GG20904@pendragon.ideasonboard.com>","References":"<20250910151857.1502329-1-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250910151857.1502329-1-barnabas.pocze@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35825,"web_url":"https://patchwork.libcamera.org/comment/35825/","msgid":"<175794322831.653594.11234335109195950517@localhost>","date":"2025-09-15T13:33:48","subject":"Re: [PATCH v2] libcamera: request: addBuffer(): Do not destroy fence\n\ton failure","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nQuoting Barnabás Pőcze (2025-09-10 17:18:57)\n> Take the unique pointer to the `Fence` object by rvalue reference\n> so that it is not destroyed if the function returns an error code\n> and does not take ownership of the unique pointer.\n> \n> Signed-off-by: Barnabás Pőcze \n\nI wonder how you found that issue. But the fix is reasonable.\nReviewed-by: Stefan Klug \n\nRegards,\nStefan\n\n> ---\n> changes in v2:\n> * modify function documentation\n> \n> v1: https://patchwork.libcamera.org/patch/22907/\n> ---\n> include/libcamera/request.h | 2 +-\n> src/libcamera/request.cpp | 6 ++++--\n> 2 files changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index e214a9d13..0c5939f7b 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -53,7 +53,7 @@ public:\n> ControlList &metadata() { return *metadata_; }\n> const BufferMap &buffers() const { return bufferMap_; }\n> int addBuffer(const Stream *stream, FrameBuffer *buffer,\n> - std::unique_ptr fence = nullptr);\n> + std::unique_ptr &&fence = {});\n> FrameBuffer *findBuffer(const Stream *stream) const;\n> \n> uint32_t sequence() const;\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 7f1e11e8f..26bba8f28 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -452,7 +452,9 @@ void Request::reuse(ReuseFlag flags)\n> *\n> * When a valid Fence is provided to this function, \\a fence is moved to \\a\n> * buffer and this Request will only be queued to the device once the\n> - * fences of all its buffers have been correctly signalled.\n> + * fences of all its buffers have been correctly signalled. Ownership of the\n> + * fence will only be taken in case of success, otherwise the fence will\n> + * be left unmodified.\n> *\n> * If the \\a fence associated with \\a buffer isn't signalled, the request will\n> * fail after a timeout. The buffer will still contain the fence, which\n> @@ -468,7 +470,7 @@ void Request::reuse(ReuseFlag flags)\n> * \\retval -EINVAL The buffer does not reference a valid Stream\n> */\n> int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> - std::unique_ptr fence)\n> + std::unique_ptr &&fence)\n> {\n> if (!stream) {\n> LOG(Request, Error) << \"Invalid stream reference\";\n> --\n> 2.51.0","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 27472C328C\n\tfor ;\n\tMon, 15 Sep 2025 13:33:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 13C446936F;\n\tMon, 15 Sep 2025 15:33:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 75C61613A0\n\tfor ;\n\tMon, 15 Sep 2025 15:33:51 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:2a47:b7e5:c39c:d1b3])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id F35168D4;\n\tMon, 15 Sep 2025 15:32:33 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PnGM5LXi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1757943154;\n\tbh=OqXs2Zq7rP9ISuL5XkxJ06bntx3hzCRuDvo8dZ7VnwE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=PnGM5LXiWlvLxpRMJB5g1UT31C2L4s/NnII0deeSSHECb9uyHnBWKcKTwxkGLqJBy\n\tIHWCOc3Dc9s/EGVffEkFL8HWjW0hmnUZeLZT5XBnstgK3FFX1KaImBprUDVXKhUicp\n\t81HR9gw6fuewDgeqSN5gNYI49UE33qttQ4lwH3jY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250910151857.1502329-1-barnabas.pocze@ideasonboard.com>","References":"<20250910151857.1502329-1-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v2] libcamera: request: addBuffer(): Do not destroy fence\n\ton failure","From":"Stefan Klug ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 15 Sep 2025 15:33:48 +0200","Message-ID":"<175794322831.653594.11234335109195950517@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]