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

[Clément Bœsch]

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 :)

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111003/76239cd9/attachment.asc>