[{"id":2394,"web_url":"https://patchwork.libcamera.org/comment/2394/","msgid":"<20190813101212.agvwk2zd6nbph5vu@uno.localdomain>","date":"2019-08-13T10:12:13","subject":"Re: [libcamera-devel] [PATCH v3 3/6] libcamera: v4l2_videodevice:\n\tSupport M2M devices","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n a few notes\n\nOn Tue, Aug 13, 2019 at 10:40:17AM +0100, Kieran Bingham wrote:\n> V4L2 M2M devices represent a V4L2Device with two queues: One output, and\n> one capture on a single device node.\n>\n> Represent this by instantiating a V4L2VideoDevice for each queue type,\n> and preparing each device for its queue.\n>\n> Signed-off-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n> ---\n> src/libcamera/include/v4l2_videodevice.h | 25 +++\n> src/libcamera/v4l2_videodevice.cpp | 194 ++++++++++++++++++++++-\n> 2 files changed, 218 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h\n> index f5c8da93fcb5..0fc2dcd81d2b 100644\n> --- a/src/libcamera/include/v4l2_videodevice.h\n> +++ b/src/libcamera/include/v4l2_videodevice.h\n> @@ -71,6 +71,11 @@ struct V4L2Capability final : v4l2_capability {\n> \t\t\t\t\tV4L2_CAP_VIDEO_OUTPUT |\n> \t\t\t\t\tV4L2_CAP_VIDEO_OUTPUT_MPLANE);\n> \t}\n> +\tbool isM2M() const\n> +\t{\n> +\t\treturn device_caps() & (V4L2_CAP_VIDEO_M2M |\n> +\t\t\t\t\tV4L2_CAP_VIDEO_M2M_MPLANE);\n> +\t}\n> \tbool isMeta() const\n> \t{\n> \t\treturn device_caps() & (V4L2_CAP_META_CAPTURE |\n> @@ -124,6 +129,7 @@ public:\n> \tV4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete;\n>\n> \tint open();\n> +\tint open(int handle, enum v4l2_buf_type type);\n> \tvoid close();\n>\n> \tconst char *driverName() const { return caps_.driver(); }\n> @@ -182,6 +188,25 @@ private:\n> \tEventNotifier *fdEvent_;\n> };\n>\n> +class V4L2M2MDevice\n> +{\n> +public:\n> +\tV4L2M2MDevice(const std::string &deviceNode);\n> +\t~V4L2M2MDevice();\n> +\n> +\tint open();\n> +\tvoid close();\n> +\n> +\tV4L2VideoDevice *output() { return output_; }\n> +\tV4L2VideoDevice *capture() { return capture_; }\n> +\n> +private:\n> +\tstd::string deviceNode_;\n> +\n> +\tV4L2VideoDevice *output_;\n> +\tV4L2VideoDevice *capture_;\n> +};\n> +\n> } /* namespace libcamera */\n>\n> #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 81098dd70190..5e41f44eeb50 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -89,6 +89,12 @@ LOG_DECLARE_CATEGORY(V4L2)\n> * \\return True if the video device can capture or output images\n> */\n>\n> +/**\n> + * \\fn V4L2Capability::isM2M()\n> + * \\brief Identify if the device is an M2M device\n\nnit: I would spell memory-to-memory. As you prefer though.\n\n> + * \\return True if the device can capture and output images using the M2M API\n> + */\n> +\n> /**\n> * \\fn V4L2Capability::isMeta()\n> * \\brief Identify if the video device captures or outputs image meta-data\n> @@ -295,7 +301,8 @@ V4L2VideoDevice::~V4L2VideoDevice()\n> }\n>\n> /**\n> - * \\brief Open a V4L2 video device and query its capabilities\n> + * \\brief Open the V4L2 video device node and query its capabilities\n> + *\n> * \\return 0 on success or a negative error code otherwise\n> */\n> int V4L2VideoDevice::open()\n> @@ -355,6 +362,92 @@ int V4L2VideoDevice::open()\n> \treturn 0;\n> }\n>\n> +/**\n> + * \\brief Open a V4L2 video device from an opened file handle and query its\n> + * capabilities\n> + * \\param[in] handle The file descriptor to set\n> + * \\param[in] type The device type to operate on\n> + *\n> + * This methods opens a video device from the existing file descriptor \\a\n> + * handle. Like open(), this method queries the capabilities of the device, but\n> + * handles it according to the given device \\a type instead of determining its\n> + * type from the capabilities. This can be used to force a given device type for\n> + * M2M devices.\n> + *\n> + * The file descriptor \\a handle is duplicated, and the caller is responsible\n> + * for closing the \\a handle when it has no further use for it. The close()\n> + * method will close the duplicated file descriptor, leaving \\a handle\n> + * untouched.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type)\n> +{\n> +\tint ret;\n> +\tint newFd;\n> +\n> +\tnewFd = dup(handle);\n> +\tif (newFd < 0) {\n> +\t\tret = -errno;\n> +\t\tLOG(V4L2, Error) << \"Failed to duplicate file handle: \"\n> +\t\t\t\t << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = V4L2Device::setFd(newFd);\n> +\tif (ret < 0) {\n> +\t\tLOG(V4L2, Error) << \"Failed to set file handle: \"\n> +\t\t\t\t << strerror(-ret);\n> +\t\t::close(newFd);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = ioctl(VIDIOC_QUERYCAP, &caps_);\n> +\tif (ret < 0) {\n> +\t\tLOG(V4L2, Error)\n> +\t\t\t<< \"Failed to query device capabilities: \"\n\nShouldn't you close newfd here and in the below error paths too?\n\n> +\t\t\t<< strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (!caps_.hasStreaming()) {\n> +\t\tLOG(V4L2, Error) << \"Device does not support streaming I/O\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\t/*\n> +\t * Set buffer type and wait for read notifications on CAPTURE video\n> +\t * devices (POLLIN), and write notifications for OUTPUT video devices\n> +\t * (POLLOUT).\n> +\t */\n> +\tswitch (type) {\n> +\tcase V4L2_BUF_TYPE_VIDEO_OUTPUT:\n> +\t\tfdEvent_ = new EventNotifier(fd(), EventNotifier::Write);\n> +\t\tbufferType_ = caps_.isMultiplanar()\n> +\t\t\t ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE\n> +\t\t\t : V4L2_BUF_TYPE_VIDEO_OUTPUT;\n> +\t\tbreak;\n> +\tcase V4L2_BUF_TYPE_VIDEO_CAPTURE:\n> +\t\tfdEvent_ = new EventNotifier(fd(), EventNotifier::Read);\n> +\t\tbufferType_ = caps_.isMultiplanar()\n> +\t\t\t ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE\n> +\t\t\t : V4L2_BUF_TYPE_VIDEO_CAPTURE;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tLOG(V4L2, Error) << \"Unsupported buffer type\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tfdEvent_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);\n> +\tfdEvent_->setEnabled(false);\n> +\n> +\tLOG(V4L2, Debug)\n> +\t\t<< \"Opened device \" << caps_.bus_info() << \": \"\n> +\t\t<< caps_.driver() << \": \" << caps_.card();\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\brief Close the video device, releasing any resources acquired by open()\n> */\n> @@ -1143,4 +1236,103 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n> \treturn new V4L2VideoDevice(mediaEntity);\n> }\n>\n> +/**\n> + * \\class V4L2M2MDevice\n> + * \\brief Memory2Memory video device\n> + *\n> + * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same\n> + * deviceNode which operate together using two queues to implement the V4L2\n> + * Memory to Memory API.\n> + *\n> + * The two devices should be opened by calling open() on the V4L2M2MDevice, and\n> + * can be closed by calling close on the V4L2M2MDevice.\n> + *\n> + * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture\n> + * or output V4L2VideoDevice is not permitted.\n> + */\n> +\n> +/**\n> + * \\fn V4L2M2MDevice::output\n> + * \\brief Retrieve the output V4L2VideoDevice instance\n> + * \\return The output V4L2Device instance\nV4L2VideoDevice\n\n> + */\n> +\n> +/**\n> + * \\fn V4L2M2MDevice::capture\n> + * \\brief Retrieve the capture V4L2VideoDevice instance\n> + * \\return The capture V4L2Device instance\n\nsame here\n\n> + */\n> +\n> +/**\n> + * \\brief Create a new V4L2M2MDevice from the \\a deviceNode\n\nmissing parameter documentation\n\n> + */\n> +V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode)\n> +\t: deviceNode_(deviceNode)\n> +{\n> +\toutput_ = new V4L2VideoDevice(deviceNode);\n> +\tcapture_ = new V4L2VideoDevice(deviceNode);\n> +}\n> +\n> +V4L2M2MDevice::~V4L2M2MDevice()\n> +{\n> +\tdelete capture_;\n> +\tdelete output_;\n> +}\n> +\n> +/**\n> + * \\brief Open a V4L2 Memory to Memory device\n> + *\n> + * Open the device node and prepare the two V4L2VideoDevice instances to handle\n> + * their respective buffer queues.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int V4L2M2MDevice::open()\n> +{\n> +\tint fd;\n> +\tint ret;\n> +\n> +\t/*\n> +\t * The output and capture V4L2VideoDevice instances use the same file\n> +\t * handle for the same device node. The local file handle can be closed\n> +\t * as the V4L2VideoDevice::open() retains a handle by duplicating the\n> +\t * fd passed in.\n> +\t */\n> +\tfd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK);\n> +\tif (fd < 0) {\n> +\t\tret = -errno;\n> +\t\tLOG(V4L2, Error)\n> +\t\t\t<< \"Failed to open V4L2 M2M device: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT);\n> +\tif (ret)\n> +\t\tgoto err;\n> +\n> +\tret = capture_->open(fd, 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> +/**\n> + * \\brief Close the memory-to-memory device, releasing any resources acquired by\n> + * open()\n> + */\n> +void V4L2M2MDevice::close()\n> +{\n> +\tcapture_->close();\n> +\toutput_->close();\n> +}\n> +\n\nMinors apart,\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> } /* namespace libcamera */\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7462260C43\n\tfor ;\n\tTue, 13 Aug 2019 12:10:47 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id C4FF0200007;\n\tTue, 13 Aug 2019 10:10:46 +0000 (UTC)"],"Date":"Tue, 13 Aug 2019 12:12:13 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190813101212.agvwk2zd6nbph5vu@uno.localdomain>","References":"<20190813094020.10277-1-kieran.bingham@ideasonboard.com>\n\t<20190813094020.10277-4-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"vc5nbsxcs6nhform\"","Content-Disposition":"inline","In-Reply-To":"<20190813094020.10277-4-kieran.bingham@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v3 3/6] libcamera: v4l2_videodevice:\n\tSupport M2M devices","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 13 Aug 2019 10:10:47 -0000"}},{"id":2397,"web_url":"https://patchwork.libcamera.org/comment/2397/","msgid":"","date":"2019-08-13T11:22:59","subject":"Re: [libcamera-devel] [PATCH v3 3/6] libcamera: v4l2_videodevice:\n\tSupport M2M devices","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 13/08/2019 11:12, Jacopo Mondi wrote:\n> Hi Kieran,\n> a few notes\n> \n> On Tue, Aug 13, 2019 at 10:40:17AM +0100, Kieran Bingham wrote:\n>> V4L2 M2M devices represent a V4L2Device with two queues: One output, and\n>> one capture on a single device node.\n>>\n>> Represent this by instantiating a V4L2VideoDevice for each queue type,\n>> and preparing each device for its queue.\n>>\n>> Signed-off-by: Kieran Bingham \n>> Reviewed-by: Laurent Pinchart \n>> ---\n>> src/libcamera/include/v4l2_videodevice.h | 25 +++\n>> src/libcamera/v4l2_videodevice.cpp | 194 ++++++++++++++++++++++-\n>> 2 files changed, 218 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h\n>> index f5c8da93fcb5..0fc2dcd81d2b 100644\n>> --- a/src/libcamera/include/v4l2_videodevice.h\n>> +++ b/src/libcamera/include/v4l2_videodevice.h\n>> @@ -71,6 +71,11 @@ struct V4L2Capability final : v4l2_capability {\n>> \t\t\t\t\tV4L2_CAP_VIDEO_OUTPUT |\n>> \t\t\t\t\tV4L2_CAP_VIDEO_OUTPUT_MPLANE);\n>> \t}\n>> +\tbool isM2M() const\n>> +\t{\n>> +\t\treturn device_caps() & (V4L2_CAP_VIDEO_M2M |\n>> +\t\t\t\t\tV4L2_CAP_VIDEO_M2M_MPLANE);\n>> +\t}\n>> \tbool isMeta() const\n>> \t{\n>> \t\treturn device_caps() & (V4L2_CAP_META_CAPTURE |\n>> @@ -124,6 +129,7 @@ public:\n>> \tV4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete;\n>>\n>> \tint open();\n>> +\tint open(int handle, enum v4l2_buf_type type);\n>> \tvoid close();\n>>\n>> \tconst char *driverName() const { return caps_.driver(); }\n>> @@ -182,6 +188,25 @@ private:\n>> \tEventNotifier *fdEvent_;\n>> };\n>>\n>> +class V4L2M2MDevice\n>> +{\n>> +public:\n>> +\tV4L2M2MDevice(const std::string &deviceNode);\n>> +\t~V4L2M2MDevice();\n>> +\n>> +\tint open();\n>> +\tvoid close();\n>> +\n>> +\tV4L2VideoDevice *output() { return output_; }\n>> +\tV4L2VideoDevice *capture() { return capture_; }\n>> +\n>> +private:\n>> +\tstd::string deviceNode_;\n>> +\n>> +\tV4L2VideoDevice *output_;\n>> +\tV4L2VideoDevice *capture_;\n>> +};\n>> +\n>> } /* namespace libcamera */\n>>\n>> #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */\n>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n>> index 81098dd70190..5e41f44eeb50 100644\n>> --- a/src/libcamera/v4l2_videodevice.cpp\n>> +++ b/src/libcamera/v4l2_videodevice.cpp\n>> @@ -89,6 +89,12 @@ LOG_DECLARE_CATEGORY(V4L2)\n>> * \\return True if the video device can capture or output images\n>> */\n>>\n>> +/**\n>> + * \\fn V4L2Capability::isM2M()\n>> + * \\brief Identify if the device is an M2M device\n> \n> nit: I would spell memory-to-memory. As you prefer though.\n\nIt is usually spelt out in full, so I've updated this, and a couple of\nother places.\n\n> \n>> + * \\return True if the device can capture and output images using the M2M API\n\nBut specifically not the one in that line, as it's just referencing the API.\n\n\n>> + */\n>> +\n>> /**\n>> * \\fn V4L2Capability::isMeta()\n>> * \\brief Identify if the video device captures or outputs image meta-data\n>> @@ -295,7 +301,8 @@ V4L2VideoDevice::~V4L2VideoDevice()\n>> }\n>>\n>> /**\n>> - * \\brief Open a V4L2 video device and query its capabilities\n>> + * \\brief Open the V4L2 video device node and query its capabilities\n>> + *\n>> * \\return 0 on success or a negative error code otherwise\n>> */\n>> int V4L2VideoDevice::open()\n>> @@ -355,6 +362,92 @@ int V4L2VideoDevice::open()\n>> \treturn 0;\n>> }\n>>\n>> +/**\n>> + * \\brief Open a V4L2 video device from an opened file handle and query its\n>> + * capabilities\n>> + * \\param[in] handle The file descriptor to set\n>> + * \\param[in] type The device type to operate on\n>> + *\n>> + * This methods opens a video device from the existing file descriptor \\a\n>> + * handle. Like open(), this method queries the capabilities of the device, but\n>> + * handles it according to the given device \\a type instead of determining its\n>> + * type from the capabilities. This can be used to force a given device type for\n>> + * M2M devices.\n>> + *\n>> + * The file descriptor \\a handle is duplicated, and the caller is responsible\n>> + * for closing the \\a handle when it has no further use for it. The close()\n>> + * method will close the duplicated file descriptor, leaving \\a handle\n>> + * untouched.\n>> + *\n>> + * \\return 0 on success or a negative error code otherwise\n>> + */\n>> +int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type)\n>> +{\n>> +\tint ret;\n>> +\tint newFd;\n>> +\n>> +\tnewFd = dup(handle);\n>> +\tif (newFd < 0) {\n>> +\t\tret = -errno;\n>> +\t\tLOG(V4L2, Error) << \"Failed to duplicate file handle: \"\n>> +\t\t\t\t << strerror(-ret);\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tret = V4L2Device::setFd(newFd);\n>> +\tif (ret < 0) {\n>> +\t\tLOG(V4L2, Error) << \"Failed to set file handle: \"\n>> +\t\t\t\t << strerror(-ret);\n>> +\t\t::close(newFd);\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tret = ioctl(VIDIOC_QUERYCAP, &caps_);\n>> +\tif (ret < 0) {\n>> +\t\tLOG(V4L2, Error)\n>> +\t\t\t<< \"Failed to query device capabilities: \"\n> \n> Shouldn't you close newfd here and in the below error paths too?\n\nI think we can get away with those, because on those error paths the\ndestructor will clean it up for us.\n\nIt's only the error path above which will not register the duplicated FD\nas fd_ and thus it will not get closed by another means. (except of\ncourse it will when the app closes but hey :-D)\n\n> \n>> +\t\t\t<< strerror(-ret);\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tif (!caps_.hasStreaming()) {\n>> +\t\tLOG(V4L2, Error) << \"Device does not support streaming I/O\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\t/*\n>> +\t * Set buffer type and wait for read notifications on CAPTURE video\n>> +\t * devices (POLLIN), and write notifications for OUTPUT video devices\n>> +\t * (POLLOUT).\n>> +\t */\n>> +\tswitch (type) {\n>> +\tcase V4L2_BUF_TYPE_VIDEO_OUTPUT:\n>> +\t\tfdEvent_ = new EventNotifier(fd(), EventNotifier::Write);\n>> +\t\tbufferType_ = caps_.isMultiplanar()\n>> +\t\t\t ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE\n>> +\t\t\t : V4L2_BUF_TYPE_VIDEO_OUTPUT;\n>> +\t\tbreak;\n>> +\tcase V4L2_BUF_TYPE_VIDEO_CAPTURE:\n>> +\t\tfdEvent_ = new EventNotifier(fd(), EventNotifier::Read);\n>> +\t\tbufferType_ = caps_.isMultiplanar()\n>> +\t\t\t ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE\n>> +\t\t\t : V4L2_BUF_TYPE_VIDEO_CAPTURE;\n>> +\t\tbreak;\n>> +\tdefault:\n>> +\t\tLOG(V4L2, Error) << \"Unsupported buffer type\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tfdEvent_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);\n>> +\tfdEvent_->setEnabled(false);\n>> +\n>> +\tLOG(V4L2, Debug)\n>> +\t\t<< \"Opened device \" << caps_.bus_info() << \": \"\n>> +\t\t<< caps_.driver() << \": \" << caps_.card();\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> /**\n>> * \\brief Close the video device, releasing any resources acquired by open()\n>> */\n>> @@ -1143,4 +1236,103 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n>> \treturn new V4L2VideoDevice(mediaEntity);\n>> }\n>>\n>> +/**\n>> + * \\class V4L2M2MDevice\n>> + * \\brief Memory2Memory video device\n>> + *\n>> + * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same\n>> + * deviceNode which operate together using two queues to implement the V4L2\n>> + * Memory to Memory API.\n>> + *\n>> + * The two devices should be opened by calling open() on the V4L2M2MDevice, and\n>> + * can be closed by calling close on the V4L2M2MDevice.\n>> + *\n>> + * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture\n>> + * or output V4L2VideoDevice is not permitted.\n>> + */\n>> +\n>> +/**\n>> + * \\fn V4L2M2MDevice::output\n>> + * \\brief Retrieve the output V4L2VideoDevice instance\n>> + * \\return The output V4L2Device instance\n> V4L2VideoDevice\n> \n>> + */\n>> +\n>> +/**\n>> + * \\fn V4L2M2MDevice::capture\n>> + * \\brief Retrieve the capture V4L2VideoDevice instance\n>> + * \\return The capture V4L2Device instance\n> \n> same here\n\nGood spots.\n\n> \n>> + */\n>> +\n>> +/**\n>> + * \\brief Create a new V4L2M2MDevice from the \\a deviceNode\n> \n> missing parameter documentation\n\nHrm... why didn't my doxygen build warn me about this ? :S\n\n\n\n> \n>> + */\n>> +V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode)\n>> +\t: deviceNode_(deviceNode)\n>> +{\n>> +\toutput_ = new V4L2VideoDevice(deviceNode);\n>> +\tcapture_ = new V4L2VideoDevice(deviceNode);\n>> +}\n>> +\n>> +V4L2M2MDevice::~V4L2M2MDevice()\n>> +{\n>> +\tdelete capture_;\n>> +\tdelete output_;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Open a V4L2 Memory to Memory device\n>> + *\n>> + * Open the device node and prepare the two V4L2VideoDevice instances to handle\n>> + * their respective buffer queues.\n>> + *\n>> + * \\return 0 on success or a negative error code otherwise\n>> + */\n>> +int V4L2M2MDevice::open()\n>> +{\n>> +\tint fd;\n>> +\tint ret;\n>> +\n>> +\t/*\n>> +\t * The output and capture V4L2VideoDevice instances use the same file\n>> +\t * handle for the same device node. The local file handle can be closed\n>> +\t * as the V4L2VideoDevice::open() retains a handle by duplicating the\n>> +\t * fd passed in.\n>> +\t */\n>> +\tfd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK);\n>> +\tif (fd < 0) {\n>> +\t\tret = -errno;\n>> +\t\tLOG(V4L2, Error)\n>> +\t\t\t<< \"Failed to open V4L2 M2M device: \" << strerror(-ret);\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT);\n>> +\tif (ret)\n>> +\t\tgoto err;\n>> +\n>> +\tret = capture_->open(fd, 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>> +/**\n>> + * \\brief Close the memory-to-memory device, releasing any resources acquired by\n>> + * open()\n>> + */\n>> +void V4L2M2MDevice::close()\n>> +{\n>> +\tcapture_->close();\n>> +\toutput_->close();\n>> +}\n>> +\n> \n> Minors apart,\n> Reviewed-by: Jacopo Mondi \n\nThanks, I've applied the following fixes:\n\n\ndiff --git a/src/libcamera/v4l2_videodevice.cpp\nb/src/libcamera/v4l2_videodevice.cpp\nindex 5e41f44eeb50..eb4e44deb4a5 100644\n--- a/src/libcamera/v4l2_videodevice.cpp\n+++ b/src/libcamera/v4l2_videodevice.cpp\n@@ -91,7 +91,7 @@ LOG_DECLARE_CATEGORY(V4L2)\n\n /**\n * \\fn V4L2Capability::isM2M()\n- * \\brief Identify if the device is an M2M device\n+ * \\brief Identify if the device is a Memory-to-Memory device\n * \\return True if the device can capture and output images using the\nM2M API\n */\n\n@@ -372,7 +372,7 @@ int V4L2VideoDevice::open()\n * handle. Like open(), this method queries the capabilities of the\ndevice, but\n * handles it according to the given device \\a type instead of\ndetermining its\n * type from the capabilities. This can be used to force a given device\ntype for\n- * M2M devices.\n+ * memory-to-memory devices.\n *\n * The file descriptor \\a handle is duplicated, and the caller is\nresponsible\n * for closing the \\a handle when it has no further use for it. The close()\n@@ -1238,7 +1238,7 @@ V4L2VideoDevice\n*V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n\n /**\n * \\class V4L2M2MDevice\n- * \\brief Memory2Memory video device\n+ * \\brief Memory-to-Memory video device\n *\n * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same\n * deviceNode which operate together using two queues to implement the V4L2\n@@ -1254,17 +1254,18 @@ V4L2VideoDevice\n*V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n /**\n * \\fn V4L2M2MDevice::output\n * \\brief Retrieve the output V4L2VideoDevice instance\n- * \\return The output V4L2Device instance\n+ * \\return The output V4L2VideoDevice instance\n */\n\n /**\n * \\fn V4L2M2MDevice::capture\n * \\brief Retrieve the capture V4L2VideoDevice instance\n- * \\return The capture V4L2Device instance\n+ * \\return The capture V4L2VideoDevice instance\n */\n\n /**\n * \\brief Create a new V4L2M2MDevice from the \\a deviceNode\n+ * \\param[in] deviceNode The file-system path to the video device node\n */\n V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode)\n : deviceNode_(deviceNode)\n\n\n\nIf you're happy with the above changesets, I'll push this later without\nreposting a v4. (excluding 6/6 of course)\n\n\nRegards\n--\nKieran","headers":{"Return-Path":"","Received":["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 B88C960E6C\n\tfor ;\n\tTue, 13 Aug 2019 13:23:03 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0B5962B2;\n\tTue, 13 Aug 2019 13:23:02 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1565695383;\n\tbh=WGYirQzxyYeoX7NMNaF8wa6UpwOBIw544n+pXRQfEZE=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=ge5gFqVA3sVy5ESihFwgew+fM2pdN+VNsJBiZQUJ/emXfB/Bsq/F6sqoLFuUH7Lt7\n\tnXBpO8ZQH/XKo3JiajOjJtXqOOxrguUroo3eXnnwQ1hwEVJJYCj8f/5WOJ3ukJMTA1\n\tzPFC9R53VnXBbeMbFOaR8sA4YmLdeided/Op0fzE=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi ","Cc":"LibCamera Devel ","References":"<20190813094020.10277-1-kieran.bingham@ideasonboard.com>\n\t<20190813094020.10277-4-kieran.bingham@ideasonboard.com>\n\t<20190813101212.agvwk2zd6nbph5vu@uno.localdomain>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"","Date":"Tue, 13 Aug 2019 12:22:59 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.8.0","MIME-Version":"1.0","In-Reply-To":"<20190813101212.agvwk2zd6nbph5vu@uno.localdomain>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3 3/6] libcamera: v4l2_videodevice:\n\tSupport M2M devices","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 13 Aug 2019 11:23:05 -0000"}}]