Patch Detail

HTTP 200 OK
Allow: GET, PUT, PATCH, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

{
    "id": 16246,
    "url": "https://patchwork.libcamera.org/api/1.1/patches/16246/?format=api",
    "web_url": "https://patchwork.libcamera.org/patch/16246/",
    "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": "<20220616150535.21742-3-laurent.pinchart@ideasonboard.com>",
    "date": "2022-06-16T15:05:33",
    "name": "[libcamera-devel,RFC,v3,2/4] libcamera: yaml_parser: Preserve order of items in dictionary",
    "commit_ref": null,
    "pull_url": null,
    "state": "accepted",
    "archived": false,
    "hash": "6573e87bc1d430d9c758c1f2ec03d4d58cce6c91",
    "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/16246/mbox/",
    "series": [
        {
            "id": 3182,
            "url": "https://patchwork.libcamera.org/api/1.1/series/3182/?format=api",
            "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3182",
            "date": "2022-06-16T15:05:31",
            "name": "Replace boost JSON parser with libyaml in Raspberry Pi IPA",
            "version": 3,
            "mbox": "https://patchwork.libcamera.org/series/3182/mbox/"
        }
    ],
    "comments": "https://patchwork.libcamera.org/api/patches/16246/comments/",
    "check": "pending",
    "checks": "https://patchwork.libcamera.org/api/patches/16246/checks/",
    "tags": {},
    "headers": {
        "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>",
        "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 66675C3274\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 16 Jun 2022 15:05:55 +0000 (UTC)",
            "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC1E06563B;\n\tThu, 16 Jun 2022 17:05: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 52E40600F0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Jun 2022 17:05:50 +0200 (CEST)",
            "from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B69AA8AF;\n\tThu, 16 Jun 2022 17:05:49 +0200 (CEST)"
        ],
        "DKIM-Signature": [
            "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655391954;\n\tbh=dBQXB41OzInljx0+jHsMKf4NLOImcVQk41QqHqkNrZk=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=2B08Maw/ISUp/NYIJTTNid01yrMVSEMIgJts3tWcXuqkhEXuWflK8udqMcIbqtO1T\n\tugEwvUlPmT6gAn+zpK1AHw/81/nNEpCp8Q9PMo7XemC1bHaRjU5HtRo9iim+IKdVZr\n\tKkr2rf/CsyBq1dcp6nKu1LWhH62ttobLnesfGbezFoLxaB4kTGPdGmKAo+c79bSLE2\n\tVYdzXfUxfVMImvDk7b2qmirLz7IULTcpLalyheRY250W/VKVOJerZx+soHE5aybZgY\n\tEqsdHHAOMNJxp/NH9tUvjFrwdtd/93LhcGp5541a2fhHAKhmR3RvIQWcD35EXEcVwN\n\tMp+P46q+idinQ==",
            "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655391950;\n\tbh=dBQXB41OzInljx0+jHsMKf4NLOImcVQk41QqHqkNrZk=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=PpKqC3E6K80JgqWQkCNE/FnspqIxw4LenVGGO3FM6XetylwIT9/iyG1pl+DUI3rjh\n\t0/QJ5LW9Mj6VzJORNDK2nF+JfOsliMt/dvoruBUdhbc+X9kRo7sjQ7FsgT/nIMi5pp\n\t1TmJN04Nj7s0yxGFnkN5IBPDa2LUZFHbawrysRb8="
        ],
        "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"PpKqC3E6\"; dkim-atps=neutral",
        "To": "libcamera-devel@lists.libcamera.org",
        "Date": "Thu, 16 Jun 2022 18:05:33 +0300",
        "Message-Id": "<20220616150535.21742-3-laurent.pinchart@ideasonboard.com>",
        "X-Mailer": "git-send-email 2.35.1",
        "In-Reply-To": "<20220616150535.21742-1-laurent.pinchart@ideasonboard.com>",
        "References": "<20220616150535.21742-1-laurent.pinchart@ideasonboard.com>",
        "MIME-Version": "1.0",
        "Content-Transfer-Encoding": "8bit",
        "Subject": "[libcamera-devel] [RFC PATCH v3 2/4] libcamera: yaml_parser:\n\tPreserve order of items in dictionary",
        "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>",
        "From": "Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>",
        "Reply-To": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>",
        "Errors-To": "libcamera-devel-bounces@lists.libcamera.org",
        "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"
    },
    "content": "The std::map container used to store dictionary items in YamlObject\ndoesn't preserve the YAML data order, as maps are ordered by key, not by\ninsertion order. While this is compliant with the YAML specification\nwhich doesn't guarantee ordering of mappings, the Raspberry Pi IPA\nrelies on elements being ordered as in the YAML data. To replace the\ndependency on boost with the YamlParser class, we thus need to guarantee\nthat the order is preserved.\n\nPreserve the order by storing items in list_ unconditionally. Turn the\nlist_ vector from storing YamlObject unique pointers to storing\nkey-value pairs, with the key being absent when the object is a list,\nnot a dictionary.\n\nThe YamlObject implementation is updated to preserve the existing API,\nwith the only difference being that YamlObject::memberNames() now\nreturns member names in the same order as in the YAML file.\n\nThe ordering is an implementation detail, so changing it doesn't violate\nthe YAML specification. The documentation is not updated to reflect\nthis, as we don't want any new user to rely on a particular ordering.\nThis commit could be reverted if desired when the Raspberry Pi IPA\nupdates to a new tuning data format and drops support for the old\nformat.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/internal/yaml_parser.h | 37 +++++++++++++++---------\n src/libcamera/yaml_parser.cpp            | 35 ++++++++++++++--------\n 2 files changed, 46 insertions(+), 26 deletions(-)",
    "diff": "diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\nindex 90d348eac74c..b0bda1fc103e 100644\n--- a/include/libcamera/internal/yaml_parser.h\n+++ b/include/libcamera/internal/yaml_parser.h\n@@ -24,12 +24,21 @@ class YamlParserContext;\n class YamlObject\n {\n private:\n-\tusing DictContainer = std::map<std::string, std::unique_ptr<YamlObject>>;\n+\tstruct Value {\n+\t\tValue(std::string &&k, std::unique_ptr<YamlObject> &&v)\n+\t\t\t: key(std::move(k)), value(std::move(v))\n+\t\t{\n+\t\t}\n+\t\tstd::string key;\n+\t\tstd::unique_ptr<YamlObject> value;\n+\t};\n+\n+\tusing Container = std::vector<Value>;\n \tusing ListContainer = std::vector<std::unique_ptr<YamlObject>>;\n \n public:\n #ifndef __DOXYGEN__\n-\ttemplate<typename Container, typename Derived>\n+\ttemplate<typename Derived>\n \tclass Iterator\n \t{\n \tpublic:\n@@ -65,10 +74,10 @@ public:\n \t\t}\n \n \tprotected:\n-\t\ttypename Container::const_iterator it_;\n+\t\tContainer::const_iterator it_;\n \t};\n \n-\ttemplate<typename Container, typename Iterator>\n+\ttemplate<typename Iterator>\n \tclass Adapter\n \t{\n \tpublic:\n@@ -91,7 +100,7 @@ public:\n \t\tconst Container &container_;\n \t};\n \n-\tclass ListIterator : public Iterator<ListContainer, ListIterator>\n+\tclass ListIterator : public Iterator<ListIterator>\n \t{\n \tpublic:\n \t\tusing value_type = const YamlObject &;\n@@ -100,16 +109,16 @@ public:\n \n \t\tvalue_type operator*() const\n \t\t{\n-\t\t\treturn *it_->get();\n+\t\t\treturn *it_->value.get();\n \t\t}\n \n \t\tpointer operator->() const\n \t\t{\n-\t\t\treturn it_->get();\n+\t\t\treturn it_->value.get();\n \t\t}\n \t};\n \n-\tclass DictIterator : public Iterator<DictContainer, DictIterator>\n+\tclass DictIterator : public Iterator<DictIterator>\n \t{\n \tpublic:\n \t\tusing value_type = std::pair<const std::string &, const YamlObject &>;\n@@ -118,17 +127,17 @@ public:\n \n \t\tvalue_type operator*() const\n \t\t{\n-\t\t\treturn { it_->first, *it_->second.get() };\n+\t\t\treturn { it_->key, *it_->value.get() };\n \t\t}\n \t};\n \n-\tclass DictAdapter : public Adapter<DictContainer, DictIterator>\n+\tclass DictAdapter : public Adapter<DictIterator>\n \t{\n \tpublic:\n \t\tusing key_type = std::string;\n \t};\n \n-\tclass ListAdapter : public Adapter<ListContainer, ListIterator>\n+\tclass ListAdapter : public Adapter<ListIterator>\n \t{\n \t};\n #endif /* __DOXYGEN__ */\n@@ -167,7 +176,7 @@ public:\n #endif\n \tT get(const T &defaultValue, bool *ok = nullptr) const;\n \n-\tDictAdapter asDict() const { return DictAdapter{ dictionary_ }; }\n+\tDictAdapter asDict() const { return DictAdapter{ list_ }; }\n \tListAdapter asList() const { return ListAdapter{ list_ }; }\n \n \tconst YamlObject &operator[](std::size_t index) const;\n@@ -189,8 +198,8 @@ private:\n \tType type_;\n \n \tstd::string value_;\n-\tListContainer list_;\n-\tDictContainer dictionary_;\n+\tContainer list_;\n+\tstd::map<std::string, YamlObject *> dictionary_;\n };\n \n class YamlParser final\ndiff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\nindex 5c45e44e49c3..e30de050a0ea 100644\n--- a/src/libcamera/yaml_parser.cpp\n+++ b/src/libcamera/yaml_parser.cpp\n@@ -91,7 +91,6 @@ std::size_t YamlObject::size() const\n {\n \tswitch (type_) {\n \tcase Type::Dictionary:\n-\t\treturn dictionary_.size();\n \tcase Type::List:\n \t\treturn list_.size();\n \tdefault:\n@@ -311,11 +310,11 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const\n \t * that ok == nullptr.\n \t */\n \tbool valid;\n-\tuint32_t width = list_[0]->get<uint32_t>(0, &valid);\n+\tuint32_t width = list_[0].value->get<uint32_t>(0, &valid);\n \tif (!valid)\n \t\treturn defaultValue;\n \n-\tuint32_t height = list_[1]->get<uint32_t>(0, &valid);\n+\tuint32_t height = list_[1].value->get<uint32_t>(0, &valid);\n \tif (!valid)\n \t\treturn defaultValue;\n \n@@ -379,7 +378,7 @@ const YamlObject &YamlObject::operator[](std::size_t index) const\n \tif (type_ != Type::List || index >= size())\n \t\treturn empty;\n \n-\treturn *list_[index];\n+\treturn *list_[index].value;\n }\n \n /**\n@@ -395,7 +394,7 @@ const YamlObject &YamlObject::operator[](std::size_t index) const\n  */\n bool YamlObject::contains(const std::string &key) const\n {\n-\tif (dictionary_.find(key) == dictionary_.end())\n+\tif (dictionary_.find(std::ref(key)) == dictionary_.end())\n \t\treturn false;\n \n \treturn true;\n@@ -667,16 +666,16 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even\n \t\tyamlObject.type_ = YamlObject::Type::List;\n \t\tauto &list = yamlObject.list_;\n \t\tauto handler = [this, &list](EventPtr evt) {\n-\t\t\tlist.emplace_back(new YamlObject());\n-\t\t\treturn parseNextYamlObject(*list.back(), std::move(evt));\n+\t\t\tlist.emplace_back(std::string{}, std::make_unique<YamlObject>());\n+\t\t\treturn parseNextYamlObject(*list.back().value, std::move(evt));\n \t\t};\n \t\treturn parseDictionaryOrList(YamlObject::Type::List, handler);\n \t}\n \n \tcase YAML_MAPPING_START_EVENT: {\n \t\tyamlObject.type_ = YamlObject::Type::Dictionary;\n-\t\tauto &dictionary = yamlObject.dictionary_;\n-\t\tauto handler = [this, &dictionary](EventPtr evtKey) {\n+\t\tauto &list = yamlObject.list_;\n+\t\tauto handler = [this, &list](EventPtr evtKey) {\n \t\t\t/* Parse key */\n \t\t\tif (evtKey->type != YAML_SCALAR_EVENT) {\n \t\t\t\tLOG(YamlParser, Error) << \"Expect key at line: \"\n@@ -694,10 +693,19 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even\n \t\t\tif (!evtValue)\n \t\t\t\treturn -EINVAL;\n \n-\t\t\tauto elem = dictionary.emplace(key, std::make_unique<YamlObject>());\n-\t\t\treturn parseNextYamlObject(*elem.first->second.get(), std::move(evtValue));\n+\t\t\tauto &elem = list.emplace_back(std::move(key),\n+\t\t\t\t\t\t       std::make_unique<YamlObject>());\n+\t\t\treturn parseNextYamlObject(*elem.value, std::move(evtValue));\n \t\t};\n-\t\treturn parseDictionaryOrList(YamlObject::Type::Dictionary, handler);\n+\t\tint ret = parseDictionaryOrList(YamlObject::Type::Dictionary, handler);\n+\t\tif (ret)\n+\t\t\treturn ret;\n+\n+\t\tauto &dictionary = yamlObject.dictionary_;\n+\t\tfor (const auto &elem : list)\n+\t\t\tdictionary.emplace(elem.key, elem.value.get());\n+\n+\t\treturn 0;\n \t}\n \n \tdefault:\n@@ -753,6 +761,9 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even\n  * The YamlParser::parse() function takes an open FILE, parses its contents, and\n  * returns a pointer to a YamlObject corresponding to the root node of the YAML\n  * document.\n+ *\n+ * The parser preserves the order of items in the YAML file, for both lists and\n+ * dictionaries.\n  */\n \n /**\n",
    "prefixes": [
        "libcamera-devel",
        "RFC",
        "v3",
        "2/4"
    ]
}