[FFmpeg-devel] [PATCH] ffprobe: generalize nesting model for the XML writer

[Stefano Sabatini]

On date Saturday 2012-09-29 00:00:08 +0200, Clément Bœsch encoded:
> On Fri, Sep 28, 2012 at 07:31:57PM +0200, Stefano Sabatini wrote:
> > Do not make use of ad-hoc "tags" code, introduce a new section flag
> > SECTION_FLAG_HAS_VARIABLE_FIELDS to deal with the tags in a
> > content-agnostic way.
> > 
> > This is required by the pending disposition change.
> > ---
> >  ffprobe.c |   23 ++++++++++++-----------
> >  1 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/ffprobe.c b/ffprobe.c
> > index 153ed8c..fc7e9ff 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -78,6 +78,7 @@ struct section {
> >  
> >  #define SECTION_FLAG_IS_WRAPPER      1 ///< the section only contains other sections, but has no data at its own level
> >  #define SECTION_FLAG_IS_ARRAY        2 ///< the section contains an array of elements of the same type
> > +#define SECTION_FLAG_HAS_VARIABLE_FIELDS 4 ///< the section contains variable fields
> >      int flags;
> >      const char *element_name; ///< name of the contained element, if provided
> >  };
> > @@ -105,10 +106,10 @@ typedef enum {
> >  static const struct section sections[] = {
> >      [SECTION_ID_ERROR] = { SECTION_ID_ERROR, "error", 0 },
> >      [SECTION_ID_FORMAT] = { SECTION_ID_FORMAT, "format", 0 },
> > -    [SECTION_ID_FORMAT_TAGS] = { SECTION_ID_FORMAT_TAGS, "tags", 0, .element_name = "tag" },
> > +    [SECTION_ID_FORMAT_TAGS] = { SECTION_ID_FORMAT_TAGS, "tags", SECTION_FLAG_HAS_VARIABLE_FIELDS, .element_name = "tag" },
> >      [SECTION_ID_FRAME] = { SECTION_ID_FRAME, "frame", 0 },
> >      [SECTION_ID_FRAMES] = { SECTION_ID_FRAMES, "frames", SECTION_FLAG_IS_ARRAY },
> > -    [SECTION_ID_FRAME_TAGS] = { SECTION_ID_FRAME_TAGS, "tags", 0, .element_name = "tag" },
> > +    [SECTION_ID_FRAME_TAGS] = { SECTION_ID_FRAME_TAGS, "tags", SECTION_FLAG_HAS_VARIABLE_FIELDS, .element_name = "tag" },
> >      [SECTION_ID_LIBRARY_VERSION] = { SECTION_ID_LIBRARY_VERSION, "library_version", 0 },
> >      [SECTION_ID_LIBRARY_VERSIONS] = { SECTION_ID_LIBRARY_VERSIONS, "library_versions", SECTION_FLAG_IS_ARRAY },
> >      [SECTION_ID_PACKET] = { SECTION_ID_PACKET, "packet", 0 },
> > @@ -118,7 +119,7 @@ static const struct section sections[] = {
> >      [SECTION_ID_ROOT] = { SECTION_ID_ROOT, "root", SECTION_FLAG_IS_WRAPPER },
> >      [SECTION_ID_STREAM] = { SECTION_ID_STREAM, "stream", 0 },
> >      [SECTION_ID_STREAMS] = { SECTION_ID_STREAMS, "streams", SECTION_FLAG_IS_ARRAY },
> > -    [SECTION_ID_STREAM_TAGS] = { SECTION_ID_STREAM_TAGS, "tags", 0, .element_name = "tag" },
> > +    [SECTION_ID_STREAM_TAGS] = { SECTION_ID_STREAM_TAGS, "tags", SECTION_FLAG_HAS_VARIABLE_FIELDS, .element_name = "tag" },
> 
> Isn't this just related to the presence of element_name field? (At least
> the flag seems to imply the presence of element_name). If so, wouldn't it
> make sense to simply check for section->element_name?

No they are two logical entitities, even if related. A section may
have an element_name (which is used by the default/compact writers),
but not being with variable fields (for example it could have been
dispositions->disposition).

Also for consistency, it is better to express section capabilities
with a flag, rather than rely on the presence of optional fields.
-- 
FFmpeg = Fabulous and Fostering Mind-dumbing Pure Eccentric Gnome