[FFmpeg-devel] [PATCH] ffprobe: add/extend disposition printing support

Stefano Sabatini stefasab at gmail.com
Sat Sep 29 10:47:29 CEST 2012


On date Saturday 2012-09-29 00:12:01 +0200, Clément Bœsch encoded:
> On Fri, Sep 28, 2012 at 07:32:07PM +0200, Stefano Sabatini wrote:
> > This generalizes the previous work on disposition printing.
> > 
> > Disposition flags are shown in a dedicated section, which should improve
> > output intellegibility, extensibility and filtering operations.
> > 
> 
> \o/
> 
> > This breaks output syntax for the recently introduced disposition
> > printing.
> > ---
> >  doc/ffprobe.xsd                |   18 +++++++++++--
> >  ffprobe.c                      |   28 ++++++++++++++++----
> >  tests/ref/fate/ffprobe_compact |    6 ++--
> >  tests/ref/fate/ffprobe_csv     |    6 ++--
> >  tests/ref/fate/ffprobe_default |   41 +++++++++++++++++++++++++------
> >  tests/ref/fate/ffprobe_flat    |   41 +++++++++++++++++++++++++------
> >  tests/ref/fate/ffprobe_ini     |   47 +++++++++++++++++++++++++++++------
> >  tests/ref/fate/ffprobe_json    |   53 +++++++++++++++++++++++++++++++--------
> >  tests/ref/fate/ffprobe_xml     |   12 +++++++--
> >  9 files changed, 199 insertions(+), 53 deletions(-)
> > 
> > diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd
> > index 403d59e..193c537 100644
> > --- a/doc/ffprobe.xsd
> > +++ b/doc/ffprobe.xsd
> > @@ -86,9 +86,24 @@
> >          </xsd:sequence>
> >      </xsd:complexType>
> >  
> > +    <xsd:complexType name="streamDispositionType">
> > +      <xsd:attribute name="default"          type="xsd:int" use="required" />
> > +      <xsd:attribute name="dub"              type="xsd:int" use="required" />
> > +      <xsd:attribute name="original"         type="xsd:int" use="required" />
> > +      <xsd:attribute name="comment"          type="xsd:int" use="required" />
> > +      <xsd:attribute name="lyrics"           type="xsd:int" use="required" />
> > +      <xsd:attribute name="karaoke"          type="xsd:int" use="required" />
> > +      <xsd:attribute name="forced"           type="xsd:int" use="required" />
> > +      <xsd:attribute name="hearing_impaired" type="xsd:int" use="required" />
> > +      <xsd:attribute name="visual_impaired"  type="xsd:int" use="required" />
> > +      <xsd:attribute name="clean_effects"    type="xsd:int" use="required" />
> > +      <xsd:attribute name="attached_pic"     type="xsd:int" use="required" />
> > +    </xsd:complexType>
> > +
> >      <xsd:complexType name="streamType">
> >        <xsd:sequence>
> >          <xsd:element name="tag" type="ffprobe:tagType" minOccurs="0" maxOccurs="unbounded"/>
> > +        <xsd:element name="disposition" type="ffprobe:streamDispositionType" minOccurs="0" maxOccurs="1"/>
> >        </xsd:sequence>
> >  
> >        <xsd:attribute name="index"            type="xsd:int" use="required"/>
> > @@ -100,8 +115,6 @@
> >        <xsd:attribute name="codec_tag"        type="xsd:string" use="required"/>
> >        <xsd:attribute name="codec_tag_string" type="xsd:string" use="required"/>
> >        <xsd:attribute name="extradata"        type="xsd:string" />
> > -      <xsd:attribute name="default"          type="xsd:int" use="required"/>
> > -      <xsd:attribute name="forced"           type="xsd:int" use="required"/>
> >  
> >        <!-- video attributes -->
> >        <xsd:attribute name="width"                type="xsd:int"/>
> > @@ -112,7 +125,6 @@
> >        <xsd:attribute name="pix_fmt"              type="xsd:string"/>
> >        <xsd:attribute name="level"                type="xsd:int"/>
> >        <xsd:attribute name="timecode"             type="xsd:string"/>
> > -      <xsd:attribute name="attached_pic"         type="xsd:int"/>
> >  
> >        <!-- audio attributes -->
> >        <xsd:attribute name="sample_fmt"       type="xsd:string"/>
> > diff --git a/ffprobe.c b/ffprobe.c
> > index fc7e9ff..217aefd 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -99,6 +99,7 @@ typedef enum {
> >      SECTION_ID_PROGRAM_VERSION,
> >      SECTION_ID_ROOT,
> >      SECTION_ID_STREAM,
> > +    SECTION_ID_STREAM_DISPOSITION,
> >      SECTION_ID_STREAMS,
> >      SECTION_ID_STREAM_TAGS
> >  } SectionID;
> > @@ -118,6 +119,7 @@ static const struct section sections[] = {
> >      [SECTION_ID_PROGRAM_VERSION] = { SECTION_ID_PROGRAM_VERSION, "program_version", 0 },
> >      [SECTION_ID_ROOT] = { SECTION_ID_ROOT, "root", SECTION_FLAG_IS_WRAPPER },
> >      [SECTION_ID_STREAM] = { SECTION_ID_STREAM, "stream", 0 },
> > +    [SECTION_ID_STREAM_DISPOSITION] = { SECTION_ID_STREAM_DISPOSITION, "disposition", 0 },
> 

> Why not use the SECTION_FLAG_HAS_VARIABLE_FIELDS/.element_name thing?
> Wouldn't it break some outputs if we add more flags?

This is not what that flag means. In this case we have fixed variable
fields.

Note that this makes a difference only in the case of the XML writer,
if the fields are fixed then it can render it with tag attributes,
otherwise it needs to wrap the variable key/values in a specific tag
(for which an "element_name" is mandatory).
 
> >      [SECTION_ID_STREAMS] = { SECTION_ID_STREAMS, "streams", SECTION_FLAG_IS_ARRAY },
> >      [SECTION_ID_STREAM_TAGS] = { SECTION_ID_STREAM_TAGS, "tags", SECTION_FLAG_HAS_VARIABLE_FIELDS, .element_name = "tag" },
> >  };
> > @@ -1681,10 +1683,6 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> >          print_str("codec_tag_string",    val_str);
> >          print_fmt("codec_tag", "0x%04x", dec_ctx->codec_tag);
> >  
> > -        /* Print useful disposition */
> > -        print_int("default", !!(stream->disposition & AV_DISPOSITION_DEFAULT));
> > -        print_int("forced", !!(stream->disposition & AV_DISPOSITION_FORCED));
> > -
> >          switch (dec_ctx->codec_type) {
> >          case AVMEDIA_TYPE_VIDEO:
> >              print_int("width",        dec_ctx->width);
> > @@ -1713,8 +1711,6 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> >              } else {
> >                  print_str_opt("timecode", "N/A");
> >              }
> > -            print_int("attached_pic",
> > -                      !!(stream->disposition & AV_DISPOSITION_ATTACHED_PIC));
> >              break;
> >  
> >          case AVMEDIA_TYPE_AUDIO:
> > @@ -1761,6 +1757,26 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> >      if (do_show_data)
> >          writer_print_data(w, "extradata", dec_ctx->extradata,
> >                                            dec_ctx->extradata_size);
> > +
> > +    /* Print disposition information */
> > +    writer_print_section_header(w, SECTION_ID_STREAM_DISPOSITION);
> > +#define PRINT_DISPOSITION(flagname, name) do {                          \
> > +        print_int(name, !!(stream->disposition & AV_DISPOSITION_##flagname)); \
> 
> nit: weird '\' positioning

Locally fixed.
 
> > +    } while (0)
> > +
> > +    PRINT_DISPOSITION(DEFAULT,          "default");
> > +    PRINT_DISPOSITION(DUB,              "dub");
> > +    PRINT_DISPOSITION(ORIGINAL,         "original");
> > +    PRINT_DISPOSITION(COMMENT,          "comment");
> > +    PRINT_DISPOSITION(LYRICS,           "lyrics");
> > +    PRINT_DISPOSITION(KARAOKE,          "karaoke");
> > +    PRINT_DISPOSITION(FORCED,           "forced");
> > +    PRINT_DISPOSITION(HEARING_IMPAIRED, "hearing_impaired");
> > +    PRINT_DISPOSITION(VISUAL_IMPAIRED,  "visual_impaired");
> > +    PRINT_DISPOSITION(CLEAN_EFFECTS,    "clean_effects");
> > +    PRINT_DISPOSITION(ATTACHED_PIC,     "attached_pic");
> > +    writer_print_section_footer(w);
> > +
> 
> Would it make sense to use WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS to avoid
> printing a lot of zero entries when unnecessary?

I thought about that. My conclusion is that a missing field means
"unspecified/unavailable", in this case all the values are specified
and are set to 0/1.

> 
> [...]
> >  filename=tests/data/ffprobe-test.nut
> > diff --git a/tests/ref/fate/ffprobe_flat b/tests/ref/fate/ffprobe_flat
> > index e9e2f87..5a0b6d8 100644
> > --- a/tests/ref/fate/ffprobe_flat
> > +++ b/tests/ref/fate/ffprobe_flat
> > @@ -425,8 +425,6 @@ streams.stream.0.codec_type="audio"
> >  streams.stream.0.codec_time_base="1/44100"
> >  streams.stream.0.codec_tag_string="[1][0][0][0]"
> >  streams.stream.0.codec_tag="0x0001"
> > -streams.stream.0.default=0
> > -streams.stream.0.forced=0
> >  streams.stream.0.sample_fmt="s16"
> >  streams.stream.0.sample_rate="44100"
> >  streams.stream.0.channels=1
> > @@ -443,6 +441,17 @@ streams.stream.0.bit_rate="705600"
> >  streams.stream.0.nb_frames="N/A"
> >  streams.stream.0.nb_read_frames="6"
> >  streams.stream.0.nb_read_packets="6"
> > +streams.stream.0.disposition.default=0
> > +streams.stream.0.disposition.dub=0
> > +streams.stream.0.disposition.original=0
> > +streams.stream.0.disposition.comment=0
> > +streams.stream.0.disposition.lyrics=0
> > +streams.stream.0.disposition.karaoke=0
> > +streams.stream.0.disposition.forced=0
> > +streams.stream.0.disposition.hearing_impaired=0
> > +streams.stream.0.disposition.visual_impaired=0
> > +streams.stream.0.disposition.clean_effects=0
> > +streams.stream.0.disposition.attached_pic=0
> 
> Sexy :)
> 
> [...]
> 
> Looks great, thanks.

:)
-- 
FFmpeg = Formidable & Friendly Multimedia Pitiful Enlightening Guide


More information about the ffmpeg-devel mailing list