[FFmpeg-devel] [PATCH] lavu/opt: add av_opt_bprint()

[Stefano Sabatini]

On date Thursday 2013-01-10 19:38:57 +0100, Nicolas George encoded:
> Le nonidi 19 nivôse, an CCXXI, Stefano Sabatini a écrit :
> > >From 9b7de355afb61bab72bce682aacbd96eb4a9cb95 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Thu, 9 Aug 2012 19:59:48 +0200
> > Subject: [PATCH] lavu/bprint: add av_bprint_options()
> > 
> > This function will be useful to serialize an object context.
> > 
> > TODO: add APIChanges entry, bump minor
> > ---
> >  libavutil/bprint.c |   23 +++++++++++++++++++++++
> >  libavutil/bprint.h |   15 +++++++++++++++
> >  libavutil/opt.c    |   10 +++++++++-
> >  3 files changed, 47 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavutil/bprint.c b/libavutil/bprint.c
> > index 64cd8db..4565698 100644
> > --- a/libavutil/bprint.c
> > +++ b/libavutil/bprint.c
> > @@ -28,6 +28,7 @@
> >  #include "common.h"
> >  #include "error.h"
> >  #include "mem.h"
> > +#include "opt.h"
> >  
> >  #define av_bprint_room(buf) ((buf)->size - FFMIN((buf)->len, (buf)->size))
> >  #define av_bprint_is_allocated(buf) ((buf)->str != (buf)->reserved_internal_buffer)
> > @@ -262,6 +263,28 @@ int av_bprint_escape(AVBPrint *dstbuf, const char *src, const char *special_char
> >      return dstbuf->len;
> >  }
> >  
> > +void av_bprint_options(struct AVBPrint *bp, void *ctx,
> > +                       const char *key_val_sep, const char *pairs_sep)
> > +{
> > +    const AVOption *opt = NULL, *prev_opt = NULL;
> > +
> > +    while (opt = av_opt_next(ctx, opt)) {
> > +        uint8_t *val;
> > +        char sep[2] = { pairs_sep[0], 0 };
> > +
> > +        /* skip duplicated option values */
> > +        if (prev_opt && prev_opt->offset == opt->offset)
> > +            continue;
> > +        av_opt_get(ctx, opt->name, AV_OPT_SEARCH_CHILDREN, &val);
> > +        if (opt->offset) {
> > +            av_bprintf(bp, "%s%s%c", prev_opt ? sep : "", opt->name, key_val_sep[0]);
> > +            av_bprint_escape(bp, val, pairs_sep, AV_ESCAPE_MODE_BACKSLASH, 0);
> > +            av_freep(&val);
> > +        }
> > +        prev_opt = opt;
> > +    }
> > +}
> > +
> >  #ifdef TEST
> >  
> >  #undef printf
> > diff --git a/libavutil/bprint.h b/libavutil/bprint.h
> > index b9039bd..316f99c 100644
> > --- a/libavutil/bprint.h
> > +++ b/libavutil/bprint.h
> > @@ -195,4 +195,19 @@ int av_bprint_finalize(AVBPrint *buf, char **ret_str);
> >  int av_bprint_escape(AVBPrint *dstbuf, const char *src, const char *special_chars,
> >                       enum AVEscapeMode mode, int flags);
> >  
> > +/**
> > + * Append a compact description of the context options to a bprint buffer.
> 
> > + * The obtained string is escape, and can be used to set options through the function
> 
> escaped

Fixed locally.

> 
> > + * av_opt_set_from_string() or equivalent.
> 
> "the foobar() function", I believe.

I don't know what you mean.

> 
> > + *
> > + * @param bp           already inited destination bprint buffer
> > + * @param ctx          pointer to a context whose first field is a pointer to an AVClass
> > + * @param key_val_sep  a 0-terminated list of characters used to separate
> > + *                     key from value, for example '='
> > + * @param pairs_sep    a 0-terminated list of characters used to separate
> > + *                     two pairs from each other, for example ':' or ','
> > + */
> > +void av_bprint_options(struct AVBPrint *bp, void *ctx,
> > +                       const char *key_val_sep, const char *pairs_sep);
> > +
> >  #endif /* AVUTIL_BPRINT_H */
> > diff --git a/libavutil/opt.c b/libavutil/opt.c
> > index 61c76da..5a02113 100644
> > --- a/libavutil/opt.c
> > +++ b/libavutil/opt.c
> > @@ -27,7 +27,7 @@
> >  
> >  #include "avutil.h"
> >  #include "avstring.h"
> > -#include "common.h"
> > +#include "bprint.h"
> >  #include "opt.h"
> >  #include "eval.h"
> >  #include "dict.h"
> > @@ -1329,6 +1329,7 @@ static const AVOption test_options[]= {
> >  {"lame",     "set lame flag ", 0,                AV_OPT_TYPE_CONST,    {.i64 = TEST_FLAG_LAME}, INT_MIN,  INT_MAX, 0, "flags" },
> >  {"mu",       "set mu flag ",   0,                AV_OPT_TYPE_CONST,    {.i64 = TEST_FLAG_MU},   INT_MIN,  INT_MAX, 0, "flags" },
> >  {"size",     "set size",       OFFSET(w),        AV_OPT_TYPE_IMAGE_SIZE,{0},             0,        0                   },
> 
> > +{"s",        "set size",       OFFSET(w),        AV_OPT_TYPE_IMAGE_SIZE,{0},             0,        0                   },
> 
> Not entirely related, but I see the point.
> 
> >  {"pix_fmt",  "set pixfmt",     OFFSET(pix_fmt),  AV_OPT_TYPE_PIXEL_FMT, {.i64 = AV_PIX_FMT_NONE}, -1, AV_PIX_FMT_NB-1},
> >  {"sample_fmt", "set samplefmt", OFFSET(sample_fmt), AV_OPT_TYPE_SAMPLE_FMT, {.i64 = AV_SAMPLE_FMT_NONE}, -1, AV_SAMPLE_FMT_NB-1},
> >  {NULL},
> > @@ -1348,6 +1349,7 @@ static const AVClass test_class = {
> >  int main(void)
> >  {
> >      int i;
> > +    AVBPrint bp;
> >  
> >      printf("\nTesting av_set_options_string()\n");
> >      {
> > @@ -1395,6 +1397,12 @@ int main(void)
> >                  av_log(&test_ctx, AV_LOG_ERROR, "Error setting options string: '%s'\n", options[i]);
> >              printf("\n");
> >          }
> > +
> > +        av_bprint_init(&bp, 1, AV_BPRINT_SIZE_UNLIMITED);
> > +        av_bprint_options(&bp, &test_ctx, "=", ":");
> > +        printf("test_ctx=%s\n", bp.str);
> > +        av_bprint_clear(&bp);
> > +
> >          av_freep(&test_ctx.string);
> >      }
> >  
> 

> Looks good. Wild idea, not blocking: try to determine whether the options
> are at their default or not.

The idea is to be able to represent the options explicitetly, so that
it is possible to get the same configuration even in case the default
values change. But I could add a flag (DONT_SHOW_DEFAULT) if you think
it is worth the added complexity.
-- 
FFmpeg = Fancy & Fostering Minimal Political Erotic Ghost