[FFmpeg-devel] [PATCH] ffprobe: report read nb_packets and nb_frames by stream, add count_frames and count_packets options

[Stefano Sabatini]

On date Monday 2012-02-06 11:51:41 +0100, Matthieu Bouron encoded:
[...]
> From ee0d32cb7d7daf1475d3a5347bf24db7e7c0ea49 Mon Sep 17 00:00:00 2001
> From: Matthieu Bouron <matthieu.bouron at smartjog.com>
> Date: Wed, 1 Feb 2012 16:23:53 +0100
> Subject: [PATCH 1/2] ffprobe: report read nb_frames and nb_packets per stream
> 
> ---
>  doc/ffprobe.xsd |    3 +++
>  ffprobe.c       |   22 ++++++++++++++++++++--
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd
> index 9ac80bb..8cdc5cc 100644
> --- a/doc/ffprobe.xsd
> +++ b/doc/ffprobe.xsd
> @@ -97,6 +97,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="nb_read_frames"       type="xsd:int"/>
>  
>        <!-- audio attributes -->
>        <xsd:attribute name="sample_fmt"       type="xsd:string"/>
> @@ -111,6 +112,8 @@
>        <xsd:attribute name="start_time"       type="xsd:float"/>
>        <xsd:attribute name="duration"         type="xsd:float"/>
>        <xsd:attribute name="nb_frames"        type="xsd:int"/>
> +      <xsd:attribute name="nb_read_packets"  type="xsd:int"/>
> +

nit++: useless empty line

>      </xsd:complexType>
>  
>      <xsd:complexType name="formatType">
> diff --git a/ffprobe.c b/ffprobe.c
> index ca6133e..2a5e24e 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -70,6 +70,8 @@ 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;

Nit: maybe nb_stream_packets/frames (without the trailing "s" in "_stream")

>  void av_noreturn exit_program(int ret)
>  {
> @@ -1368,8 +1370,10 @@ static void show_packets(WriterContext *w, AVFormatContext *fmt_ctx)
>      av_init_packet(&pkt);
>  
>      while (!av_read_frame(fmt_ctx, &pkt)) {
> -        if (do_show_packets)
> +        if (do_show_packets) {
>              show_packet(w, fmt_ctx, &pkt, i++);
> +            nb_streams_packets[pkt.stream_index]++;
> +        }
>          if (do_show_frames) {
>              pkt1 = pkt;
>              while (1) {
> @@ -1380,6 +1384,7 @@ static void show_packets(WriterContext *w, AVFormatContext *fmt_ctx)
>                  show_frame(w, &frame, fmt_ctx->streams[pkt.stream_index]);
>                  pkt1.data += ret;
>                  pkt1.size -= ret;
> +                nb_streams_frames[pkt.stream_index]++;
>              }
>          }
>          av_free_packet(&pkt);
> @@ -1390,8 +1395,10 @@ static void show_packets(WriterContext *w, AVFormatContext *fmt_ctx)
>      //Flush remaining frames that are cached in the decoder
>      for (i = 0; i < fmt_ctx->nb_streams; i++) {
>          pkt.stream_index = i;
> -        while (get_decoded_frame(fmt_ctx, &frame, &got_frame, &pkt) >= 0 && got_frame)
> +        while (get_decoded_frame(fmt_ctx, &frame, &got_frame, &pkt) >= 0 && got_frame) {
>              show_frame(w, &frame, fmt_ctx->streams[pkt.stream_index]);
> +            nb_streams_frames[pkt.stream_index]++;
> +        }
>      }
>  }
>  
> @@ -1498,6 +1505,10 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
>      print_time("duration",      stream->duration,   &stream->time_base);
>      if (stream->nb_frames) print_fmt    ("nb_frames", "%"PRId64, stream->nb_frames);
>      else                   print_str_opt("nb_frames", "N/A");

> +    if (nb_streams_frames[stream_idx] && dec_ctx->codec_type == AVMEDIA_TYPE_VIDEO)
> +        print_fmt("nb_read_frames", "%"PRIu64, nb_streams_frames[stream_idx]);

Is the check on type == VIDEO really required? I think it makes sense
to print the number of audio frames as well.

> +    if (nb_streams_packets[stream_idx])
> +        print_fmt("nb_read_packets", "%"PRIu64, nb_streams_packets[stream_idx]);

Also you should employ the print_str_opt() trick to avoid breaking
fixed-number-of-fields format parsing.

>      show_tags(stream->metadata);
>  
>      print_section_footer("stream");
> @@ -1607,6 +1618,10 @@ static int probe_file(WriterContext *wctx, const char *filename)
>      int ret, i;
>  
>      ret = open_input_file(&fmt_ctx, filename);
> +    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));
>      if (ret >= 0) {
>          if (do_show_packets || do_show_frames) {
>              const char *chapter;
> @@ -1629,6 +1644,9 @@ static int probe_file(WriterContext *wctx, const char *filename)
>          avformat_close_input(&fmt_ctx);
>      }
>  
> +    av_freep(&nb_streams_frames);
> +    av_freep(&nb_streams_packets);
> +
>      return ret;

Looks fine otherwise.

>  
> -- 
> 1.7.8.3
> 

> From 6bc2944af82299274b7801af470e722ae41a2e47 Mon Sep 17 00:00:00 2001
> From: Matthieu Bouron <matthieu.bouron at smartjog.com>
> Date: Wed, 1 Feb 2012 17:37:29 +0100
> Subject: [PATCH 2/2] ffprobe: add count_frames and count_packets options
> 
> ---
>  doc/ffprobe.texi |    8 ++++++++
>  ffprobe.c        |   19 +++++++++++++------
>  2 files changed, 21 insertions(+), 6 deletions(-)
[...]

Still looks good.
-- 
FFmpeg = Fanciful and Fascinating Multimedia Perfectionist Elastic Guide