[FFmpeg-devel] [PATCH v2] ffprobe: Add a few common disposition cases

[Clément Bœsch]

On Tue, Sep 18, 2012 at 11:24:31PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2012-09-18 16:12:01 -0400, Derek Buitenhuis encoded:
> > This info is crucial in knowing which stream to pick in an
> > automated setup.
> > 
> > Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
> > ---
> > Since these must be printed unconditionally, I've kept attached_pic
> > under the video case.
> > ---
> >  doc/ffprobe.xsd |    3 +++
> >  ffprobe.c       |    6 ++++++
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd
> > index c9213de..b4887ae 100644
> > --- a/doc/ffprobe.xsd
> > +++ b/doc/ffprobe.xsd
> > @@ -100,6 +100,8 @@
> >        <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"/>
> > @@ -110,6 +112,7 @@
> >        <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 a0aee83..771c11d 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -1665,6 +1665,10 @@ 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);
> > @@ -1693,6 +1697,8 @@ 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;
> 
> After looking at the code, I see that disposition can have many
> possibly useful values:
> 
> #define AV_DISPOSITION_DEFAULT   0x0001
> #define AV_DISPOSITION_DUB       0x0002
> #define AV_DISPOSITION_ORIGINAL  0x0004
> #define AV_DISPOSITION_COMMENT   0x0008
> #define AV_DISPOSITION_LYRICS    0x0010
> #define AV_DISPOSITION_KARAOKE   0x0020
> #define AV_DISPOSITION_FORCED    0x0040
> #define AV_DISPOSITION_HEARING_IMPAIRED  0x0080  /**< stream for hearing impaired audiences */
> #define AV_DISPOSITION_VISUAL_IMPAIRED   0x0100  /**< stream for visual impaired audiences */
> #define AV_DISPOSITION_CLEAN_EFFECTS     0x0200  /**< stream without voice */
> #define AV_DISPOSITION_ATTACHED_PIC      0x0400
> 
> Having a single field for each separate value is going to be pretty
> verbose and would require manual updates, so I wonder what's the best
> way to show flags in this case and in general (may be useful to show
> codec properties for example).
> 
> It could be rendered as a string, something like:
> disposition=default+attached_pic+forced
> 
> On the other hand we don't have a clean way to convert numerical
> flags to a string (we could add options support to AVStream, and add a
> flags->string serializer, but that would require some effort and
> possibly an ABI break), and that would require more parsing on the
> scripting side (as you need to split the flags in the various
> components).
> 
> An intermediary solution could consist in a manual flags->int
> conversion, then we may add options support to AVStream and a generic
> av_opt_get_flags_string(), and rely on that when we will bump major.
> 
> What do people think?

I guess the sustainable solution will be to use your previous section work
and introduce a disposition section for the flag at some point if it's
worth the effort. But I'm not even sure from a user point of view it's
that interesting.

I believed the current patch was good enough and didn't think a more
complex solution was worth blocking it. The field default field is indeed
quite useful.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120919/776e919f/attachment.asc>