[FFmpeg-devel] [PATCH] ffprobe: add INI writer

[Clément Bœsch]

On Sun, May 27, 2012 at 01:37:13AM +0200, Stefano Sabatini wrote:
> Liberally based on the work of Luca Barbato <lu_zero at gentoo.org>, done
> for libav/avprobe.
> ---
>  doc/ffprobe.texi |   24 +++++++++
>  ffprobe.c        |  144 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 167 insertions(+), 1 deletions(-)
> 
> diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> index ed96575..b6af5df 100644
> --- a/doc/ffprobe.texi
> +++ b/doc/ffprobe.texi
> @@ -269,6 +269,30 @@ CSV format.
>  This writer is equivalent to
>  @code{compact=item_sep=,:nokey=1:escape=csv}.
>  
> + at section ini
> +INI format output.
> +
> +Print output in an INI based format.
> +
> +The following convenctions are adopted:
> +

conventions?

> + at itemize
> + at item
> +all key and values are UTF8

UTF-8

> + at item
> +'.' is the subgroup separator
> + at item
> +newline, tab, form feed, and back space characters are escaped

I'd explicit the \n, \t, ...

> + at item
> +'\' is the escape character
> + at item
> +'#' is the comment indicator
> + at item
> +'=' is the key/value separators

separator?

> + at item
> +':' is not used but usually parsed as key/value separator
> + at end itemize
> +
>  @section json
>  JSON based format.
>  
> diff --git a/ffprobe.c b/ffprobe.c
> index 1375f14..54ab9cd 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -715,6 +715,147 @@ static const Writer csv_writer = {
>      .flags = WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS,
>  };
>  
> +/*
> + * INI format output
> + *
> + * - all key and values are UTF8
> + * - '.' is the subgroup separator
> + * - newlines and the following characters are escaped
> + * - '\' is the escape character
> + * - '#' is the comment
> + * - '=' is the key/value separators
> + * - ':' is not used but usually parsed as key/value separator
> + */
> +

This is already in the documentation with the same typo errors, not sure
it's worth duplicating them.

> +typedef struct IniContext {

Note: do we really need to name these struct?

> +    AVBPrint section_name;
> +    int print_packets_and_frames;
> +    int nb_frame;
> +    int nb_packet;
> +} IniContext;
> +

nit: INIContext?

> +static av_cold int ini_init(WriterContext *wctx, const char *args, void *opaque)
> +{
> +    IniContext *ini = wctx->priv;
> +    av_bprint_init(&ini->section_name, 1, AV_BPRINT_SIZE_UNLIMITED);
> +    ini->nb_frame = ini->nb_packet = 0;
> +
> +    if (args) {
> +        av_log(wctx, AV_LOG_ERROR, "No options supported by the ini format writer: '%s'\n", args);

nit: "INI"? (we should change the strcmp to strcasecmp in the writers
selection).

> +        return AVERROR(EINVAL);
> +    }
> +
> +    return 0;
> +}
> +
> +static av_cold void ini_uninit(WriterContext *wctx)
> +{
> +    IniContext *ini = wctx->priv;
> +    av_bprint_finalize(&ini->section_name, NULL);
> +}
> +
> +static void ini_print_header(WriterContext *wctx)
> +{
> +    printf("# ffprobe output\n\n");
> +}
> +
> +static char *ini_escape_str(AVBPrint *dst, const char *src)
> +{
> +    int i = 0;
> +    char c = 0;
> +
> +    while (c = src[i++]) {
> +        switch (c) {
> +        case '\b': av_bprintf(dst, "%s", "\\b"); break;
> +        case '\f': av_bprintf(dst, "%s", "\\f"); break;
> +        case '\n': av_bprintf(dst, "%s", "\\n"); break;
> +        case '\r': av_bprintf(dst, "%s", "\\r"); break;
> +        case '\t': av_bprintf(dst, "%s", "\\t"); break;
> +        case '\\':
> +        case '#' :
> +        case '=' :
> +        case ':' : av_bprint_chars(dst, '\\', 1);
> +        default:
> +            if ((unsigned char)c < 32)
> +                av_bprintf(dst, "\\x00%02x", c & 0xff);
> +            else
> +                av_bprint_chars(dst, c, 1);
> +        break;

nit: align breaker

> +        }
> +    }
> +    return dst->str;
> +}
> +
> +static void ini_print_chapter_header(WriterContext *wctx, const char *chapter)
> +{
> +    IniContext *ini = wctx->priv;
> +    if (wctx->nb_chapter)
> +        printf("\n");
> +    ini->print_packets_and_frames = !strcmp("packets_and_frames", chapter);
> +}
> +
> +static void ini_print_section_header(WriterContext *wctx, const char *section)
> +{
> +    IniContext *ini = wctx->priv;
> +    int n;
> +    if (wctx->nb_section)
> +        printf("\n");
> +    av_bprint_clear(&ini->section_name);
> +    av_bprintf(&ini->section_name, "%s", section);
> +
> +   if (ini->print_packets_and_frames)

align

> +        n = !strcmp(section, "packet") ? ini->nb_packet++ : ini->nb_frame++;
> +    else
> +        n = wctx->nb_section;
> +    if (wctx->multiple_sections)
> +        av_bprintf(&ini->section_name, ".%d", n);
> +    printf("[%s]\n", ini->section_name.str);
> +}
> +
> +static void ini_print_str(WriterContext *wctx, const char *key, const char *value)
> +{
> +    AVBPrint buf;
> +    av_bprint_init(&buf, 1, AV_BPRINT_SIZE_UNLIMITED);
> +
> +    printf("%s=", ini_escape_str(&buf, key));
> +    av_bprint_clear(&buf);
> +    printf("%s\n", ini_escape_str(&buf, value));

I know this is meant to change, but using AV_BPRINT_SIZE_UNLIMITED needs a
finalize to avoid leaks in some cases.

Beside that, the rest LGTM, thanks.

-- 
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/20120527/6e30d1f3/attachment.asc>