[PATCH 1/3] libcamera: Implement YamlEmitter

[Barnabás Pőcze]

Hi


2024. november 29., péntek 19:24 keltezéssel, Jacopo Mondi <jacopo.mondi at ideasonboard.com> írta:

> Hi Barnabás
> 
> On Fri, Nov 29, 2024 at 05:28:01PM +0000, Barnabás Pőcze wrote:
> > Hi
> >
> >
> > 2024. november 29., péntek 09:44 keltezéssel, Laurent Pinchart <laurent.pinchart at ideasonboard.com> írta:
> >
> > > On Fri, Nov 29, 2024 at 09:35:06AM +0100, Jacopo Mondi wrote:
> > > > On Wed, Nov 27, 2024 at 08:04:06AM +0200, Laurent Pinchart wrote:
> > > > > On Wed, Nov 06, 2024 at 06:58:51PM +0100, Jacopo Mondi wrote:
> > > > > > Implement a helpers to allow outputting text in YAML format.
> > > > > [...]
> > > > > > + *
> > > > > > + * To emit YAML users of the these helper classes create a root node with
> > > > > > + *
> > > > > > + * \code
> > > > > > +	std::string filePath("...");
> > > > > > +	auto root = YamlEmitter::root(filePath);
> > > > > > +   \endcode
> > > > > > + *
> > > > > > + * and start populating it with dictionaries and lists with the YamlRoot::dict()
> > > > > > + * and YamlRoot::list() functions.
> > > > >
> > > > > I would write "and start emitting dictionaries and lists", "populating"
> > > > > sounds more like a YAML writer.
> > > > >
> > > > ditto
> > > >
> > > > > > + *
> > > > > > + * The classes part of this file implement RAII-style handling of YAML
> > > > > > + * events. By creating a YamlList and YamlDict instance the associated YAML
> > > > > > + * sequence start and mapping start events are emitted and once the instances
> > > > > > + * gets destroyed the corresponding sequence end and mapping end events are
> > > > > > + * emitted.
> > > > > > + *
> > > > > > + * From an initialized YamlRoot instance is possible to create YAML list and
> > > > > > + * dictionaries.
> > > > > > + *
> > > > > > + * \code
> > > > > > +	YamlDict dict = root.dict();
> > > > > > +	YamlList list = root.list();
> > > > > > +   \endcode
> > > > > > + *
> > > > > > + * YamlDict instances can be populated with scalars associated with a key
> > > > > > + *
> > > > > > + * \code
> > > > > > +	dict["key"] = "value";
> > > > > > +   \endcode
> > > > > > + *
> > > > > > + * and it is possible to create lists and dictionaries, associated with a key
> > > > > > + *
> > > > > > + * \code
> > > > > > +	YamlDict subDict = dict.dict("newDict");
> > > > > > +	YamlList subList = dict.list("newList");
> > > > > > +   \endcode
> > > > > > + *
> > > > > > + * YamlList instances can be populated with scalar elements
> > > > > > + *
> > > > > > + * \code
> > > > > > +	list.scalar("x");
> > > > > > +	list.scalar("y");
> > > > > > +   \endcode
> > > > > > + *
> > > > > > + * and with dictionaries and lists too
> > > > > > + *
> > > > > > + * \code
> > > > > > +	YamlDict subDict = list.dict();
> > > > > > +	YamlList subList = list.list();
> > > > > > +   \endcode
> > > > >
> > > > > The biggest issue with the API is that it can be easily misused:
> > > > >
> > > > > 	YamlDict list1 = dict.list("foo");
> > > > > 	YamlDict list2 = dict.list("bar");
> > > > > 	list1.scalar("0");
> > > > >
> > > > > I'm still annoyed that I haven't found a neat way to prevent this. I
> > > > > wonder if we could make it a bit safer by at least catching incorrect
> > > > > usage at runtime. What I'm thinking is
> > > >
> > > > Yes, unfortunately I don't think we can easily catch this at build
> > > > time
> > > >
> > > > > - When creating a child, recording the child pointer in the parent, and
> > > > >   the parent pointer in the child
> > > > > - If the parent already has a child when a new child is created, reset
> > > > >   the parent pointer of the previous child. This makes the previous
> > > > >   child invalid.
> > > > > - Any invalidation of a child needs to be propagated to its children.
> > > > > - Any operation on an invalid child would be caught and logged.
> > > > > - When a child is destroyed, if its parent is not null, reset the child
> > > > >   pointer in the parent to null.
> > > > > - If a parent is destroyed while it has a child pointer, reset the
> > > > >   child's parent pointer to null and log an error.
> > > > >
> > > >
> > > > That should be enough, I wonder if we can make access to an invalid
> > > > childer a compiler error...
> > >
> > > I'd love to, but haven't found a nice way to do so :-S
> > > [...]
> >
> > My guess is that the C++ type system is "too weak" in this sense, to implement
> > the kind of checking you envision.
> >
> > If one is willing to trade indentation for a smaller chance of misuse, then one could:
> > (Or at least I think such an API where the structure of the resulting YAML file
> >  is in a sense encapsulated in the layout of the source code leads to fewer misuses.)
> >
> >    YamlDict root;
> >
> >    root.dict("foo", [](auto& d) {
> >      d.list("x", [](auto& l) {
> >        l.scalar(1);
> >        l.scalar(true);
> >      });
> >      d.scalar("y", 42);
> >    });
> >
> >    root.list("bar", [](auto&) {
> >      ...
> >    });
> >
> >    /* the methods could return an appropriate reference to make some kind of chaining possible: */
> >
> >    YamlDict()
> >      .dict("foo", [](auto& d) {
> >         d
> >           .list("x", [](auto& l) {
> >              l.scalar(1)
> >               .scalar(true);
> >           })
> >           .scalar("y", 42);
> >       })
> >      .list("bar", [](auto&) { ... });
> >
> > Or one could even go fully in on some kind of chaining:
> >
> > YamlDict()
> > 	.dict("foo")
> > 		.list("x")
> > 			.scalar(1)
> > 			.scalar(true)
> > 		.finish() /* end list */
> > 		.scalar()
> > 	.finish() /* end dict */
> > 	.list("bar")
> > 		...
> >
> >
> > I am not sure if this has already been considered, sorry if it has been.
> 
> It hasn't, thanks for the input.
> 
> The problem I see, as you can see in the next patches, is that we need
> to emit yaml, in example, every frame, programmatically. I don't think
> the above can be constructed iteratively, could it ?
> [...]

Ahh, sorry, you're right. One item is added to a list in every call of `dumpRequest()`.
In this case there does not seem to be a good way to use the type system to
enforce much of anything without runtime checking.

YAML allows multiple documents in a stream, so theoretically every request could
be described by its own separate document, which would avoid the need for keeping
unterminated YAML structures around between calls. But I am not sure what kind
of challenges that would bring (especially on the parsing side), and if it is
worth it.


Regards,
Barnabás Pőcze