[FFmpeg-devel] [PATCH 4/4] ffprobe: add flat output format.

[Stefano Sabatini]

On date Tuesday 2012-05-29 23:24:42 +0200, Clément Bœsch encoded:
> ---
>  doc/ffprobe.texi |   24 ++++++++
>  ffprobe.c        |  163 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 186 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> index a725c37..e1c64ae 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 flat
> +Flat format.
> +

> +A free-form output with each line contains an explicit key=value, such as
> +"streams.stream.3.tags.foo=bar"

Mention that the output is sh-escaped, so can be directly embedded in
sh scripts (if using a valid separator).

> +
> +This writer accepts options as a list of @var{key}=@var{value} pairs,
> +separated by ":".

Reminder: we should make the interface consistent with all writers,
and maybe factorize some init code.

> +
> +The description of the accepted options follows.
> +
> + at table @option
> + at item chr_sep, c

Nit: sep_char, s? (also, consistent with compact writer)
Random abbreviations are usually confusing.

> +Separator used between chapters/section/etc. Default is '.'.

"chapter"

Also maybe more clear:
"Separator character used to separate the chapter and the section name
in the printed field key."

> +
> + at item hierarchical, h
> +Specify if the section name specification should be hierarchical. If
> +set to 1, and if there is more than one section in the current
> +chapter, the section name will be prefixed by the name of the
> +chapter. A value of 0 will disable this behavior.
> +
> +Default value is 1.
> + at end table
> +
>  @section ini
>  INI format output.
>  
> diff --git a/ffprobe.c b/ffprobe.c
> index 27ab62e..c64a687 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -727,6 +727,166 @@ static const Writer csv_writer = {
>      .flags = WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS,
>  };
>  
> +/* Flat output */
> +
> +typedef struct FlatContext {
> +    const AVClass *class;
> +    const char *section, *chapter;

> +    const char *sep;
> +    char chr_sep;

nit: sep_str, sep, should be more consistent (e.g. check
CompactContext), and I wonder if you can directly use the string.

> +    int hierarchical;
> +} FlatContext;
> +
> +#undef OFFSET
> +#define OFFSET(x) offsetof(FlatContext, x)
> +
> +static const AVOption flat_options[]= {
> +    {"chr_sep",  "set separator",    OFFSET(sep),    AV_OPT_TYPE_STRING, {.str="."},  CHAR_MIN, CHAR_MAX },
> +    {"c",        "set separator",    OFFSET(sep),    AV_OPT_TYPE_STRING, {.str="."},  CHAR_MIN, CHAR_MAX },
> +    {"hierachical", "specify if the section specification should be hierarchical", OFFSET(hierarchical), AV_OPT_TYPE_INT, {.dbl=1}, 0, 1 },
> +    {"h",           "specify if the section specification should be hierarchical", OFFSET(hierarchical), AV_OPT_TYPE_INT, {.dbl=1}, 0, 1 },
> +    {NULL},
> +};
> +
> +static const char *flat_get_name(void *ctx)
> +{
> +    return "flat";
> +}
> +
> +static const AVClass flat_class = {
> +    "FlatContext",
> +    flat_get_name,
> +    flat_options
> +};
> +
> +static av_cold int flat_init(WriterContext *wctx, const char *args, void *opaque)
> +{
> +    FlatContext *flat = wctx->priv;
> +    int err;
> +
> +    flat->class = &flat_class;
> +    av_opt_set_defaults(flat);
> +
> +    if (args &&
> +        (err = (av_set_options_string(flat, args, "=", ":"))) < 0) {
> +        av_log(wctx, AV_LOG_ERROR, "Error parsing options string: '%s'\n", args);
> +        return err;
> +    }
> +    if (strlen(flat->sep) != 1) {
> +        av_log(wctx, AV_LOG_ERROR, "Item separator '%s' specified, but must contain a single character\n",
> +               flat->sep);
> +        return AVERROR(EINVAL);
> +    }
> +    flat->chr_sep = flat->sep[0];
> +    return 0;
> +}
> +
> +static const char *flat_escape_key_str(AVBPrint *dst, const char *src, const char sep)
> +{
> +    const char *p;
> +
> +    for (p = src; *p; p++) {
> +        if (!((*p >= '0' && *p <= '9') ||
> +              (*p >= 'a' && *p <= 'z') ||
> +              (*p >= 'A' && *p <= 'Z') ||
> +              *p == sep))
> +            av_bprint_chars(dst, '_', 1);
> +        else
> +            av_bprint_chars(dst, *p, 1);
> +    }
> +    return dst->str;
> +}
> +
> +static const char *flat_escape_value_str(AVBPrint *dst, const char *src)
> +{
> +    const char *p;
> +
> +    for (p = src; *p; p++) {
> +        switch (*p) {
> +        case '\n': av_bprintf(dst, "%s", "\\n");  break;
> +        case '\r': av_bprintf(dst, "%s", "\\r");  break;
> +        case '\\': av_bprintf(dst, "%s", "\\\\"); break;
> +        case '"':  av_bprintf(dst, "%s", "\\\""); break;
> +        case '\'': av_bprintf(dst, "%s", "\\'");  break;
> +        default:   av_bprint_chars(dst, *p, 1);   break;
> +        }
> +    }
> +    return dst->str;
> +}
> +
> +static void flat_print_section(WriterContext *wctx)
> +{
> +    int n = wctx->is_packets_and_frames ? wctx->nb_section_packet_frame
> +                                        : wctx->nb_section;
> +    FlatContext *flat = wctx->priv;

nit+++: private context is usually the first item in the declaration
section

> +
> +    if (flat->hierarchical && wctx->multiple_sections)
> +        printf("%s%c", flat->chapter, flat->chr_sep);
> +    printf("%s%c", flat->section, flat->chr_sep);
> +    if (wctx->multiple_sections)
> +        printf("%d%c", n, flat->chr_sep);
> +}

Would it make sense to use '_' like default separator character? This
way you have sh code output by default.

Also: you could cache the section name in the private context, like it
is done in the INI writer.

> +
> +static void flat_print_chapter_header(WriterContext *wctx, const char *chapter)
> +{
> +    FlatContext *flat = wctx->priv;
> +    flat->chapter = chapter;
> +}
> +
> +static void flat_print_section_header(WriterContext *wctx, const char *section)
> +{
> +    FlatContext *flat = wctx->priv;
> +    flat->section = section;
> +}

nit: please declare these in order of printing, seems more
logical/easier to read

[...]
-- 
FFmpeg = Fancy & Frightening Maxi Pure Earthshaking Geek