[FFmpeg-devel] [PATCH] ffprobe: extend writers API, and move the writers up in the file

[Stefano Sabatini]

On date Monday 2011-10-03 21:06:02 +0200, Clément Bœsch encoded:
> On Sun, Oct 02, 2011 at 12:40:40PM +0200, Stefano Sabatini wrote:
> > The new provided API is more flexible and is is decoupled with the
> > application level code, so it is easier to maintain.
> > ---
> >  ffprobe.c |  580 +++++++++++++++++++++++++++++++++++++++----------------------
> >  1 files changed, 375 insertions(+), 205 deletions(-)
> > 
> > diff --git a/ffprobe.c b/ffprobe.c
> > index 2c354ad..f1f77d0 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> 
> I believe this is the first patch of the serie, so here we go…
> 
> [...]
> > -static void json_print_footer(const char *section)
> > +static inline void writer_print_chapter_header(WriterContext *wctx,
> > +                                                  const char *header)
> 
> Weird align in the prototype here and below.
> 
> [...]
> > -static void default_print_footer(const char *section)
> > +#define MAX_REGISTERED_WRITERS_NB 64
> > +
> > +static Writer *registered_writers[MAX_REGISTERED_WRITERS_NB + 1];
> > +
> > +static int next_registered_writer_idx = 0;
> > +
> 
> Why not local to writer_register()?
> 
> > +static Writer *writer_get_by_name(const char *name)
> >  {
> > -    printf("\n[/%s]", section);
> > +    int i;
> > +
> > +    for (i = 0; registered_writers[i]; i++)
> > +        if (!strcmp(registered_writers[i]->name, name))
> > +            return registered_writers[i];
> > +
> 
> This leads to a crash if name is NULL.
> 
> Rest looks OK and interesting, much less hacky than my initial work. But I
> really believe some ffprobe fate tests should be added somehow ASAP :)

Updated with various fixes. Regarding the test, maybe a simple
metadata file would be enough, but I'm not really sure about it.

Going to push it in a few days if I see no more comments.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ffprobe-extend-writers-API-and-move-the-writers-up-i.patch
Type: text/x-diff
Size: 24804 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111008/ddc7b62c/attachment.bin>