[FFmpeg-devel] [PATCH] ffprobe: add -select_streams option

[Stefano Sabatini]

On date Friday 2012-10-05 18:29:31 +0200, Clément Bœsch encoded:
> On Thu, Oct 04, 2012 at 05:40:44PM +0200, Stefano Sabatini wrote:
> > NIT: would -specify_streams be a better name?
> > 
> 
> As said on IRC, I prefer the -select_streams as it is in the current patch
> since "specify" sounds wrong: you're not asking ffprobe to specify
> anything. "filter" or "pick" might work, but "select" really is OK IMO.

+1
 
> > TODO: add Changelog entry
> > ---
> >  doc/ffprobe.texi |   15 +++++++++++++++
> >  ffprobe.c        |   29 +++++++++++++++++++++++++++--
> >  2 files changed, 42 insertions(+), 2 deletions(-)
> > 
> > diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> > index 58ea383..ee69fbf 100644
> > --- a/doc/ffprobe.texi
> > +++ b/doc/ffprobe.texi
> > @@ -94,6 +94,21 @@ For example for printing the output in JSON format, specify:
> >  For more details on the available output printing formats, see the
> >  Writers section below.
> >  
> > + at item -select_streams @var{stream_specifier}
> > +Select only the streams specified by @var{stream_specifier}. This
> > +option affect only the options related to streams
> 
> affects?

Fixed.
 
> > +(e.g. @code{show_streams}, @code{show_packets}, etc.).
> > +
> > +For example to show only audio streams, you can use the command:
> > + at example
> > +ffprobe -show_streams -select_streams a INPUT
> > + at end example
> > +
> > +To show only video packets belonging to the video stream with index 1:
> > + at example
> > +ffprobe -show_packets -select_streams v:1 INPUT
> > + at end example
> > +
> >  @item -show_data
> >  Show payload data, as an hexadecimal and ASCII dump. Coupled with
> >  @option{-show_packets}, it will dump the packets' data. Coupled with
> > diff --git a/ffprobe.c b/ffprobe.c
> > index 4fb4982..c8a9022 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -69,6 +69,7 @@ static int use_value_sexagesimal_format = 0;
> >  static int show_private_data            = 1;
> >  
> >  static char *print_format;
> > +static char *stream_specifier;
> >  
> >  /* section structure definition */
> >  
> > @@ -138,8 +139,10 @@ static const char unit_second_str[]         = "s"    ;
> >  static const char unit_hertz_str[]          = "Hz"   ;
> >  static const char unit_byte_str[]           = "byte" ;
> >  static const char unit_bit_per_second_str[] = "bit/s";
> > +
> >  static uint64_t *nb_streams_packets;
> >  static uint64_t *nb_streams_frames;
> > +static int *selected_streams;
> >  
> >  static void exit_program(void)
> >  {
> > @@ -1614,6 +1617,8 @@ static void read_packets(WriterContext *w, AVFormatContext *fmt_ctx)
> >      av_init_packet(&pkt);
> >  
> >      while (!av_read_frame(fmt_ctx, &pkt)) {
> > +        if (!selected_streams[pkt.stream_index])
> > +            continue;
> 

> Isn't av_free_packet() still needed?

Ouch, fixed.
 
> >          if (do_read_packets) {
> >              if (do_show_packets)
> >                  show_packet(w, fmt_ctx, &pkt, i++);
> > @@ -1789,7 +1794,8 @@ static void show_streams(WriterContext *w, AVFormatContext *fmt_ctx)
> >      int i;
> >      writer_print_section_header(w, SECTION_ID_STREAMS);
> >      for (i = 0; i < fmt_ctx->nb_streams; i++)
> > -        show_stream(w, fmt_ctx, i);
> > +        if (selected_streams[i])
> > +            show_stream(w, fmt_ctx, i);
> >      writer_print_section_footer(w);
> >  }
> >  
> > @@ -1896,7 +1902,7 @@ static void close_input_file(AVFormatContext **ctx_ptr)
> >  static int probe_file(WriterContext *wctx, const char *filename)
> >  {
> >      AVFormatContext *fmt_ctx;
> > -    int ret;
> > +    int ret, i;
> >      int section_id;
> >  
> >      do_read_frames = do_show_frames || do_count_frames;
> > @@ -1906,6 +1912,22 @@ static int probe_file(WriterContext *wctx, const char *filename)
> >      if (ret >= 0) {
> >          nb_streams_frames  = av_calloc(fmt_ctx->nb_streams, sizeof(*nb_streams_frames));
> >          nb_streams_packets = av_calloc(fmt_ctx->nb_streams, sizeof(*nb_streams_packets));
> > +        selected_streams   = av_calloc(fmt_ctx->nb_streams, sizeof(*selected_streams));
> > +
> > +        for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > +            if (stream_specifier) {
> > +                ret = avformat_match_stream_specifier(fmt_ctx,
> > +                                                      fmt_ctx->streams[i],
> > +                                                      stream_specifier);
> > +                if (ret < 0)
> > +                    goto end;
> > +                else
> > +                    selected_streams[i] = ret;
> > +            } else {
> > +                selected_streams[i] = 1;
> > +            }
> > +        }
> > +
> >          if (do_read_frames || do_read_packets) {
> >              if (do_show_frames && do_show_packets &&
> >                  wctx->writer->flags & WRITER_FLAG_PUT_PACKETS_AND_FRAMES_IN_SAME_CHAPTER)
> > @@ -1925,9 +1947,11 @@ static int probe_file(WriterContext *wctx, const char *filename)
> >          if (do_show_format)
> >              show_format(wctx, fmt_ctx);
> >  
> > +    end:
> >          close_input_file(&fmt_ctx);
> >          av_freep(&nb_streams_frames);
> >          av_freep(&nb_streams_packets);
> > +        av_freep(&selected_streams);
> >      }
> >      return ret;
> >  }
> > @@ -2062,6 +2086,7 @@ static const OptionDef real_options[] = {
> >      { "print_format", OPT_STRING | HAS_ARG, {(void*)&print_format},
> >        "set the output printing format (available formats are: default, compact, csv, flat, ini, json, xml)", "format" },
> >      { "of", OPT_STRING | HAS_ARG, {(void*)&print_format}, "alias for -print_format", "format" },
> > +    { "select_streams", OPT_STRING | HAS_ARG, {(void*)&stream_specifier}, "select the specified streams", "stream_specifier" },
> >      { "show_data",    OPT_BOOL, {(void*)&do_show_data}, "show packets data" },
> >      { "show_error",   OPT_BOOL, {(void*)&do_show_error} ,  "show probing error" },
> >      { "show_format",  OPT_BOOL, {&do_show_format} , "show format/container info" },
> 
> Rest looks OK.
> 

> In a perfect world we would even have a FATE test (or maybe you're looking
> for using the feature in some particular already existing tests?).

Not yet, but if there are bugs people will found out.

Pushed, thanks for the review.
-- 
FFmpeg = Free Fantastic Mythic Philosofic Exciting Gadget