[FFmpeg-devel] [PATCH 1/2] ffprobe: introduce output format writers.

[Stefano Sabatini]

On date Sunday 2011-08-28 12:18:48 +0200, Clément Bœsch encoded:
> On Sun, Aug 28, 2011 at 11:09:15AM +0200, Stefano Sabatini wrote:
> [...]
> > > -static void show_packet(AVFormatContext *fmt_ctx, AVPacket *pkt)
> > > +
> > > +struct writer {
> > > +    const char *name;
> > > +    const char *item_sep, *items_sep, *section_sep;
> > 
> > what's exactly the difference between item_sep and items_sep? Doxies are
> > welcome here.
> > 
> 
> Added.
> 
> [...]
> > > +    print_fmt("pos", "%"PRId64, pkt->pos);
> > > +    print_fmt("flags",    "%c", pkt->flags & AV_PKT_FLAG_KEY ? 'K' : '_');
> > 
> > Nit++: this may be vertically aligned
> > 
> 
> It was a weird align, but aligned anyway. Changed :)
> 
> > > +    w->print_footer("PACKET");
> > >      fflush(stdout);
> > >  }
> > [...]  
> > 
> > > +#define SECTION_PRINT(name, multiple_entries, left) do {      \
> > > +    if (do_show_ ## name) {                                   \
> > > +        show_ ## name (w, fmt_ctx);                           \
> > > +        if (left)                                             \
> > > +            printf("%s", w->section_sep);                     \
> > > +    }                                                         \
> > > +} while (0)
> > 
> > what's the use of multiple_entries?
> > 
> 
> It was a leftover from the JSON patch; streams and packets sections
> contain several streams and packets, so in JSON you need to open/close the
> lists with [ ], but this is not true for the format section.
> multiple_entries flag is for that purpose.
> 
> Since it was introduced for JSON, that chunk is moved to the appropriate
> patch (I'll resend it in a minute).
> 
> [...]
> > >  
> > > -    if (do_show_streams)
> > > -        for (i = 0; i < fmt_ctx->nb_streams; i++)
> > > -            show_stream(fmt_ctx, i);
> > > +    SECTION_PRINT(packets, 1, do_show_streams || do_show_format);
> > > +    SECTION_PRINT(streams, 1, do_show_format);
> > > +    SECTION_PRINT(format,  0, 0);
> > 
> > Looks fine to me, although I wonder if there is a simpler way to
> > handle this without the use of macros.
> 
> The macro gets bigger in JSON patch (use of multiple_entries mentionned
> above). I can't think of something better if we want to avoid probe_file()
> reimplementation in each writers, but feel free to propose anything…
> 
> -- 
> Clément B.

> From 5bdd793d764af83e1c101f4c178d17aade38cfbc Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Sat, 27 Aug 2011 20:19:19 +0200
> Subject: [PATCH 1/2] ffprobe: introduce output format writers.
> 
> ---
>  ffprobe.c |  295 +++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 211 insertions(+), 84 deletions(-)

No more comments from me. Just verify the output is not changed in
significant ways (so scripts relying on the previous behavior won't
break), maybe just wait one day or two before committing in case other
devs want to comment on it.

And nice work :-).
-- 
FFmpeg = Fanciful and Fascinating Meaningless Pitiless Explosive Gigant