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

[Clément Bœsch]

On Sun, Sep 25, 2011 at 02:09:24AM +0200, Stefano Sabatini wrote:
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> 
> ---
>  doc/ffprobe.texi |   17 +++++++++++++++++
>  ffprobe.c        |   39 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 55 insertions(+), 1 deletions(-)
> 
> diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> index e9b26ac..a41ce1d 100644
> --- a/doc/ffprobe.texi
> +++ b/doc/ffprobe.texi
> @@ -93,6 +93,23 @@ Current available formats are:
>  
>  @table @option
>  
> + at item compact
> +Compact format.
> +
> +Each section is printed on a single line, and has the form:
> + at example
> +[SECTION] key1=val1 ... keyN=valN [/SECTION]
> + at end example
> +
> + at item compactnk
> +Compact format with no key printing.
> +
> +Like the @option{compact} format, but no key is printed.
> +Each section is printed on a single line, and has the form:
> + at example
> +[SECTION] val1 ... valN [/SECTION]
> + at end example
> +
>  @item default
>  It is default format.
>  

Maybe keep the default on top?

> diff --git a/ffprobe.c b/ffprobe.c
> index d12c98c..b975cf8 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -238,6 +238,25 @@ static void default_print_footer(const char *section)
>      printf("\n[/%s]", section);
>  }
>  
> +static void compact_print_header(const char *section)
> +{
> +    printf("[%s] ", section);
> +}
> +
> +static void compact_print_footer(const char *section)
> +{
> +    printf(" [/%s]\n", section);
> +}
> +

I wonder if keeping this nesting really is important. We might be limited
in width, so better keep it really compact. What about something like:

SECTION: k=v ...
SECTION: k=v ...
...

> +static void compactnk_print_int(const char *key, int value)
> +{
> +    printf("%d", value);
> +}
> +
> +static void compactnk_print_str(const char *key, const char *value)
> +{
> +    printf("%s", value);
> +}
>  

Maybe some quoting would be worth here? It's sometimes hard to tell what
are the limit of a key. Another solution might be to use some '|'.

Those are just suggestions, I won't push for this.

>  /* Print helpers */
>  
> @@ -539,6 +558,24 @@ static struct writer writers[] = {{
>          .print_section_start = json_print_section_start,
>          .print_section_end   = json_print_section_end,
>          WRITER_FUNC(json),
> +    },{
> +        .name          = "compact",
> +        .item_sep      = " ",
> +        .items_sep     = "",
> +        .print_header  = compact_print_header,
> +        .print_footer  = compact_print_footer,
> +        .print_integer = default_print_int,
> +        .print_string  = default_print_str,
> +        .show_tags     = default_show_tags,
> +    },{
> +        .name          = "compactnk",
> +        .item_sep      = " ",
> +        .items_sep     = "",
> +        .print_header  = compact_print_header,
> +        .print_footer  = compact_print_footer,
> +        .print_integer = compactnk_print_int,
> +        .print_string  = compactnk_print_str,
> +        .show_tags     = default_show_tags,
>      }
>  };
>  
> @@ -656,7 +693,7 @@ static const OptionDef options[] = {
>        "use sexagesimal format HOURS:MM:SS.MICROSECONDS for time units" },
>      { "pretty", 0, {(void*)&opt_pretty},
>        "prettify the format of displayed values, make it more human readable" },
> -    { "print_format", OPT_STRING | HAS_ARG, {(void*)&print_format}, "set the output printing format (available formats are: default, json)", "format" },
> +    { "print_format", OPT_STRING | HAS_ARG, {(void*)&print_format}, "set the output printing format (available formats are: default, json, compact, compactnk)", "format" },

nit: let's keep the same order as the documentation: default, compact,
compactnk, json. (default + alphabetical order).

LGTM otherwise.

-- 
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/20110925/a631e719/attachment.asc>