[{"id":31742,"web_url":"https://patchwork.libcamera.org/comment/31742/","msgid":"<20241015102133.GB5682@pendragon.ideasonboard.com>","date":"2024-10-15T10:21:33","subject":"Re: [RFC 3/4] 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\nI've kept the review high-level to start with.\n\nOn Mon, Oct 14, 2024 at 11:59:35AM +0200, Jacopo Mondi wrote:\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  include/libcamera/internal/meson.build    |   1 +\n>  include/libcamera/internal/yaml_emitter.h | 172 +++++++++\n>  src/libcamera/meson.build                 |   1 +\n>  src/libcamera/yaml_emitter.cpp            | 427 ++++++++++++++++++++++\n>  4 files changed, 601 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..3fe05f97de70\n> --- /dev/null\n> +++ b/include/libcamera/internal/yaml_emitter.h\n> @@ -0,0 +1,172 @@\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 <memory>\n> +#include <string_view>\n> +#include <unordered_map>\n> +\n> +#include <libcamera/base/class.h>\n> +#include <libcamera/base/file.h>\n> +#include <libcamera/orientation.h>\n> +\n> +#include <yaml.h>\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() = default;\n> +\n> +\tstatic std::unique_ptr<YamlRoot> root(std::string_view path);\n> +\n> +\tint emit(YamlEvent *event);\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY(YamlEmitter)\n> +\n> +\tclass Emitter\n> +\t{\n> +\tpublic:\n> +\t\tEmitter() = default;\n> +\t\t~Emitter();\n> +\n> +\t\tvoid init(File *file);\n> +\n> +\t\tint emit(YamlEvent *event);\n> +\n> +\tprivate:\n> +\t\tvoid logError();\n> +\n> +\t\tyaml_emitter_t emitter_;\n> +\t};\n> +\n> +\tYamlEmitter() = default;\n> +\n> +\tvoid init();\n> +\n> +\tstd::unique_ptr<File> file_;\n> +\tEmitter emitter_;\n> +};\n> +\n> +class YamlOutput\n> +{\n> +public:\n\nShouldn't you disable copying of this class ?\n\n> +\tvirtual ~YamlOutput() = default;\n> +\n> +\tvoid close()\n> +\t{\n> +\t\tclosed_ = true;\n> +\t}\n\nCan we avoid this and close on destruction only ? Having an explicit\nclose is error-prone, what if someone calls one of the functions below\nafter calling close() ? The API should make it very hard, if not\nimpossible, to do the wrong thing.\n\n> +\n> +\tstd::unique_ptr<YamlScalar> scalar();\n> +\tstd::unique_ptr<YamlDict> dict();\n> +\tstd::unique_ptr<YamlList> list();\n\nIn many cases the YamlScalar, YamlDict and YamlList classes will be\nlocal to the caller function. It would be best to avoid dynamic heap\nallocation. You could do so by returning values here. Same below. You\nwill likely need to implement move constructors and operators.\n\n> +\n> +protected:\n> +\tYamlOutput(YamlEmitter *emitter)\n> +\t\t: emitter_(emitter), closed_(false)\n> +\t{\n> +\t}\n> +\n> +\tint emitScalar(std::string_view scalar);\n> +\tint emitMappingStart();\n> +\tint emitMappingEnd();\n> +\tint emitSequenceStart();\n> +\tint emitSequenceEnd();\n> +\n> +\tYamlEmitter *emitter_;\n> +\n> +\tbool closed_;\n> +};\n> +\n> +class YamlRoot : public YamlOutput\n> +{\n> +public:\n> +\t~YamlRoot();\n> +\tvoid close() {}\n> +\n> +\tstd::unique_ptr<YamlList> list();\n> +\tstd::unique_ptr<YamlDict> dict();\n> +\tvoid scalar(std::string_view scalar);\n> +\n> +private:\n> +\tfriend class YamlEmitter;\n> +\n> +\tYamlRoot(YamlEmitter *emitter)\n> +\t\t: YamlOutput(emitter)\n> +\t{\n> +\t\temitterRoot_ = std::unique_ptr<YamlEmitter>(emitter);\n> +\t}\n> +\n> +\tstd::unique_ptr<YamlEmitter> emitterRoot_;\n> +};\n> +\n> +class YamlScalar : public YamlOutput\n> +{\n> +public:\n> +\t~YamlScalar() = default;\n> +\n> +\tvoid close() {}\n> +\n> +\tvoid operator=(const Orientation &orientation);\n\nI don't think this should be a member of the YamlScalar class. You\nshould convert the Orientation to a string in the caller.\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> +\t~YamlList();\n> +\n> +\tvoid close();\n> +\n> +\tstd::unique_ptr<YamlList> list();\n> +\tstd::unique_ptr<YamlDict> 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> +\t\t private std::unordered_map<std::string_view,\n> +\t\t\t\t\t    std::unique_ptr<YamlScalar>>\n\nWhy do you need to store content ? That doesn't sound right,the\nemitter's job is to emit YAML, we shouldn't need to store anything.\n\n> +{\n> +public:\n> +\tusing Map = std::unordered_map<std::string_view, YamlScalar>;\n> +\n> +\t~YamlDict();\n> +\n> +\tvoid close();\n> +\n> +\tstd::unique_ptr<YamlList> list(std::string_view key);\n> +\tstd::unique_ptr<YamlDict> dict(std::string_view key);\n> +\n> +\tYamlScalar &operator[](const Map::key_type &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..1f7651ffcb24\n> --- /dev/null\n> +++ b/src/libcamera/yaml_emitter.cpp\n> @@ -0,0 +1,427 @@\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 <libcamera/base/log.h>\n> +\n> +/**\n> + * \\file yaml_emitter.h\n> + * \\brief A YAML emitter helper\n> + *\n> + * The YAML Emitter helpers allows users to emit output in YAML format.\n> + *\n> + * To emit YAML users of this classes should create a root node with\n> + *\n> + * \\code\n> +\tstd::string filePath(\"...\");\n> +\tauto root = YamlEmitter::root(filePath);\n> +   \\endcode\n> + *\n> + * A YamlRoot implements RAII-style handling of YAML sequence and document\n> + * events handling. Creating a YamlRoot emits the STREAM_START and DOC_START\n> + * events. Once a YamlRoot gets deleted the DOC_SEND and STREAM_END events\n> + * gets automatically deleted.\n> + *\n> + * Once a root node has been created it is possible to populate it with\n> + * scalars, list or dictionaries.\n> + *\n> + * YamlList and YamlDict can only be created wrapped in unique_ptr<> instances,\n> + * to implement a RAII-style handling of YAML of lists and dictionaries.\n> + * Creating a YamlList and a YamlDict emits the YAML sequence and mapping start\n> + * events. Once an instance gets deleted, the sequence and mapping gets\n> + * automatically emitted. If a list of dictionary needs to be closed before\n> + * it gets deleted, it can be explicitly closed with the close() function.\n> + *\n> + * A YamlScalar is a simpler object that can be assigned to different types\n> + * to emit them as strings.\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<File *>(data);\n> +\n> +\tSpan<unsigned char> buf{ buffer, size };\n> +\tssize_t ret = file->write(buf);\n> +\tif (ret < 0) {\n> +\t\tret = errno;\n> +\t\tLOG(YamlEmitter, Error) << \"Write error : \" << strerror(ret);\n> +\t\treturn 0;\n> +\t}\n> +\n> +\treturn 1;\n> +}\n> +\n> +} /* namespace */\n> +\n> +/**\n> + * \\class YamlEvent\n> + *\n> + * Class that represents a yaml_event that automatically cleans-up on\n> + * destruction.\n> + */\n> +class YamlEvent\n> +{\n> +public:\n> +\tstatic std::unique_ptr<YamlEvent> create();\n> +\n> +\tconst yaml_event_t *event() const\n> +\t{\n> +\t\treturn &event_;\n> +\t}\n> +\n> +\tyaml_event_t *event()\n> +\t{\n> +\t\treturn &event_;\n> +\t}\n> +\n> +private:\n> +\tYamlEvent() = default;\n> +\n> +\tyaml_event_t event_;\n> +};\n> +\n> +std::unique_ptr<YamlEvent> YamlEvent::create()\n> +{\n> +\tstruct Deleter : std::default_delete<YamlEvent> {\n> +\t\tvoid operator()(YamlEvent *yamlEvent)\n> +\t\t{\n> +\t\t\tyaml_event_delete(yamlEvent->event());\n> +\t\t}\n> +\t};\n> +\n> +\treturn std::unique_ptr<YamlEvent>(new YamlEvent(), Deleter());\n\nHere too, we should avoid dynamic allocations on the heap. I don't think\nyou need a custom deleter, you can simply call yaml_event_delete() in\nthe destructor of the YamlEvent class, and you then don't need to use\nunique pointers.\n\n> +}\n> +\n> +/**\n> + * \\class YamlEmitter\n> + *\n> + * Yaml Emitter entry point. Allows to create a YamlRoot object that users\n> + * can populate.\n> + */\n> +\n> +/**\n> + * \\brief Create a YamlRoot that applications can start populating with YamlOutput\n> + * \\param[in] path The YAML output file path\n> + * \\return A unique pointer to a YamlRoot\n> + */\n> +std::unique_ptr<YamlRoot> YamlEmitter::root(std::string_view path)\n> +{\n> +\tYamlEmitter *emitter = new YamlEmitter();\n> +\n> +\tstd::string filePath(path);\n> +\temitter->file_ = std::make_unique<File>(filePath);\n> +\temitter->file_->open(File::OpenModeFlag::WriteOnly);\n> +\n> +\temitter->init();\n> +\n> +\tYamlRoot *root = new YamlRoot(emitter);\n> +\treturn std::unique_ptr<YamlRoot>(root);\n> +}\n> +\n> +/**\n> + * \\brief Emit a YamlEvent\n> + */\n> +int YamlEmitter::emit(YamlEvent *event)\n> +{\n> +\treturn emitter_.emit(event);\n> +}\n> +\n> +void YamlEmitter::init()\n> +{\n> +\temitter_.init(file_.get());\n> +\n> +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> +\n> +\tyaml_stream_start_event_initialize(event->event(), YAML_UTF8_ENCODING);\n> +\temitter_.emit(event.get());\n> +\n> +\tyaml_document_start_event_initialize(event->event(), NULL, NULL,\n> +\t\t\t\t\t     NULL, 0);\n> +\temitter_.emit(event.get());\n> +}\n> +\n> +/**\n> + * \\class YamlEmitter::Emitter\n> + *\n> + * yaml_emitter_t wrapper. Initialize the yaml_emitter_t on creation allows\n> + * YamlOutput classes to emit events.\n> + */\n> +\n> +YamlEmitter::Emitter::~Emitter()\n> +{\n> +\tyaml_emitter_delete(&emitter_);\n> +}\n> +\n> +void YamlEmitter::Emitter::init(File *file)\n> +{\n> +\tyaml_emitter_initialize(&emitter_);\n> +\tyaml_emitter_set_output(&emitter_, yamlWrite, file);\n> +}\n> +\n> +void YamlEmitter::Emitter::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> +\t\tbreak;\n> +\t}\n> +}\n> +\n> +int YamlEmitter::Emitter::emit(YamlEvent *event)\n> +{\n> +\tint ret = yaml_emitter_emit(&emitter_, event->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 YamlScalar, YamlList and YamlDict\n> + * are derived.\n> + *\n> + * The YamlOutput class provides functions to create a scalar, a list or a\n> + * dictionary.\n> + *\n> + * The class cannot be instantiated directly by applications.\n> + */\n> +\n> +std::unique_ptr<YamlScalar> YamlOutput::scalar()\n> +{\n> +\treturn std::unique_ptr<YamlScalar>(new YamlScalar(emitter_));\n> +}\n> +\n> +std::unique_ptr<YamlDict> YamlOutput::dict()\n> +{\n> +\treturn std::unique_ptr<YamlDict>(new YamlDict(emitter_));\n> +}\n> +\n> +std::unique_ptr<YamlList> YamlOutput::list()\n> +{\n> +\treturn std::unique_ptr<YamlList>(new YamlList(emitter_));\n> +}\n> +\n> +int YamlOutput::emitScalar(std::string_view scalar)\n> +{\n> +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> +\n> +\tconst unsigned char *value = reinterpret_cast<const unsigned char *>\n> +\t\t\t\t     (scalar.data());\n> +\tyaml_scalar_event_initialize(event->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(event.get());\n> +}\n> +\n> +int YamlOutput::emitMappingStart()\n> +{\n> +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> +\tyaml_mapping_start_event_initialize(event->event(), NULL, NULL,\n> +\t\t\t\t\t    true, YAML_BLOCK_MAPPING_STYLE);\n> +\treturn emitter_->emit(event.get());\n> +}\n> +\n> +int YamlOutput::emitMappingEnd()\n> +{\n> +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> +\tyaml_mapping_end_event_initialize(event->event());\n> +\treturn emitter_->emit(event.get());\n> +}\n> +\n> +int YamlOutput::emitSequenceStart()\n> +{\n> +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> +\tyaml_sequence_start_event_initialize(event->event(), NULL, NULL, true,\n> +\t\t\t\t\t     YAML_BLOCK_SEQUENCE_STYLE);\n> +\treturn emitter_->emit(event.get());\n> +}\n> +\n> +int YamlOutput::emitSequenceEnd()\n> +{\n> +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> +\tyaml_sequence_end_event_initialize(event->event());\n> +\treturn emitter_->emit(event.get());\n> +}\n> +\n> +/**\n> + * \\class YamlRoot\n> + *\n> + * Yaml root node. A root node can be populated with a scalar, a list or a dict.\n> + */\n> +\n> +YamlRoot::~YamlRoot()\n> +{\n> +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> +\n> +\tyaml_document_end_event_initialize(event->event(), 0);\n> +\temitterRoot_->emit(event.get());\n> +\n> +\tyaml_stream_end_event_initialize(event->event());\n> +\temitterRoot_->emit(event.get());\n> +}\n> +\n> +std::unique_ptr<YamlDict> YamlRoot::dict()\n> +{\n> +\temitMappingStart();\n> +\n> +\treturn YamlOutput::dict();\n> +}\n> +\n> +std::unique_ptr<YamlList> YamlRoot::list()\n> +{\n> +\temitSequenceStart();\n> +\n> +\treturn YamlOutput::list();\n> +}\n> +\n> +/**\n> + * \\class YamlScalar\n> + *\n> + * A YamlScalar can be assigned to an std::string_view and other libcamera\n> + * types to emit them as YAML scalars.\n> + */\n> +\n> +YamlScalar::YamlScalar(YamlEmitter *emitter)\n> +\t: YamlOutput(emitter)\n> +{\n> +}\n> +\n> +void YamlScalar::operator=(const libcamera::Orientation &orientation)\n> +{\n> +\tstd::stringstream o;\n> +\to << orientation;\n> +\n> +\temitScalar(o.str());\n> +}\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 scalar and allows to create other lists\n> + * and dictionaries.\n> + */\n> +\n> +YamlList::YamlList(YamlEmitter *emitter)\n> +\t: YamlOutput(emitter)\n> +{\n> +}\n> +\n> +YamlList::~YamlList()\n> +{\n> +\tif (!closed_)\n> +\t\tclose();\n> +}\n> +\n> +void YamlList::close()\n> +{\n> +\temitSequenceEnd();\n> +\tYamlOutput::close();\n> +}\n> +\n> +void YamlList::scalar(std::string_view scalar)\n> +{\n> +\temitScalar(scalar);\n> +}\n> +\n> +std::unique_ptr<YamlList> YamlList::list()\n> +{\n> +\temitSequenceStart();\n> +\n> +\treturn YamlOutput::list();\n> +}\n> +\n> +std::unique_ptr<YamlDict> YamlList::dict()\n> +{\n> +\temitMappingStart();\n> +\n> +\treturn YamlOutput::dict();\n> +}\n> +\n> +/**\n> + * \\class YamlDict\n> + *\n> + * A YamlDict derives an unordered_map that maps strings to YAML scalar.\n> + *\n> + * A YamlDict can be populated with scalars using operator[] and allows to\n> + * create other lists and dictionaries.\n> + */\n> +\n> +YamlDict::YamlDict(YamlEmitter *emitter)\n> +\t: YamlOutput(emitter)\n> +{\n> +}\n> +\n> +YamlDict::~YamlDict()\n> +{\n> +\tif (!closed_)\n> +\t\tclose();\n> +\n> +\tclear();\n> +}\n> +\n> +void YamlDict::close()\n> +{\n> +\temitMappingEnd();\n> +\tYamlOutput::close();\n> +}\n> +\n> +std::unique_ptr<YamlList> YamlDict::list(std::string_view key)\n> +{\n> +\temitScalar(key);\n> +\temitSequenceStart();\n> +\n> +\treturn YamlOutput::list();\n> +}\n> +\n> +std::unique_ptr<YamlDict> YamlDict::dict(std::string_view key)\n> +{\n> +\temitScalar(key);\n> +\temitMappingStart();\n> +\n> +\treturn YamlOutput::dict();\n> +}\n> +\n> +YamlScalar &YamlDict::operator[](const YamlDict::Map::key_type &key)\n> +{\n> +\templace(key, YamlOutput::scalar());\n> +\temitScalar(key);\n> +\n> +\treturn *at(key);\n> +}\n> +\n> +} /* namespace libcamera */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 58807C326C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 15 Oct 2024 10:21:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 432DF6537E;\n\tTue, 15 Oct 2024 12:21:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E80A260537\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Oct 2024 12:21:36 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [185.143.39.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A26C9CE;\n\tTue, 15 Oct 2024 12:19:54 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"io6vz741\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728987594;\n\tbh=lzwMa47sLEdvaFLLmRsmII85Fiioc3Nf7s5XIXUSKbA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=io6vz7415XYSomtQs19gbpdhxdY/GG6HfxLrrMn9T1y+D+tsHJHCXgTWf/88qoNB0\n\t9KYerKE1/Hq4XpUJvrxuS93FOfAQ7KLoaogBfdd4kuB6lYhslLbjX0ysniklZYfO2B\n\t2WEVA1V7b7/B8CfCqq8k63tKjj3LGp1NkKcnL+3A=","Date":"Tue, 15 Oct 2024 13:21:33 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC 3/4] libcamera: Implement YamlEmitter","Message-ID":"<20241015102133.GB5682@pendragon.ideasonboard.com>","References":"<20241014095937.24924-1-jacopo.mondi@ideasonboard.com>\n\t<20241014095937.24924-4-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241014095937.24924-4-jacopo.mondi@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31749,"web_url":"https://patchwork.libcamera.org/comment/31749/","msgid":"<n5c7pmelrwnvonktaujclhnui4sshzcm6rdcpdxahlm23v5uo2@ijx7y36cwnch>","date":"2024-10-15T12:16:11","subject":"Re: [RFC 3/4] 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,\n    thanks for review\n\nOn Tue, Oct 15, 2024 at 01:21:33PM GMT, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> I've kept the review high-level to start with.\n>\n> On Mon, Oct 14, 2024 at 11:59:35AM +0200, Jacopo Mondi wrote:\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >  include/libcamera/internal/meson.build    |   1 +\n> >  include/libcamera/internal/yaml_emitter.h | 172 +++++++++\n> >  src/libcamera/meson.build                 |   1 +\n> >  src/libcamera/yaml_emitter.cpp            | 427 ++++++++++++++++++++++\n> >  4 files changed, 601 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..3fe05f97de70\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/yaml_emitter.h\n> > @@ -0,0 +1,172 @@\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 <memory>\n> > +#include <string_view>\n> > +#include <unordered_map>\n> > +\n> > +#include <libcamera/base/class.h>\n> > +#include <libcamera/base/file.h>\n> > +#include <libcamera/orientation.h>\n> > +\n> > +#include <yaml.h>\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() = default;\n> > +\n> > +\tstatic std::unique_ptr<YamlRoot> root(std::string_view path);\n> > +\n> > +\tint emit(YamlEvent *event);\n> > +\n> > +private:\n> > +\tLIBCAMERA_DISABLE_COPY(YamlEmitter)\n> > +\n> > +\tclass Emitter\n> > +\t{\n> > +\tpublic:\n> > +\t\tEmitter() = default;\n> > +\t\t~Emitter();\n> > +\n> > +\t\tvoid init(File *file);\n> > +\n> > +\t\tint emit(YamlEvent *event);\n> > +\n> > +\tprivate:\n> > +\t\tvoid logError();\n> > +\n> > +\t\tyaml_emitter_t emitter_;\n> > +\t};\n> > +\n> > +\tYamlEmitter() = default;\n> > +\n> > +\tvoid init();\n> > +\n> > +\tstd::unique_ptr<File> file_;\n> > +\tEmitter emitter_;\n> > +};\n> > +\n> > +class YamlOutput\n> > +{\n> > +public:\n>\n> Shouldn't you disable copying of this class ?\n>\n\nyup\n\n> > +\tvirtual ~YamlOutput() = default;\n> > +\n> > +\tvoid close()\n> > +\t{\n> > +\t\tclosed_ = true;\n> > +\t}\n>\n> Can we avoid this and close on destruction only ? Having an explicit\n> close is error-prone, what if someone calls one of the functions below\n> after calling close() ? The API should make it very hard, if not\n> impossible, to do the wrong thing.\n>\n\nEh\n\nmy idea was ideed to rely on object's destruction to emit the correct\nyaml events, then I thought it was worth to allow users to close a\nsequence/mapping earlier if they didn't want to enforce this through\nthe object lifetime. I think I had a use case for this, but maybe it\nhas gone during the development, I'll try remove close() and solely\nrely on object's lifetime for the correct events emission.\n\n> > +\n> > +\tstd::unique_ptr<YamlScalar> scalar();\n> > +\tstd::unique_ptr<YamlDict> dict();\n> > +\tstd::unique_ptr<YamlList> list();\n>\n> In many cases the YamlScalar, YamlDict and YamlList classes will be\n> local to the caller function. It would be best to avoid dynamic heap\n> allocation. You could do so by returning values here. Same below. You\n> will likely need to implement move constructors and operators.\n>\n\n\ngood idea, I'll try\n\n> > +\n> > +protected:\n> > +\tYamlOutput(YamlEmitter *emitter)\n> > +\t\t: emitter_(emitter), closed_(false)\n> > +\t{\n> > +\t}\n> > +\n> > +\tint emitScalar(std::string_view scalar);\n> > +\tint emitMappingStart();\n> > +\tint emitMappingEnd();\n> > +\tint emitSequenceStart();\n> > +\tint emitSequenceEnd();\n> > +\n> > +\tYamlEmitter *emitter_;\n> > +\n> > +\tbool closed_;\n> > +};\n> > +\n> > +class YamlRoot : public YamlOutput\n> > +{\n> > +public:\n> > +\t~YamlRoot();\n> > +\tvoid close() {}\n> > +\n> > +\tstd::unique_ptr<YamlList> list();\n> > +\tstd::unique_ptr<YamlDict> dict();\n> > +\tvoid scalar(std::string_view scalar);\n> > +\n> > +private:\n> > +\tfriend class YamlEmitter;\n> > +\n> > +\tYamlRoot(YamlEmitter *emitter)\n> > +\t\t: YamlOutput(emitter)\n> > +\t{\n> > +\t\temitterRoot_ = std::unique_ptr<YamlEmitter>(emitter);\n> > +\t}\n> > +\n> > +\tstd::unique_ptr<YamlEmitter> emitterRoot_;\n> > +};\n> > +\n> > +class YamlScalar : public YamlOutput\n> > +{\n> > +public:\n> > +\t~YamlScalar() = default;\n> > +\n> > +\tvoid close() {}\n> > +\n> > +\tvoid operator=(const Orientation &orientation);\n>\n> I don't think this should be a member of the YamlScalar class. You\n> should convert the Orientation to a string in the caller.\n>\n\nThis is something I wanted indeed feedbacks on. I see it two ways,\neither we augment this interface with libcamera types as we need them\nto ease for users of this class to emit them, or we require users to\nconvert to string for. I understand your preference is for the latter\noption.\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> > +\t~YamlList();\n> > +\n> > +\tvoid close();\n> > +\n> > +\tstd::unique_ptr<YamlList> list();\n> > +\tstd::unique_ptr<YamlDict> 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> > +\t\t private std::unordered_map<std::string_view,\n> > +\t\t\t\t\t    std::unique_ptr<YamlScalar>>\n>\n> Why do you need to store content ? That doesn't sound right,the\n> emitter's job is to emit YAML, we shouldn't need to store anything.\n>\n\nbecause I wanted to override operator[] for YamlDict and it returns a\nrefernce, so I need to have a storage. I can try to remove it.\n\n> > +{\n> > +public:\n> > +\tusing Map = std::unordered_map<std::string_view, YamlScalar>;\n> > +\n> > +\t~YamlDict();\n> > +\n> > +\tvoid close();\n> > +\n> > +\tstd::unique_ptr<YamlList> list(std::string_view key);\n> > +\tstd::unique_ptr<YamlDict> dict(std::string_view key);\n> > +\n> > +\tYamlScalar &operator[](const Map::key_type &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..1f7651ffcb24\n> > --- /dev/null\n> > +++ b/src/libcamera/yaml_emitter.cpp\n> > @@ -0,0 +1,427 @@\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 <libcamera/base/log.h>\n> > +\n> > +/**\n> > + * \\file yaml_emitter.h\n> > + * \\brief A YAML emitter helper\n> > + *\n> > + * The YAML Emitter helpers allows users to emit output in YAML format.\n> > + *\n> > + * To emit YAML users of this classes should create a root node with\n> > + *\n> > + * \\code\n> > +\tstd::string filePath(\"...\");\n> > +\tauto root = YamlEmitter::root(filePath);\n> > +   \\endcode\n> > + *\n> > + * A YamlRoot implements RAII-style handling of YAML sequence and document\n> > + * events handling. Creating a YamlRoot emits the STREAM_START and DOC_START\n> > + * events. Once a YamlRoot gets deleted the DOC_SEND and STREAM_END events\n> > + * gets automatically deleted.\n> > + *\n> > + * Once a root node has been created it is possible to populate it with\n> > + * scalars, list or dictionaries.\n> > + *\n> > + * YamlList and YamlDict can only be created wrapped in unique_ptr<> instances,\n> > + * to implement a RAII-style handling of YAML of lists and dictionaries.\n> > + * Creating a YamlList and a YamlDict emits the YAML sequence and mapping start\n> > + * events. Once an instance gets deleted, the sequence and mapping gets\n> > + * automatically emitted. If a list of dictionary needs to be closed before\n> > + * it gets deleted, it can be explicitly closed with the close() function.\n> > + *\n> > + * A YamlScalar is a simpler object that can be assigned to different types\n> > + * to emit them as strings.\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<File *>(data);\n> > +\n> > +\tSpan<unsigned char> buf{ buffer, size };\n> > +\tssize_t ret = file->write(buf);\n> > +\tif (ret < 0) {\n> > +\t\tret = errno;\n> > +\t\tLOG(YamlEmitter, Error) << \"Write error : \" << strerror(ret);\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\treturn 1;\n> > +}\n> > +\n> > +} /* namespace */\n> > +\n> > +/**\n> > + * \\class YamlEvent\n> > + *\n> > + * Class that represents a yaml_event that automatically cleans-up on\n> > + * destruction.\n> > + */\n> > +class YamlEvent\n> > +{\n> > +public:\n> > +\tstatic std::unique_ptr<YamlEvent> create();\n> > +\n> > +\tconst yaml_event_t *event() const\n> > +\t{\n> > +\t\treturn &event_;\n> > +\t}\n> > +\n> > +\tyaml_event_t *event()\n> > +\t{\n> > +\t\treturn &event_;\n> > +\t}\n> > +\n> > +private:\n> > +\tYamlEvent() = default;\n> > +\n> > +\tyaml_event_t event_;\n> > +};\n> > +\n> > +std::unique_ptr<YamlEvent> YamlEvent::create()\n> > +{\n> > +\tstruct Deleter : std::default_delete<YamlEvent> {\n> > +\t\tvoid operator()(YamlEvent *yamlEvent)\n> > +\t\t{\n> > +\t\t\tyaml_event_delete(yamlEvent->event());\n> > +\t\t}\n> > +\t};\n> > +\n> > +\treturn std::unique_ptr<YamlEvent>(new YamlEvent(), Deleter());\n>\n> Here too, we should avoid dynamic allocations on the heap. I don't think\n> you need a custom deleter, you can simply call yaml_event_delete() in\n> the destructor of the YamlEvent class, and you then don't need to use\n> unique pointers.\n>\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlEmitter\n> > + *\n> > + * Yaml Emitter entry point. Allows to create a YamlRoot object that users\n> > + * can populate.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Create a YamlRoot that applications can start populating with YamlOutput\n> > + * \\param[in] path The YAML output file path\n> > + * \\return A unique pointer to a YamlRoot\n> > + */\n> > +std::unique_ptr<YamlRoot> YamlEmitter::root(std::string_view path)\n> > +{\n> > +\tYamlEmitter *emitter = new YamlEmitter();\n> > +\n> > +\tstd::string filePath(path);\n> > +\temitter->file_ = std::make_unique<File>(filePath);\n> > +\temitter->file_->open(File::OpenModeFlag::WriteOnly);\n> > +\n> > +\temitter->init();\n> > +\n> > +\tYamlRoot *root = new YamlRoot(emitter);\n> > +\treturn std::unique_ptr<YamlRoot>(root);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Emit a YamlEvent\n> > + */\n> > +int YamlEmitter::emit(YamlEvent *event)\n> > +{\n> > +\treturn emitter_.emit(event);\n> > +}\n> > +\n> > +void YamlEmitter::init()\n> > +{\n> > +\temitter_.init(file_.get());\n> > +\n> > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > +\n> > +\tyaml_stream_start_event_initialize(event->event(), YAML_UTF8_ENCODING);\n> > +\temitter_.emit(event.get());\n> > +\n> > +\tyaml_document_start_event_initialize(event->event(), NULL, NULL,\n> > +\t\t\t\t\t     NULL, 0);\n> > +\temitter_.emit(event.get());\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlEmitter::Emitter\n> > + *\n> > + * yaml_emitter_t wrapper. Initialize the yaml_emitter_t on creation allows\n> > + * YamlOutput classes to emit events.\n> > + */\n> > +\n> > +YamlEmitter::Emitter::~Emitter()\n> > +{\n> > +\tyaml_emitter_delete(&emitter_);\n> > +}\n> > +\n> > +void YamlEmitter::Emitter::init(File *file)\n> > +{\n> > +\tyaml_emitter_initialize(&emitter_);\n> > +\tyaml_emitter_set_output(&emitter_, yamlWrite, file);\n> > +}\n> > +\n> > +void YamlEmitter::Emitter::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> > +\t\tbreak;\n> > +\t}\n> > +}\n> > +\n> > +int YamlEmitter::Emitter::emit(YamlEvent *event)\n> > +{\n> > +\tint ret = yaml_emitter_emit(&emitter_, event->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 YamlScalar, YamlList and YamlDict\n> > + * are derived.\n> > + *\n> > + * The YamlOutput class provides functions to create a scalar, a list or a\n> > + * dictionary.\n> > + *\n> > + * The class cannot be instantiated directly by applications.\n> > + */\n> > +\n> > +std::unique_ptr<YamlScalar> YamlOutput::scalar()\n> > +{\n> > +\treturn std::unique_ptr<YamlScalar>(new YamlScalar(emitter_));\n> > +}\n> > +\n> > +std::unique_ptr<YamlDict> YamlOutput::dict()\n> > +{\n> > +\treturn std::unique_ptr<YamlDict>(new YamlDict(emitter_));\n> > +}\n> > +\n> > +std::unique_ptr<YamlList> YamlOutput::list()\n> > +{\n> > +\treturn std::unique_ptr<YamlList>(new YamlList(emitter_));\n> > +}\n> > +\n> > +int YamlOutput::emitScalar(std::string_view scalar)\n> > +{\n> > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > +\n> > +\tconst unsigned char *value = reinterpret_cast<const unsigned char *>\n> > +\t\t\t\t     (scalar.data());\n> > +\tyaml_scalar_event_initialize(event->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(event.get());\n> > +}\n> > +\n> > +int YamlOutput::emitMappingStart()\n> > +{\n> > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > +\tyaml_mapping_start_event_initialize(event->event(), NULL, NULL,\n> > +\t\t\t\t\t    true, YAML_BLOCK_MAPPING_STYLE);\n> > +\treturn emitter_->emit(event.get());\n> > +}\n> > +\n> > +int YamlOutput::emitMappingEnd()\n> > +{\n> > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > +\tyaml_mapping_end_event_initialize(event->event());\n> > +\treturn emitter_->emit(event.get());\n> > +}\n> > +\n> > +int YamlOutput::emitSequenceStart()\n> > +{\n> > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > +\tyaml_sequence_start_event_initialize(event->event(), NULL, NULL, true,\n> > +\t\t\t\t\t     YAML_BLOCK_SEQUENCE_STYLE);\n> > +\treturn emitter_->emit(event.get());\n> > +}\n> > +\n> > +int YamlOutput::emitSequenceEnd()\n> > +{\n> > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > +\tyaml_sequence_end_event_initialize(event->event());\n> > +\treturn emitter_->emit(event.get());\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlRoot\n> > + *\n> > + * Yaml root node. A root node can be populated with a scalar, a list or a dict.\n> > + */\n> > +\n> > +YamlRoot::~YamlRoot()\n> > +{\n> > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > +\n> > +\tyaml_document_end_event_initialize(event->event(), 0);\n> > +\temitterRoot_->emit(event.get());\n> > +\n> > +\tyaml_stream_end_event_initialize(event->event());\n> > +\temitterRoot_->emit(event.get());\n> > +}\n> > +\n> > +std::unique_ptr<YamlDict> YamlRoot::dict()\n> > +{\n> > +\temitMappingStart();\n> > +\n> > +\treturn YamlOutput::dict();\n> > +}\n> > +\n> > +std::unique_ptr<YamlList> YamlRoot::list()\n> > +{\n> > +\temitSequenceStart();\n> > +\n> > +\treturn YamlOutput::list();\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlScalar\n> > + *\n> > + * A YamlScalar can be assigned to an std::string_view and other libcamera\n> > + * types to emit them as YAML scalars.\n> > + */\n> > +\n> > +YamlScalar::YamlScalar(YamlEmitter *emitter)\n> > +\t: YamlOutput(emitter)\n> > +{\n> > +}\n> > +\n> > +void YamlScalar::operator=(const libcamera::Orientation &orientation)\n> > +{\n> > +\tstd::stringstream o;\n> > +\to << orientation;\n> > +\n> > +\temitScalar(o.str());\n> > +}\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 scalar and allows to create other lists\n> > + * and dictionaries.\n> > + */\n> > +\n> > +YamlList::YamlList(YamlEmitter *emitter)\n> > +\t: YamlOutput(emitter)\n> > +{\n> > +}\n> > +\n> > +YamlList::~YamlList()\n> > +{\n> > +\tif (!closed_)\n> > +\t\tclose();\n> > +}\n> > +\n> > +void YamlList::close()\n> > +{\n> > +\temitSequenceEnd();\n> > +\tYamlOutput::close();\n> > +}\n> > +\n> > +void YamlList::scalar(std::string_view scalar)\n> > +{\n> > +\temitScalar(scalar);\n> > +}\n> > +\n> > +std::unique_ptr<YamlList> YamlList::list()\n> > +{\n> > +\temitSequenceStart();\n> > +\n> > +\treturn YamlOutput::list();\n> > +}\n> > +\n> > +std::unique_ptr<YamlDict> YamlList::dict()\n> > +{\n> > +\temitMappingStart();\n> > +\n> > +\treturn YamlOutput::dict();\n> > +}\n> > +\n> > +/**\n> > + * \\class YamlDict\n> > + *\n> > + * A YamlDict derives an unordered_map that maps strings to YAML scalar.\n> > + *\n> > + * A YamlDict can be populated with scalars using operator[] and allows to\n> > + * create other lists and dictionaries.\n> > + */\n> > +\n> > +YamlDict::YamlDict(YamlEmitter *emitter)\n> > +\t: YamlOutput(emitter)\n> > +{\n> > +}\n> > +\n> > +YamlDict::~YamlDict()\n> > +{\n> > +\tif (!closed_)\n> > +\t\tclose();\n> > +\n> > +\tclear();\n> > +}\n> > +\n> > +void YamlDict::close()\n> > +{\n> > +\temitMappingEnd();\n> > +\tYamlOutput::close();\n> > +}\n> > +\n> > +std::unique_ptr<YamlList> YamlDict::list(std::string_view key)\n> > +{\n> > +\temitScalar(key);\n> > +\temitSequenceStart();\n> > +\n> > +\treturn YamlOutput::list();\n> > +}\n> > +\n> > +std::unique_ptr<YamlDict> YamlDict::dict(std::string_view key)\n> > +{\n> > +\temitScalar(key);\n> > +\temitMappingStart();\n> > +\n> > +\treturn YamlOutput::dict();\n> > +}\n> > +\n> > +YamlScalar &YamlDict::operator[](const YamlDict::Map::key_type &key)\n> > +{\n> > +\templace(key, YamlOutput::scalar());\n> > +\temitScalar(key);\n> > +\n> > +\treturn *at(key);\n> > +}\n> > +\n> > +} /* namespace libcamera */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 694B4C326C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 15 Oct 2024 12:16:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2EBF96537E;\n\tTue, 15 Oct 2024 14:16:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9DC4860537\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Oct 2024 14:16:16 +0200 (CEST)","from ideasonboard.com (mob-5-91-56-97.net.vodafone.it [5.91.56.97])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 964B79CE;\n\tTue, 15 Oct 2024 14:14:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L6KfVS+p\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728994474;\n\tbh=hwG2LVr9/sPV2uWt9DQicpdC5WUP368q83wiev0LOAM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L6KfVS+puGOv0jhOh4VUxefnGx9ECKi3ME8XR3AhAyoaZus78T3A+fMsqo8rEMzlm\n\t3LNPScJNuVZmR6HP+M1sB1qMHydNYfxV/Kk23R9ZzjMTVcEq5l6q2jfiUMpdTRMiKG\n\t4EFm7PcvUEEMfgJB4/iSMVPt53JRkmOHWNQAD1k4=","Date":"Tue, 15 Oct 2024 14:16:11 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [RFC 3/4] libcamera: Implement YamlEmitter","Message-ID":"<n5c7pmelrwnvonktaujclhnui4sshzcm6rdcpdxahlm23v5uo2@ijx7y36cwnch>","References":"<20241014095937.24924-1-jacopo.mondi@ideasonboard.com>\n\t<20241014095937.24924-4-jacopo.mondi@ideasonboard.com>\n\t<20241015102133.GB5682@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241015102133.GB5682@pendragon.ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31776,"web_url":"https://patchwork.libcamera.org/comment/31776/","msgid":"<20241016224749.GI30496@pendragon.ideasonboard.com>","date":"2024-10-16T22:47:49","subject":"Re: [RFC 3/4] 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\nOn Tue, Oct 15, 2024 at 02:16:11PM +0200, Jacopo Mondi wrote:\n> On Tue, Oct 15, 2024 at 01:21:33PM GMT, Laurent Pinchart wrote:\n> > On Mon, Oct 14, 2024 at 11:59:35AM +0200, Jacopo Mondi wrote:\n> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > ---\n> > >  include/libcamera/internal/meson.build    |   1 +\n> > >  include/libcamera/internal/yaml_emitter.h | 172 +++++++++\n> > >  src/libcamera/meson.build                 |   1 +\n> > >  src/libcamera/yaml_emitter.cpp            | 427 ++++++++++++++++++++++\n> > >  4 files changed, 601 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..3fe05f97de70\n> > > --- /dev/null\n> > > +++ b/include/libcamera/internal/yaml_emitter.h\n> > > @@ -0,0 +1,172 @@\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 <memory>\n> > > +#include <string_view>\n> > > +#include <unordered_map>\n> > > +\n> > > +#include <libcamera/base/class.h>\n> > > +#include <libcamera/base/file.h>\n> > > +#include <libcamera/orientation.h>\n> > > +\n> > > +#include <yaml.h>\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() = default;\n> > > +\n> > > +\tstatic std::unique_ptr<YamlRoot> root(std::string_view path);\n> > > +\n> > > +\tint emit(YamlEvent *event);\n> > > +\n> > > +private:\n> > > +\tLIBCAMERA_DISABLE_COPY(YamlEmitter)\n> > > +\n> > > +\tclass Emitter\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tEmitter() = default;\n> > > +\t\t~Emitter();\n> > > +\n> > > +\t\tvoid init(File *file);\n> > > +\n> > > +\t\tint emit(YamlEvent *event);\n> > > +\n> > > +\tprivate:\n> > > +\t\tvoid logError();\n> > > +\n> > > +\t\tyaml_emitter_t emitter_;\n> > > +\t};\n> > > +\n> > > +\tYamlEmitter() = default;\n> > > +\n> > > +\tvoid init();\n> > > +\n> > > +\tstd::unique_ptr<File> file_;\n> > > +\tEmitter emitter_;\n> > > +};\n> > > +\n> > > +class YamlOutput\n> > > +{\n> > > +public:\n> >\n> > Shouldn't you disable copying of this class ?\n> \n> yup\n> \n> > > +\tvirtual ~YamlOutput() = default;\n> > > +\n> > > +\tvoid close()\n> > > +\t{\n> > > +\t\tclosed_ = true;\n> > > +\t}\n> >\n> > Can we avoid this and close on destruction only ? Having an explicit\n> > close is error-prone, what if someone calls one of the functions below\n> > after calling close() ? The API should make it very hard, if not\n> > impossible, to do the wrong thing.\n> \n> Eh\n> \n> my idea was ideed to rely on object's destruction to emit the correct\n> yaml events, then I thought it was worth to allow users to close a\n> sequence/mapping earlier if they didn't want to enforce this through\n> the object lifetime. I think I had a use case for this, but maybe it\n> has gone during the development, I'll try remove close() and solely\n> rely on object's lifetime for the correct events emission.\n\nI'd like to at least hear about shortcomings of this option, if there\nare any.\n\nOne thing I'm concerned about is how this kind of pattern is invalid:\n\n\tYamlList &parent = ...;\n\n\tYamlList child = parent.list();\n\n\tparent.scalar(\"value\");\n\n\tchild.scalar(\"value\");\n\nIt would be nice if we could design an API that would prevent this at\ncompile time. I don't know if that's possible (with reasonable\nimplementation and runtime complexity).\n\n> > > +\n> > > +\tstd::unique_ptr<YamlScalar> scalar();\n> > > +\tstd::unique_ptr<YamlDict> dict();\n> > > +\tstd::unique_ptr<YamlList> list();\n> >\n> > In many cases the YamlScalar, YamlDict and YamlList classes will be\n> > local to the caller function. It would be best to avoid dynamic heap\n> > allocation. You could do so by returning values here. Same below. You\n> > will likely need to implement move constructors and operators.\n> \n> good idea, I'll try\n> \n> > > +\n> > > +protected:\n> > > +\tYamlOutput(YamlEmitter *emitter)\n> > > +\t\t: emitter_(emitter), closed_(false)\n> > > +\t{\n> > > +\t}\n> > > +\n> > > +\tint emitScalar(std::string_view scalar);\n> > > +\tint emitMappingStart();\n> > > +\tint emitMappingEnd();\n> > > +\tint emitSequenceStart();\n> > > +\tint emitSequenceEnd();\n> > > +\n> > > +\tYamlEmitter *emitter_;\n> > > +\n> > > +\tbool closed_;\n> > > +};\n> > > +\n> > > +class YamlRoot : public YamlOutput\n> > > +{\n> > > +public:\n> > > +\t~YamlRoot();\n> > > +\tvoid close() {}\n> > > +\n> > > +\tstd::unique_ptr<YamlList> list();\n> > > +\tstd::unique_ptr<YamlDict> dict();\n> > > +\tvoid scalar(std::string_view scalar);\n> > > +\n> > > +private:\n> > > +\tfriend class YamlEmitter;\n> > > +\n> > > +\tYamlRoot(YamlEmitter *emitter)\n> > > +\t\t: YamlOutput(emitter)\n> > > +\t{\n> > > +\t\temitterRoot_ = std::unique_ptr<YamlEmitter>(emitter);\n> > > +\t}\n> > > +\n> > > +\tstd::unique_ptr<YamlEmitter> emitterRoot_;\n> > > +};\n> > > +\n> > > +class YamlScalar : public YamlOutput\n> > > +{\n> > > +public:\n> > > +\t~YamlScalar() = default;\n> > > +\n> > > +\tvoid close() {}\n> > > +\n> > > +\tvoid operator=(const Orientation &orientation);\n> >\n> > I don't think this should be a member of the YamlScalar class. You\n> > should convert the Orientation to a string in the caller.\n> \n> This is something I wanted indeed feedbacks on. I see it two ways,\n> either we augment this interface with libcamera types as we need them\n> to ease for users of this class to emit them, or we require users to\n> convert to string for. I understand your preference is for the latter\n> option.\n\nOne option would be to add an operator<<() implementation to output a\nlibcamera type to a YamlScaler. That way the YamlScalar implementation\nwould not be dependent on specific types, and each type could provide\nits own implementation. Let's see how the callers look like, it may be\noverkill if we only use the YamlEmitter in a single place.\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> > > +\t~YamlList();\n> > > +\n> > > +\tvoid close();\n> > > +\n> > > +\tstd::unique_ptr<YamlList> list();\n> > > +\tstd::unique_ptr<YamlDict> 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> > > +\t\t private std::unordered_map<std::string_view,\n> > > +\t\t\t\t\t    std::unique_ptr<YamlScalar>>\n> >\n> > Why do you need to store content ? That doesn't sound right,the\n> > emitter's job is to emit YAML, we shouldn't need to store anything.\n> \n> because I wanted to override operator[] for YamlDict and it returns a\n> refernce, so I need to have a storage. I can try to remove it.\n\nIt would be nice to remove it. Let's discuss the drawbacks if you run\ninto issues.\n\n> > > +{\n> > > +public:\n> > > +\tusing Map = std::unordered_map<std::string_view, YamlScalar>;\n> > > +\n> > > +\t~YamlDict();\n> > > +\n> > > +\tvoid close();\n> > > +\n> > > +\tstd::unique_ptr<YamlList> list(std::string_view key);\n> > > +\tstd::unique_ptr<YamlDict> dict(std::string_view key);\n> > > +\n> > > +\tYamlScalar &operator[](const Map::key_type &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..1f7651ffcb24\n> > > --- /dev/null\n> > > +++ b/src/libcamera/yaml_emitter.cpp\n> > > @@ -0,0 +1,427 @@\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 <libcamera/base/log.h>\n> > > +\n> > > +/**\n> > > + * \\file yaml_emitter.h\n> > > + * \\brief A YAML emitter helper\n> > > + *\n> > > + * The YAML Emitter helpers allows users to emit output in YAML format.\n> > > + *\n> > > + * To emit YAML users of this classes should create a root node with\n> > > + *\n> > > + * \\code\n> > > +\tstd::string filePath(\"...\");\n> > > +\tauto root = YamlEmitter::root(filePath);\n> > > +   \\endcode\n> > > + *\n> > > + * A YamlRoot implements RAII-style handling of YAML sequence and document\n> > > + * events handling. Creating a YamlRoot emits the STREAM_START and DOC_START\n> > > + * events. Once a YamlRoot gets deleted the DOC_SEND and STREAM_END events\n> > > + * gets automatically deleted.\n> > > + *\n> > > + * Once a root node has been created it is possible to populate it with\n> > > + * scalars, list or dictionaries.\n> > > + *\n> > > + * YamlList and YamlDict can only be created wrapped in unique_ptr<> instances,\n> > > + * to implement a RAII-style handling of YAML of lists and dictionaries.\n> > > + * Creating a YamlList and a YamlDict emits the YAML sequence and mapping start\n> > > + * events. Once an instance gets deleted, the sequence and mapping gets\n> > > + * automatically emitted. If a list of dictionary needs to be closed before\n> > > + * it gets deleted, it can be explicitly closed with the close() function.\n> > > + *\n> > > + * A YamlScalar is a simpler object that can be assigned to different types\n> > > + * to emit them as strings.\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<File *>(data);\n> > > +\n> > > +\tSpan<unsigned char> buf{ buffer, size };\n> > > +\tssize_t ret = file->write(buf);\n> > > +\tif (ret < 0) {\n> > > +\t\tret = errno;\n> > > +\t\tLOG(YamlEmitter, Error) << \"Write error : \" << strerror(ret);\n> > > +\t\treturn 0;\n> > > +\t}\n> > > +\n> > > +\treturn 1;\n> > > +}\n> > > +\n> > > +} /* namespace */\n> > > +\n> > > +/**\n> > > + * \\class YamlEvent\n> > > + *\n> > > + * Class that represents a yaml_event that automatically cleans-up on\n> > > + * destruction.\n> > > + */\n> > > +class YamlEvent\n> > > +{\n> > > +public:\n> > > +\tstatic std::unique_ptr<YamlEvent> create();\n> > > +\n> > > +\tconst yaml_event_t *event() const\n> > > +\t{\n> > > +\t\treturn &event_;\n> > > +\t}\n> > > +\n> > > +\tyaml_event_t *event()\n> > > +\t{\n> > > +\t\treturn &event_;\n> > > +\t}\n> > > +\n> > > +private:\n> > > +\tYamlEvent() = default;\n> > > +\n> > > +\tyaml_event_t event_;\n> > > +};\n> > > +\n> > > +std::unique_ptr<YamlEvent> YamlEvent::create()\n> > > +{\n> > > +\tstruct Deleter : std::default_delete<YamlEvent> {\n> > > +\t\tvoid operator()(YamlEvent *yamlEvent)\n> > > +\t\t{\n> > > +\t\t\tyaml_event_delete(yamlEvent->event());\n> > > +\t\t}\n> > > +\t};\n> > > +\n> > > +\treturn std::unique_ptr<YamlEvent>(new YamlEvent(), Deleter());\n> >\n> > Here too, we should avoid dynamic allocations on the heap. I don't think\n> > you need a custom deleter, you can simply call yaml_event_delete() in\n> > the destructor of the YamlEvent class, and you then don't need to use\n> > unique pointers.\n> >\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlEmitter\n> > > + *\n> > > + * Yaml Emitter entry point. Allows to create a YamlRoot object that users\n> > > + * can populate.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Create a YamlRoot that applications can start populating with YamlOutput\n> > > + * \\param[in] path The YAML output file path\n> > > + * \\return A unique pointer to a YamlRoot\n> > > + */\n> > > +std::unique_ptr<YamlRoot> YamlEmitter::root(std::string_view path)\n> > > +{\n> > > +\tYamlEmitter *emitter = new YamlEmitter();\n> > > +\n> > > +\tstd::string filePath(path);\n> > > +\temitter->file_ = std::make_unique<File>(filePath);\n> > > +\temitter->file_->open(File::OpenModeFlag::WriteOnly);\n> > > +\n> > > +\temitter->init();\n> > > +\n> > > +\tYamlRoot *root = new YamlRoot(emitter);\n> > > +\treturn std::unique_ptr<YamlRoot>(root);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Emit a YamlEvent\n> > > + */\n> > > +int YamlEmitter::emit(YamlEvent *event)\n> > > +{\n> > > +\treturn emitter_.emit(event);\n> > > +}\n> > > +\n> > > +void YamlEmitter::init()\n> > > +{\n> > > +\temitter_.init(file_.get());\n> > > +\n> > > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > > +\n> > > +\tyaml_stream_start_event_initialize(event->event(), YAML_UTF8_ENCODING);\n> > > +\temitter_.emit(event.get());\n> > > +\n> > > +\tyaml_document_start_event_initialize(event->event(), NULL, NULL,\n> > > +\t\t\t\t\t     NULL, 0);\n> > > +\temitter_.emit(event.get());\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlEmitter::Emitter\n> > > + *\n> > > + * yaml_emitter_t wrapper. Initialize the yaml_emitter_t on creation allows\n> > > + * YamlOutput classes to emit events.\n> > > + */\n> > > +\n> > > +YamlEmitter::Emitter::~Emitter()\n> > > +{\n> > > +\tyaml_emitter_delete(&emitter_);\n> > > +}\n> > > +\n> > > +void YamlEmitter::Emitter::init(File *file)\n> > > +{\n> > > +\tyaml_emitter_initialize(&emitter_);\n> > > +\tyaml_emitter_set_output(&emitter_, yamlWrite, file);\n> > > +}\n> > > +\n> > > +void YamlEmitter::Emitter::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> > > +\t\tbreak;\n> > > +\t}\n> > > +}\n> > > +\n> > > +int YamlEmitter::Emitter::emit(YamlEvent *event)\n> > > +{\n> > > +\tint ret = yaml_emitter_emit(&emitter_, event->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 YamlScalar, YamlList and YamlDict\n> > > + * are derived.\n> > > + *\n> > > + * The YamlOutput class provides functions to create a scalar, a list or a\n> > > + * dictionary.\n> > > + *\n> > > + * The class cannot be instantiated directly by applications.\n> > > + */\n> > > +\n> > > +std::unique_ptr<YamlScalar> YamlOutput::scalar()\n> > > +{\n> > > +\treturn std::unique_ptr<YamlScalar>(new YamlScalar(emitter_));\n> > > +}\n> > > +\n> > > +std::unique_ptr<YamlDict> YamlOutput::dict()\n> > > +{\n> > > +\treturn std::unique_ptr<YamlDict>(new YamlDict(emitter_));\n> > > +}\n> > > +\n> > > +std::unique_ptr<YamlList> YamlOutput::list()\n> > > +{\n> > > +\treturn std::unique_ptr<YamlList>(new YamlList(emitter_));\n> > > +}\n> > > +\n> > > +int YamlOutput::emitScalar(std::string_view scalar)\n> > > +{\n> > > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > > +\n> > > +\tconst unsigned char *value = reinterpret_cast<const unsigned char *>\n> > > +\t\t\t\t     (scalar.data());\n> > > +\tyaml_scalar_event_initialize(event->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(event.get());\n> > > +}\n> > > +\n> > > +int YamlOutput::emitMappingStart()\n> > > +{\n> > > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > > +\tyaml_mapping_start_event_initialize(event->event(), NULL, NULL,\n> > > +\t\t\t\t\t    true, YAML_BLOCK_MAPPING_STYLE);\n> > > +\treturn emitter_->emit(event.get());\n> > > +}\n> > > +\n> > > +int YamlOutput::emitMappingEnd()\n> > > +{\n> > > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > > +\tyaml_mapping_end_event_initialize(event->event());\n> > > +\treturn emitter_->emit(event.get());\n> > > +}\n> > > +\n> > > +int YamlOutput::emitSequenceStart()\n> > > +{\n> > > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > > +\tyaml_sequence_start_event_initialize(event->event(), NULL, NULL, true,\n> > > +\t\t\t\t\t     YAML_BLOCK_SEQUENCE_STYLE);\n> > > +\treturn emitter_->emit(event.get());\n> > > +}\n> > > +\n> > > +int YamlOutput::emitSequenceEnd()\n> > > +{\n> > > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > > +\tyaml_sequence_end_event_initialize(event->event());\n> > > +\treturn emitter_->emit(event.get());\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlRoot\n> > > + *\n> > > + * Yaml root node. A root node can be populated with a scalar, a list or a dict.\n> > > + */\n> > > +\n> > > +YamlRoot::~YamlRoot()\n> > > +{\n> > > +\tstd::unique_ptr<YamlEvent> event = YamlEvent::create();\n> > > +\n> > > +\tyaml_document_end_event_initialize(event->event(), 0);\n> > > +\temitterRoot_->emit(event.get());\n> > > +\n> > > +\tyaml_stream_end_event_initialize(event->event());\n> > > +\temitterRoot_->emit(event.get());\n> > > +}\n> > > +\n> > > +std::unique_ptr<YamlDict> YamlRoot::dict()\n> > > +{\n> > > +\temitMappingStart();\n> > > +\n> > > +\treturn YamlOutput::dict();\n> > > +}\n> > > +\n> > > +std::unique_ptr<YamlList> YamlRoot::list()\n> > > +{\n> > > +\temitSequenceStart();\n> > > +\n> > > +\treturn YamlOutput::list();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlScalar\n> > > + *\n> > > + * A YamlScalar can be assigned to an std::string_view and other libcamera\n> > > + * types to emit them as YAML scalars.\n> > > + */\n> > > +\n> > > +YamlScalar::YamlScalar(YamlEmitter *emitter)\n> > > +\t: YamlOutput(emitter)\n> > > +{\n> > > +}\n> > > +\n> > > +void YamlScalar::operator=(const libcamera::Orientation &orientation)\n> > > +{\n> > > +\tstd::stringstream o;\n> > > +\to << orientation;\n> > > +\n> > > +\temitScalar(o.str());\n> > > +}\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 scalar and allows to create other lists\n> > > + * and dictionaries.\n> > > + */\n> > > +\n> > > +YamlList::YamlList(YamlEmitter *emitter)\n> > > +\t: YamlOutput(emitter)\n> > > +{\n> > > +}\n> > > +\n> > > +YamlList::~YamlList()\n> > > +{\n> > > +\tif (!closed_)\n> > > +\t\tclose();\n> > > +}\n> > > +\n> > > +void YamlList::close()\n> > > +{\n> > > +\temitSequenceEnd();\n> > > +\tYamlOutput::close();\n> > > +}\n> > > +\n> > > +void YamlList::scalar(std::string_view scalar)\n> > > +{\n> > > +\temitScalar(scalar);\n> > > +}\n> > > +\n> > > +std::unique_ptr<YamlList> YamlList::list()\n> > > +{\n> > > +\temitSequenceStart();\n> > > +\n> > > +\treturn YamlOutput::list();\n> > > +}\n> > > +\n> > > +std::unique_ptr<YamlDict> YamlList::dict()\n> > > +{\n> > > +\temitMappingStart();\n> > > +\n> > > +\treturn YamlOutput::dict();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class YamlDict\n> > > + *\n> > > + * A YamlDict derives an unordered_map that maps strings to YAML scalar.\n> > > + *\n> > > + * A YamlDict can be populated with scalars using operator[] and allows to\n> > > + * create other lists and dictionaries.\n> > > + */\n> > > +\n> > > +YamlDict::YamlDict(YamlEmitter *emitter)\n> > > +\t: YamlOutput(emitter)\n> > > +{\n> > > +}\n> > > +\n> > > +YamlDict::~YamlDict()\n> > > +{\n> > > +\tif (!closed_)\n> > > +\t\tclose();\n> > > +\n> > > +\tclear();\n> > > +}\n> > > +\n> > > +void YamlDict::close()\n> > > +{\n> > > +\temitMappingEnd();\n> > > +\tYamlOutput::close();\n> > > +}\n> > > +\n> > > +std::unique_ptr<YamlList> YamlDict::list(std::string_view key)\n> > > +{\n> > > +\temitScalar(key);\n> > > +\temitSequenceStart();\n> > > +\n> > > +\treturn YamlOutput::list();\n> > > +}\n> > > +\n> > > +std::unique_ptr<YamlDict> YamlDict::dict(std::string_view key)\n> > > +{\n> > > +\temitScalar(key);\n> > > +\temitMappingStart();\n> > > +\n> > > +\treturn YamlOutput::dict();\n> > > +}\n> > > +\n> > > +YamlScalar &YamlDict::operator[](const YamlDict::Map::key_type &key)\n> > > +{\n> > > +\templace(key, YamlOutput::scalar());\n> > > +\temitScalar(key);\n> > > +\n> > > +\treturn *at(key);\n> > > +}\n> > > +\n> > > +} /* namespace libcamera */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8F206C326C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 16 Oct 2024 22:47:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9001065383;\n\tThu, 17 Oct 2024 00:47:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DBA1260537\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 17 Oct 2024 00:47:53 +0200 (CEST)","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 EC861581;\n\tThu, 17 Oct 2024 00:46:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"IF1ikbe0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729118771;\n\tbh=of46w114pkWO/xb6rs4v1KdbPUM8yzjvVWIJVO8Ysro=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IF1ikbe0QZJCVGWSsu6ptIxGHWRf1Thjzy4tNEaAtXqRwarZaZdr654qga/UJIAog\n\t9ICwuEkcsfTpT7d+O7PRIIfsMUaRV9oBvkFGlEcEEryHoS8HQZbP/U2JVl3LK7HiHG\n\t0nQLk5PQreaLD4NeqyFz5ymyrFF7bQd8jatdHcqo=","Date":"Thu, 17 Oct 2024 01:47:49 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC 3/4] libcamera: Implement YamlEmitter","Message-ID":"<20241016224749.GI30496@pendragon.ideasonboard.com>","References":"<20241014095937.24924-1-jacopo.mondi@ideasonboard.com>\n\t<20241014095937.24924-4-jacopo.mondi@ideasonboard.com>\n\t<20241015102133.GB5682@pendragon.ideasonboard.com>\n\t<n5c7pmelrwnvonktaujclhnui4sshzcm6rdcpdxahlm23v5uo2@ijx7y36cwnch>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<n5c7pmelrwnvonktaujclhnui4sshzcm6rdcpdxahlm23v5uo2@ijx7y36cwnch>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]