Patch Detail
Show a patch.
GET /api/1.1/patches/3229/?format=api
{ "id": 3229, "url": "https://patchwork.libcamera.org/api/1.1/patches/3229/?format=api", "web_url": "https://patchwork.libcamera.org/patch/3229/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200321003640.2156-2-laurent.pinchart@ideasonboard.com>", "date": "2020-03-21T00:36:34", "name": "[libcamera-devel,v2,1/7] libcamera: controls: Add zero-copy set API for ControlValue", "commit_ref": "8daf20485b90af2065e3db0e3fd0cd5b72fd7ac4", "pull_url": null, "state": "accepted", "archived": false, "hash": "9e66f563854c21089e3d3ff99639fae234d0e316", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/3229/mbox/", "series": [ { "id": 754, "url": "https://patchwork.libcamera.org/api/1.1/series/754/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=754", "date": "2020-03-21T00:36:33", "name": "Add support for V4L2 array controls", "version": 2, "mbox": "https://patchwork.libcamera.org/series/754/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/3229/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/3229/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06A9662BA9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 21 Mar 2020 01:36:51 +0100 (CET)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8B5B31265;\n\tSat, 21 Mar 2020 01:36:50 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584751010;\n\tbh=Rx24ezjq6uZEMrSny/+gAL7tGn9NMgYHXjSoiZdj9VE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=GmjI+T/K/d4DwAEDbH+AVSNWm3J16GepE3ThjJPuCWBAuv5YyMCZcu5L3ygoONgtH\n\ttnprOMuNK2EVITMVF6FOzoL9/yjVTpOYdzEE3+QdvPpvVJkgRSzmssx52NuEiYd+eO\n\tUkiuD/AkxULdSp2i5eTEEUCcq6T7d1RPYJEHucE8=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 21 Mar 2020 02:36:34 +0200", "Message-Id": "<20200321003640.2156-2-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200321003640.2156-1-laurent.pinchart@ideasonboard.com>", "References": "<20200321003640.2156-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 1/7] libcamera: controls: Add zero-copy\n\tset API for ControlValue", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Sat, 21 Mar 2020 00:36:51 -0000" }, "content": "Extend the ControlValue class with a reserve() function to set the value\nwithout actually copying data, and a non-const data() function that\nallows writing data directly to the ControlValue storage. This allows\nallocating memory directly in ControlValue, potentially removing a data\ncopy.\n\nNote that this change was implemented before ByteStreamBuffer gained the\nzero-copy read() variant, and doesn't actually save a copy in the\ncontrol serializer. It however still simplifies\nControlSerializer::loadControlValue().\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/controls.h | 4 ++\n src/libcamera/control_serializer.cpp | 61 ++--------------------\n src/libcamera/controls.cpp | 54 ++++++++++++++-----\n src/libcamera/include/control_serializer.h | 3 --\n 4 files changed, 50 insertions(+), 72 deletions(-)", "diff": "diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex 2a6657128f17..4b2e7e9cdd6c 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -115,6 +115,7 @@ public:\n \tbool isArray() const { return isArray_; }\n \tstd::size_t numElements() const { return numElements_; }\n \tSpan<const uint8_t> data() const;\n+\tSpan<uint8_t> data();\n \n \tstd::string toString() const;\n \n@@ -174,6 +175,9 @@ public:\n \t\t value.data(), value.size(), sizeof(typename T::value_type));\n \t}\n \n+\tvoid reserve(ControlType type, bool isArray = false,\n+\t\t std::size_t numElements = 1);\n+\n private:\n \tControlType type_ : 8;\n \tbool isArray_;\ndiff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\nindex 808419f246c0..fcff5e56fbf7 100644\n--- a/src/libcamera/control_serializer.cpp\n+++ b/src/libcamera/control_serializer.cpp\n@@ -295,70 +295,17 @@ int ControlSerializer::serialize(const ControlList &list,\n \treturn 0;\n }\n \n-template<typename T>\n-ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,\n-\t\t\t\t\t\t bool isArray,\n-\t\t\t\t\t\t unsigned int count)\n-{\n-\tControlValue value;\n-\n-\tconst T *data = buffer.read<T>(count);\n-\tif (!data)\n-\t\treturn value;\n-\n-\tif (isArray)\n-\t\tvalue.set(Span<const T>{ data, count });\n-\telse\n-\t\tvalue.set(*data);\n-\n-\treturn value;\n-}\n-\n-template<>\n-ControlValue ControlSerializer::loadControlValue<std::string>(ByteStreamBuffer &buffer,\n-\t\t\t\t\t\t bool isArray,\n-\t\t\t\t\t\t unsigned int count)\n-{\n-\tControlValue value;\n-\n-\tconst char *data = buffer.read<char>(count);\n-\tif (!data)\n-\t\treturn value;\n-\n-\tvalue.set(std::string{ data, count });\n-\n-\treturn value;\n-}\n-\n ControlValue ControlSerializer::loadControlValue(ControlType type,\n \t\t\t\t\t\t ByteStreamBuffer &buffer,\n \t\t\t\t\t\t bool isArray,\n \t\t\t\t\t\t unsigned int count)\n {\n-\tswitch (type) {\n-\tcase ControlTypeBool:\n-\t\treturn loadControlValue<bool>(buffer, isArray, count);\n-\n-\tcase ControlTypeByte:\n-\t\treturn loadControlValue<uint8_t>(buffer, isArray, count);\n-\n-\tcase ControlTypeInteger32:\n-\t\treturn loadControlValue<int32_t>(buffer, isArray, count);\n-\n-\tcase ControlTypeInteger64:\n-\t\treturn loadControlValue<int64_t>(buffer, isArray, count);\n-\n-\tcase ControlTypeFloat:\n-\t\treturn loadControlValue<float>(buffer, isArray, count);\n-\n-\tcase ControlTypeString:\n-\t\treturn loadControlValue<std::string>(buffer, isArray, count);\n+\tControlValue value;\n \n-\tcase ControlTypeNone:\n-\t\treturn ControlValue();\n-\t}\n+\tvalue.reserve(type, isArray, count);\n+\tbuffer.read(value.data());\n \n-\treturn ControlValue();\n+\treturn value;\n }\n \n ControlInfo ControlSerializer::loadControlInfo(ControlType type,\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 2568447478d5..540cc026417a 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -189,6 +189,15 @@ Span<const uint8_t> ControlValue::data() const\n \treturn { data, size };\n }\n \n+/**\n+ * \\copydoc ControlValue::data() const\n+ */\n+Span<uint8_t> ControlValue::data()\n+{\n+\tSpan<const uint8_t> data = const_cast<const ControlValue *>(this)->data();\n+\treturn { const_cast<uint8_t *>(data.data()), data.size() };\n+}\n+\n /**\n * \\brief Assemble and return a string describing the value\n * \\return A string describing the ControlValue\n@@ -312,23 +321,44 @@ void ControlValue::set(ControlType type, bool isArray, const void *data,\n {\n \tASSERT(elementSize == ControlValueSize[type]);\n \n-\trelease();\n+\treserve(type, isArray, numElements);\n+\n+\tSpan<uint8_t> storage = ControlValue::data();\n+\tmemcpy(storage.data(), data, storage.size());\n+}\n+\n+/**\n+ * \\brief Set the control type and reserve memory\n+ * \\param[in] type The control type\n+ * \\param[in] isArray True to make the value an array\n+ * \\param[in] numElements The number of elements\n+ *\n+ * This function sets the type of the control value to \\a type, and reserves\n+ * memory to store the control value. If \\a isArray is true, the instance\n+ * becomes an array control and storage for \\a numElements is reserved.\n+ * Otherwise the instance becomes a simple control, numElements is ignored, and\n+ * storage for the single element is reserved.\n+ */\n+void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElements)\n+{\n+\tif (!isArray)\n+\t\tnumElements = 1;\n+\n+\tstd::size_t oldSize = numElements_ * ControlValueSize[type_];\n+\tstd::size_t newSize = numElements * ControlValueSize[type];\n+\n+\tif (oldSize != newSize)\n+\t\trelease();\n \n \ttype_ = type;\n-\tnumElements_ = numElements;\n \tisArray_ = isArray;\n+\tnumElements_ = numElements;\n \n-\tstd::size_t size = elementSize * numElements;\n-\tvoid *storage;\n-\n-\tif (size > sizeof(value_)) {\n-\t\tstorage_ = reinterpret_cast<void *>(new uint8_t[size]);\n-\t\tstorage = storage_;\n-\t} else {\n-\t\tstorage = reinterpret_cast<void *>(&value_);\n-\t}\n+\tif (oldSize == newSize)\n+\t\treturn;\n \n-\tmemcpy(storage, data, size);\n+\tif (newSize > sizeof(value_))\n+\t\tstorage_ = reinterpret_cast<void *>(new uint8_t[newSize]);\n }\n \n /**\ndiff --git a/src/libcamera/include/control_serializer.h b/src/libcamera/include/control_serializer.h\nindex 70aa28fd5f58..99bacd920fce 100644\n--- a/src/libcamera/include/control_serializer.h\n+++ b/src/libcamera/include/control_serializer.h\n@@ -40,9 +40,6 @@ private:\n \tstatic void store(const ControlValue &value, ByteStreamBuffer &buffer);\n \tstatic void store(const ControlInfo &info, ByteStreamBuffer &buffer);\n \n-\ttemplate<typename T>\n-\tControlValue loadControlValue(ByteStreamBuffer &buffer, bool isArray,\n-\t\t\t\t unsigned int count);\n \tControlValue loadControlValue(ControlType type, ByteStreamBuffer &buffer,\n \t\t\t\t bool isArray = false, unsigned int count = 1);\n \tControlInfo loadControlInfo(ControlType type, ByteStreamBuffer &buffer);\n", "prefixes": [ "libcamera-devel", "v2", "1/7" ] }