[{"id":32400,"web_url":"https://patchwork.libcamera.org/comment/32400/","msgid":"<20241127060406.GP5461@pendragon.ideasonboard.com>","date":"2024-11-27T06:04:06","subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:\n> Implement a helpers to allow outputting text in YAML format.\n\ns/helpers/helper/\n\n> \n> The class of helpers allows to create list and dictionaries and emit\n\ns/list/lists/\n\n> scalar values starting from a YamlRoot object initialized with\n> a file path.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/internal/yaml_emitter.h | 164 ++++++\n> src/libcamera/meson.build | 1 +\n> src/libcamera/yaml_emitter.cpp | 577 ++++++++++++++++++++++\n> 4 files changed, 743 insertions(+)\n> create mode 100644 include/libcamera/internal/yaml_emitter.h\n> create mode 100644 src/libcamera/yaml_emitter.cpp\n> \n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 1c5eef9cab80..7533b075fde2 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -41,6 +41,7 @@ libcamera_internal_headers = files([\n> 'v4l2_pixelformat.h',\n> 'v4l2_subdevice.h',\n> 'v4l2_videodevice.h',\n> + 'yaml_emitter.h',\n> 'yaml_parser.h',\n> ])\n> \n> diff --git a/include/libcamera/internal/yaml_emitter.h b/include/libcamera/internal/yaml_emitter.h\n> new file mode 100644\n> index 000000000000..78196a7f147f\n> --- /dev/null\n> +++ b/include/libcamera/internal/yaml_emitter.h\n> @@ -0,0 +1,164 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board Oy\n> + *\n> + * libcamera YAML emitter helper\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class YamlDict;\n> +class YamlEvent;\n> +class YamlList;\n> +class YamlRoot;\n> +class YamlScalar;\n> +\n> +class YamlEmitter final\n> +{\n> +public:\n> +\t~YamlEmitter();\n> +\n> +\tstatic YamlRoot root(const std::string &path);\n> +\n> +private:\n> +\tfriend class YamlOutput;\n> +\tfriend class YamlRoot;\n> +\n> +\tLIBCAMERA_DISABLE_COPY(YamlEmitter)\n> +\n> +\tYamlEmitter(const std::string &path);\n> +\n> +\tvoid logError();\n> +\tvoid init();\n> +\tint emit();\n> +\n> +\tFile file_;\n> +\tyaml_event_t event_;\n> +\tyaml_emitter_t emitter_;\n> +};\n> +\n> +class YamlOutput\n> +{\n> +public:\n> +\tbool valid() const { return !!emitter_; }\n> +\n> +protected:\n> +\tYamlOutput() = default;\n> +\n> +\tYamlOutput(YamlEmitter *emitter)\n> +\t\t: emitter_(emitter)\n> +\t{\n> +\t}\n> +\n> +\tYamlOutput &operator=(YamlOutput &&other)\n> +\t{\n> +\t\temitter_ = other.emitter_;\n> +\t\tother.emitter_ = nullptr;\n> +\n> +\t\treturn *this;\n> +\t}\n> +\n> +\tint emitScalar(std::string_view scalar);\n> +\tint emitMappingStart();\n> +\tint emitMappingEnd();\n> +\tint emitSequenceStart();\n> +\tint emitSequenceEnd();\n> +\n> +\tYamlScalar scalar();\n> +\tYamlDict dict();\n> +\tYamlList list();\n> +\n> +\tYamlEmitter *emitter_ = nullptr;\n> +\tyaml_event_t event_;\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY(YamlOutput)\n> +};\n> +\n> +class YamlRoot : public YamlOutput\n> +{\n> +public:\n> +\tYamlRoot() = default;\n> +\t~YamlRoot();\n> +\n> +\tYamlRoot &operator=(YamlRoot &&other) = default;\n> +\n> +\tYamlList list();\n> +\tYamlDict dict();\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY(YamlRoot);\n> +\n> +\tfriend class YamlEmitter;\n> +\n> +\tYamlRoot(std::unique_ptr emitter)\n> +\t\t: YamlOutput(emitter.get()), emitterRoot_(std::move(emitter))\n> +\t{\n> +\t}\n> +\n> +\tstd::unique_ptr emitterRoot_;\n> +};\n> +\n> +class YamlScalar : public YamlOutput\n> +{\n> +public:\n> +\tYamlScalar() = default;\n> +\t~YamlScalar() = default;\n> +\n> +\tvoid operator=(std::string_view scalar);\n> +\n> +private:\n> +\tfriend class YamlOutput;\n> +\n> +\tYamlScalar(YamlEmitter *emitter);\n> +};\n> +\n> +class YamlList : public YamlOutput\n> +{\n> +public:\n> +\tYamlList() = default;\n> +\t~YamlList();\n> +\n> +\tYamlList &operator=(YamlList &&other) = default;\n> +\n> +\tYamlList list();\n> +\tYamlDict dict();\n> +\tvoid scalar(std::string_view scalar);\n> +\n> +private:\n> +\tfriend class YamlOutput;\n> +\n> +\tYamlList(YamlEmitter *emitter);\n> +};\n> +\n> +class YamlDict : public YamlOutput\n> +{\n> +public:\n> +\tYamlDict() = default;\n> +\t~YamlDict();\n> +\n> +\tYamlDict &operator=(YamlDict &&other) = default;\n> +\n> +\tYamlList list(std::string_view key);\n> +\tYamlDict dict(std::string_view key);\n> +\n> +\tYamlScalar operator[](std::string_view key);\n> +\n> +private:\n> +\tfriend class YamlOutput;\n> +\n> +\tYamlDict(YamlEmitter *emitter);\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index aa9ab0291854..5b8af4103085 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -51,6 +51,7 @@ libcamera_internal_sources = files([\n> 'v4l2_pixelformat.cpp',\n> 'v4l2_subdevice.cpp',\n> 'v4l2_videodevice.cpp',\n> + 'yaml_emitter.cpp',\n> 'yaml_parser.cpp',\n> ])\n> \n> diff --git a/src/libcamera/yaml_emitter.cpp b/src/libcamera/yaml_emitter.cpp\n> new file mode 100644\n> index 000000000000..fa3de91ce988\n> --- /dev/null\n> +++ b/src/libcamera/yaml_emitter.cpp\n> @@ -0,0 +1,577 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board Oy\n> + *\n> + * libcamera YAML emitter helper\n> + */\n> +\n> +#include \"libcamera/internal/yaml_emitter.h\"\n> +\n> +#include \n\nMissing\n\n#include \n\nYou should also include errno.h.\n\n> +\n> +/**\n> + * \\file yaml_emitter.h\n> + * \\brief A YAML emitter helper\n> + *\n> + * The YAML emitter helpers allow users to emit output in YAML format.\n\nI would outline here the key difference with a YAML writer.\n\n * Unlike YAML writers that operate on a fully populated representation of the\n * data, the YAML emitter outputs YAML data on the fly. It is suitable for\n * outputting large amount of data with low overhead and no runtime heap\n * allocation.\n\n> + *\n> + * To emit YAML users of the these helper classes create a root node with\n> + *\n> + * \\code\n> +\tstd::string filePath(\"...\");\n> +\tauto root = YamlEmitter::root(filePath);\n> + \\endcode\n> + *\n> + * and start populating it with dictionaries and lists with the YamlRoot::dict()\n> + * and YamlRoot::list() functions.\n\nI would write \"and start emitting dictionaries and lists\", \"populating\"\nsounds more like a YAML writer.\n\n> + *\n> + * The classes part of this file implement RAII-style handling of YAML\n> + * events. By creating a YamlList and YamlDict instance the associated YAML\n> + * sequence start and mapping start events are emitted and once the instances\n> + * gets destroyed the corresponding sequence end and mapping end events are\n> + * emitted.\n> + *\n> + * From an initialized YamlRoot instance is possible to create YAML list and\n> + * dictionaries.\n> + *\n> + * \\code\n> +\tYamlDict dict = root.dict();\n> +\tYamlList list = root.list();\n> + \\endcode\n> + *\n> + * YamlDict instances can be populated with scalars associated with a key\n> + *\n> + * \\code\n> +\tdict[\"key\"] = \"value\";\n> + \\endcode\n> + *\n> + * and it is possible to create lists and dictionaries, associated with a key\n> + *\n> + * \\code\n> +\tYamlDict subDict = dict.dict(\"newDict\");\n> +\tYamlList subList = dict.list(\"newList\");\n> + \\endcode\n> + *\n> + * YamlList instances can be populated with scalar elements\n> + *\n> + * \\code\n> +\tlist.scalar(\"x\");\n> +\tlist.scalar(\"y\");\n> + \\endcode\n> + *\n> + * and with dictionaries and lists too\n> + *\n> + * \\code\n> +\tYamlDict subDict = list.dict();\n> +\tYamlList subList = list.list();\n> + \\endcode\n\nThe biggest issue with the API is that it can be easily misused:\n\n\tYamlDict list1 = dict.list(\"foo\");\n\tYamlDict list2 = dict.list(\"bar\");\n\tlist1.scalar(\"0\");\n\nI'm still annoyed that I haven't found a neat way to prevent this. I\nwonder if we could make it a bit safer by at least catching incorrect\nusage at runtime. What I'm thinking is\n\n- When creating a child, recording the child pointer in the parent, and\n the parent pointer in the child\n- If the parent already has a child when a new child is created, reset\n the parent pointer of the previous child. This makes the previous\n child invalid.\n- Any invalidation of a child needs to be propagated to its children.\n- Any operation on an invalid child would be caught and logged.\n- When a child is destroyed, if its parent is not null, reset the child\n pointer in the parent to null.\n- If a parent is destroyed while it has a child pointer, reset the\n child's parent pointer to null and log an error.\n\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(YamlEmitter)\n> +\n> +namespace {\n> +\n> +int yamlWrite(void *data, unsigned char *buffer, size_t size)\n> +{\n> +\tFile *file = static_cast(data);\n> +\n> +\tSpan buf{ buffer, size };\n> +\tssize_t ret = file->write(buf);\n> +\tif (ret < 0) {\n> +\t\tLOG(YamlEmitter, Error) << \"Write error : \" << strerror(ret);\n\ns/error :/error:/\n\n> +\t\treturn 0;\n> +\t}\n> +\n> +\treturn 1;\n> +}\n> +\n> +} /* namespace */\n> +\n> +/**\n> + * \\class YamlEmitter\n> + *\n> + * YAML helper classes entry point. This class allows to create a YamlRoot\n> + * instances, using the YamlEmitter::root() function, that users can populate\n\ns/instances/instance/\n\n> + * with lists, dictionaries and scalars.\n> + */\n> +\n> +YamlEmitter::YamlEmitter(const std::string &path)\n> +{\n> +\tstd::string filePath(path);\n> +\tfile_.setFileName(filePath);\n\nI don't think you need the local variable, you can write\n\n\tfile_.setFileName(path);\n\n> +\tfile_.open(File::OpenModeFlag::WriteOnly);\n> +}\n> +\n> +/**\n> + * \\brief Destroy the YamlEmitter\n> + */\n\nI think you can drop this, the documentation doesn't bring much, and the\nfunction isn't part of the API exposed to users anyway.\n\n> +YamlEmitter::~YamlEmitter()\n> +{\n> +\tyaml_event_delete(&event_);\n> +\tyaml_emitter_delete(&emitter_);\n> +}\n> +\n> +/**\n> + * \\brief Create an initialized instance of YamlRoot\n> + * \\param[in] path The YAML output file path\n> + *\n> + * Create an initialized instance of the YamlRoot class that users can start\n> + * using and populating with scalers, lists and dictionaries.\n\ns/scalers/scalars/\n\n> + *\n> + * \\return An initialized YamlRoot instance\n> + */\n> +YamlRoot YamlEmitter::root(const std::string &path)\n\nI asked in the review of the preview version if we should pass a\nstd::ostream to make this more generic. I'm fine ignoring it for now, as\nit's unclear if we would have use cases for that.\n\n> +{\n> +\tstd::unique_ptr emitter{ new YamlEmitter(path) };\n> +\n> +\temitter->init();\n> +\n> +\treturn YamlRoot(std::move(emitter));\n> +}\n> +\n> +void YamlEmitter::logError()\n> +{\n> +\tswitch (emitter_.error) {\n> +\tcase YAML_MEMORY_ERROR:\n> +\t\tLOG(YamlEmitter, Error)\n> +\t\t\t<< \"Memory error: Not enough memory for emitting\";\n> +\t\tbreak;\n> +\n> +\tcase YAML_WRITER_ERROR:\n> +\t\tLOG(YamlEmitter, Error)\n> +\t\t\t<< \"Writer error: \" << emitter_.problem;\n> +\t\tbreak;\n> +\n> +\tcase YAML_EMITTER_ERROR:\n> +\t\tLOG(YamlEmitter, Error)\n> +\t\t\t<< \"Emitter error: \" << emitter_.problem;\n> +\t\tbreak;\n> +\n> +\tdefault:\n> +\t\tLOG(YamlEmitter, Error) << \"Internal problem\";\n\n\"Internal error\" ?\n\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> +void YamlEmitter::init()\n> +{\n> +\tyaml_emitter_initialize(&emitter_);\n> +\tyaml_emitter_set_output(&emitter_, yamlWrite, &file_);\n> +\n> +\tyaml_stream_start_event_initialize(&event_, YAML_UTF8_ENCODING);\n> +\temit();\n> +\n> +\tyaml_document_start_event_initialize(&event_, NULL, NULL, NULL, 0);\n\nnullptr ? Same elsewhere.\n\n> +\temit();\n> +}\n> +\n> +int YamlEmitter::emit()\n> +{\n> +\tint ret = yaml_emitter_emit(&emitter_, &event_);\n> +\tif (!ret) {\n> +\t\tlogError();\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\class YamlOutput\n> + *\n> + * The YamlOutput base class. From this class are derived the YamlScalar,\n> + * YamlList and YamlDict classes which are meant to be used by users of the\n> + * YAML emitter helpers.\n> + *\n> + * The YamlOutput base class provides functions to create YAML lists and\n> + * dictionaries and to populate them.\n> + *\n> + * Instances of this class cannot be instantiated directly by applications.\n\ns/Instances of this/This/\ns/ by applications// as none of this is exposed to applications :-)\n\n> + */\n> +\n> +/**\n> + * \\fn YamlOutput::valid()\n> + * \\brief Check if a YamlOutput instance has been correctly initialized\n> + * \\return True if the instance has been initialized, false otherwise\n> + */\n\nWe usually document constructors first.\n\n> +\n> +/**\n> + * \\fn YamlOutput::YamlOutput(YamlEmitter *emitter)\n> + * \\brief Create a YamlOutput instance with an associated emitter\n> + * \\param[in] emitter The YAML emitter\n> + */\n> +\n> +/**\n> + * \\fn YamlOutput &YamlOutput::operator=(YamlOutput &&other)\n> + * \\brief The move-assignment operator\n> + * \\param[in] other The instance to be moved\n> + */\n> +\n> +/**\n> + * \\brief Emit \\a scalar as a YAML scalar\n> + * \\param[in] scalar The element to emit\n> + * \\return 0 in case of success, a negative error value otherwise\n> + */\n> +int YamlOutput::emitScalar(std::string_view scalar)\n> +{\n> +\tif (!valid())\n> +\t\treturn -EINVAL;\n> +\n> +\tconst yaml_char_t *value = reinterpret_cast\n> +\t\t\t\t (scalar.data());\n> +\tyaml_scalar_event_initialize(&emitter_->event_, NULL, NULL, value,\n> +\t\t\t\t scalar.length(), true, false,\n> +\t\t\t\t YAML_PLAIN_SCALAR_STYLE);\n> +\treturn emitter_->emit();\n> +}\n> +\n> +/**\n> + * \\brief Emit the mapping start YAML event\n> + * \\return 0 in case of success, a negative error value otherwise\n> + */\n> +int YamlOutput::emitMappingStart()\n> +{\n> +\tif (!valid())\n> +\t\treturn -EINVAL;\n> +\n> +\tyaml_mapping_start_event_initialize(&emitter_->event_, NULL, NULL,\n> +\t\t\t\t\t true, YAML_BLOCK_MAPPING_STYLE);\n> +\treturn emitter_->emit();\n> +}\n> +\n> +/**\n> + * \\brief Emit the mapping end YAML event\n> + * \\return 0 in case of success, a negative error value otherwise\n> + */\n> +int YamlOutput::emitMappingEnd()\n> +{\n> +\tif (!valid())\n> +\t\treturn -EINVAL;\n> +\n> +\tyaml_mapping_end_event_initialize(&emitter_->event_);\n> +\treturn emitter_->emit();\n> +}\n> +\n> +/**\n> + * \\brief Emit the sequence start YAML event\n> + * \\return 0 in case of success, a negative error value otherwise\n> + */\n> +int YamlOutput::emitSequenceStart()\n> +{\n> +\tif (!valid())\n> +\t\treturn -EINVAL;\n> +\n> +\tyaml_sequence_start_event_initialize(&emitter_->event_, NULL, NULL,\n> +\t\t\t\t\t true, YAML_BLOCK_SEQUENCE_STYLE);\n> +\treturn emitter_->emit();\n> +}\n> +\n> +/**\n> + * \\brief Emit the sequence end YAML event\n> + * \\return 0 in case of success, a negative error value otherwise\n> + */\n> +int YamlOutput::emitSequenceEnd()\n> +{\n> +\tif (!valid())\n> +\t\treturn -EINVAL;\n> +\n> +\tyaml_sequence_end_event_initialize(&emitter_->event_);\n> +\treturn emitter_->emit();\n> +}\n> +\n> +/**\n> + * \\brief Create a scalar instance\n> + * \\return An instance of YamlScalar\n> + */\n> +YamlScalar YamlOutput::scalar()\n> +{\n> +\treturn YamlScalar(emitter_);\n> +}\n> +\n> +/**\n> + * \\brief Create a dictionary instance\n> + * \\return An instance of YamlDict\n> + */\n> +YamlDict YamlOutput::dict()\n> +{\n> +\treturn YamlDict(emitter_);\n> +}\n> +\n> +/**\n> + * \\brief Create a list instance\n> + * \\return An instance of YamlList\n> + */\n> +YamlList YamlOutput::list()\n> +{\n> +\treturn YamlList(emitter_);\n> +}\n> +\n> +/**\n> + * \\var YamlOutput::emitter_\n> + * \\brief The emitter used by this YamlObject to output YAML events\n> + */\n> +\n> +/**\n> + * \\var YamlOutput::event_\n> + * \\brief The YAML event used by this YamlObject\n> + */\n> +\n> +/**\n> + * \\class YamlRoot\n> + *\n> + * The YAML root node. A valid YamlRoot instance can only be created using the\n> + * YamlEmitter::root() function. The typical initialization pattern of users of\n> + * this class is similar to the one in the following example:\n> + *\n> + * \\code\n> +\tclass YamlUser\n> +\t{\n> +\tpublic:\n> +\t\tYamlUser();\n> +\n> +\tprivate:\n> +\t\tYamlRool root_;\n> +\t};\n> +\n> +\tYamlUser::YamlUser()\n> +\t{\n> +\t\troot_ = YamlEmitter::root(\"/path/to/yaml/file.yml\");\n> +\t}\n> + \\endcode\n> + *\n> + * A YamlRoot element can be populated with list and dictionaries.\n> + */\n> +\n> +/**\n> + * \\fn YamlRoot::YamlRoot()\n> + * \\brief Construct a YamlRoot instance without initializing it\n> + *\n> + * A YamlRoot instance can be created in non-initialized state typically to be\n> + * stored as a class member by the users of this class. In order to start using\n> + * and populating the YamlRoot instance a valid and initialized instance,\n> + * created using the YamlEmitter::root() function, has to be move-assigned to\n> + * the non-initialized instance.\n> + *\n> + * \\code\n> +\tYamlRoot root;\n> +\n> +\troot = YamlEmitter::root(\"/path/to/yaml/file.yml\");\n> + \\endcode\n> + */\n> +\n> +/**\n> + * \\brief Delete a YamlRoot\n> + */\n> +YamlRoot::~YamlRoot()\n> +{\n> +\tif (!valid())\n> +\t\treturn;\n> +\n> +\tyaml_document_end_event_initialize(&emitter_->event_, 0);\n> +\temitterRoot_->emit();\n> +\n> +\tyaml_stream_end_event_initialize(&emitter_->event_);\n> +\temitterRoot_->emit();\n> +}\n> +\n> +/**\n> + * \\fn YamlRoot &YamlRoot::operator=(YamlRoot &&other)\n> + * \\brief Move assignment operator\n> + *\n> + * Move-assign a YamlRoot instance. This function is typically used to assign an\n> + * initialized instance returned by YamlEmitter::root() to a non-initialized\n> + * one.\n> + *\n> + * \\return A reference to this instance of YamlRoot\n> + */\n> +\n> +/**\n> + * \\copydoc YamlOutput::dict()\n> + */\n> +YamlDict YamlRoot::dict()\n> +{\n> +\tint ret = emitMappingStart();\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\treturn YamlOutput::dict();\n> +}\n> +\n> +/**\n> + * \\copydoc YamlOutput::list()\n> + */\n> +YamlList YamlRoot::list()\n> +{\n> +\tint ret = emitSequenceStart();\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\treturn YamlOutput::list();\n> +}\n> +\n> +/**\n> + * \\class YamlScalar\n> + *\n> + * A YamlScalar can be assigned to an std::string_view to emit them as YAML\n> + * elements.\n> + */\n> +\n> +/**\n> + * \\brief Create a YamlScalar instance\n> + */\n> +YamlScalar::YamlScalar(YamlEmitter *emitter)\n> +\t: YamlOutput(emitter)\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Emit \\a scalar as a YAML scalar\n> + * \\param[in] scalar The element to emit in the YAML output\n> + */\n> +void YamlScalar::operator=(std::string_view scalar)\n> +{\n> +\temitScalar(scalar);\n> +}\n> +\n> +/**\n> + * \\class YamlList\n> + *\n> + * A YamlList can be populated with scalars and allows to create nested lists\n> + * and dictionaries.\n> + */\n> +\n> +/**\n> + * \\brief Create a YamlList\n> + */\n> +YamlList::YamlList(YamlEmitter *emitter)\n> +\t: YamlOutput(emitter)\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Destroy a YamlList instance\n> + */\n> +YamlList::~YamlList()\n> +{\n> +\temitSequenceEnd();\n> +}\n> +\n> +/**\n> + * \\fn YamlList &YamlList::operator=(YamlList &&other)\n> + * \\brief Move-assignment operator\n> + * \\param[inout] other The instance to move\n> + */\n> +\n> +/**\n> + * \\brief Append \\a scalar to the list\n> + * \\param[in] scalar The element to append to the list\n> + */\n> +void YamlList::scalar(std::string_view scalar)\n> +{\n> +\temitScalar(scalar);\n> +}\n> +\n> +/**\n> + * \\copydoc YamlOutput::list()\n> + */\n> +YamlList YamlList::list()\n> +{\n> +\tint ret = emitSequenceStart();\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\treturn YamlOutput::list();\n> +}\n> +\n> +/**\n> + * \\copydoc YamlOutput::dict()\n> + */\n> +YamlDict YamlList::dict()\n> +{\n> +\tint ret = emitMappingStart();\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\treturn YamlOutput::dict();\n> +}\n> +\n> +/**\n> + * \\class YamlDict\n> + *\n> + * A YamlDict can be populated with scalars using operator[] and allows to\n> + * create other lists and dictionaries associated with a key.\n> + */\n> +\n> +/**\n> + * \\fn YamlDict::YamlDict()\n> + * \\brief Create a non-initialized instance of a YamlDict\n> + */\n> +\n> +YamlDict::YamlDict(YamlEmitter *emitter)\n> +\t: YamlOutput(emitter)\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Destroy a YamlDict instance\n> + */\n> +YamlDict::~YamlDict()\n> +{\n> +\temitMappingEnd();\n> +}\n> +\n> +/**\n> + * \\fn YamlDict &YamlDict::operator=(YamlDict &&other)\n> + * \\brief Move-assignment operator\n> + * \\param[inout] other The instance to move\n\nI think we usually use param[in] for move constructors and assignment\noperators. Granted, it's a bit borderline.\n\n> + */\n> +\n> +/**\n> + * \\copydoc YamlOutput::list()\n\nThat won't document the key parameter.\n\n> + */\n> +YamlList YamlDict::list(std::string_view key)\n> +{\n> +\tint ret = emitScalar(key);\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\tret = emitSequenceStart();\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\treturn YamlOutput::list();\n> +}\n> +\n> +/**\n> + * \\copydoc YamlOutput::dict()\n\nSame here.\n\n> + */\n> +YamlDict YamlDict::dict(std::string_view key)\n> +{\n> +\tint ret = emitScalar(key);\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\tret = emitMappingStart();\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\treturn YamlOutput::dict();\n> +}\n> +\n> +/**\n> + * \\brief Create a scalar associated with \\a key in the dictionary\n> + * \\param[in] key The key associated with the newly created scalar\n> + * \\return A YamlScalar that application can use to output text\n> + */\n> +YamlScalar YamlDict::operator[](std::string_view key)\n\nI'm still not entirely thrilled by this, for a few reasons. None are\nshowstoppers, you can decide what to do.\n\nFirst there's an asymmetry in the API. To emit scalars on lists you have\na scalar() function, while here you use the [] operator. Similarly, in\nthe YamlDict class, you have explicitly named functions to emit a list\nor dictionary, but not for scalars. Now asymmetries are not always bad,\nsometimes specific APIs are best for specific jobs.\n\nAnother API design point that bothers me possibly a bit more is the\ndifference in behaviour between YamlDict::operator[]() and\nstd::map::operator[](). Using the [] operators, users could expect that\n\n\tdict[\"foo\"] = \"bar\";\n\tdict[\"foo\"] = \"baz\";\n\nwill result in the second statement overwriting the first one. The\nYamlDict::operator[]() API is a bit misleading. As it's an internal\nclass it's less of an issue than it would be for the libcamera public\nAPI, but still.\n\nFinally, returning a YamlScalar object opens the door for misuse. The\nfollowing code will compile, but is invalid:\n\n\tYamlScalar scalar = dict[\"foo\"];\n\tdict[\"bar\"] = \"0\";\n\tscalar = \"1\";\n\nUsing an explicit function would solve all this:\n\n\tdict.scalar(\"foo\", \"0\");\n\nThe YamlScalar class could then be moved from the .h file to the .cpp\nfile, and possibly even dropped completely.\n\n> +{\n> +\tint ret = emitScalar(key);\n> +\tif (ret)\n> +\t\treturn {};\n> +\n> +\treturn YamlOutput::scalar();\n> +}\n> +\n> +} /* namespace libcamera */","headers":{"Return-Path":"","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 69559BE08B\n\tfor ;\n\tWed, 27 Nov 2024 06:04:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A5F86607E;\n\tWed, 27 Nov 2024 07:04:19 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6822A65FA0\n\tfor ;\n\tWed, 27 Nov 2024 07:04:17 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 423AD792;\n\tWed, 27 Nov 2024 07:03:54 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"twCPRvb2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732687434;\n\tbh=Vent3QF2dI6Ug3Qu1THIzJGv1jf7pEkZVZNjWSOSLyg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=twCPRvb2fJxGbXUvJ4JXeIrqAEUwt59g3GDJQypXQcJ5P6tTYGX7AJBFWkuylumn7\n\t9mTtLV0kHuPq88i7eCLCbt0AUhqaO14QpxHal2yy1ZDkfZOWPloNtP1Rai8dqOqFN3\n\tQimm+dXp42M07+3zvpPCh3s06l/FeMelRlsjplTI=","Date":"Wed, 27 Nov 2024 08:04:06 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","Message-ID":"<20241127060406.GP5461@pendragon.ideasonboard.com>","References":"<20241106175901.83960-1-jacopo.mondi@ideasonboard.com>\n\t<20241106175901.83960-2-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241106175901.83960-2-jacopo.mondi@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32452,"web_url":"https://patchwork.libcamera.org/comment/32452/","msgid":"","date":"2024-11-29T08:35:06","subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent thanks for review\n\nOn Wed, Nov 27, 2024 at 08:04:06AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:\n> > Implement a helpers to allow outputting text in YAML format.\n>\n> s/helpers/helper/\n>\n> >\n> > The class of helpers allows to create list and dictionaries and emit\n>\n> s/list/lists/\n>\n> > scalar values starting from a YamlRoot object initialized with\n> > a file path.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > include/libcamera/internal/meson.build | 1 +\n> > include/libcamera/internal/yaml_emitter.h | 164 ++++++\n> > src/libcamera/meson.build | 1 +\n> > src/libcamera/yaml_emitter.cpp | 577 ++++++++++++++++++++++\n> > 4 files changed, 743 insertions(+)\n> > create mode 100644 include/libcamera/internal/yaml_emitter.h\n> > create mode 100644 src/libcamera/yaml_emitter.cpp\n> >\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 1c5eef9cab80..7533b075fde2 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -41,6 +41,7 @@ libcamera_internal_headers = files([\n> > 'v4l2_pixelformat.h',\n> > 'v4l2_subdevice.h',\n> > 'v4l2_videodevice.h',\n> > + 'yaml_emitter.h',\n> > 'yaml_parser.h',\n> > ])\n> >\n> > diff --git a/include/libcamera/internal/yaml_emitter.h b/include/libcamera/internal/yaml_emitter.h\n> > new file mode 100644\n> > index 000000000000..78196a7f147f\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/yaml_emitter.h\n> > @@ -0,0 +1,164 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board Oy\n> > + *\n> > + * libcamera YAML emitter helper\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class YamlDict;\n> > +class YamlEvent;\n> > +class YamlList;\n> > +class YamlRoot;\n> > +class YamlScalar;\n> > +\n> > +class YamlEmitter final\n> > +{\n> > +public:\n> > +\t~YamlEmitter();\n> > +\n> > +\tstatic YamlRoot root(const std::string &path);\n> > +\n> > +private:\n> > +\tfriend class YamlOutput;\n> > +\tfriend class YamlRoot;\n> > +\n> > +\tLIBCAMERA_DISABLE_COPY(YamlEmitter)\n> > +\n> > +\tYamlEmitter(const std::string &path);\n> > +\n> > +\tvoid logError();\n> > +\tvoid init();\n> > +\tint emit();\n> > +\n> > +\tFile file_;\n> > +\tyaml_event_t event_;\n> > +\tyaml_emitter_t emitter_;\n> > +};\n> > +\n> > +class YamlOutput\n> > +{\n> > +public:\n> > +\tbool valid() const { return !!emitter_; }\n> > +\n> > +protected:\n> > +\tYamlOutput() = default;\n> > +\n> > +\tYamlOutput(YamlEmitter *emitter)\n> > +\t\t: emitter_(emitter)\n> > +\t{\n> > +\t}\n> > +\n> > +\tYamlOutput &operator=(YamlOutput &&other)\n> > +\t{\n> > +\t\temitter_ = other.emitter_;\n> > +\t\tother.emitter_ = nullptr;\n> > +\n> > +\t\treturn *this;\n> > +\t}\n> > +\n> > +\tint emitScalar(std::string_view scalar);\n> > +\tint emitMappingStart();\n> > +\tint emitMappingEnd();\n> > +\tint emitSequenceStart();\n> > +\tint emitSequenceEnd();\n> > +\n> > +\tYamlScalar scalar();\n> > +\tYamlDict dict();\n> > +\tYamlList list();\n> > +\n> > +\tYamlEmitter *emitter_ = nullptr;\n> > +\tyaml_event_t event_;\n> > +\n> > +private:\n> > +\tLIBCAMERA_DISABLE_COPY(YamlOutput)\n> > +};\n> > +\n> > +class YamlRoot : public YamlOutput\n> > +{\n> > +public:\n> > +\tYamlRoot() = default;\n> > +\t~YamlRoot();\n> > +\n> > +\tYamlRoot &operator=(YamlRoot &&other) = default;\n> > +\n> > +\tYamlList list();\n> > +\tYamlDict dict();\n> > +\n> > +private:\n> > +\tLIBCAMERA_DISABLE_COPY(YamlRoot);\n> > +\n> > +\tfriend class YamlEmitter;\n> > +\n> > +\tYamlRoot(std::unique_ptr emitter)\n> > +\t\t: YamlOutput(emitter.get()), emitterRoot_(std::move(emitter))\n> > +\t{\n> > +\t}\n> > +\n> > +\tstd::unique_ptr emitterRoot_;\n> > +};\n> > +\n> > +class YamlScalar : public YamlOutput\n> > +{\n> > +public:\n> > +\tYamlScalar() = default;\n> > +\t~YamlScalar() = default;\n> > +\n> > +\tvoid operator=(std::string_view scalar);\n> > +\n> > +private:\n> > +\tfriend class YamlOutput;\n> > +\n> > +\tYamlScalar(YamlEmitter *emitter);\n> > +};\n> > +\n> > +class YamlList : public YamlOutput\n> > +{\n> > +public:\n> > +\tYamlList() = default;\n> > +\t~YamlList();\n> > +\n> > +\tYamlList &operator=(YamlList &&other) = default;\n> > +\n> > +\tYamlList list();\n> > +\tYamlDict dict();\n> > +\tvoid scalar(std::string_view scalar);\n> > +\n> > +private:\n> > +\tfriend class YamlOutput;\n> > +\n> > +\tYamlList(YamlEmitter *emitter);\n> > +};\n> > +\n> > +class YamlDict : public YamlOutput\n> > +{\n> > +public:\n> > +\tYamlDict() = default;\n> > +\t~YamlDict();\n> > +\n> > +\tYamlDict &operator=(YamlDict &&other) = default;\n> > +\n> > +\tYamlList list(std::string_view key);\n> > +\tYamlDict dict(std::string_view key);\n> > +\n> > +\tYamlScalar operator[](std::string_view key);\n> > +\n> > +private:\n> > +\tfriend class YamlOutput;\n> > +\n> > +\tYamlDict(YamlEmitter *emitter);\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index aa9ab0291854..5b8af4103085 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -51,6 +51,7 @@ libcamera_internal_sources = files([\n> > 'v4l2_pixelformat.cpp',\n> > 'v4l2_subdevice.cpp',\n> > 'v4l2_videodevice.cpp',\n> > + 'yaml_emitter.cpp',\n> > 'yaml_parser.cpp',\n> > ])\n> >\n> > diff --git a/src/libcamera/yaml_emitter.cpp b/src/libcamera/yaml_emitter.cpp\n> > new file mode 100644\n> > index 000000000000..fa3de91ce988\n> > --- /dev/null\n> > +++ b/src/libcamera/yaml_emitter.cpp\n> > @@ -0,0 +1,577 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board Oy\n> > + *\n> > + * libcamera YAML emitter helper\n> > + */\n> > +\n> > +#include \"libcamera/internal/yaml_emitter.h\"\n> > +\n> > +#include \n>\n> Missing\n>\n> #include \n>\n> You should also include errno.h.\n>\n> > +\n> > +/**\n> > + * \\file yaml_emitter.h\n> > + * \\brief A YAML emitter helper\n> > + *\n> > + * The YAML emitter helpers allow users to emit output in YAML format.\n>\n> I would outline here the key difference with a YAML writer.\n>\n> * Unlike YAML writers that operate on a fully populated representation of the\n> * data, the YAML emitter outputs YAML data on the fly. It is suitable for\n> * outputting large amount of data with low overhead and no runtime heap\n> * allocation.\n>\n\nI like this\n\n> > + *\n> > + * To emit YAML users of the these helper classes create a root node with\n> > + *\n> > + * \\code\n> > +\tstd::string filePath(\"...\");\n> > +\tauto root = YamlEmitter::root(filePath);\n> > + \\endcode\n> > + *\n> > + * and start populating it with dictionaries and lists with the YamlRoot::dict()\n> > + * and YamlRoot::list() functions.\n>\n> I would write \"and start emitting dictionaries and lists\", \"populating\"\n> sounds more like a YAML writer.\n>\nditto\n\n> > + *\n> > + * The classes part of this file implement RAII-style handling of YAML\n> > + * events. By creating a YamlList and YamlDict instance the associated YAML\n> > + * sequence start and mapping start events are emitted and once the instances\n> > + * gets destroyed the corresponding sequence end and mapping end events are\n> > + * emitted.\n> > + *\n> > + * From an initialized YamlRoot instance is possible to create YAML list and\n> > + * dictionaries.\n> > + *\n> > + * \\code\n> > +\tYamlDict dict = root.dict();\n> > +\tYamlList list = root.list();\n> > + \\endcode\n> > + *\n> > + * YamlDict instances can be populated with scalars associated with a key\n> > + *\n> > + * \\code\n> > +\tdict[\"key\"] = \"value\";\n> > + \\endcode\n> > + *\n> > + * and it is possible to create lists and dictionaries, associated with a key\n> > + *\n> > + * \\code\n> > +\tYamlDict subDict = dict.dict(\"newDict\");\n> > +\tYamlList subList = dict.list(\"newList\");\n> > + \\endcode\n> > + *\n> > + * YamlList instances can be populated with scalar elements\n> > + *\n> > + * \\code\n> > +\tlist.scalar(\"x\");\n> > +\tlist.scalar(\"y\");\n> > + \\endcode\n> > + *\n> > + * and with dictionaries and lists too\n> > + *\n> > + * \\code\n> > +\tYamlDict subDict = list.dict();\n> > +\tYamlList subList = list.list();\n> > + \\endcode\n>\n> The biggest issue with the API is that it can be easily misused:\n>\n> \tYamlDict list1 = dict.list(\"foo\");\n> \tYamlDict list2 = dict.list(\"bar\");\n> \tlist1.scalar(\"0\");\n>\n> I'm still annoyed that I haven't found a neat way to prevent this. I\n> wonder if we could make it a bit safer by at least catching incorrect\n> usage at runtime. What I'm thinking is\n\nYes, unfortunately I don't think we can easily catch this at build\ntime\n\n>\n> - When creating a child, recording the child pointer in the parent, and\n> the parent pointer in the child\n> - If the parent already has a child when a new child is created, reset\n> the parent pointer of the previous child. This makes the previous\n> child invalid.\n> - Any invalidation of a child needs to be propagated to its children.\n> - Any operation on an invalid child would be caught and logged.\n> - When a child is destroyed, if its parent is not null, reset the child\n> pointer in the parent to null.\n> - If a parent is destroyed while it has a child pointer, reset the\n> child's parent pointer to null and log an error.\n>\n\nThat should be enough, I wonder if we can make access to an invalid\nchilder a compiler error...\n\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(YamlEmitter)\n> > +\n> > +namespace {\n> > +\n> > +int yamlWrite(void *data, unsigned char *buffer, size_t size)\n> > +{\n> > +\tFile *file = static_cast(data);\n> > +\n> > +\tSpan buf{ buffer, size };\n> > +\tssize_t ret = file->write(buf);\n> > +\tif (ret < 0) {\n> > +\t\tLOG(YamlEmitter, Error) << \"Write error : \" << strerror(ret);\n>\n> s/error :/error:/\n>\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\treturn 1;\n> > +}\n> > +\n> > +} /* namespace */\n> > +\n> > +/**\n> > + * \\class YamlEmitter\n> > + *\n> > + * YAML helper classes entry point. This class allows to create a YamlRoot\n> > + * instances, using the YamlEmitter::root() function, that users can populate\n>\n> s/instances/instance/\n>\n> > + * with lists, dictionaries and scalars.\n> > + */\n> > +\n> > +YamlEmitter::YamlEmitter(const std::string &path)\n> > +{\n> > +\tstd::string filePath(path);\n> > +\tfile_.setFileName(filePath);\n>\n> I don't think you need the local variable, you can write\n>\n> \tfile_.setFileName(path);\n>\n> > +\tfile_.open(File::OpenModeFlag::WriteOnly);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Destroy the YamlEmitter\n> > + */\n>\n> I think you can drop this, the documentation doesn't bring much, and the\n> function isn't part of the API exposed to users anyway.\n>\n\nIsn't it a public function ??\n\n> > +YamlEmitter::~YamlEmitter()\n> > +{\n> > +\tyaml_event_delete(&event_);\n> > +\tyaml_emitter_delete(&emitter_);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create an initialized instance of YamlRoot\n> > + * \\param[in] path The YAML output file path\n> > + *\n> > + * Create an initialized instance of the YamlRoot class that users can start\n> > + * using and populating with scalers, lists and dictionaries.\n>\n> s/scalers/scalars/\n>\n> > + *\n> > + * \\return An initialized YamlRoot instance\n> > + */\n> > +YamlRoot YamlEmitter::root(const std::string &path)\n>\n> I asked in the review of the preview version if we should pass a\n> std::ostream to make this more generic. I'm fine ignoring it for now, as\n> it's unclear if we would have use cases for that.\n>\n\nI might have missed this. I recall I had a string_view most probably\nhere. Let's post-pone it for when we have such a use case.\n\n> > +{\n> > +\tstd::unique_ptr emitter{ new YamlEmitter(path) };\n> > +\n> > +\temitter->init();\n> > +\n> > +\treturn YamlRoot(std::move(emitter));\n> > +}\n> > +\n> > +void YamlEmitter::logError()\n> > +{\n> > +\tswitch (emitter_.error) {\n> > +\tcase YAML_MEMORY_ERROR:\n> > +\t\tLOG(YamlEmitter, Error)\n> > +\t\t\t<< \"Memory error: Not enough memory for emitting\";\n> > +\t\tbreak;\n> > +\n> > +\tcase YAML_WRITER_ERROR:\n> > +\t\tLOG(YamlEmitter, Error)\n> > +\t\t\t<< \"Writer error: \" << emitter_.problem;\n> > +\t\tbreak;\n> > +\n> > +\tcase YAML_EMITTER_ERROR:\n> > +\t\tLOG(YamlEmitter, Error)\n> > +\t\t\t<< \"Emitter error: \" << emitter_.problem;\n> > +\t\tbreak;\n> > +\n> > +\tdefault:\n> > +\t\tLOG(YamlEmitter, Error) << \"Internal problem\";\n>\n> \"Internal error\" ?\n>\n\nack\n\n> > +\t\tbreak;\n> > +\t}\n> > +}\n> > +\n> > +void YamlEmitter::init()\n> > +{\n> > +\tyaml_emitter_initialize(&emitter_);\n> > +\tyaml_emitter_set_output(&emitter_, yamlWrite, &file_);\n> > +\n> > +\tyaml_stream_start_event_initialize(&event_, YAML_UTF8_ENCODING);\n> > +\temit();\n> > +\n> > +\tyaml_document_start_event_initialize(&event_, NULL, NULL, NULL, 0);\n>\n> nullptr ? Same elsewhere.\n>\n> > +\temit();\n> > +}\n> > +\n> > +int YamlEmitter::emit()\n> > +{\n> > +\tint ret = yaml_emitter_emit(&emitter_, &event_);\n> > +\tif (!ret) {\n> > +\t\tlogError();\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlOutput\n> > + *\n> > + * The YamlOutput base class. From this class are derived the YamlScalar,\n> > + * YamlList and YamlDict classes which are meant to be used by users of the\n> > + * YAML emitter helpers.\n> > + *\n> > + * The YamlOutput base class provides functions to create YAML lists and\n> > + * dictionaries and to populate them.\n> > + *\n> > + * Instances of this class cannot be instantiated directly by applications.\n>\n> s/Instances of this/This/\n> s/ by applications// as none of this is exposed to applications :-)\n>\n\nYeah, I mean users of this class\n\n> > + */\n> > +\n> > +/**\n> > + * \\fn YamlOutput::valid()\n> > + * \\brief Check if a YamlOutput instance has been correctly initialized\n> > + * \\return True if the instance has been initialized, false otherwise\n> > + */\n>\n> We usually document constructors first.\n>\n> > +\n> > +/**\n> > + * \\fn YamlOutput::YamlOutput(YamlEmitter *emitter)\n> > + * \\brief Create a YamlOutput instance with an associated emitter\n> > + * \\param[in] emitter The YAML emitter\n> > + */\n> > +\n> > +/**\n> > + * \\fn YamlOutput &YamlOutput::operator=(YamlOutput &&other)\n> > + * \\brief The move-assignment operator\n> > + * \\param[in] other The instance to be moved\n> > + */\n> > +\n> > +/**\n> > + * \\brief Emit \\a scalar as a YAML scalar\n> > + * \\param[in] scalar The element to emit\n> > + * \\return 0 in case of success, a negative error value otherwise\n> > + */\n> > +int YamlOutput::emitScalar(std::string_view scalar)\n> > +{\n> > +\tif (!valid())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tconst yaml_char_t *value = reinterpret_cast\n> > +\t\t\t\t (scalar.data());\n> > +\tyaml_scalar_event_initialize(&emitter_->event_, NULL, NULL, value,\n> > +\t\t\t\t scalar.length(), true, false,\n> > +\t\t\t\t YAML_PLAIN_SCALAR_STYLE);\n> > +\treturn emitter_->emit();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Emit the mapping start YAML event\n> > + * \\return 0 in case of success, a negative error value otherwise\n> > + */\n> > +int YamlOutput::emitMappingStart()\n> > +{\n> > +\tif (!valid())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tyaml_mapping_start_event_initialize(&emitter_->event_, NULL, NULL,\n> > +\t\t\t\t\t true, YAML_BLOCK_MAPPING_STYLE);\n> > +\treturn emitter_->emit();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Emit the mapping end YAML event\n> > + * \\return 0 in case of success, a negative error value otherwise\n> > + */\n> > +int YamlOutput::emitMappingEnd()\n> > +{\n> > +\tif (!valid())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tyaml_mapping_end_event_initialize(&emitter_->event_);\n> > +\treturn emitter_->emit();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Emit the sequence start YAML event\n> > + * \\return 0 in case of success, a negative error value otherwise\n> > + */\n> > +int YamlOutput::emitSequenceStart()\n> > +{\n> > +\tif (!valid())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tyaml_sequence_start_event_initialize(&emitter_->event_, NULL, NULL,\n> > +\t\t\t\t\t true, YAML_BLOCK_SEQUENCE_STYLE);\n> > +\treturn emitter_->emit();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Emit the sequence end YAML event\n> > + * \\return 0 in case of success, a negative error value otherwise\n> > + */\n> > +int YamlOutput::emitSequenceEnd()\n> > +{\n> > +\tif (!valid())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tyaml_sequence_end_event_initialize(&emitter_->event_);\n> > +\treturn emitter_->emit();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create a scalar instance\n> > + * \\return An instance of YamlScalar\n> > + */\n> > +YamlScalar YamlOutput::scalar()\n> > +{\n> > +\treturn YamlScalar(emitter_);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create a dictionary instance\n> > + * \\return An instance of YamlDict\n> > + */\n> > +YamlDict YamlOutput::dict()\n> > +{\n> > +\treturn YamlDict(emitter_);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create a list instance\n> > + * \\return An instance of YamlList\n> > + */\n> > +YamlList YamlOutput::list()\n> > +{\n> > +\treturn YamlList(emitter_);\n> > +}\n> > +\n> > +/**\n> > + * \\var YamlOutput::emitter_\n> > + * \\brief The emitter used by this YamlObject to output YAML events\n> > + */\n> > +\n> > +/**\n> > + * \\var YamlOutput::event_\n> > + * \\brief The YAML event used by this YamlObject\n> > + */\n> > +\n> > +/**\n> > + * \\class YamlRoot\n> > + *\n> > + * The YAML root node. A valid YamlRoot instance can only be created using the\n> > + * YamlEmitter::root() function. The typical initialization pattern of users of\n> > + * this class is similar to the one in the following example:\n> > + *\n> > + * \\code\n> > +\tclass YamlUser\n> > +\t{\n> > +\tpublic:\n> > +\t\tYamlUser();\n> > +\n> > +\tprivate:\n> > +\t\tYamlRool root_;\n> > +\t};\n> > +\n> > +\tYamlUser::YamlUser()\n> > +\t{\n> > +\t\troot_ = YamlEmitter::root(\"/path/to/yaml/file.yml\");\n> > +\t}\n> > + \\endcode\n> > + *\n> > + * A YamlRoot element can be populated with list and dictionaries.\n> > + */\n> > +\n> > +/**\n> > + * \\fn YamlRoot::YamlRoot()\n> > + * \\brief Construct a YamlRoot instance without initializing it\n> > + *\n> > + * A YamlRoot instance can be created in non-initialized state typically to be\n> > + * stored as a class member by the users of this class. In order to start using\n> > + * and populating the YamlRoot instance a valid and initialized instance,\n> > + * created using the YamlEmitter::root() function, has to be move-assigned to\n> > + * the non-initialized instance.\n> > + *\n> > + * \\code\n> > +\tYamlRoot root;\n> > +\n> > +\troot = YamlEmitter::root(\"/path/to/yaml/file.yml\");\n> > + \\endcode\n> > + */\n> > +\n> > +/**\n> > + * \\brief Delete a YamlRoot\n> > + */\n> > +YamlRoot::~YamlRoot()\n> > +{\n> > +\tif (!valid())\n> > +\t\treturn;\n> > +\n> > +\tyaml_document_end_event_initialize(&emitter_->event_, 0);\n> > +\temitterRoot_->emit();\n> > +\n> > +\tyaml_stream_end_event_initialize(&emitter_->event_);\n> > +\temitterRoot_->emit();\n> > +}\n> > +\n> > +/**\n> > + * \\fn YamlRoot &YamlRoot::operator=(YamlRoot &&other)\n> > + * \\brief Move assignment operator\n> > + *\n> > + * Move-assign a YamlRoot instance. This function is typically used to assign an\n> > + * initialized instance returned by YamlEmitter::root() to a non-initialized\n> > + * one.\n> > + *\n> > + * \\return A reference to this instance of YamlRoot\n> > + */\n> > +\n> > +/**\n> > + * \\copydoc YamlOutput::dict()\n> > + */\n> > +YamlDict YamlRoot::dict()\n> > +{\n> > +\tint ret = emitMappingStart();\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\treturn YamlOutput::dict();\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc YamlOutput::list()\n> > + */\n> > +YamlList YamlRoot::list()\n> > +{\n> > +\tint ret = emitSequenceStart();\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\treturn YamlOutput::list();\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlScalar\n> > + *\n> > + * A YamlScalar can be assigned to an std::string_view to emit them as YAML\n> > + * elements.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Create a YamlScalar instance\n> > + */\n> > +YamlScalar::YamlScalar(YamlEmitter *emitter)\n> > +\t: YamlOutput(emitter)\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\brief Emit \\a scalar as a YAML scalar\n> > + * \\param[in] scalar The element to emit in the YAML output\n> > + */\n> > +void YamlScalar::operator=(std::string_view scalar)\n> > +{\n> > +\temitScalar(scalar);\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlList\n> > + *\n> > + * A YamlList can be populated with scalars and allows to create nested lists\n> > + * and dictionaries.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Create a YamlList\n> > + */\n> > +YamlList::YamlList(YamlEmitter *emitter)\n> > +\t: YamlOutput(emitter)\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\brief Destroy a YamlList instance\n> > + */\n> > +YamlList::~YamlList()\n> > +{\n> > +\temitSequenceEnd();\n> > +}\n> > +\n> > +/**\n> > + * \\fn YamlList &YamlList::operator=(YamlList &&other)\n> > + * \\brief Move-assignment operator\n> > + * \\param[inout] other The instance to move\n> > + */\n> > +\n> > +/**\n> > + * \\brief Append \\a scalar to the list\n> > + * \\param[in] scalar The element to append to the list\n> > + */\n> > +void YamlList::scalar(std::string_view scalar)\n> > +{\n> > +\temitScalar(scalar);\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc YamlOutput::list()\n> > + */\n> > +YamlList YamlList::list()\n> > +{\n> > +\tint ret = emitSequenceStart();\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\treturn YamlOutput::list();\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc YamlOutput::dict()\n> > + */\n> > +YamlDict YamlList::dict()\n> > +{\n> > +\tint ret = emitMappingStart();\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\treturn YamlOutput::dict();\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlDict\n> > + *\n> > + * A YamlDict can be populated with scalars using operator[] and allows to\n> > + * create other lists and dictionaries associated with a key.\n> > + */\n> > +\n> > +/**\n> > + * \\fn YamlDict::YamlDict()\n> > + * \\brief Create a non-initialized instance of a YamlDict\n> > + */\n> > +\n> > +YamlDict::YamlDict(YamlEmitter *emitter)\n> > +\t: YamlOutput(emitter)\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\brief Destroy a YamlDict instance\n> > + */\n> > +YamlDict::~YamlDict()\n> > +{\n> > +\temitMappingEnd();\n> > +}\n> > +\n> > +/**\n> > + * \\fn YamlDict &YamlDict::operator=(YamlDict &&other)\n> > + * \\brief Move-assignment operator\n> > + * \\param[inout] other The instance to move\n>\n> I think we usually use param[in] for move constructors and assignment\n> operators. Granted, it's a bit borderline.\n>\n\nack\n\n> > + */\n> > +\n> > +/**\n> > + * \\copydoc YamlOutput::list()\n>\n> That won't document the key parameter.\n>\n> > + */\n> > +YamlList YamlDict::list(std::string_view key)\n> > +{\n> > +\tint ret = emitScalar(key);\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\tret = emitSequenceStart();\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\treturn YamlOutput::list();\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc YamlOutput::dict()\n>\n> Same here.\n>\n\nI'll manually copy the documentation\n\n> > + */\n> > +YamlDict YamlDict::dict(std::string_view key)\n> > +{\n> > +\tint ret = emitScalar(key);\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\tret = emitMappingStart();\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\treturn YamlOutput::dict();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create a scalar associated with \\a key in the dictionary\n> > + * \\param[in] key The key associated with the newly created scalar\n> > + * \\return A YamlScalar that application can use to output text\n> > + */\n> > +YamlScalar YamlDict::operator[](std::string_view key)\n>\n> I'm still not entirely thrilled by this, for a few reasons. None are\n> showstoppers, you can decide what to do.\n>\n> First there's an asymmetry in the API. To emit scalars on lists you have\n> a scalar() function, while here you use the [] operator. Similarly, in\n> the YamlDict class, you have explicitly named functions to emit a list\n> or dictionary, but not for scalars. Now asymmetries are not always bad,\n> sometimes specific APIs are best for specific jobs.\n\nTo be hones I find the usage of operator[] to emit a scalar\n(associated with a key) quite neat as it directly connect to the\n'dictionary' API (or a map, as this is C++ and not Python).\n\nYou're right, it's asymmetric, but in lists you can just emit a\nscalar, in dict you always associate it (as well as anything else)\nwith a key. Operator[] makes sure you pass a key in :)\n\n>\n> Another API design point that bothers me possibly a bit more is the\n> difference in behaviour between YamlDict::operator[]() and\n> std::map::operator[](). Using the [] operators, users could expect that\n>\n> \tdict[\"foo\"] = \"bar\";\n> \tdict[\"foo\"] = \"baz\";\n>\n> will result in the second statement overwriting the first one. The\n> YamlDict::operator[]() API is a bit misleading. As it's an internal\n> class it's less of an issue than it would be for the libcamera public\n> API, but still.\n\nAs we don't retain data but emit them as soon as we can, yes, this\nwon't work as std::map.\n\n>\n> Finally, returning a YamlScalar object opens the door for misuse. The\n> following code will compile, but is invalid:\n>\n> \tYamlScalar scalar = dict[\"foo\"];\n> \tdict[\"bar\"] = \"0\";\n> \tscalar = \"1\";\n>\n> Using an explicit function would solve all this:\n>\n> \tdict.scalar(\"foo\", \"0\");\n>\n\nI can indeed use a 'scalar' function if it's safer\n\n> The YamlScalar class could then be moved from the .h file to the .cpp\n> file, and possibly even dropped completely.\n>\n\nyes, users now do not need to manage scalars directly now\n\nThanks\n j\n\n> > +{\n> > +\tint ret = emitScalar(key);\n> > +\tif (ret)\n> > +\t\treturn {};\n> > +\n> > +\treturn YamlOutput::scalar();\n> > +}\n> > +\n> > +} /* namespace libcamera */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","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 C35BAC31E9\n\tfor ;\n\tFri, 29 Nov 2024 08:35:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F0B6065FFA;\n\tFri, 29 Nov 2024 09:35:11 +0100 (CET)","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 DF41860CE6\n\tfor ;\n\tFri, 29 Nov 2024 09:35:09 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:6462:5de2:459e:1ee6:26ea:2d31])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C242A8F;\n\tFri, 29 Nov 2024 09:34:45 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ILv3gdLy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732869285;\n\tbh=2TcNxPh6SdHLOZ25xyUWTpYEsSqCPo6BJr7awOjMrPg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ILv3gdLydZrYiHkarXIYUndvySx6jIlh5I4sRKULdvzUSfokRxt4CtQuJHzYg06fC\n\tZ0DaW8kHVGim1yVvX4getklP1CyePbGJ7mzXJQC1U40ZikUfskbWb+o4tyJmUU6T5u\n\tiOevVOE+k25NSggbDsM7jFGYA9u7mKA77EKL2gOA=","Date":"Fri, 29 Nov 2024 09:35:06 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","Message-ID":"","References":"<20241106175901.83960-1-jacopo.mondi@ideasonboard.com>\n\t<20241106175901.83960-2-jacopo.mondi@ideasonboard.com>\n\t<20241127060406.GP5461@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241127060406.GP5461@pendragon.ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32453,"web_url":"https://patchwork.libcamera.org/comment/32453/","msgid":"<20241129084423.GJ25731@pendragon.ideasonboard.com>","date":"2024-11-29T08:44:23","subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Nov 29, 2024 at 09:35:06AM +0100, Jacopo Mondi wrote:\n> On Wed, Nov 27, 2024 at 08:04:06AM +0200, Laurent Pinchart wrote:\n> > On Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:\n> > > Implement a helpers to allow outputting text in YAML format.\n> >\n> > s/helpers/helper/\n> >\n> > > The class of helpers allows to create list and dictionaries and emit\n> >\n> > s/list/lists/\n> >\n> > > scalar values starting from a YamlRoot object initialized with\n> > > a file path.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > include/libcamera/internal/meson.build | 1 +\n> > > include/libcamera/internal/yaml_emitter.h | 164 ++++++\n> > > src/libcamera/meson.build | 1 +\n> > > src/libcamera/yaml_emitter.cpp | 577 ++++++++++++++++++++++\n> > > 4 files changed, 743 insertions(+)\n> > > create mode 100644 include/libcamera/internal/yaml_emitter.h\n> > > create mode 100644 src/libcamera/yaml_emitter.cpp\n> > >\n> > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > > index 1c5eef9cab80..7533b075fde2 100644\n> > > --- a/include/libcamera/internal/meson.build\n> > > +++ b/include/libcamera/internal/meson.build\n> > > @@ -41,6 +41,7 @@ libcamera_internal_headers = files([\n> > > 'v4l2_pixelformat.h',\n> > > 'v4l2_subdevice.h',\n> > > 'v4l2_videodevice.h',\n> > > + 'yaml_emitter.h',\n> > > 'yaml_parser.h',\n> > > ])\n> > >\n> > > diff --git a/include/libcamera/internal/yaml_emitter.h b/include/libcamera/internal/yaml_emitter.h\n> > > new file mode 100644\n> > > index 000000000000..78196a7f147f\n> > > --- /dev/null\n> > > +++ b/include/libcamera/internal/yaml_emitter.h\n> > > @@ -0,0 +1,164 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2024, Ideas On Board Oy\n> > > + *\n> > > + * libcamera YAML emitter helper\n> > > + */\n> > > +\n> > > +#pragma once\n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +class YamlDict;\n> > > +class YamlEvent;\n> > > +class YamlList;\n> > > +class YamlRoot;\n> > > +class YamlScalar;\n> > > +\n> > > +class YamlEmitter final\n> > > +{\n> > > +public:\n> > > +\t~YamlEmitter();\n> > > +\n> > > +\tstatic YamlRoot root(const std::string &path);\n> > > +\n> > > +private:\n> > > +\tfriend class YamlOutput;\n> > > +\tfriend class YamlRoot;\n> > > +\n> > > +\tLIBCAMERA_DISABLE_COPY(YamlEmitter)\n> > > +\n> > > +\tYamlEmitter(const std::string &path);\n> > > +\n> > > +\tvoid logError();\n> > > +\tvoid init();\n> > > +\tint emit();\n> > > +\n> > > +\tFile file_;\n> > > +\tyaml_event_t event_;\n> > > +\tyaml_emitter_t emitter_;\n> > > +};\n> > > +\n> > > +class YamlOutput\n> > > +{\n> > > +public:\n> > > +\tbool valid() const { return !!emitter_; }\n> > > +\n> > > +protected:\n> > > +\tYamlOutput() = default;\n> > > +\n> > > +\tYamlOutput(YamlEmitter *emitter)\n> > > +\t\t: emitter_(emitter)\n> > > +\t{\n> > > +\t}\n> > > +\n> > > +\tYamlOutput &operator=(YamlOutput &&other)\n> > > +\t{\n> > > +\t\temitter_ = other.emitter_;\n> > > +\t\tother.emitter_ = nullptr;\n> > > +\n> > > +\t\treturn *this;\n> > > +\t}\n> > > +\n> > > +\tint emitScalar(std::string_view scalar);\n> > > +\tint emitMappingStart();\n> > > +\tint emitMappingEnd();\n> > > +\tint emitSequenceStart();\n> > > +\tint emitSequenceEnd();\n> > > +\n> > > +\tYamlScalar scalar();\n> > > +\tYamlDict dict();\n> > > +\tYamlList list();\n> > > +\n> > > +\tYamlEmitter *emitter_ = nullptr;\n> > > +\tyaml_event_t event_;\n> > > +\n> > > +private:\n> > > +\tLIBCAMERA_DISABLE_COPY(YamlOutput)\n> > > +};\n> > > +\n> > > +class YamlRoot : public YamlOutput\n> > > +{\n> > > +public:\n> > > +\tYamlRoot() = default;\n> > > +\t~YamlRoot();\n> > > +\n> > > +\tYamlRoot &operator=(YamlRoot &&other) = default;\n> > > +\n> > > +\tYamlList list();\n> > > +\tYamlDict dict();\n> > > +\n> > > +private:\n> > > +\tLIBCAMERA_DISABLE_COPY(YamlRoot);\n> > > +\n> > > +\tfriend class YamlEmitter;\n> > > +\n> > > +\tYamlRoot(std::unique_ptr emitter)\n> > > +\t\t: YamlOutput(emitter.get()), emitterRoot_(std::move(emitter))\n> > > +\t{\n> > > +\t}\n> > > +\n> > > +\tstd::unique_ptr emitterRoot_;\n> > > +};\n> > > +\n> > > +class YamlScalar : public YamlOutput\n> > > +{\n> > > +public:\n> > > +\tYamlScalar() = default;\n> > > +\t~YamlScalar() = default;\n> > > +\n> > > +\tvoid operator=(std::string_view scalar);\n> > > +\n> > > +private:\n> > > +\tfriend class YamlOutput;\n> > > +\n> > > +\tYamlScalar(YamlEmitter *emitter);\n> > > +};\n> > > +\n> > > +class YamlList : public YamlOutput\n> > > +{\n> > > +public:\n> > > +\tYamlList() = default;\n> > > +\t~YamlList();\n> > > +\n> > > +\tYamlList &operator=(YamlList &&other) = default;\n> > > +\n> > > +\tYamlList list();\n> > > +\tYamlDict dict();\n> > > +\tvoid scalar(std::string_view scalar);\n> > > +\n> > > +private:\n> > > +\tfriend class YamlOutput;\n> > > +\n> > > +\tYamlList(YamlEmitter *emitter);\n> > > +};\n> > > +\n> > > +class YamlDict : public YamlOutput\n> > > +{\n> > > +public:\n> > > +\tYamlDict() = default;\n> > > +\t~YamlDict();\n> > > +\n> > > +\tYamlDict &operator=(YamlDict &&other) = default;\n> > > +\n> > > +\tYamlList list(std::string_view key);\n> > > +\tYamlDict dict(std::string_view key);\n> > > +\n> > > +\tYamlScalar operator[](std::string_view key);\n> > > +\n> > > +private:\n> > > +\tfriend class YamlOutput;\n> > > +\n> > > +\tYamlDict(YamlEmitter *emitter);\n> > > +};\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index aa9ab0291854..5b8af4103085 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -51,6 +51,7 @@ libcamera_internal_sources = files([\n> > > 'v4l2_pixelformat.cpp',\n> > > 'v4l2_subdevice.cpp',\n> > > 'v4l2_videodevice.cpp',\n> > > + 'yaml_emitter.cpp',\n> > > 'yaml_parser.cpp',\n> > > ])\n> > >\n> > > diff --git a/src/libcamera/yaml_emitter.cpp b/src/libcamera/yaml_emitter.cpp\n> > > new file mode 100644\n> > > index 000000000000..fa3de91ce988\n> > > --- /dev/null\n> > > +++ b/src/libcamera/yaml_emitter.cpp\n> > > @@ -0,0 +1,577 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2024, Ideas On Board Oy\n> > > + *\n> > > + * libcamera YAML emitter helper\n> > > + */\n> > > +\n> > > +#include \"libcamera/internal/yaml_emitter.h\"\n> > > +\n> > > +#include \n> >\n> > Missing\n> >\n> > #include \n> >\n> > You should also include errno.h.\n> >\n> > > +\n> > > +/**\n> > > + * \\file yaml_emitter.h\n> > > + * \\brief A YAML emitter helper\n> > > + *\n> > > + * The YAML emitter helpers allow users to emit output in YAML format.\n> >\n> > I would outline here the key difference with a YAML writer.\n> >\n> > * Unlike YAML writers that operate on a fully populated representation of the\n> > * data, the YAML emitter outputs YAML data on the fly. It is suitable for\n> > * outputting large amount of data with low overhead and no runtime heap\n> > * allocation.\n> \n> I like this\n> \n> > > + *\n> > > + * To emit YAML users of the these helper classes create a root node with\n> > > + *\n> > > + * \\code\n> > > +\tstd::string filePath(\"...\");\n> > > +\tauto root = YamlEmitter::root(filePath);\n> > > + \\endcode\n> > > + *\n> > > + * and start populating it with dictionaries and lists with the YamlRoot::dict()\n> > > + * and YamlRoot::list() functions.\n> >\n> > I would write \"and start emitting dictionaries and lists\", \"populating\"\n> > sounds more like a YAML writer.\n> >\n> ditto\n> \n> > > + *\n> > > + * The classes part of this file implement RAII-style handling of YAML\n> > > + * events. By creating a YamlList and YamlDict instance the associated YAML\n> > > + * sequence start and mapping start events are emitted and once the instances\n> > > + * gets destroyed the corresponding sequence end and mapping end events are\n> > > + * emitted.\n> > > + *\n> > > + * From an initialized YamlRoot instance is possible to create YAML list and\n> > > + * dictionaries.\n> > > + *\n> > > + * \\code\n> > > +\tYamlDict dict = root.dict();\n> > > +\tYamlList list = root.list();\n> > > + \\endcode\n> > > + *\n> > > + * YamlDict instances can be populated with scalars associated with a key\n> > > + *\n> > > + * \\code\n> > > +\tdict[\"key\"] = \"value\";\n> > > + \\endcode\n> > > + *\n> > > + * and it is possible to create lists and dictionaries, associated with a key\n> > > + *\n> > > + * \\code\n> > > +\tYamlDict subDict = dict.dict(\"newDict\");\n> > > +\tYamlList subList = dict.list(\"newList\");\n> > > + \\endcode\n> > > + *\n> > > + * YamlList instances can be populated with scalar elements\n> > > + *\n> > > + * \\code\n> > > +\tlist.scalar(\"x\");\n> > > +\tlist.scalar(\"y\");\n> > > + \\endcode\n> > > + *\n> > > + * and with dictionaries and lists too\n> > > + *\n> > > + * \\code\n> > > +\tYamlDict subDict = list.dict();\n> > > +\tYamlList subList = list.list();\n> > > + \\endcode\n> >\n> > The biggest issue with the API is that it can be easily misused:\n> >\n> > \tYamlDict list1 = dict.list(\"foo\");\n> > \tYamlDict list2 = dict.list(\"bar\");\n> > \tlist1.scalar(\"0\");\n> >\n> > I'm still annoyed that I haven't found a neat way to prevent this. I\n> > wonder if we could make it a bit safer by at least catching incorrect\n> > usage at runtime. What I'm thinking is\n> \n> Yes, unfortunately I don't think we can easily catch this at build\n> time\n> \n> > - When creating a child, recording the child pointer in the parent, and\n> > the parent pointer in the child\n> > - If the parent already has a child when a new child is created, reset\n> > the parent pointer of the previous child. This makes the previous\n> > child invalid.\n> > - Any invalidation of a child needs to be propagated to its children.\n> > - Any operation on an invalid child would be caught and logged.\n> > - When a child is destroyed, if its parent is not null, reset the child\n> > pointer in the parent to null.\n> > - If a parent is destroyed while it has a child pointer, reset the\n> > child's parent pointer to null and log an error.\n> >\n> \n> That should be enough, I wonder if we can make access to an invalid\n> childer a compiler error...\n\nI'd love to, but haven't found a nice way to do so :-S\n\n> > > + */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +LOG_DEFINE_CATEGORY(YamlEmitter)\n> > > +\n> > > +namespace {\n> > > +\n> > > +int yamlWrite(void *data, unsigned char *buffer, size_t size)\n> > > +{\n> > > +\tFile *file = static_cast(data);\n> > > +\n> > > +\tSpan buf{ buffer, size };\n> > > +\tssize_t ret = file->write(buf);\n> > > +\tif (ret < 0) {\n> > > +\t\tLOG(YamlEmitter, Error) << \"Write error : \" << strerror(ret);\n> >\n> > s/error :/error:/\n> >\n> > > +\t\treturn 0;\n> > > +\t}\n> > > +\n> > > +\treturn 1;\n> > > +}\n> > > +\n> > > +} /* namespace */\n> > > +\n> > > +/**\n> > > + * \\class YamlEmitter\n> > > + *\n> > > + * YAML helper classes entry point. This class allows to create a YamlRoot\n> > > + * instances, using the YamlEmitter::root() function, that users can populate\n> >\n> > s/instances/instance/\n> >\n> > > + * with lists, dictionaries and scalars.\n> > > + */\n> > > +\n> > > +YamlEmitter::YamlEmitter(const std::string &path)\n> > > +{\n> > > +\tstd::string filePath(path);\n> > > +\tfile_.setFileName(filePath);\n> >\n> > I don't think you need the local variable, you can write\n> >\n> > \tfile_.setFileName(path);\n> >\n> > > +\tfile_.open(File::OpenModeFlag::WriteOnly);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Destroy the YamlEmitter\n> > > + */\n> >\n> > I think you can drop this, the documentation doesn't bring much, and the\n> > function isn't part of the API exposed to users anyway.\n> \n> Isn't it a public function ??\n\nIt is public, but isn't called by users. The YamlEmitter instance is\ncreated internally by YamlEmitter::root(), and stored in the YamlRoot\nclass. The user never interacts with the YamlEmitter instance directly.\n\n> > > +YamlEmitter::~YamlEmitter()\n> > > +{\n> > > +\tyaml_event_delete(&event_);\n> > > +\tyaml_emitter_delete(&emitter_);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Create an initialized instance of YamlRoot\n> > > + * \\param[in] path The YAML output file path\n> > > + *\n> > > + * Create an initialized instance of the YamlRoot class that users can start\n> > > + * using and populating with scalers, lists and dictionaries.\n> >\n> > s/scalers/scalars/\n> >\n> > > + *\n> > > + * \\return An initialized YamlRoot instance\n> > > + */\n> > > +YamlRoot YamlEmitter::root(const std::string &path)\n> >\n> > I asked in the review of the preview version if we should pass a\n> > std::ostream to make this more generic. I'm fine ignoring it for now, as\n> > it's unclear if we would have use cases for that.\n> \n> I might have missed this. I recall I had a string_view most probably\n> here. Let's post-pone it for when we have such a use case.\n> \n> > > +{\n> > > +\tstd::unique_ptr emitter{ new YamlEmitter(path) };\n> > > +\n> > > +\temitter->init();\n> > > +\n> > > +\treturn YamlRoot(std::move(emitter));\n> > > +}\n> > > +\n> > > +void YamlEmitter::logError()\n> > > +{\n> > > +\tswitch (emitter_.error) {\n> > > +\tcase YAML_MEMORY_ERROR:\n> > > +\t\tLOG(YamlEmitter, Error)\n> > > +\t\t\t<< \"Memory error: Not enough memory for emitting\";\n> > > +\t\tbreak;\n> > > +\n> > > +\tcase YAML_WRITER_ERROR:\n> > > +\t\tLOG(YamlEmitter, Error)\n> > > +\t\t\t<< \"Writer error: \" << emitter_.problem;\n> > > +\t\tbreak;\n> > > +\n> > > +\tcase YAML_EMITTER_ERROR:\n> > > +\t\tLOG(YamlEmitter, Error)\n> > > +\t\t\t<< \"Emitter error: \" << emitter_.problem;\n> > > +\t\tbreak;\n> > > +\n> > > +\tdefault:\n> > > +\t\tLOG(YamlEmitter, Error) << \"Internal problem\";\n> >\n> > \"Internal error\" ?\n> \n> ack\n> \n> > > +\t\tbreak;\n> > > +\t}\n> > > +}\n> > > +\n> > > +void YamlEmitter::init()\n> > > +{\n> > > +\tyaml_emitter_initialize(&emitter_);\n> > > +\tyaml_emitter_set_output(&emitter_, yamlWrite, &file_);\n> > > +\n> > > +\tyaml_stream_start_event_initialize(&event_, YAML_UTF8_ENCODING);\n> > > +\temit();\n> > > +\n> > > +\tyaml_document_start_event_initialize(&event_, NULL, NULL, NULL, 0);\n> >\n> > nullptr ? Same elsewhere.\n> >\n> > > +\temit();\n> > > +}\n> > > +\n> > > +int YamlEmitter::emit()\n> > > +{\n> > > +\tint ret = yaml_emitter_emit(&emitter_, &event_);\n> > > +\tif (!ret) {\n> > > +\t\tlogError();\n> > > +\t\treturn -EINVAL;\n> > > +\t}\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlOutput\n> > > + *\n> > > + * The YamlOutput base class. From this class are derived the YamlScalar,\n> > > + * YamlList and YamlDict classes which are meant to be used by users of the\n> > > + * YAML emitter helpers.\n> > > + *\n> > > + * The YamlOutput base class provides functions to create YAML lists and\n> > > + * dictionaries and to populate them.\n> > > + *\n> > > + * Instances of this class cannot be instantiated directly by applications.\n> >\n> > s/Instances of this/This/\n> > s/ by applications// as none of this is exposed to applications :-)\n> \n> Yeah, I mean users of this class\n> \n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn YamlOutput::valid()\n> > > + * \\brief Check if a YamlOutput instance has been correctly initialized\n> > > + * \\return True if the instance has been initialized, false otherwise\n> > > + */\n> >\n> > We usually document constructors first.\n> >\n> > > +\n> > > +/**\n> > > + * \\fn YamlOutput::YamlOutput(YamlEmitter *emitter)\n> > > + * \\brief Create a YamlOutput instance with an associated emitter\n> > > + * \\param[in] emitter The YAML emitter\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn YamlOutput &YamlOutput::operator=(YamlOutput &&other)\n> > > + * \\brief The move-assignment operator\n> > > + * \\param[in] other The instance to be moved\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Emit \\a scalar as a YAML scalar\n> > > + * \\param[in] scalar The element to emit\n> > > + * \\return 0 in case of success, a negative error value otherwise\n> > > + */\n> > > +int YamlOutput::emitScalar(std::string_view scalar)\n> > > +{\n> > > +\tif (!valid())\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\tconst yaml_char_t *value = reinterpret_cast\n> > > +\t\t\t\t (scalar.data());\n> > > +\tyaml_scalar_event_initialize(&emitter_->event_, NULL, NULL, value,\n> > > +\t\t\t\t scalar.length(), true, false,\n> > > +\t\t\t\t YAML_PLAIN_SCALAR_STYLE);\n> > > +\treturn emitter_->emit();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Emit the mapping start YAML event\n> > > + * \\return 0 in case of success, a negative error value otherwise\n> > > + */\n> > > +int YamlOutput::emitMappingStart()\n> > > +{\n> > > +\tif (!valid())\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\tyaml_mapping_start_event_initialize(&emitter_->event_, NULL, NULL,\n> > > +\t\t\t\t\t true, YAML_BLOCK_MAPPING_STYLE);\n> > > +\treturn emitter_->emit();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Emit the mapping end YAML event\n> > > + * \\return 0 in case of success, a negative error value otherwise\n> > > + */\n> > > +int YamlOutput::emitMappingEnd()\n> > > +{\n> > > +\tif (!valid())\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\tyaml_mapping_end_event_initialize(&emitter_->event_);\n> > > +\treturn emitter_->emit();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Emit the sequence start YAML event\n> > > + * \\return 0 in case of success, a negative error value otherwise\n> > > + */\n> > > +int YamlOutput::emitSequenceStart()\n> > > +{\n> > > +\tif (!valid())\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\tyaml_sequence_start_event_initialize(&emitter_->event_, NULL, NULL,\n> > > +\t\t\t\t\t true, YAML_BLOCK_SEQUENCE_STYLE);\n> > > +\treturn emitter_->emit();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Emit the sequence end YAML event\n> > > + * \\return 0 in case of success, a negative error value otherwise\n> > > + */\n> > > +int YamlOutput::emitSequenceEnd()\n> > > +{\n> > > +\tif (!valid())\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\tyaml_sequence_end_event_initialize(&emitter_->event_);\n> > > +\treturn emitter_->emit();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Create a scalar instance\n> > > + * \\return An instance of YamlScalar\n> > > + */\n> > > +YamlScalar YamlOutput::scalar()\n> > > +{\n> > > +\treturn YamlScalar(emitter_);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Create a dictionary instance\n> > > + * \\return An instance of YamlDict\n> > > + */\n> > > +YamlDict YamlOutput::dict()\n> > > +{\n> > > +\treturn YamlDict(emitter_);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Create a list instance\n> > > + * \\return An instance of YamlList\n> > > + */\n> > > +YamlList YamlOutput::list()\n> > > +{\n> > > +\treturn YamlList(emitter_);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\var YamlOutput::emitter_\n> > > + * \\brief The emitter used by this YamlObject to output YAML events\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var YamlOutput::event_\n> > > + * \\brief The YAML event used by this YamlObject\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\class YamlRoot\n> > > + *\n> > > + * The YAML root node. A valid YamlRoot instance can only be created using the\n> > > + * YamlEmitter::root() function. The typical initialization pattern of users of\n> > > + * this class is similar to the one in the following example:\n> > > + *\n> > > + * \\code\n> > > +\tclass YamlUser\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tYamlUser();\n> > > +\n> > > +\tprivate:\n> > > +\t\tYamlRool root_;\n> > > +\t};\n> > > +\n> > > +\tYamlUser::YamlUser()\n> > > +\t{\n> > > +\t\troot_ = YamlEmitter::root(\"/path/to/yaml/file.yml\");\n> > > +\t}\n> > > + \\endcode\n> > > + *\n> > > + * A YamlRoot element can be populated with list and dictionaries.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn YamlRoot::YamlRoot()\n> > > + * \\brief Construct a YamlRoot instance without initializing it\n> > > + *\n> > > + * A YamlRoot instance can be created in non-initialized state typically to be\n> > > + * stored as a class member by the users of this class. In order to start using\n> > > + * and populating the YamlRoot instance a valid and initialized instance,\n> > > + * created using the YamlEmitter::root() function, has to be move-assigned to\n> > > + * the non-initialized instance.\n> > > + *\n> > > + * \\code\n> > > +\tYamlRoot root;\n> > > +\n> > > +\troot = YamlEmitter::root(\"/path/to/yaml/file.yml\");\n> > > + \\endcode\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Delete a YamlRoot\n> > > + */\n> > > +YamlRoot::~YamlRoot()\n> > > +{\n> > > +\tif (!valid())\n> > > +\t\treturn;\n> > > +\n> > > +\tyaml_document_end_event_initialize(&emitter_->event_, 0);\n> > > +\temitterRoot_->emit();\n> > > +\n> > > +\tyaml_stream_end_event_initialize(&emitter_->event_);\n> > > +\temitterRoot_->emit();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\fn YamlRoot &YamlRoot::operator=(YamlRoot &&other)\n> > > + * \\brief Move assignment operator\n> > > + *\n> > > + * Move-assign a YamlRoot instance. This function is typically used to assign an\n> > > + * initialized instance returned by YamlEmitter::root() to a non-initialized\n> > > + * one.\n> > > + *\n> > > + * \\return A reference to this instance of YamlRoot\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\copydoc YamlOutput::dict()\n> > > + */\n> > > +YamlDict YamlRoot::dict()\n> > > +{\n> > > +\tint ret = emitMappingStart();\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\treturn YamlOutput::dict();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc YamlOutput::list()\n> > > + */\n> > > +YamlList YamlRoot::list()\n> > > +{\n> > > +\tint ret = emitSequenceStart();\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\treturn YamlOutput::list();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlScalar\n> > > + *\n> > > + * A YamlScalar can be assigned to an std::string_view to emit them as YAML\n> > > + * elements.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Create a YamlScalar instance\n> > > + */\n> > > +YamlScalar::YamlScalar(YamlEmitter *emitter)\n> > > +\t: YamlOutput(emitter)\n> > > +{\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Emit \\a scalar as a YAML scalar\n> > > + * \\param[in] scalar The element to emit in the YAML output\n> > > + */\n> > > +void YamlScalar::operator=(std::string_view scalar)\n> > > +{\n> > > +\temitScalar(scalar);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlList\n> > > + *\n> > > + * A YamlList can be populated with scalars and allows to create nested lists\n> > > + * and dictionaries.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Create a YamlList\n> > > + */\n> > > +YamlList::YamlList(YamlEmitter *emitter)\n> > > +\t: YamlOutput(emitter)\n> > > +{\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Destroy a YamlList instance\n> > > + */\n> > > +YamlList::~YamlList()\n> > > +{\n> > > +\temitSequenceEnd();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\fn YamlList &YamlList::operator=(YamlList &&other)\n> > > + * \\brief Move-assignment operator\n> > > + * \\param[inout] other The instance to move\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Append \\a scalar to the list\n> > > + * \\param[in] scalar The element to append to the list\n> > > + */\n> > > +void YamlList::scalar(std::string_view scalar)\n> > > +{\n> > > +\temitScalar(scalar);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc YamlOutput::list()\n> > > + */\n> > > +YamlList YamlList::list()\n> > > +{\n> > > +\tint ret = emitSequenceStart();\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\treturn YamlOutput::list();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc YamlOutput::dict()\n> > > + */\n> > > +YamlDict YamlList::dict()\n> > > +{\n> > > +\tint ret = emitMappingStart();\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\treturn YamlOutput::dict();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlDict\n> > > + *\n> > > + * A YamlDict can be populated with scalars using operator[] and allows to\n> > > + * create other lists and dictionaries associated with a key.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn YamlDict::YamlDict()\n> > > + * \\brief Create a non-initialized instance of a YamlDict\n> > > + */\n> > > +\n> > > +YamlDict::YamlDict(YamlEmitter *emitter)\n> > > +\t: YamlOutput(emitter)\n> > > +{\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Destroy a YamlDict instance\n> > > + */\n> > > +YamlDict::~YamlDict()\n> > > +{\n> > > +\temitMappingEnd();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\fn YamlDict &YamlDict::operator=(YamlDict &&other)\n> > > + * \\brief Move-assignment operator\n> > > + * \\param[inout] other The instance to move\n> >\n> > I think we usually use param[in] for move constructors and assignment\n> > operators. Granted, it's a bit borderline.\n> \n> ack\n> \n> > > + */\n> > > +\n> > > +/**\n> > > + * \\copydoc YamlOutput::list()\n> >\n> > That won't document the key parameter.\n> >\n> > > + */\n> > > +YamlList YamlDict::list(std::string_view key)\n> > > +{\n> > > +\tint ret = emitScalar(key);\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\tret = emitSequenceStart();\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\treturn YamlOutput::list();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc YamlOutput::dict()\n> >\n> > Same here.\n> \n> I'll manually copy the documentation\n> \n> > > + */\n> > > +YamlDict YamlDict::dict(std::string_view key)\n> > > +{\n> > > +\tint ret = emitScalar(key);\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\tret = emitMappingStart();\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\treturn YamlOutput::dict();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Create a scalar associated with \\a key in the dictionary\n> > > + * \\param[in] key The key associated with the newly created scalar\n> > > + * \\return A YamlScalar that application can use to output text\n> > > + */\n> > > +YamlScalar YamlDict::operator[](std::string_view key)\n> >\n> > I'm still not entirely thrilled by this, for a few reasons. None are\n> > showstoppers, you can decide what to do.\n> >\n> > First there's an asymmetry in the API. To emit scalars on lists you have\n> > a scalar() function, while here you use the [] operator. Similarly, in\n> > the YamlDict class, you have explicitly named functions to emit a list\n> > or dictionary, but not for scalars. Now asymmetries are not always bad,\n> > sometimes specific APIs are best for specific jobs.\n> \n> To be hones I find the usage of operator[] to emit a scalar\n> (associated with a key) quite neat as it directly connect to the\n> 'dictionary' API (or a map, as this is C++ and not Python).\n> \n> You're right, it's asymmetric, but in lists you can just emit a\n> scalar, in dict you always associate it (as well as anything else)\n> with a key. Operator[] makes sure you pass a key in :)\n> \n> >\n> > Another API design point that bothers me possibly a bit more is the\n> > difference in behaviour between YamlDict::operator[]() and\n> > std::map::operator[](). Using the [] operators, users could expect that\n> >\n> > \tdict[\"foo\"] = \"bar\";\n> > \tdict[\"foo\"] = \"baz\";\n> >\n> > will result in the second statement overwriting the first one. The\n> > YamlDict::operator[]() API is a bit misleading. As it's an internal\n> > class it's less of an issue than it would be for the libcamera public\n> > API, but still.\n> \n> As we don't retain data but emit them as soon as we can, yes, this\n> won't work as std::map.\n> \n> >\n> > Finally, returning a YamlScalar object opens the door for misuse. The\n> > following code will compile, but is invalid:\n> >\n> > \tYamlScalar scalar = dict[\"foo\"];\n> > \tdict[\"bar\"] = \"0\";\n> > \tscalar = \"1\";\n> >\n> > Using an explicit function would solve all this:\n> >\n> > \tdict.scalar(\"foo\", \"0\");\n> >\n> \n> I can indeed use a 'scalar' function if it's safer\n\nI think it would be :-)\n\n> > The YamlScalar class could then be moved from the .h file to the .cpp\n> > file, and possibly even dropped completely.\n> \n> yes, users now do not need to manage scalars directly now\n> \n> > > +{\n> > > +\tint ret = emitScalar(key);\n> > > +\tif (ret)\n> > > +\t\treturn {};\n> > > +\n> > > +\treturn YamlOutput::scalar();\n> > > +}\n> > > +\n> > > +} /* namespace libcamera */","headers":{"Return-Path":"","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 83319C3220\n\tfor ;\n\tFri, 29 Nov 2024 08:44:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6644965FFC;\n\tFri, 29 Nov 2024 09:44:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45BB360CE6\n\tfor ;\n\tFri, 29 Nov 2024 09:44:33 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8C460A8F;\n\tFri, 29 Nov 2024 09:44:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kHcfgAjD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732869848;\n\tbh=GS3Ql7EIFNk71pFdVnf9jacOC3MdhH6YdLp42Jn3QiI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kHcfgAjDLD5lcGCSROFT6IAI1lN7WtU9lK7kJz4NZMPS6rCXd6dQeIM8nZVHqAjpk\n\t/pqvoV1ve6QgQ6wnIf8PmojlMqpUD6l15eRBD1m9e8+xX7mvB2wgHYtwzaHLg2WGmi\n\tlovGsgU/MSlAjWr/PzjCnn+2VE4rN8BlDyUi0u3w=","Date":"Fri, 29 Nov 2024 10:44:23 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","Message-ID":"<20241129084423.GJ25731@pendragon.ideasonboard.com>","References":"<20241106175901.83960-1-jacopo.mondi@ideasonboard.com>\n\t<20241106175901.83960-2-jacopo.mondi@ideasonboard.com>\n\t<20241127060406.GP5461@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32464,"web_url":"https://patchwork.libcamera.org/comment/32464/","msgid":"","date":"2024-11-29T17:28:01","subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. november 29., péntek 09:44 keltezéssel, Laurent Pinchart írta:\n\n> On Fri, Nov 29, 2024 at 09:35:06AM +0100, Jacopo Mondi wrote:\n> > On Wed, Nov 27, 2024 at 08:04:06AM +0200, Laurent Pinchart wrote:\n> > > On Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:\n> > > > Implement a helpers to allow outputting text in YAML format.\n> > > [...]\n> > > > + *\n> > > > + * To emit YAML users of the these helper classes create a root node with\n> > > > + *\n> > > > + * \\code\n> > > > +\tstd::string filePath(\"...\");\n> > > > +\tauto root = YamlEmitter::root(filePath);\n> > > > + \\endcode\n> > > > + *\n> > > > + * and start populating it with dictionaries and lists with the YamlRoot::dict()\n> > > > + * and YamlRoot::list() functions.\n> > >\n> > > I would write \"and start emitting dictionaries and lists\", \"populating\"\n> > > sounds more like a YAML writer.\n> > >\n> > ditto\n> >\n> > > > + *\n> > > > + * The classes part of this file implement RAII-style handling of YAML\n> > > > + * events. By creating a YamlList and YamlDict instance the associated YAML\n> > > > + * sequence start and mapping start events are emitted and once the instances\n> > > > + * gets destroyed the corresponding sequence end and mapping end events are\n> > > > + * emitted.\n> > > > + *\n> > > > + * From an initialized YamlRoot instance is possible to create YAML list and\n> > > > + * dictionaries.\n> > > > + *\n> > > > + * \\code\n> > > > +\tYamlDict dict = root.dict();\n> > > > +\tYamlList list = root.list();\n> > > > + \\endcode\n> > > > + *\n> > > > + * YamlDict instances can be populated with scalars associated with a key\n> > > > + *\n> > > > + * \\code\n> > > > +\tdict[\"key\"] = \"value\";\n> > > > + \\endcode\n> > > > + *\n> > > > + * and it is possible to create lists and dictionaries, associated with a key\n> > > > + *\n> > > > + * \\code\n> > > > +\tYamlDict subDict = dict.dict(\"newDict\");\n> > > > +\tYamlList subList = dict.list(\"newList\");\n> > > > + \\endcode\n> > > > + *\n> > > > + * YamlList instances can be populated with scalar elements\n> > > > + *\n> > > > + * \\code\n> > > > +\tlist.scalar(\"x\");\n> > > > +\tlist.scalar(\"y\");\n> > > > + \\endcode\n> > > > + *\n> > > > + * and with dictionaries and lists too\n> > > > + *\n> > > > + * \\code\n> > > > +\tYamlDict subDict = list.dict();\n> > > > +\tYamlList subList = list.list();\n> > > > + \\endcode\n> > >\n> > > The biggest issue with the API is that it can be easily misused:\n> > >\n> > > \tYamlDict list1 = dict.list(\"foo\");\n> > > \tYamlDict list2 = dict.list(\"bar\");\n> > > \tlist1.scalar(\"0\");\n> > >\n> > > I'm still annoyed that I haven't found a neat way to prevent this. I\n> > > wonder if we could make it a bit safer by at least catching incorrect\n> > > usage at runtime. What I'm thinking is\n> >\n> > Yes, unfortunately I don't think we can easily catch this at build\n> > time\n> >\n> > > - When creating a child, recording the child pointer in the parent, and\n> > > the parent pointer in the child\n> > > - If the parent already has a child when a new child is created, reset\n> > > the parent pointer of the previous child. This makes the previous\n> > > child invalid.\n> > > - Any invalidation of a child needs to be propagated to its children.\n> > > - Any operation on an invalid child would be caught and logged.\n> > > - When a child is destroyed, if its parent is not null, reset the child\n> > > pointer in the parent to null.\n> > > - If a parent is destroyed while it has a child pointer, reset the\n> > > child's parent pointer to null and log an error.\n> > >\n> >\n> > That should be enough, I wonder if we can make access to an invalid\n> > childer a compiler error...\n> \n> I'd love to, but haven't found a nice way to do so :-S\n> [...]\n\nMy guess is that the C++ type system is \"too weak\" in this sense, to implement\nthe kind of checking you envision.\n\nIf one is willing to trade indentation for a smaller chance of misuse, then one could:\n(Or at least I think such an API where the structure of the resulting YAML file\n is in a sense encapsulated in the layout of the source code leads to fewer misuses.)\n\n YamlDict root;\n\n root.dict(\"foo\", [](auto& d) {\n d.list(\"x\", [](auto& l) {\n l.scalar(1);\n l.scalar(true);\n });\n d.scalar(\"y\", 42);\n });\n\n root.list(\"bar\", [](auto&) {\n ...\n });\n\n /* the methods could return an appropriate reference to make some kind of chaining possible: */\n \n YamlDict()\n .dict(\"foo\", [](auto& d) {\n d\n .list(\"x\", [](auto& l) {\n l.scalar(1)\n .scalar(true);\n })\n .scalar(\"y\", 42);\n })\n .list(\"bar\", [](auto&) { ... });\n\nOr one could even go fully in on some kind of chaining:\n\nYamlDict()\n\t.dict(\"foo\")\n\t\t.list(\"x\")\n\t\t\t.scalar(1)\n\t\t\t.scalar(true)\n\t\t.finish() /* end list */\n\t\t.scalar()\n\t.finish() /* end dict */\n\t.list(\"bar\")\n\t\t...\n\n\nI am not sure if this has already been considered, sorry if it has been.\n\nRegards,\nBarnabás Pőcze","headers":{"Return-Path":"","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 701E6BE175\n\tfor ;\n\tFri, 29 Nov 2024 17:28:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF6D166031;\n\tFri, 29 Nov 2024 18:28:08 +0100 (CET)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B9C6E6601A\n\tfor ;\n\tFri, 29 Nov 2024 18:28:07 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"MbUXckjJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1732901287; x=1733160487;\n\tbh=fpp2MmtuYLNGHU+3PNSpRNtXdAdty6/SlNEtVxJ65Eg=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=MbUXckjJC6MSxcr60jpXPvy1MfIrmbLbYkkfeL+Gdnm8mJ8BG4NFo+dNHAPs3+XTK\n\tDKL71h1KAP1h9c/QXBSDamsOUo9qk45gvOw6BtTEXekvMr06j2lAmltuL8C/p7NgxV\n\tbrqcfYq+sORN7q4ffU2q8ys6SvnczSkqSNNBOebZcBEG31FlrDVRU9dl9RGU4RvLhQ\n\tG6KzjsHV15RFPKnYxSGfMha3JipiuY97iPr6dZs+N4+P5heKQ9FXsuo8C7WTymxO4v\n\tBloaBd9ymu/p0tvAu69escfSNOTtcV12JlOskKf+R08jTy5lH+rkfs6bhiUDQ2guBm\n\tSgUEcqTKkzxqQ==","Date":"Fri, 29 Nov 2024 17:28:01 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","Message-ID":"","In-Reply-To":"<20241129084423.GJ25731@pendragon.ideasonboard.com>","References":"<20241106175901.83960-1-jacopo.mondi@ideasonboard.com>\n\t<20241106175901.83960-2-jacopo.mondi@ideasonboard.com>\n\t<20241127060406.GP5461@pendragon.ideasonboard.com>\n\t\n\t<20241129084423.GJ25731@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"1c7e9cb46323f2fefa0dff70aad1caaf46b59cf8","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32466,"web_url":"https://patchwork.libcamera.org/comment/32466/","msgid":"","date":"2024-11-29T18:24:35","subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Fri, Nov 29, 2024 at 05:28:01PM +0000, Barnabás Pőcze wrote:\n> Hi\n>\n>\n> 2024. november 29., péntek 09:44 keltezéssel, Laurent Pinchart írta:\n>\n> > On Fri, Nov 29, 2024 at 09:35:06AM +0100, Jacopo Mondi wrote:\n> > > On Wed, Nov 27, 2024 at 08:04:06AM +0200, Laurent Pinchart wrote:\n> > > > On Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:\n> > > > > Implement a helpers to allow outputting text in YAML format.\n> > > > [...]\n> > > > > + *\n> > > > > + * To emit YAML users of the these helper classes create a root node with\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tstd::string filePath(\"...\");\n> > > > > +\tauto root = YamlEmitter::root(filePath);\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * and start populating it with dictionaries and lists with the YamlRoot::dict()\n> > > > > + * and YamlRoot::list() functions.\n> > > >\n> > > > I would write \"and start emitting dictionaries and lists\", \"populating\"\n> > > > sounds more like a YAML writer.\n> > > >\n> > > ditto\n> > >\n> > > > > + *\n> > > > > + * The classes part of this file implement RAII-style handling of YAML\n> > > > > + * events. By creating a YamlList and YamlDict instance the associated YAML\n> > > > > + * sequence start and mapping start events are emitted and once the instances\n> > > > > + * gets destroyed the corresponding sequence end and mapping end events are\n> > > > > + * emitted.\n> > > > > + *\n> > > > > + * From an initialized YamlRoot instance is possible to create YAML list and\n> > > > > + * dictionaries.\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tYamlDict dict = root.dict();\n> > > > > +\tYamlList list = root.list();\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * YamlDict instances can be populated with scalars associated with a key\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tdict[\"key\"] = \"value\";\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * and it is possible to create lists and dictionaries, associated with a key\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tYamlDict subDict = dict.dict(\"newDict\");\n> > > > > +\tYamlList subList = dict.list(\"newList\");\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * YamlList instances can be populated with scalar elements\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tlist.scalar(\"x\");\n> > > > > +\tlist.scalar(\"y\");\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * and with dictionaries and lists too\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tYamlDict subDict = list.dict();\n> > > > > +\tYamlList subList = list.list();\n> > > > > + \\endcode\n> > > >\n> > > > The biggest issue with the API is that it can be easily misused:\n> > > >\n> > > > \tYamlDict list1 = dict.list(\"foo\");\n> > > > \tYamlDict list2 = dict.list(\"bar\");\n> > > > \tlist1.scalar(\"0\");\n> > > >\n> > > > I'm still annoyed that I haven't found a neat way to prevent this. I\n> > > > wonder if we could make it a bit safer by at least catching incorrect\n> > > > usage at runtime. What I'm thinking is\n> > >\n> > > Yes, unfortunately I don't think we can easily catch this at build\n> > > time\n> > >\n> > > > - When creating a child, recording the child pointer in the parent, and\n> > > > the parent pointer in the child\n> > > > - If the parent already has a child when a new child is created, reset\n> > > > the parent pointer of the previous child. This makes the previous\n> > > > child invalid.\n> > > > - Any invalidation of a child needs to be propagated to its children.\n> > > > - Any operation on an invalid child would be caught and logged.\n> > > > - When a child is destroyed, if its parent is not null, reset the child\n> > > > pointer in the parent to null.\n> > > > - If a parent is destroyed while it has a child pointer, reset the\n> > > > child's parent pointer to null and log an error.\n> > > >\n> > >\n> > > That should be enough, I wonder if we can make access to an invalid\n> > > childer a compiler error...\n> >\n> > I'd love to, but haven't found a nice way to do so :-S\n> > [...]\n>\n> My guess is that the C++ type system is \"too weak\" in this sense, to implement\n> the kind of checking you envision.\n>\n> If one is willing to trade indentation for a smaller chance of misuse, then one could:\n> (Or at least I think such an API where the structure of the resulting YAML file\n> is in a sense encapsulated in the layout of the source code leads to fewer misuses.)\n>\n> YamlDict root;\n>\n> root.dict(\"foo\", [](auto& d) {\n> d.list(\"x\", [](auto& l) {\n> l.scalar(1);\n> l.scalar(true);\n> });\n> d.scalar(\"y\", 42);\n> });\n>\n> root.list(\"bar\", [](auto&) {\n> ...\n> });\n>\n> /* the methods could return an appropriate reference to make some kind of chaining possible: */\n>\n> YamlDict()\n> .dict(\"foo\", [](auto& d) {\n> d\n> .list(\"x\", [](auto& l) {\n> l.scalar(1)\n> .scalar(true);\n> })\n> .scalar(\"y\", 42);\n> })\n> .list(\"bar\", [](auto&) { ... });\n>\n> Or one could even go fully in on some kind of chaining:\n>\n> YamlDict()\n> \t.dict(\"foo\")\n> \t\t.list(\"x\")\n> \t\t\t.scalar(1)\n> \t\t\t.scalar(true)\n> \t\t.finish() /* end list */\n> \t\t.scalar()\n> \t.finish() /* end dict */\n> \t.list(\"bar\")\n> \t\t...\n>\n>\n> I am not sure if this has already been considered, sorry if it has been.\n\nIt hasn't, thanks for the input.\n\nThe problem I see, as you can see in the next patches, is that we need\nto emit yaml, in example, every frame, programmatically. I don't think\nthe above can be constructed iteratively, could it ?\n\n>\n> Regards,\n> Barnabás Pőcze","headers":{"Return-Path":"","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 C05B2BE175\n\tfor ;\n\tFri, 29 Nov 2024 18:24:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E0BFC66031;\n\tFri, 29 Nov 2024 19:24:40 +0100 (CET)","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 6D15D6601A\n\tfor ;\n\tFri, 29 Nov 2024 19:24:39 +0100 (CET)","from ideasonboard.com (mob-5-90-136-201.net.vodafone.it\n\t[5.90.136.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7470A80A;\n\tFri, 29 Nov 2024 19:24:14 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HVWC1WFq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732904654;\n\tbh=BFAYtKe23to1lvX1hhQXxPBVzXQTBBCscZnHZoGL6+8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HVWC1WFqqxfL/qMJ4SWesg4zjgaRV/i5QtgwMpB5XNrK8PA6x8EKdkdgG58vaB6fW\n\tyd4nKMyqABK8JKa8OYGi6Fmc3pMKJD0SJyOMXdyABoDLP8RRlQklpu8zs78cHVqlvV\n\tLa4RdJ5pJveR1wrj6y/BiHP/0rCmVMxJpZ0qvUEs=","Date":"Fri, 29 Nov 2024 19:24:35 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Laurent Pinchart , \n\tJacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","Message-ID":"","References":"<20241106175901.83960-1-jacopo.mondi@ideasonboard.com>\n\t<20241106175901.83960-2-jacopo.mondi@ideasonboard.com>\n\t<20241127060406.GP5461@pendragon.ideasonboard.com>\n\t\n\t<20241129084423.GJ25731@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32473,"web_url":"https://patchwork.libcamera.org/comment/32473/","msgid":"","date":"2024-11-29T22:52:44","subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. november 29., péntek 19:24 keltezéssel, Jacopo Mondi írta:\n\n> Hi Barnabás\n> \n> On Fri, Nov 29, 2024 at 05:28:01PM +0000, Barnabás Pőcze wrote:\n> > Hi\n> >\n> >\n> > 2024. november 29., péntek 09:44 keltezéssel, Laurent Pinchart írta:\n> >\n> > > On Fri, Nov 29, 2024 at 09:35:06AM +0100, Jacopo Mondi wrote:\n> > > > On Wed, Nov 27, 2024 at 08:04:06AM +0200, Laurent Pinchart wrote:\n> > > > > On Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:\n> > > > > > Implement a helpers to allow outputting text in YAML format.\n> > > > > [...]\n> > > > > > + *\n> > > > > > + * To emit YAML users of the these helper classes create a root node with\n> > > > > > + *\n> > > > > > + * \\code\n> > > > > > +\tstd::string filePath(\"...\");\n> > > > > > +\tauto root = YamlEmitter::root(filePath);\n> > > > > > + \\endcode\n> > > > > > + *\n> > > > > > + * and start populating it with dictionaries and lists with the YamlRoot::dict()\n> > > > > > + * and YamlRoot::list() functions.\n> > > > >\n> > > > > I would write \"and start emitting dictionaries and lists\", \"populating\"\n> > > > > sounds more like a YAML writer.\n> > > > >\n> > > > ditto\n> > > >\n> > > > > > + *\n> > > > > > + * The classes part of this file implement RAII-style handling of YAML\n> > > > > > + * events. By creating a YamlList and YamlDict instance the associated YAML\n> > > > > > + * sequence start and mapping start events are emitted and once the instances\n> > > > > > + * gets destroyed the corresponding sequence end and mapping end events are\n> > > > > > + * emitted.\n> > > > > > + *\n> > > > > > + * From an initialized YamlRoot instance is possible to create YAML list and\n> > > > > > + * dictionaries.\n> > > > > > + *\n> > > > > > + * \\code\n> > > > > > +\tYamlDict dict = root.dict();\n> > > > > > +\tYamlList list = root.list();\n> > > > > > + \\endcode\n> > > > > > + *\n> > > > > > + * YamlDict instances can be populated with scalars associated with a key\n> > > > > > + *\n> > > > > > + * \\code\n> > > > > > +\tdict[\"key\"] = \"value\";\n> > > > > > + \\endcode\n> > > > > > + *\n> > > > > > + * and it is possible to create lists and dictionaries, associated with a key\n> > > > > > + *\n> > > > > > + * \\code\n> > > > > > +\tYamlDict subDict = dict.dict(\"newDict\");\n> > > > > > +\tYamlList subList = dict.list(\"newList\");\n> > > > > > + \\endcode\n> > > > > > + *\n> > > > > > + * YamlList instances can be populated with scalar elements\n> > > > > > + *\n> > > > > > + * \\code\n> > > > > > +\tlist.scalar(\"x\");\n> > > > > > +\tlist.scalar(\"y\");\n> > > > > > + \\endcode\n> > > > > > + *\n> > > > > > + * and with dictionaries and lists too\n> > > > > > + *\n> > > > > > + * \\code\n> > > > > > +\tYamlDict subDict = list.dict();\n> > > > > > +\tYamlList subList = list.list();\n> > > > > > + \\endcode\n> > > > >\n> > > > > The biggest issue with the API is that it can be easily misused:\n> > > > >\n> > > > > \tYamlDict list1 = dict.list(\"foo\");\n> > > > > \tYamlDict list2 = dict.list(\"bar\");\n> > > > > \tlist1.scalar(\"0\");\n> > > > >\n> > > > > I'm still annoyed that I haven't found a neat way to prevent this. I\n> > > > > wonder if we could make it a bit safer by at least catching incorrect\n> > > > > usage at runtime. What I'm thinking is\n> > > >\n> > > > Yes, unfortunately I don't think we can easily catch this at build\n> > > > time\n> > > >\n> > > > > - When creating a child, recording the child pointer in the parent, and\n> > > > > the parent pointer in the child\n> > > > > - If the parent already has a child when a new child is created, reset\n> > > > > the parent pointer of the previous child. This makes the previous\n> > > > > child invalid.\n> > > > > - Any invalidation of a child needs to be propagated to its children.\n> > > > > - Any operation on an invalid child would be caught and logged.\n> > > > > - When a child is destroyed, if its parent is not null, reset the child\n> > > > > pointer in the parent to null.\n> > > > > - If a parent is destroyed while it has a child pointer, reset the\n> > > > > child's parent pointer to null and log an error.\n> > > > >\n> > > >\n> > > > That should be enough, I wonder if we can make access to an invalid\n> > > > childer a compiler error...\n> > >\n> > > I'd love to, but haven't found a nice way to do so :-S\n> > > [...]\n> >\n> > My guess is that the C++ type system is \"too weak\" in this sense, to implement\n> > the kind of checking you envision.\n> >\n> > If one is willing to trade indentation for a smaller chance of misuse, then one could:\n> > (Or at least I think such an API where the structure of the resulting YAML file\n> > is in a sense encapsulated in the layout of the source code leads to fewer misuses.)\n> >\n> > YamlDict root;\n> >\n> > root.dict(\"foo\", [](auto& d) {\n> > d.list(\"x\", [](auto& l) {\n> > l.scalar(1);\n> > l.scalar(true);\n> > });\n> > d.scalar(\"y\", 42);\n> > });\n> >\n> > root.list(\"bar\", [](auto&) {\n> > ...\n> > });\n> >\n> > /* the methods could return an appropriate reference to make some kind of chaining possible: */\n> >\n> > YamlDict()\n> > .dict(\"foo\", [](auto& d) {\n> > d\n> > .list(\"x\", [](auto& l) {\n> > l.scalar(1)\n> > .scalar(true);\n> > })\n> > .scalar(\"y\", 42);\n> > })\n> > .list(\"bar\", [](auto&) { ... });\n> >\n> > Or one could even go fully in on some kind of chaining:\n> >\n> > YamlDict()\n> > \t.dict(\"foo\")\n> > \t\t.list(\"x\")\n> > \t\t\t.scalar(1)\n> > \t\t\t.scalar(true)\n> > \t\t.finish() /* end list */\n> > \t\t.scalar()\n> > \t.finish() /* end dict */\n> > \t.list(\"bar\")\n> > \t\t...\n> >\n> >\n> > I am not sure if this has already been considered, sorry if it has been.\n> \n> It hasn't, thanks for the input.\n> \n> The problem I see, as you can see in the next patches, is that we need\n> to emit yaml, in example, every frame, programmatically. I don't think\n> the above can be constructed iteratively, could it ?\n> [...]\n\nAhh, sorry, you're right. One item is added to a list in every call of `dumpRequest()`.\nIn this case there does not seem to be a good way to use the type system to\nenforce much of anything without runtime checking.\n\nYAML allows multiple documents in a stream, so theoretically every request could\nbe described by its own separate document, which would avoid the need for keeping\nunterminated YAML structures around between calls. But I am not sure what kind\nof challenges that would bring (especially on the parsing side), and if it is\nworth it.\n\n\nRegards,\nBarnabás Pőcze","headers":{"Return-Path":"","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 20CEFC3237\n\tfor ;\n\tFri, 29 Nov 2024 22:52:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 116DD66041;\n\tFri, 29 Nov 2024 23:52:50 +0100 (CET)","from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A5E1666025\n\tfor ;\n\tFri, 29 Nov 2024 23:52:48 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"lzIszXGC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1732920768; x=1733179968;\n\tbh=yeqr/WjSUa2wyvZGYW9/op/lWbdTECcs6RqUihgK4A8=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=lzIszXGCT1H0rx8GLMZjsdcMPBNZWL8MbFBk9v3OHVOYXwVWJ+IWumz1RGJDAkttL\n\tV7yUDGB+9htYhOfZpQ8wKmdF+BvB2NoG7zwG9h/3U4KxlCdiSvRQyxOIWcCmxcDTAG\n\t+dcWhI+go7XP9jq3E0zH9x5ohUpOrq79WoDpUTkT2DQlfbVxZDyy7SB8/0eos1GOWL\n\tAGb8AUSSwr2o2AjNhe/fc1gwa0XKAWg+2JBllz+Mibz1BY517/+aMCO6thOwpXW9DP\n\tWA2ph8vBVCYq0Jepa1FHRj4acGs9jFyBBzPU4q7ITQaTltDXgZlCm6IrPjBp1yYZVj\n\tdj3yd1XHsfa5A==","Date":"Fri, 29 Nov 2024 22:52:44 +0000","To":"Jacopo Mondi ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","Message-ID":"","In-Reply-To":"","References":"<20241106175901.83960-1-jacopo.mondi@ideasonboard.com>\n\t<20241106175901.83960-2-jacopo.mondi@ideasonboard.com>\n\t<20241127060406.GP5461@pendragon.ideasonboard.com>\n\t\n\t<20241129084423.GJ25731@pendragon.ideasonboard.com>\n\t\n\t","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"638ce7e3d326612dc78b6d0c07e39e5aae8a8bb9","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32479,"web_url":"https://patchwork.libcamera.org/comment/32479/","msgid":"<20241130170138.GE2652@pendragon.ideasonboard.com>","date":"2024-11-30T17:01:38","subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Nov 29, 2024 at 05:28:01PM +0000, Barnabás Pőcze wrote:\n> 2024. november 29., péntek 09:44 keltezéssel, Laurent Pinchart írta:\n> > On Fri, Nov 29, 2024 at 09:35:06AM +0100, Jacopo Mondi wrote:\n> > > On Wed, Nov 27, 2024 at 08:04:06AM +0200, Laurent Pinchart wrote:\n> > > > On Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:\n> > > > > Implement a helpers to allow outputting text in YAML format.\n> > > > [...]\n> > > > > + *\n> > > > > + * To emit YAML users of the these helper classes create a root node with\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tstd::string filePath(\"...\");\n> > > > > +\tauto root = YamlEmitter::root(filePath);\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * and start populating it with dictionaries and lists with the YamlRoot::dict()\n> > > > > + * and YamlRoot::list() functions.\n> > > >\n> > > > I would write \"and start emitting dictionaries and lists\", \"populating\"\n> > > > sounds more like a YAML writer.\n> > > >\n> > > ditto\n> > >\n> > > > > + *\n> > > > > + * The classes part of this file implement RAII-style handling of YAML\n> > > > > + * events. By creating a YamlList and YamlDict instance the associated YAML\n> > > > > + * sequence start and mapping start events are emitted and once the instances\n> > > > > + * gets destroyed the corresponding sequence end and mapping end events are\n> > > > > + * emitted.\n> > > > > + *\n> > > > > + * From an initialized YamlRoot instance is possible to create YAML list and\n> > > > > + * dictionaries.\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tYamlDict dict = root.dict();\n> > > > > +\tYamlList list = root.list();\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * YamlDict instances can be populated with scalars associated with a key\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tdict[\"key\"] = \"value\";\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * and it is possible to create lists and dictionaries, associated with a key\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tYamlDict subDict = dict.dict(\"newDict\");\n> > > > > +\tYamlList subList = dict.list(\"newList\");\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * YamlList instances can be populated with scalar elements\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tlist.scalar(\"x\");\n> > > > > +\tlist.scalar(\"y\");\n> > > > > + \\endcode\n> > > > > + *\n> > > > > + * and with dictionaries and lists too\n> > > > > + *\n> > > > > + * \\code\n> > > > > +\tYamlDict subDict = list.dict();\n> > > > > +\tYamlList subList = list.list();\n> > > > > + \\endcode\n> > > >\n> > > > The biggest issue with the API is that it can be easily misused:\n> > > >\n> > > > \tYamlDict list1 = dict.list(\"foo\");\n> > > > \tYamlDict list2 = dict.list(\"bar\");\n> > > > \tlist1.scalar(\"0\");\n> > > >\n> > > > I'm still annoyed that I haven't found a neat way to prevent this. I\n> > > > wonder if we could make it a bit safer by at least catching incorrect\n> > > > usage at runtime. What I'm thinking is\n> > >\n> > > Yes, unfortunately I don't think we can easily catch this at build\n> > > time\n> > >\n> > > > - When creating a child, recording the child pointer in the parent, and\n> > > > the parent pointer in the child\n> > > > - If the parent already has a child when a new child is created, reset\n> > > > the parent pointer of the previous child. This makes the previous\n> > > > child invalid.\n> > > > - Any invalidation of a child needs to be propagated to its children.\n> > > > - Any operation on an invalid child would be caught and logged.\n> > > > - When a child is destroyed, if its parent is not null, reset the child\n> > > > pointer in the parent to null.\n> > > > - If a parent is destroyed while it has a child pointer, reset the\n> > > > child's parent pointer to null and log an error.\n> > > >\n> > >\n> > > That should be enough, I wonder if we can make access to an invalid\n> > > childer a compiler error...\n> > \n> > I'd love to, but haven't found a nice way to do so :-S\n> > [...]\n> \n> My guess is that the C++ type system is \"too weak\" in this sense, to implement\n> the kind of checking you envision.\n\nWe would need a way, when creating and destroying child objects, to\nalter the state of the parent object in a way that could be checked at\ncompile time. I don't think that's doable indeed just with the C++\nlanguage (and I won't explore the option of developing a compiler\nplugin).\n\n> If one is willing to trade indentation for a smaller chance of misuse, then one could:\n> (Or at least I think such an API where the structure of the resulting YAML file\n> is in a sense encapsulated in the layout of the source code leads to fewer misuses.)\n> \n> YamlDict root;\n> \n> root.dict(\"foo\", [](auto& d) {\n> d.list(\"x\", [](auto& l) {\n> l.scalar(1);\n> l.scalar(true);\n> });\n> d.scalar(\"y\", 42);\n> });\n> \n> root.list(\"bar\", [](auto&) {\n> ...\n> });\n> \n> /* the methods could return an appropriate reference to make some kind of chaining possible: */\n> \n> YamlDict()\n> .dict(\"foo\", [](auto& d) {\n> d\n> .list(\"x\", [](auto& l) {\n> l.scalar(1)\n> .scalar(true);\n> })\n> .scalar(\"y\", 42);\n> })\n> .list(\"bar\", [](auto&) { ... });\n> \n> Or one could even go fully in on some kind of chaining:\n> \n> YamlDict()\n> \t.dict(\"foo\")\n> \t\t.list(\"x\")\n> \t\t\t.scalar(1)\n> \t\t\t.scalar(true)\n> \t\t.finish() /* end list */\n> \t\t.scalar()\n> \t.finish() /* end dict */\n> \t.list(\"bar\")\n> \t\t...\n> \n> \n> I am not sure if this has already been considered, sorry if it has been.","headers":{"Return-Path":"","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 DC79FBEFBE\n\tfor ;\n\tSat, 30 Nov 2024 17:01:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E2C5F66041;\n\tSat, 30 Nov 2024 18:01:51 +0100 (CET)","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 6DE5C618B9\n\tfor ;\n\tSat, 30 Nov 2024 18:01:49 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4EBF348;\n\tSat, 30 Nov 2024 18:01:23 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HCvJqg1g\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732986084;\n\tbh=iJl0nLTe5PzjdqkLseW9U/NgtDVoWCdbO5gvbM7cFnc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HCvJqg1g0+3BriJkGiZ17tx5kWrowzkiyjoK8A+ygagHvfL1iu4YgatN9wBNLDmbo\n\t84HMFYAg/klHgTf+iqYCQ4cATEoU/yRPDSS0OSb0qpjYmE2qFiBZtSYdHdMiL1TFhr\n\tmSKF6An8MLuMA7ey9wVBg5cHkcUdyc6jDIoYkgrc=","Date":"Sat, 30 Nov 2024 19:01:38 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/3] libcamera: Implement YamlEmitter","Message-ID":"<20241130170138.GE2652@pendragon.ideasonboard.com>","References":"<20241106175901.83960-1-jacopo.mondi@ideasonboard.com>\n\t<20241106175901.83960-2-jacopo.mondi@ideasonboard.com>\n\t<20241127060406.GP5461@pendragon.ideasonboard.com>\n\t\n\t<20241129084423.GJ25731@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]