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

[Stefano Sabatini]

On date Wednesday 2012-02-15 11:02:43 +0100, Matthieu Bouron encoded:
> 2012/2/9 Stefano Sabatini <stefasab at gmail.com>:
> > On date Monday 2012-02-06 14:18:04 +0100, Matthieu Bouron encoded:
> > [...]
> >
> > I noticed some problems when I was testing the patches.
> >
> >> From d2b6d2b3c08475f066356bd19b95063bfa7d890f 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/3] ffprobe: report read nb_frames and nb_packets per stream
> >>
> >> ---
> >>  doc/ffprobe.xsd |    2 ++
> >>  ffprobe.c       |   22 ++++++++++++++++++++--
> >>  2 files changed, 22 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd
> >> index 9ac80bb..e7104ef 100644
> >> --- a/doc/ffprobe.xsd
> >> +++ b/doc/ffprobe.xsd
> >> @@ -111,6 +111,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_frames"   type="xsd:int"/>
> >> +      <xsd:attribute name="nb_read_packets"  type="xsd:int"/>
> >>      </xsd:complexType>
> >>
> >>      <xsd:complexType name="formatType">
> >> diff --git a/ffprobe.c b/ffprobe.c
> >> index ca6133e..0b82092 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;
> >>
> >>  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]++;
> >>              }
> >>          }
> >
> > The problem with this is that frames/packets are counted *only* if
> > they are also shown, so the -count_frames/packets option won't have
> > effect unless -show_frames/packets are selected.
> 
> That was the purpose of the first patch, counting/frames packets when
> show_frames/packets are enabled. Should i merge this patch with the
> following one ?

I mean when I select -count_frames/packets I'd expect to see them
counted, even if frames/packets are not shown, so count should imply
-> read.

[...]
> >> From 7e3c9df1dc3619ebd9eead840595014922d6de09 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/3] ffprobe: add count_frames and count_packets options
> >>
> >> ---
> >>  doc/ffprobe.texi |    8 ++++++++
> >>  ffprobe.c        |   19 +++++++++++++------
> >>  2 files changed, 21 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> >> index 829a46e..060be02 100644
> >> --- a/doc/ffprobe.texi
> >> +++ b/doc/ffprobe.texi
> >> @@ -127,6 +127,14 @@ multimedia stream.
> >>  Each media stream information is printed within a dedicated section
> >>  with name "STREAM".
> >>
> >> + at item -count_frames
> >> +Count the number of frames per stream and report it in corresponding
> >> +stream section.
> >> +
> >> + at item -count_packets
> >> +Count the number of packets per stream and report it in corresponding
> >> +stream section.
> >> +
> >>  @item -show_private_data, -private
> >>  Show private data, that is data depending on the format of the
> >>  particular shown element.
> >> diff --git a/ffprobe.c b/ffprobe.c
> >> index 0b82092..8ef9180 100644
> >> --- a/ffprobe.c
> >> +++ b/ffprobe.c
> >> @@ -48,6 +48,8 @@ static int do_show_packets = 0;
> >>  static int do_show_streams = 0;
> >>  static int do_show_program_version  = 0;
> >>  static int do_show_library_versions = 0;
> >> +static int do_count_frames = 0;
> >> +static int do_count_packets = 0;
> >>
> >>  static int show_value_unit              = 0;
> >>  static int use_value_prefix             = 0;
> >> @@ -1370,18 +1372,20 @@ static void show_packets(WriterContext *w, AVFormatContext *fmt_ctx)
> >>      av_init_packet(&pkt);
> >>
> >
> >>      while (!av_read_frame(fmt_ctx, &pkt)) {
> >> -        if (do_show_packets) {
> >> -            show_packet(w, fmt_ctx, &pkt, i++);
> >> +        if (do_show_packets || do_count_packets) {
> >> +            if (do_show_packets)
> >> +                show_packet(w, fmt_ctx, &pkt, i++);
> >>              nb_streams_packets[pkt.stream_index]++;
> >>          }
> >> +        if (do_show_packets || do_count_packets) {
> >> +            if (do_show_packets)
> >> +                show_packet(w, fmt_ctx, &pkt, i++);
> >> -        if (do_show_frames) {
> >> +        if (do_show_frames || do_count_frames) {
> >>              pkt1 = pkt;
> >>              while (1) {
> >>                  avcodec_get_frame_defaults(&frame);
> >>                  ret = get_decoded_frame(fmt_ctx, &frame, &got_frame, &pkt1);
> >>                  if (ret < 0 || !got_frame)
> >>                      break;
> >> -                show_frame(w, &frame, fmt_ctx->streams[pkt.stream_index]);
> >> +                if (do_show_frames)
> >> +                    show_frame(w, &frame, fmt_ctx->streams[pkt.stream_index]);
> >
> > The problem here is that show_packets will print an header if -count
> > is selected, but no packets will be shown.
> >
> > So I believe the logic should be changed to:
> >
> > do_show_packets or do_count_packets -> do_read_packets
> 
> Since do_show_frames or do_count_frames trigger packets reading:
> Should do_read_packets = do_show_packets or do_count_packets or
> do_show_frames or do_read_frames ?
> 
> >
> > show_packets() -> read_packets()
> 
> Do you mean show_packets renamed in read_packets ?

Yes, and change the logic according to the natural interpretation of
the name of the function, so that the count is done even if the
frames/packets are not shown.

[...]
-- 
FFmpeg = Formidable Fancy Multipurpose Powerful Exciting Gnome