[FFmpeg-devel] [PATCH] ffprobe: add compact and compactnk writers

[Clément Bœsch]

On Wed, Sep 28, 2011 at 01:57:19AM +0200, Stefano Sabatini wrote:
> On date Wednesday 2011-09-28 01:19:06 +0200, Alexander Strasser encoded:
> > Hi
> > 
> > Stefano Sabatini wrote:
> > > On date Sunday 2011-09-25 16:26:55 +0200, Alexander Strasser encoded:
> > > > Stefano Sabatini wrote:
> > [...]
> > > > > I added escaping (which may be used e.g. for CSV output), and change
> > > > 
> > > >   I think for CSV output it would be better to adhere to RFC 4180.
> > > > Probably with the exception of "Each line should contain the same
> > > > number of fields throughout the file.", but that is "should" anyway.
> > > 
> > > MS-escaping (as described in RFC4180) is mal-designed and thus I'm not
> > > eager to make of this the default escaping algo, I suppose the best
> > > option is to make the escaping algorithm configurable, which requires
> > > some major redesign.
> > 
> >   If it should be default or not I cannot judge, but it would make sense
> > because it is what people usually expect when talking about CSV (if that
> > can at all be said about such a adhoc/no-real-spec file format).
> > 
> >   But as you say, being able to choose the escaping algo would probably
> > be nice to have.
> 
> Well, actually *this* is my idea, maybe overkill, possibly extensible
> to other objects in libav* (e.g. *showinfo filters).
> 

Yeah, a bit overkill, but I like the idea of the writer context.

[...]
> From 49101b8269c1d0df6b1300023c11f6f48a6ca603 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Tue, 27 Sep 2011 20:07:51 +0200
> Subject: [PATCH] ffprobe: move writers to separate files, and use them
> 
> Generalize writers API, and move them to separate dedicated files.
> ---
>  Makefile  |    6 +-
>  ffprobe.c |  335 +++++++++----------------------------------------------------
>  writers.c |  312 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  writers.h |  132 ++++++++++++++++++++++++
>  4 files changed, 493 insertions(+), 292 deletions(-)
>  create mode 100644 writers.c
>  create mode 100644 writers.h

The item separator isn't honored correctly here in JSON output after this
change:

    "streams": [{...}{...}...]
                    ^^
                 missing sep

Also, the JSON show tags is broken:

    "tags": {
    ,
    ,
    "filename": "BAARS___.TTF",
    "mimetype": "application/x-truetype-font"
    }


[...]
> -#define SECTION_PRINT(name, multiple_entries, left) do {      \
> -    if (do_show_ ## name) {                                   \
> -        if (w->print_section_start)                           \
> -            w->print_section_start(#name, multiple_entries);  \
> -        show_ ## name (w, fmt_ctx);                           \
> -        if (w->print_section_end)                             \
> -            w->print_section_end(#name, multiple_entries);    \
> -        if (left)                                             \
> -            printf("%s", w->section_sep);                     \
> -    }                                                         \

> +#define PRINT_CHAPTER(name) do {                                        \
> +    if (do_show_ ## name) {                                             \
> +        av_writer_print_chapter_header(wctx, #name);                    \
> +        show_ ## name (wctx, fmt_ctx);                                  \
> +        av_writer_print_chapter_footer(wctx, #name);                    \
> +    }                                                                   \

What happened to the section separator which have to be displayed only if
another section is following?

[...]
> +AVWriter default_writer = {
> +    .name         = "default",
> +    .print_footer = default_print_footer,
> +    WRITER_FUNC(default),
> +};
> +
> +/* JSON output */
> +
> +typedef struct {
> +    int multiple_entries; ///< tells if the given chapter requires multiple entries
> +} JSONContext;

Why do you think this should be handled in the writer context?  Maybe
other writers might need this information as well.

[...]

> +struct print_buf {
> +    char *s;
> +    int len;
> +};
> +

Can't this be put in the AVWriterContext?

> +char *ff_fast_asprintf(struct print_buf *pbuf, const char *fmt, ...);
> -- 
> 1.7.2.5
> 

> From a0fafe146c7f0245267ca388e743f6d22cd8c76f Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Wed, 28 Sep 2011 01:42:07 +0200
> Subject: [PATCH] ffprobe: add compact writer
> 
> ---
>  ffprobe.c |   19 ++++++---
>  writers.c |  126 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 139 insertions(+), 6 deletions(-)
> 
[...]
> +    w_args = print_format +idx+1;

weird spacing :)


-- 
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/20110928/7162aa98/attachment.asc>