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

[Stefano Sabatini]

On date Saturday 2011-10-08 15:23:07 +0200, Stefano Sabatini encoded:
> 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.

Pushed.