[{"id":21324,"web_url":"https://patchwork.libcamera.org/comment/21324/","msgid":"","date":"2021-11-29T14:00:17","subject":"Re: [libcamera-devel] [PATCH v3 14/17] libcamera: v4l2_videodevice:\n\tUse fd for a file descriptor","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent,\n\nOn Mon, Nov 29, 2021 at 8:58 AM Laurent Pinchart\n wrote:\n>\n> From: Hirokazu Honda \n>\n> Manages file descriptors owned by V4L2VideoDevice by UniqueFD.\n> This also changes the return type of exportDmabufFd to UniqueFD\n> from FileDescriptor in order to represent a caller owns the\n> returned file file descriptor.\n>\n> Signed-off-by: Hirokazu Honda \n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v2:\n>\n> - Use FileDescriptor(UniqueFD &&) constructor\nI think this should be Use FileDescriptor(UniqueFD) constructor, but\nit doesn't matter absolutely.\n\nReviewed-by: Hirokazu Honda if applicable.\n\n> - Fix errno handling\n> - Return {}\n> ---\n> include/libcamera/internal/v4l2_videodevice.h | 4 +--\n> src/libcamera/v4l2_videodevice.cpp | 30 ++++++++-----------\n> 2 files changed, 15 insertions(+), 19 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 4a44b7fd53b1..80b665771782 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -19,6 +19,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n> #include \n> @@ -31,7 +32,6 @@\n> namespace libcamera {\n>\n> class EventNotifier;\n> -class FileDescriptor;\n> class MediaDevice;\n> class MediaEntity;\n>\n> @@ -238,7 +238,7 @@ private:\n> int createBuffers(unsigned int count,\n> std::vector> *buffers);\n> std::unique_ptr createBuffer(unsigned int index);\n> - FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane);\n> + UniqueFD exportDmabufFd(unsigned int index, unsigned int plane);\n>\n> void bufferAvailable();\n> FrameBuffer *dequeueBuffer();\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index c95626d33cc3..3966483a365f 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -1320,12 +1320,12 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index)\n>\n> std::vector planes;\n> for (unsigned int nplane = 0; nplane < numPlanes; nplane++) {\n> - FileDescriptor fd = exportDmabufFd(buf.index, nplane);\n> + UniqueFD fd = exportDmabufFd(buf.index, nplane);\n> if (!fd.isValid())\n> return nullptr;\n>\n> FrameBuffer::Plane plane;\n> - plane.fd = std::move(fd);\n> + plane.fd = FileDescriptor(std::move(fd));\n> /*\n> * V4L2 API doesn't provide dmabuf offset information of plane.\n> * Set 0 as a placeholder offset.\n> @@ -1380,8 +1380,8 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index)\n> return std::make_unique(planes);\n> }\n>\n> -FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index,\n> - unsigned int plane)\n> +UniqueFD V4L2VideoDevice::exportDmabufFd(unsigned int index,\n> + unsigned int plane)\n> {\n> struct v4l2_exportbuffer expbuf = {};\n> int ret;\n> @@ -1395,10 +1395,10 @@ FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index,\n> if (ret < 0) {\n> LOG(V4L2, Error)\n> << \"Failed to export buffer: \" << strerror(-ret);\n> - return FileDescriptor();\n> + return {};\n> }\n>\n> - return FileDescriptor(std::move(expbuf.fd));\n> + return UniqueFD(expbuf.fd);\n> }\n>\n> /**\n> @@ -1896,7 +1896,6 @@ V4L2M2MDevice::~V4L2M2MDevice()\n> */\n> int V4L2M2MDevice::open()\n> {\n> - int fd;\n> int ret;\n>\n> /*\n> @@ -1905,30 +1904,27 @@ int V4L2M2MDevice::open()\n> * as the V4L2VideoDevice::open() retains a handle by duplicating the\n> * fd passed in.\n> */\n> - fd = syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(),\n> - O_RDWR | O_NONBLOCK);\n> - if (fd < 0) {\n> + UniqueFD fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(),\n> + O_RDWR | O_NONBLOCK));\n> + if (!fd.isValid()) {\n> ret = -errno;\n> - LOG(V4L2, Error)\n> - << \"Failed to open V4L2 M2M device: \" << strerror(-ret);\n> + LOG(V4L2, Error) << \"Failed to open V4L2 M2M device: \"\n> + << strerror(-ret);\n> return ret;\n> }\n>\n> - ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT);\n> + ret = output_->open(fd.get(), V4L2_BUF_TYPE_VIDEO_OUTPUT);\n> if (ret)\n> goto err;\n>\n> - ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE);\n> + ret = capture_->open(fd.get(), V4L2_BUF_TYPE_VIDEO_CAPTURE);\n> if (ret)\n> goto err;\n>\n> - ::close(fd);\n> -\n> return 0;\n>\n> err:\n> close();\n> - ::close(fd);\n>\n> return ret;\n> }\n> --\n> Regards,\n>\n> Laurent Pinchart\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 4711BBDB13\n\tfor ;\n\tMon, 29 Nov 2021 14:00:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9EAEA605A4;\n\tMon, 29 Nov 2021 15:00:32 +0100 (CET)","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 1E5E260592\n\tfor ;\n\tMon, 29 Nov 2021 15:00:31 +0100 (CET)","by mail-ed1-x52a.google.com with SMTP id z5so7058352edd.3\n\tfor ;\n\tMon, 29 Nov 2021 06:00:31 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"ky1Mbf86\"; 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=QiDimiVV5CgCYUWHLjE7itBkuG+hymQkhsX0LgeyWA8=;\n\tb=ky1Mbf86UP0yPX5P3OuwTfMMpr5AIW+r/Sxl+UUBWJJcB4a7mRQwhR3e+5GiPe+cNn\n\t1TbmGlvE4jHyjlxBfnTaO+5+6JDWsy84M8yBzhdvG/ybYA8MEltBy6j9jIkdryJhn1Fh\n\tEwNLH8yZHioraizHHH9RP/53XGHPMB3JtkIYg=","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=QiDimiVV5CgCYUWHLjE7itBkuG+hymQkhsX0LgeyWA8=;\n\tb=2ypBNpgMGopp/40ene0oBU05CFGSi53l7TzQ3P0RSp5VoyJoJov1pJ6Og+18cuYfUz\n\tU5nChDMgYhiNMaNS5ux1M6pvNB6BAPuRrkmhg4hLIRZGh4ALZmMkGuSqMfoT3VuXaENt\n\tP7PlyTLLg/0FKn6Ok7akqiePaVlJ/CK4orkNTXSBc0Gf6Z2BHpia30zLxVBNHNydKY9J\n\t8/ObQuERxkV+ALMBLFtFCSYaEAUMDWCDTaZP85UbRRep+Tr2mYT9+IIKDpZ/8MnUv31A\n\tA+ImmUboi3Vuq86k5kmPMkada12ZbJkY9tPWUY3W6Eszyv1qWLD6STuRELFjfRW8xWFq\n\tsw2Q==","X-Gm-Message-State":"AOAM531CHtR248Sin64/HOWgeBV46Vw8/qy2rRah8mM5mSmrIU8d46o4\n\tb6fQ9frXxCZk3Ip1SkBvV3Gmp/wtQzKX+unO9QvJNQ==","X-Google-Smtp-Source":"ABdhPJyi8jf4KKizMih7Sspdz0/TJqAo8Bq3mx/WEl68xALervjkzWYLWoexmhPLYLs1SP1oWvPPdw8GqEgrnejeGuM=","X-Received":"by 2002:a05:6402:60c:: with SMTP id\n\tn12mr76014643edv.17.1638194428973; \n\tMon, 29 Nov 2021 06:00:28 -0800 (PST)","MIME-Version":"1.0","References":"<20211128235752.10836-1-laurent.pinchart@ideasonboard.com>\n\t<20211128235752.10836-15-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20211128235752.10836-15-laurent.pinchart@ideasonboard.com>","From":"Hirokazu Honda ","Date":"Mon, 29 Nov 2021 23:00:17 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 14/17] libcamera: v4l2_videodevice:\n\tUse fd for a file descriptor","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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21345,"web_url":"https://patchwork.libcamera.org/comment/21345/","msgid":"<20211129155333.wwzdbsxku2ua3rde@uno.localdomain>","date":"2021-11-29T15:53:33","subject":"Re: [libcamera-devel] [PATCH v3 14/17] libcamera: v4l2_videodevice:\n\tUse fd for a file descriptor","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Mon, Nov 29, 2021 at 01:57:49AM +0200, Laurent Pinchart wrote:\n> From: Hirokazu Honda \n>\n> Manages file descriptors owned by V4L2VideoDevice by UniqueFD.\n> This also changes the return type of exportDmabufFd to UniqueFD\n> from FileDescriptor in order to represent a caller owns the\n> returned file file descriptor.\n>\n> Signed-off-by: Hirokazu Honda \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> Changes since v2:\n>\n> - Use FileDescriptor(UniqueFD &&) constructor\n> - Fix errno handling\n> - Return {}\n> ---\n> include/libcamera/internal/v4l2_videodevice.h | 4 +--\n> src/libcamera/v4l2_videodevice.cpp | 30 ++++++++-----------\n> 2 files changed, 15 insertions(+), 19 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 4a44b7fd53b1..80b665771782 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -19,6 +19,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n> #include \n> @@ -31,7 +32,6 @@\n> namespace libcamera {\n>\n> class EventNotifier;\n> -class FileDescriptor;\n> class MediaDevice;\n> class MediaEntity;\n>\n> @@ -238,7 +238,7 @@ private:\n> \tint createBuffers(unsigned int count,\n> \t\t\t std::vector> *buffers);\n> \tstd::unique_ptr createBuffer(unsigned int index);\n> -\tFileDescriptor exportDmabufFd(unsigned int index, unsigned int plane);\n> +\tUniqueFD exportDmabufFd(unsigned int index, unsigned int plane);\n>\n> \tvoid bufferAvailable();\n> \tFrameBuffer *dequeueBuffer();\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index c95626d33cc3..3966483a365f 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -1320,12 +1320,12 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index)\n>\n> \tstd::vector planes;\n> \tfor (unsigned int nplane = 0; nplane < numPlanes; nplane++) {\n> -\t\tFileDescriptor fd = exportDmabufFd(buf.index, nplane);\n> +\t\tUniqueFD fd = exportDmabufFd(buf.index, nplane);\n> \t\tif (!fd.isValid())\n> \t\t\treturn nullptr;\n>\n> \t\tFrameBuffer::Plane plane;\n> -\t\tplane.fd = std::move(fd);\n> +\t\tplane.fd = FileDescriptor(std::move(fd));\n> \t\t/*\n> \t\t * V4L2 API doesn't provide dmabuf offset information of plane.\n> \t\t * Set 0 as a placeholder offset.\n> @@ -1380,8 +1380,8 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index)\n> \treturn std::make_unique(planes);\n> }\n>\n> -FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index,\n> -\t\t\t\t\t unsigned int plane)\n> +UniqueFD V4L2VideoDevice::exportDmabufFd(unsigned int index,\n> +\t\t\t\t\t unsigned int plane)\n> {\n> \tstruct v4l2_exportbuffer expbuf = {};\n> \tint ret;\n> @@ -1395,10 +1395,10 @@ FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index,\n> \tif (ret < 0) {\n> \t\tLOG(V4L2, Error)\n> \t\t\t<< \"Failed to export buffer: \" << strerror(-ret);\n> -\t\treturn FileDescriptor();\n> +\t\treturn {};\n> \t}\n>\n> -\treturn FileDescriptor(std::move(expbuf.fd));\n> +\treturn UniqueFD(expbuf.fd);\n> }\n>\n> /**\n> @@ -1896,7 +1896,6 @@ V4L2M2MDevice::~V4L2M2MDevice()\n> */\n> int V4L2M2MDevice::open()\n> {\n> -\tint fd;\n> \tint ret;\n>\n> \t/*\n> @@ -1905,30 +1904,27 @@ int V4L2M2MDevice::open()\n> \t * as the V4L2VideoDevice::open() retains a handle by duplicating the\n> \t * fd passed in.\n> \t */\n> -\tfd = syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(),\n> -\t\t O_RDWR | O_NONBLOCK);\n> -\tif (fd < 0) {\n> +\tUniqueFD fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(),\n> +\t\t\t O_RDWR | O_NONBLOCK));\n> +\tif (!fd.isValid()) {\n> \t\tret = -errno;\n> -\t\tLOG(V4L2, Error)\n> -\t\t\t<< \"Failed to open V4L2 M2M device: \" << strerror(-ret);\n> +\t\tLOG(V4L2, Error) << \"Failed to open V4L2 M2M device: \"\n> +\t\t\t\t << strerror(-ret);\n> \t\treturn ret;\n> \t}\n>\n> -\tret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT);\n> +\tret = output_->open(fd.get(), V4L2_BUF_TYPE_VIDEO_OUTPUT);\n> \tif (ret)\n> \t\tgoto err;\n>\n> -\tret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE);\n> +\tret = capture_->open(fd.get(), V4L2_BUF_TYPE_VIDEO_CAPTURE);\n> \tif (ret)\n> \t\tgoto err;\n>\n> -\t::close(fd);\n> -\n> \treturn 0;\n>\n> err:\n> \tclose();\n> -\t::close(fd);\n>\n> \treturn ret;\n> }\n> --\n> Regards,\n>\n> Laurent Pinchart\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 4EB38BF415\n\tfor ;\n\tMon, 29 Nov 2021 15:52:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A2BB1605A4;\n\tMon, 29 Nov 2021 16:52:42 +0100 (CET)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 34A7060592\n\tfor ;\n\tMon, 29 Nov 2021 16:52:41 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 9CC0D1BF213;\n\tMon, 29 Nov 2021 15:52:40 +0000 (UTC)"],"Date":"Mon, 29 Nov 2021 16:53:33 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20211129155333.wwzdbsxku2ua3rde@uno.localdomain>","References":"<20211128235752.10836-1-laurent.pinchart@ideasonboard.com>\n\t<20211128235752.10836-15-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211128235752.10836-15-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 14/17] libcamera: v4l2_videodevice:\n\tUse fd for a file descriptor","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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]