libcamera: yaml_parser: Preserve order of items in dictionary

The std::map container used to store dictionary items in YamlObject
doesn't preserve the YAML data order, as maps are ordered by key, not by
insertion order. While this is compliant with the YAML specification
which doesn't guarantee ordering of mappings, the Raspberry Pi IPA
relies on elements being ordered as in the YAML data. To replace the
dependency on boost with the YamlParser class, we thus need to guarantee
that the order is preserved.

Preserve the order by storing items in list_ unconditionally. Turn the
list_ vector from storing YamlObject unique pointers to storing
key-value pairs, with the key being absent when the object is a list,
not a dictionary.

The YamlObject implementation is updated to preserve the existing API,
with the only difference being that YamlObject::memberNames() now
returns member names in the same order as in the YAML file.

The ordering is an implementation detail, so changing it doesn't violate
the YAML specification. The documentation is not updated to reflect
this, as we don't want any new user to rely on a particular ordering.
This commit could be reverted if desired when the Raspberry Pi IPA
updates to a new tuning data format and drops support for the old
format.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: Naushir Patuck <naush@raspberrypi.com>

include/libcamera/internal/yaml_parser.h

@@ -25,12 +25,21 @@ class YamlParserContext;
 class YamlObject
 {
 private:
-	using DictContainer = std::map<std::string, std::unique_ptr<YamlObject>>;
+	struct Value {
+		Value(std::string &&k, std::unique_ptr<YamlObject> &&v)
+			: key(std::move(k)), value(std::move(v))
+		{
+		}
+		std::string key;
+		std::unique_ptr<YamlObject> value;
+	};
+
+	using Container = std::vector<Value>;
 	using ListContainer = std::vector<std::unique_ptr<YamlObject>>;
 
 public:
 #ifndef __DOXYGEN__
-	template<typename Container, typename Derived>
+	template<typename Derived>
 	class Iterator
 	{
 	public:
@@ -66,10 +75,10 @@ public:
 		}
 
 	protected:
-		typename Container::const_iterator it_;
+		Container::const_iterator it_;
 	};
 
-	template<typename Container, typename Iterator>
+	template<typename Iterator>
 	class Adapter
 	{
 	public:
@@ -92,7 +101,7 @@ public:
 		const Container &container_;
 	};
 
-	class ListIterator : public Iterator<ListContainer, ListIterator>
+	class ListIterator : public Iterator<ListIterator>
 	{
 	public:
 		using value_type = const YamlObject &;
@@ -101,16 +110,16 @@ public:
 
 		value_type operator*() const
 		{
-			return *it_->get();
+			return *it_->value.get();
 		}
 
 		pointer operator->() const
 		{
-			return it_->get();
+			return it_->value.get();
 		}
 	};
 
-	class DictIterator : public Iterator<DictContainer, DictIterator>
+	class DictIterator : public Iterator<DictIterator>
 	{
 	public:
 		using value_type = std::pair<const std::string &, const YamlObject &>;
@@ -119,17 +128,17 @@ public:
 
 		value_type operator*() const
 		{
-			return { it_->first, *it_->second.get() };
+			return { it_->key, *it_->value.get() };
 		}
 	};
 
-	class DictAdapter : public Adapter<DictContainer, DictIterator>
+	class DictAdapter : public Adapter<DictIterator>
 	{
 	public:
 		using key_type = std::string;
 	};
 
-	class ListAdapter : public Adapter<ListContainer, ListIterator>
+	class ListAdapter : public Adapter<ListIterator>
 	{
 	};
 #endif /* __DOXYGEN__ */
@@ -174,7 +183,7 @@ public:
 		return get<T>().value_or(defaultValue);
 	}
 
-	DictAdapter asDict() const { return DictAdapter{ dictionary_ }; }
+	DictAdapter asDict() const { return DictAdapter{ list_ }; }
 	ListAdapter asList() const { return ListAdapter{ list_ }; }
 
 	const YamlObject &operator[](std::size_t index) const;
@@ -196,8 +205,8 @@ private:
 	Type type_;
 
 	std::string value_;
-	ListContainer list_;
+	Container list_;
-	DictContainer dictionary_;
+	std::map<std::string, YamlObject *> dictionary_;
 };
 
 class YamlParser final

src/libcamera/yaml_parser.cpp

@@ -85,7 +85,6 @@ std::size_t YamlObject::size() const
 {
 	switch (type_) {
 	case Type::Dictionary:
-		return dictionary_.size();
 	case Type::List:
 		return list_.size();
 	default:
@@ -280,11 +279,11 @@ std::optional<Size> YamlObject::get() const
 	if (list_.size() != 2)
 		return {};
 
-	auto width = list_[0]->get<uint32_t>();
+	auto width = list_[0].value->get<uint32_t>();
 	if (!width)
 		return {};
 
-	auto height = list_[1]->get<uint32_t>();
+	auto height = list_[1].value->get<uint32_t>();
 	if (!height)
 		return {};
 
@@ -347,7 +346,7 @@ const YamlObject &YamlObject::operator[](std::size_t index) const
 	if (type_ != Type::List || index >= size())
 		return empty;
 
-	return *list_[index];
+	return *list_[index].value;
 }
 
 /**
@@ -363,7 +362,7 @@ const YamlObject &YamlObject::operator[](std::size_t index) const
  */
 bool YamlObject::contains(const std::string &key) const
 {
-	if (dictionary_.find(key) == dictionary_.end())
+	if (dictionary_.find(std::ref(key)) == dictionary_.end())
 		return false;
 
 	return true;
@@ -635,16 +634,16 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
 		yamlObject.type_ = YamlObject::Type::List;
 		auto &list = yamlObject.list_;
 		auto handler = [this, &list](EventPtr evt) {
-			list.emplace_back(new YamlObject());
+			list.emplace_back(std::string{}, std::make_unique<YamlObject>());
-			return parseNextYamlObject(*list.back(), std::move(evt));
+			return parseNextYamlObject(*list.back().value, std::move(evt));
 		};
 		return parseDictionaryOrList(YamlObject::Type::List, handler);
 	}
 
 	case YAML_MAPPING_START_EVENT: {
 		yamlObject.type_ = YamlObject::Type::Dictionary;
-		auto &dictionary = yamlObject.dictionary_;
+		auto &list = yamlObject.list_;
-		auto handler = [this, &dictionary](EventPtr evtKey) {
+		auto handler = [this, &list](EventPtr evtKey) {
 			/* Parse key */
 			if (evtKey->type != YAML_SCALAR_EVENT) {
 				LOG(YamlParser, Error) << "Expect key at line: "
@@ -662,10 +661,19 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
 			if (!evtValue)
 				return -EINVAL;
 
-			auto elem = dictionary.emplace(key, std::make_unique<YamlObject>());
+			auto &elem = list.emplace_back(std::move(key),
-			return parseNextYamlObject(*elem.first->second.get(), std::move(evtValue));
+						       std::make_unique<YamlObject>());
+			return parseNextYamlObject(*elem.value, std::move(evtValue));
 		};
-		return parseDictionaryOrList(YamlObject::Type::Dictionary, handler);
+		int ret = parseDictionaryOrList(YamlObject::Type::Dictionary, handler);
+		if (ret)
+			return ret;
+
+		auto &dictionary = yamlObject.dictionary_;
+		for (const auto &elem : list)
+			dictionary.emplace(elem.key, elem.value.get());
+
+		return 0;
 	}
 
 	default:
@@ -721,6 +729,9 @@ int YamlParserContext::parseNextYamlObject(YamlObject &yamlObject, EventPtr even
  * The YamlParser::parse() function takes an open FILE, parses its contents, and
  * returns a pointer to a YamlObject corresponding to the root node of the YAML
  * document.
+ *
+ * The parser preserves the order of items in the YAML file, for both lists and
+ * dictionaries.
  */
 
 /**