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

Nicolas George nicolas.george at normalesup.org
Tue Nov 6 17:22:18 CET 2012


Le sextidi 16 brumaire, an CCXXI, Stefano Sabatini a écrit :
> ---
>  libavutil/opt.c |   25 ++++++++++++++++++++++++-
>  libavutil/opt.h |    6 ++++++
>  2 files changed, 30 insertions(+), 1 deletions(-)
> 
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 11e757e..640459a 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"
> @@ -1027,6 +1027,22 @@ void *av_opt_ptr(const AVClass *class, void *obj, const char *name)
>      return (uint8_t*)obj + opt->offset;
>  }
>  
> +void av_opt_bprint(struct AVBPrint *bp, void *obj)

I wonder if av_bprint_object() would not be better.

> +{
> +    const AVOption *opt = NULL;
> +    int is_first = 1;
> +
> +    while (opt = av_opt_next(obj, opt)) {
> +        uint8_t *val;
> +        av_opt_get(obj, opt->name, AV_OPT_SEARCH_CHILDREN, &val);
> +        if (opt->offset) {

Enhancement: keep a pointer to the previous option, and compare if opt and
prev_opt are actually the same option (same offset and type?). It will avoid
printing twice the same options when aliases are defined (s and size).

It also eliminates the need for is_first: just check that prev_opt is not
null.

> +            av_bprintf(bp, "%s%s=%s", is_first ? "" : " : ", opt->name, val);
> +            av_freep(&val);
> +        }
> +        is_first = 0;
> +    }
> +}
> +
>  #ifdef TEST
>  
>  #undef printf
> @@ -1079,6 +1095,7 @@ static const AVClass test_class = {
>  int main(void)
>  {
>      int i;
> +    AVBPrint bp;
>  
>      printf("\nTesting av_set_options_string()\n");
>      {
> @@ -1126,6 +1143,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_opt_bprint(&bp, &test_ctx);
> +        printf("test_ctx = %s\n", bp.str);
> +        av_bprint_clear(&bp);
> +
>          av_freep(&test_ctx.string);
>      }
>  
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 4a3b7f5..e97beaf 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -663,6 +663,12 @@ int av_opt_get_q     (void *obj, const char *name, int search_flags, AVRational
>   *          or written to.
>   */
>  void *av_opt_ptr(const AVClass *avclass, void *obj, const char *name);
> +
> +struct AVBPrint;
> +/**
> + * Append a compact description of a context to a bprint buffer.
> + */

A more detailed description would be nice.

> Consider this more like an RFC.
> 
> Now it shows something like:
> 
> num=42 : toggle=0 : rational=1/2 : string=blahblah : flags=0x00000004 : size=720x576 : pix_fmt=rgb24 : sample_fmt=s32

Looks reasonable.

> A possible extension:
> 
> av_opt_bprint(struct AVBPrint *bp, void *obj, const char *delim, const char *key_val_sep);
> 
> (and at this point we will also need to implement an escaping function
> in the lib, like the one in tools/ffescape).

I think a quoting API is a good idea anyway.

> The function would be useful to inspect a context "configuration"
> without to rely on programming/debugging. Also the generated string
> may be used to literally copy a configuration, and set it elsewhere
> (e.g. with av_opt_set_from_string()).

If the resulting string is guaranteed to be parseable by the options system
(and I believe it would be a good idea), it should be stated in the docs.

(Of course, it would not work for objects that have conflicting options that
should not be set separately, but there is nothing simple to do about it.

Good idea on the whole, thanks.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121106/7cdeada1/attachment.asc>


More information about the ffmpeg-devel mailing list