[{"id":330,"web_url":"https://patchwork.libcamera.org/comment/330/","msgid":"<20190115163951.5a6hsfvpp663j7rw@uno.localdomain>","date":"2019-01-15T16:39:51","subject":"Re: [libcamera-devel] [PATCH v2 1/2] lib: Add V4L2 Device object","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n thanks for the patch\n\nOn Tue, Jan 15, 2019 at 04:02:11PM +0000, Kieran Bingham wrote:\n> Provide a helper V4L2 device object capable of interacting with the\n> V4L2 Linux Kernel APIs.\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> src/libcamera/include/v4l2_device.h | 43 ++++++++++\n> src/libcamera/meson.build | 2 +\n> src/libcamera/v4l2_device.cpp | 127 ++++++++++++++++++++++++++++\n> 3 files changed, 172 insertions(+)\n> create mode 100644 src/libcamera/include/v4l2_device.h\n> create mode 100644 src/libcamera/v4l2_device.cpp\n>\n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> new file mode 100644\n> index 000000000000..ddcb17af2187\n> --- /dev/null\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -0,0 +1,43 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * v4l2_device.h - V4L2 Device API Abstractions\n\nI would leave \"API\" out (and \"abstraction\" too tbh...)\n\n> + */\n> +#ifndef __LIBCAMERA_V4L2_DEVICE_H__\n> +#define __LIBCAMERA_V4L2_DEVICE_H__\n> +\n> +#include \n> +\n> +#include \n\nPlease add this header to include/linux, otherwise you're relying on\nthe system wide installed one\n\n> +\n> +namespace libcamera {\n> +\n> +class V4L2Capability : public v4l2_capability\n\nIs this the kernel header provided one? If so, I'm not that sure is a\ngood idea to extend it directly. True, you get compatibility with the\ncurrent header version automatically, but that might hide other issues\nif the kernel API changes. I would prefer accessing the\nv4l2_capability directly, and fail at compile time if there have been\nchanges.\n\nFurthermore, you only use the 'capabilities' field of this structure,\nam I wrong?\n\n> +{\n> +public:\n> +\tbool isCapture() { return capabilities & V4L2_CAP_VIDEO_CAPTURE; }\n> +\tbool isMplane() { return capabilities & V4L2_CAP_VIDEO_CAPTURE_MPLANE; }\n> +\tbool hasStreaming() { return capabilities & V4L2_CAP_STREAMING; }\n> +};\n> +\n> +class V4L2Device\n> +{\n> +public:\n> +\tV4L2Device() = delete;\n> +\tV4L2Device(const std::string &device);\n> +\t~V4L2Device();\n> +\n> +\tint open();\n> +\tbool isOpen() const;\n> +\tvoid close();\n> +\n> +private:\n> +\tstd::string device_;\n\ndevnode_ to standardize with MediaDevice ?\n\n> +\tint fd_;\n> +\tV4L2Capability caps_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_V4L2_DEVICE_H__ */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index abf9a71d4172..f9f25c0ecf15 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -11,6 +11,7 @@ libcamera_sources = files([\n> 'pipeline_handler.cpp',\n> 'signal.cpp',\n> 'timer.cpp',\n> + 'v4l2_device.cpp',\n> ])\n>\n> libcamera_headers = files([\n> @@ -21,6 +22,7 @@ libcamera_headers = files([\n> 'include/media_object.h',\n> 'include/pipeline_handler.h',\n> 'include/utils.h',\n> + 'include/v4l2_device.h',\n> ])\n>\n> libcamera_internal_includes = include_directories('include')\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> new file mode 100644\n> index 000000000000..3133bfe4ffb0\n> --- /dev/null\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -0,0 +1,127 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * v4l2_device.cpp - V4L2 Device API\n\nKeep in sync with the header please\n\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"log.h\"\n> +#include \"v4l2_device.h\"\n> +\n> +/**\n> + * \\file v4l2_device.h\n> + * \\brief V4L2 Device API\n> + */\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class V4L2Capability\n> + * \\brief struct v4l2_capability object wrapper and helpers\n> + *\n> + * The V4L2Capability wraps the V4L2 API interactions for capabilities and\n> + * device flag parsing.\n> + */\n\nI would mention that the class uses informations provided by the V4L2\nAPIs defined 'struct v4l2_capabilities', but if you agree with what\nI've said above, this is not a wrapper.\n\n> +\n> +/**\n> + * \\fn bool V4L2Capability::isCapture()\n> + * \\brief Identify if the device is capable of capturing video\n\nFor getter we used the \"Retrieve ... \" syntax in \\brief descriptions.\nThis is not strictly a getters, I'm fine with \"Identify\" as long as it\nis used consistently in other 'identity' function descriptions.\n\n> + * \\return boolean true if the device provides video frames\n\ns/boolean//\n\", false otherwise\" at the end\n\n> + */\n> +\n> +/**\n> + * \\fn bool V4L2Capability::isMplane()\n> + * \\brief Identify if the device uses MPLANE formats\n> + * \\return boolean true if the device uses multiplanar buffer formats\ns/boolean//\n\", false otherwise\" at the end\n\nno need for a isSinglePlane() ?\nDo we need to define isMulti/isSingle, or should we return a flag from\na 'V4L2Capability::plane()' function ?\n\n> + */\n> +\n> +/**\n> + * \\fn bool V4L2Capability::hasStreaming()\n> + * \\brief Determine if the device can perform Streaming IO\n> + * \\return boolean true if the device provides Streaming IO IOCTLs\n\ns/boolean//\n\", false otherwise\" at the end\n\n> + */\n> +\n> +/**\n> + * \\class V4L2Device\n> + * \\brief V4L2Device object and API.\n> + *\n> + * The V4L2 Device API class models an instance of a v4l2 device node.\n\nPlease expand this with indications on how the class is expected to be\nconstructed (by its devnode path, if the devnode is checked to be\nvalid), if it has to be opened before it can be operated on, how and who\nshall destroy it etc..\n\nAnd I think it's either V4L2 or v4l2.\n\n> + */\n> +\n> +/**\n> + * \\brief Constructor for a V4L2Device object. The \\a device specifies the path\n> + * to the video device node.\n\nFollowing the suggestions I have received on MediaObject\n\n\\brief Construct a V4L2Device\n\\param The V4L2 device node path\n\nAh, and the 'object' term comes from Java. I tried to avoid using it\nand refer to classes/instances\n\n> + * \\param device The file-system path to the video device node\n> + */\n> +V4L2Device::V4L2Device(const std::string &device)\n> +{\n> +\tdevice_ = device;\n> +\tfd_ = -1;\n> +\tcaps_ = { };\n\nInitializers list?\n\n> +}\n> +\n> +V4L2Device::~V4L2Device()\n> +{\n> +\tclose();\n> +}\n> +\n> +/**\n> + * \\brief Opens a v4l2 device and queries properties from the device.\n\nI dont' see other \\briefs using the third person. We can change this\nthough\n\n> + * \\return 0 on success, or a negative errno\n\nCopying from the classes we have in at the moment:\n\n\"or a negative error code otherwise\"\n\n> + */\n> +int V4L2Device::open()\n> +{\n> +\tint ret;\n> +\n> +\tif (isOpen()) {\n> +\t\tLOG(Error) << \"Device already open\";\n> +\t\treturn -EBUSY;\n> +\t}\n> +\n> +\tfd_ = ::open(device_.c_str(), O_RDWR);\n> +\tif (fd_ < 0) {\n> +\t\tLOG(Error) << \"Failed to open V4L2 device \" << device_\n> +\t\t\t << \" : \" << strerror(errno);\n> +\t\treturn -errno;\n\nWe standardized on the following error handling pattern:\n\n\tint ret = ::open(devnode_.c_str(), O_RDWR);\n\tif (ret < 0) {\n\t\tret = -errno;\n\t\tLOG(Error) << \"Failed to open v4l2 device at \" << devnode_\n\t\t\t << \": \" << strerror(-ret);\n\t\treturn ret;\n\t}\n\tfd_ = ret;\n\nSo that you don't have to assign fd_ before we know open succeeded\n\n> +\t}\n> +\n> +\tret = ioctl(fd_, VIDIOC_QUERYCAP, &caps_);\n\nIs this the V4L2Capability instance? Is it safe to pass to an IOCTL\nsomething larger that what it might expect? I'm genuinely asking this,\nnot sure...\n\n> +\tif (ret < 0) {\n> +\t\tLOG(Error) << \"Failed to query device capabilities: \" << strerror(errno);\n> +\t\treturn -errno;\n> +\t}\n> +\n> +\tif (!(caps_.hasStreaming())) {\n> +\t\tLOG(Error) << \"Device does not support streaming IO\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Checks to see if we have successfully opened a v4l2 video device.\n\nCheck (or Verify, or Return) if the device is open\n?\n\n> + */\n> +bool V4L2Device::isOpen() const\n> +{\n> +\treturn (fd_ != -1);\n> +}\n> +\n> +/**\n>+ * \\brief Close the device, releasing any resources acquired by \\a open().\n\nopen() is not an argument. You might use \\sa open() but I don't think\nit is necessary.\n\n> + */\n> +void V4L2Device::close()\n> +{\n> +\tif (fd_ < 0)\n> +\t\treturn;\n> +\n> +\t::close(fd_);\n> +\tfd_ = -1;\n> +}\n> +\n> +} /* namespace libcamera */\n> --\n> 2.17.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 relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7DC860C88\n\tfor ;\n\tTue, 15 Jan 2019 17:39:42 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 5A62624000B;\n\tTue, 15 Jan 2019 16:39:42 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 15 Jan 2019 17:39:51 +0100","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190115163951.5a6hsfvpp663j7rw@uno.localdomain>","References":"<20190115160212.30100-1-kieran.bingham@ideasonboard.com>\n\t<20190115160212.30100-2-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"4jmuux4nzvcunzuu\"","Content-Disposition":"inline","In-Reply-To":"<20190115160212.30100-2-kieran.bingham@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] lib: Add V4L2 Device object","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, 15 Jan 2019 16:39:43 -0000"}},{"id":336,"web_url":"https://patchwork.libcamera.org/comment/336/","msgid":"","date":"2019-01-15T20:46:59","subject":"Re: [libcamera-devel] [PATCH v2 1/2] lib: Add V4L2 Device object","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 15/01/2019 16:39, Jacopo Mondi wrote:\n> Hi Kieran,\n> thanks for the patch\n> \n> On Tue, Jan 15, 2019 at 04:02:11PM +0000, Kieran Bingham wrote:\n>> Provide a helper V4L2 device object capable of interacting with the\n>> V4L2 Linux Kernel APIs.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> src/libcamera/include/v4l2_device.h | 43 ++++++++++\n>> src/libcamera/meson.build | 2 +\n>> src/libcamera/v4l2_device.cpp | 127 ++++++++++++++++++++++++++++\n>> 3 files changed, 172 insertions(+)\n>> create mode 100644 src/libcamera/include/v4l2_device.h\n>> create mode 100644 src/libcamera/v4l2_device.cpp\n>>\n>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n>> new file mode 100644\n>> index 000000000000..ddcb17af2187\n>> --- /dev/null\n>> +++ b/src/libcamera/include/v4l2_device.h\n>> @@ -0,0 +1,43 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2019, Google Inc.\n>> + *\n>> + * v4l2_device.h - V4L2 Device API Abstractions\n> \n> I would leave \"API\" out (and \"abstraction\" too tbh...)\n\nThat leaves v4l2_device.h - V4L2 Device....\n\nSeems to make \"V4L2 Device\" a bit redundant, as that's expressed by the\nfilename?\n\nWe are abstracting the V4L2 device node API ... which is what I think I\nmeant in the file brief...\n\n\n\n>> + */\n>> +#ifndef __LIBCAMERA_V4L2_DEVICE_H__\n>> +#define __LIBCAMERA_V4L2_DEVICE_H__\n>> +\n>> +#include \n>> +\n>> +#include \n> \n> Please add this header to include/linux, otherwise you're relying on\n> the system wide installed one\n\n/me shudders again\n\nOk :-(\n\nWhich version? 4.19? 4.20? (git-master?)\nWhich version are the other files? (the README states 4.19)\nHow do we keep them in sync?\n\n\n>> +\n>> +namespace libcamera {\n>> +\n>> +class V4L2Capability : public v4l2_capability\n> \n> Is this the kernel header provided one?\n\nYes, the public v4l2_capability is the\n linux/videodev2.h::struct v4l2_capability\n\n> If so, I'm not that sure is a\n> good idea to extend it directly. True, you get compatibility with the\n> current header version automatically, but that might hide other issues\n> if the kernel API changes.\n\nI'm not sure I understand that point?\n\n> I would prefer accessing the\n> v4l2_capability directly, and fail at compile time if there have been\n> changes.\n\nIf the v4l2_capability struct changes drastically it will still fail at\ncompile time.\n\nI'm only adding member functions which operate on the struct\nv4l2_capability.\n\nWrapping them in the class V4L2Capability means that the functions are\ntied to the type.\n\nThe V4L2Capability could be extended to add more features and helpers as\nnecessary as they are needed.\n\n\n> Furthermore, you only use the 'capabilities' field of this structure,\n> am I wrong?\n\n\nYes, currently I do - because this is the simplified version based on\nLaurent's review comments to my previous queryCap().\n\n\n> \n>> +{\n>> +public:\n>> +\tbool isCapture() { return capabilities & V4L2_CAP_VIDEO_CAPTURE; }\n\nI guess this implementation is wrong. It should probably read:\n\n capabilites & (V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_CAPTURE_MPLANE);\n\nThe check is to tell if the device being opened will provide frames...\nas opposed to being an M2M device or an Output device.\n\n\n\n>> +\tbool isMplane() { return capabilities & V4L2_CAP_VIDEO_CAPTURE_MPLANE; }\n\nThis then determines if the device can support the mplane API...\n\n>> +\tbool hasStreaming() { return capabilities & V4L2_CAP_STREAMING; }\n>> +};\n>> +\n>> +class V4L2Device\n>> +{\n>> +public:\n>> +\tV4L2Device() = delete;\n>> +\tV4L2Device(const std::string &device);\n>> +\t~V4L2Device();\n>> +\n>> +\tint open();\n>> +\tbool isOpen() const;\n>> +\tvoid close();\n>> +\n>> +private:\n>> +\tstd::string device_;\n> \n> devnode_ to standardize with MediaDevice ?\n\n\nAnother reason why I think we should have a base Device class!\n\nIt's not just about code sharing - it's about defining the common interface!\n\nTo me a device node is a major and minor device number... Shouldn't this\ninstead be devicePath_ ?\n\n\n>> +\tint fd_;\n>> +\tV4L2Capability caps_;\n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> +\n>> +#endif /* __LIBCAMERA_V4L2_DEVICE_H__ */\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index abf9a71d4172..f9f25c0ecf15 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -11,6 +11,7 @@ libcamera_sources = files([\n>> 'pipeline_handler.cpp',\n>> 'signal.cpp',\n>> 'timer.cpp',\n>> + 'v4l2_device.cpp',\n>> ])\n>>\n>> libcamera_headers = files([\n>> @@ -21,6 +22,7 @@ libcamera_headers = files([\n>> 'include/media_object.h',\n>> 'include/pipeline_handler.h',\n>> 'include/utils.h',\n>> + 'include/v4l2_device.h',\n>> ])\n>>\n>> libcamera_internal_includes = include_directories('include')\n>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n>> new file mode 100644\n>> index 000000000000..3133bfe4ffb0\n>> --- /dev/null\n>> +++ b/src/libcamera/v4l2_device.cpp\n>> @@ -0,0 +1,127 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2019, Google Inc.\n>> + *\n>> + * v4l2_device.cpp - V4L2 Device API\n> \n> Keep in sync with the header please\n> \n>> + */\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \"log.h\"\n>> +#include \"v4l2_device.h\"\n>> +\n>> +/**\n>> + * \\file v4l2_device.h\n>> + * \\brief V4L2 Device API\n>> + */\n>> +namespace libcamera {\n>> +\n>> +/**\n>> + * \\class V4L2Capability\n>> + * \\brief struct v4l2_capability object wrapper and helpers\n>> + *\n>> + * The V4L2Capability wraps the V4L2 API interactions for capabilities and\n>> + * device flag parsing.\n>> + */\n> \n> I would mention that the class uses informations provided by the V4L2\n> APIs defined 'struct v4l2_capabilities', but if you agree with what\n> I've said above, this is not a wrapper.\n> \n>> +\n>> +/**\n>> + * \\fn bool V4L2Capability::isCapture()\n>> + * \\brief Identify if the device is capable of capturing video\n> \n> For getter we used the \"Retrieve ... \" syntax in \\brief descriptions.\n> This is not strictly a getters, I'm fine with \"Identify\" as long as it\n> is used consistently in other 'identity' function descriptions.\n\nThis is not a getter. It does not return an internal value. It parses\nthe internal state for information and classifies that state to\ndetermine if the device is a capture device ...\n\n\n\n> \n>> + * \\return boolean true if the device provides video frames\n> \n> s/boolean//\n> \", false otherwise\" at the end\n\nReally ?\n\n Do we have to say that a boolean returns either true or false?\n\n\n>> + */\n>> +\n>> +/**\n>> + * \\fn bool V4L2Capability::isMplane()\n>> + * \\brief Identify if the device uses MPLANE formats\n>> + * \\return boolean true if the device uses multiplanar buffer formats\n> s/boolean//\n> \", false otherwise\" at the end\n\n... er ...\n\nOh - I see you were removing the worked boolean. But that's the part\nthat describes it will be false otherwise..\n\n\n> no need for a isSinglePlane() ?\n> Do we need to define isMulti/isSingle, or should we return a flag from\n> a 'V4L2Capability::plane()' function ?\n\nThis isn't a full implementation of the parsing of the\nv4l2_capabilities. It can grow with time based on what is needed.\n\nI very soon need to know if the device is an MPLANE device (i.e.\n\n\n>> + */\n>> +\n>> +/**\n>> + * \\fn bool V4L2Capability::hasStreaming()\n>> + * \\brief Determine if the device can perform Streaming IO\n>> + * \\return boolean true if the device provides Streaming IO IOCTLs\n> \n> s/boolean//\n> \", false otherwise\" at the end\n> \n>> + */\n>> +\n>> +/**\n>> + * \\class V4L2Device\n>> + * \\brief V4L2Device object and API.\n>> + *\n>> + * The V4L2 Device API class models an instance of a v4l2 device node.\n> \n> Please expand this with indications on how the class is expected to be\n> constructed (by its devnode path, if the devnode is checked to be\n> valid), if it has to be opened before it can be operated on, how and who\n> shall destroy it etc..\n> \n> And I think it's either V4L2 or v4l2.\n\nGood spot - I should make this V4L2\n\n> \n>> + */\n>> +\n>> +/**\n>> + * \\brief Constructor for a V4L2Device object. The \\a device specifies the path\n>> + * to the video device node.\n> \n> Following the suggestions I have received on MediaObject\n> \n> \\brief Construct a V4L2Device\n> \\param The V4L2 device node path\n\nAh yes - I should move to the \\param ...\n\n> \n> Ah, and the 'object' term comes from Java. I tried to avoid using it\n> and refer to classes/instances\n\nDoes it (come from Java?) ? I thought C++ was an object orientated\nlanguage.. where an object was an instantiation of the class type.\n\nThis this is the constructor for that object instantiation...\n\n\n>> + * \\param device The file-system path to the video device node\n>> + */\n>> +V4L2Device::V4L2Device(const std::string &device)\n>> +{\n>> +\tdevice_ = device;\n>> +\tfd_ = -1;\n>> +\tcaps_ = { };\n> \n> Initializers list?\n\nYup.\n\n\n> \n>> +}\n>> +\n>> +V4L2Device::~V4L2Device()\n>> +{\n>> +\tclose();\n>> +}\n>> +\n>> +/**\n>> + * \\brief Opens a v4l2 device and queries properties from the device.\n> \n> I dont' see other \\briefs using the third person. We can change this\n> though\n> \n>> + * \\return 0 on success, or a negative errno\n> \n> Copying from the classes we have in at the moment:\n> \n> \"or a negative error code otherwise\"\n\nSure that's a bit clearer\n\n\n>> + */\n>> +int V4L2Device::open()\n>> +{\n>> +\tint ret;\n>> +\n>> +\tif (isOpen()) {\n>> +\t\tLOG(Error) << \"Device already open\";\n>> +\t\treturn -EBUSY;\n>> +\t}\n>> +\n>> +\tfd_ = ::open(device_.c_str(), O_RDWR);\n>> +\tif (fd_ < 0) {\n>> +\t\tLOG(Error) << \"Failed to open V4L2 device \" << device_\n>> +\t\t\t << \" : \" << strerror(errno);\n>> +\t\treturn -errno;\n> \n> We standardized on the following error handling pattern:\n> \n> \tint ret = ::open(devnode_.c_str(), O_RDWR);\n> \tif (ret < 0) {\n> \t\tret = -errno;\n> \t\tLOG(Error) << \"Failed to open v4l2 device at \" << devnode_\n> \t\t\t << \": \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n> \tfd_ = ret;\n> \n> So that you don't have to assign fd_ before we know open succeeded\n> \n>> +\t}\n>> +\n>> +\tret = ioctl(fd_, VIDIOC_QUERYCAP, &caps_);\n> \n> Is this the V4L2Capability instance? Is it safe to pass to an IOCTL\n> something larger that what it might expect? I'm genuinely asking this,\n> not sure...\n\nIt's not larger. It's the same size. I validated that\nsizeof(V4L2Capability) and sizeof(struct v4l2_capability) were identical\nwhen putting the code together.\n\nI was actually trying to work out how to make a function to 'get' a\npointer to the base class type, i.e. a (struct v4l2_capabillity *)self\ngetter - and I somehow ended up with this which generates no compile\nwarnings and works.\n\nI've also validated that the data is aligned the same etc whether you\npass a struct v4l2_capabiltiy or a V4L2Capability.\n\nI originally thought that this was the compiler perfoming the\npolymorphism correctly to the base type because of the type information\nin the ioctl() parameter - but now I'm not sure if it's perhaps just\ncoincidental (or rather design) that the base class is the first part of\nthe subclass.\n\nThe V4L2Capability is not expecting to store extra data anyway. It's\njust a way to tie methods to the type.\n\n\n>> +\tif (ret < 0) {\n>> +\t\tLOG(Error) << \"Failed to query device capabilities: \" << strerror(errno);\n>> +\t\treturn -errno;\n>> +\t}\n>> +\n>> +\tif (!(caps_.hasStreaming())) {\n>> +\t\tLOG(Error) << \"Device does not support streaming IO\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Checks to see if we have successfully opened a v4l2 video device.\n> \n> Check (or Verify, or Return) if the device is open\n> ?\n> \n\nIs your question based on the tense again here?\n\n\n>> + */\n>> +bool V4L2Device::isOpen() const\n>> +{\n>> +\treturn (fd_ != -1);\n>> +}\n>> +\n>> +/**\n>> + * \\brief Close the device, releasing any resources acquired by \\a open().\n> \n> open() is not an argument. You might use \\sa open() but I don't think\n> it is necessary.\n> \n\nOh - I thought \\a was 'anchor' as in to generate a hyperlink anchor to a\nnamed thing.\n\nI want open() to be linked to the open() documentation.\nMaybe \\sa is also appropriate - I'll see how it affects the Doxygen output.\n\n>> + */\n>> +void V4L2Device::close()\n>> +{\n>> +\tif (fd_ < 0)\n>> +\t\treturn;\n>> +\n>> +\t::close(fd_);\n>> +\tfd_ = -1;\n>> +}\n>> +\n>> +} /* namespace libcamera */\n>> --\n>> 2.17.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 perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4779560C96\n\tfor ;\n\tTue, 15 Jan 2019 21:47:03 +0100 (CET)","from [192.168.0.21]\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 93FD94F8;\n\tTue, 15 Jan 2019 21:47:02 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547585222;\n\tbh=PGZychn5PeyV+7PDgI1s3W4t8PYt4rnu1QmiwulQK80=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=Pk80GJcnHSCIoF39q1rtDPFjkCO9ofq0wLcQGBgaKclurjq3vkFORT+/ZHZsJS+NZ\n\tzuYdua5IDV8qdc4tmm93qNtYbMmieN1cl6TOh7FXRzPzOr2Z7mTdxgpj/4nz9OYfE4\n\tblUp6PQ3B/FOKg6IXwTXk7aO/eq0YQyLjF+QIwRU=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi ","Cc":"LibCamera Devel ","References":"<20190115160212.30100-1-kieran.bingham@ideasonboard.com>\n\t<20190115160212.30100-2-kieran.bingham@ideasonboard.com>\n\t<20190115163951.5a6hsfvpp663j7rw@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, 15 Jan 2019 20:46:59 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190115163951.5a6hsfvpp663j7rw@uno.localdomain>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] lib: Add V4L2 Device object","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, 15 Jan 2019 20:47:03 -0000"}},{"id":339,"web_url":"https://patchwork.libcamera.org/comment/339/","msgid":"","date":"2019-01-15T21:35:17","subject":"Re: [libcamera-devel] [PATCH v2 1/2] lib: Add V4L2 Device object","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 15/01/2019 20:46, Kieran Bingham wrote:\n> Hi Jacopo,\n\n\n\n\n>>> + * \\param device The file-system path to the video device node\n>>> + */\n>>> +V4L2Device::V4L2Device(const std::string &device)\n>>> +{\n>>> +\tdevice_ = device;\n>>> +\tfd_ = -1;\n>>> +\tcaps_ = { };\n\nInterestingly, because the V4L2Capability caps_ is a class - it\nautomatically gets initialised to 0, where as a struct v4l2_capability\nhere does not.\n\n>> Initializers list?\n> \n> Yup.\n\nFixed.\n\nWorking through the rest now.\n\n","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 46C1460C6A\n\tfor ;\n\tTue, 15 Jan 2019 22:35:20 +0100 (CET)","from [192.168.0.21]\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 ACBDA530;\n\tTue, 15 Jan 2019 22:35:19 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547588119;\n\tbh=qTYeLJH4oXfjX9YPKrbxLbp7UwTm5ESdA5L4VqERwE4=;\n\th=Reply-To:Subject:From:To:Cc:References:Date:In-Reply-To:From;\n\tb=iP95JxMY+mYtWYEfush8rXQTOqRxgY0ERJLE9H9H3ZhFNTX7OZvOn/my/AMGrgdME\n\t2bFXK2bj5rUTVJTS38YMYPHVWy2PREzqwWiM2lVDzK0DJ4ZpRT0EyJrX8UCtujmTzr\n\tZPzsNde2UVrp//1+lRms+mGt/1MDDnA33hilfYrE=","Reply-To":"kieran.bingham@ideasonboard.com","From":"Kieran Bingham ","To":"Jacopo Mondi ","Cc":"LibCamera Devel ","References":"<20190115160212.30100-1-kieran.bingham@ideasonboard.com>\n\t<20190115160212.30100-2-kieran.bingham@ideasonboard.com>\n\t<20190115163951.5a6hsfvpp663j7rw@uno.localdomain>\n\t","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, 15 Jan 2019 21:35:17 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] lib: Add V4L2 Device object","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, 15 Jan 2019 21:35:20 -0000"}}]