On date Wednesday 2014-11-12 22:32:04 +0100, Lukasz Marek encoded:
> On 12.11.2014 17:45, Stefano Sabatini wrote:
> >On date Tuesday 2014-11-11 21:24:45 +0100, Lukasz Marek encoded:
> >>On 11.11.2014 17:19, Stefano Sabatini wrote:
> >>>We have already av_get_opt() which serializes the value. Also we should
> >>>probably escape the values.
> >>
> >>I saw that function, but don't remember why I didn't use. I was
> >>wrong obviously.
> >>BTW, I found few bugs in it, sent separate patch for it.
> >
> >>Updated patch is attached. It is without escaping. I left it for
> >>separate patch.
> >
> >Not sure this is a good idea. Indeed the API allows to specify several
> >key/value and option separators, so this may be a limitation.
>
> I don't want to say it is not needed or so, but it can be done in
> separate patch, thats all.
Yes but it would break public API.
>
>
> >>+int av_opt_serialize(void *obj, int opt_flags, int skip_defaults, char
> >>**buffer,
> >>+ const char key_val_sep, const char pairs_sep)
> >
> >skip_defaults could be a flag, in case we want to extend it further
> >(for example suppose we only want to print long or short option
> >names).
>
> Yes, this is good and I changed it.
>
> I don't know if we should also add a parameter for choosing
> >the escaping algorithm, probably not.
>
> I don't see any reason for it. If any in future, it can still be
> forced by flag.
>
> >>+{
> >>+ const AVOption *o = NULL;
> >>+ uint8_t *buf;
> >>+ AVBPrint bprint;
> >>+ int ret, cnt = 0;
> >>+
> >>+ if (!obj || !buffer)
> >>+ return AVERROR(EINVAL);
> >>+
> >>+ *buffer = NULL;
> >>+ av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
> >>+
> >>+ while (o = av_opt_next(obj, o)) {
> >>+ if (o->flags & opt_flags != opt_flags || o->type ==
> >>AV_OPT_TYPE_CONST)
> >>+ continue;
> >>+ if (skip_defaults && av_opt_is_set_to_default(obj, o) > 0)
> >>+ continue;
> >
> >>+ if ((ret = av_opt_get(obj, o->name, 0, &buf)) < 0) {
> >
> >Here there is a potential problem. At the moment there is no way to
> >distinguish between a string set to NULL and a string set to "", since
> >av_opt_get() will return "" for a string set to NULL. For some
> >configurable elements this will make a difference. The only solution
> >in this case is to use "skip defaults". Also in general the user won't
> >be able to set an option to NULL if not using the binary interface.
>
> I know it cannot be distinguish now. It is possible to serialize
> null as \0 for example (where \ is escaping char), but such string
> have to be unescaped before passing to set_from_string function, or
> such function support escaping itself.
>
>
> >>+ av_bprint_finalize(&bprint, NULL);
> >>+ return ret;
> >>+ }
> >
> >This will print alias options as well. This was my solution:
>
> I'm not sure it is always safe. Options with the same offset may
> have different opt_flags and different defaults.
I think this would be a bug, since for example set_defaults with set
only the value of the last specified option. Do you have examples for
this?
> user may want to serialize object if opt_flags = 0, but apply to
> object with specific mask.
> This can be controlled by a flag I think.
>
> Updated doxy, I hope it is better.
> From 715f2ef32c85112a4d7dced62afcb6d89274a927 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.lu...@gmail.com>
> Date: Mon, 10 Nov 2014 22:28:44 +0100
> Subject: [PATCH] lavu/opt: introduce av_opt_serialize()
>
> TODO: bump minor version, update doc/APIchanges
>
> Function allows to create string containing object's serialized options.
> Such string may be passed back to av_set_options_string() in order to restore
> options.
>
> Signed-off-by: Lukasz Marek <lukasz.m.lu...@gmail.com>
> ---
> libavutil/opt.c | 64
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> libavutil/opt.h | 20 +++++++++++++++++
> tests/ref/fate/opt | 8 +++++++
> 3 files changed, 92 insertions(+)
>
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 1381cc9..f0bb3cf 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -37,6 +37,7 @@
> #include "pixdesc.h"
> #include "mathematics.h"
> #include "samplefmt.h"
> +#include "bprint.h"
>
> #include <float.h>
>
> @@ -1835,6 +1836,40 @@ int av_opt_is_set_to_default_by_name(void *obj, const
> char *name, int search_fla
> return av_opt_is_set_to_default(target, o);
> }
>
> +int av_opt_serialize(void *obj, int opt_flags, int flags, char **buffer,
> + const char key_val_sep, const char pairs_sep)
> +{
> + const AVOption *o = NULL;
> + uint8_t *buf;
> + AVBPrint bprint;
> + int ret, cnt = 0;
> +
> + if (!obj || !buffer)
> + return AVERROR(EINVAL);
> +
> + *buffer = NULL;
> + av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
> +
> + while (o = av_opt_next(obj, o)) {
> + if (o->flags & opt_flags != opt_flags || o->type ==
> AV_OPT_TYPE_CONST)
> + continue;
> + if (flags & AV_OPT_SERIALIZE_SKIP_DEFAULTS &&
> av_opt_is_set_to_default(obj, o) > 0)
> + continue;
> + if ((ret = av_opt_get(obj, o->name, 0, &buf)) < 0) {
> + av_bprint_finalize(&bprint, NULL);
> + return ret;
> + }
> + if (buf) {
> + if (cnt++)
> + av_bprint_append_data(&bprint, &pairs_sep, 1);
> + av_bprintf(&bprint, "%s%c%s", o->name, key_val_sep, buf);
> + av_freep(&buf);
> + }
> + }
> + av_bprint_finalize(&bprint, buffer);
> + return 0;
> +}
> +
Not sure we should skip escaping here...
> #ifdef TEST
>
> typedef struct TestContext
> @@ -1854,6 +1889,10 @@ typedef struct TestContext
> int64_t channel_layout;
> void *binary;
> int binary_size;
> + void *binary1;
> + int binary_size1;
> + void *binary2;
> + int binary_size2;
> int64_t num64;
> float flt;
> double dbl;
> @@ -1882,6 +1921,8 @@ static const AVOption test_options[]= {
> {"color", "set color", OFFSET(color), AV_OPT_TYPE_COLOR, {.str = "pink"},
> 0, 0},
> {"cl", "set channel layout", OFFSET(channel_layout),
> AV_OPT_TYPE_CHANNEL_LAYOUT, {.i64 = AV_CH_LAYOUT_HEXAGONAL}, 0, INT64_MAX},
> {"bin", "set binary value", OFFSET(binary), AV_OPT_TYPE_BINARY,
> {.str="62696e00"}, 0, 0 },
> +{"bin1", "set binary value", OFFSET(binary1), AV_OPT_TYPE_BINARY,
> {.str=NULL}, 0, 0 },
> +{"bin2", "set binary value", OFFSET(binary2), AV_OPT_TYPE_BINARY,
> {.str=""}, 0, 0 },
> {"num64", "set num 64bit", OFFSET(num64), AV_OPT_TYPE_INT64, {.i64
> = 1}, 0, 100 },
> {"flt", "set float", OFFSET(flt), AV_OPT_TYPE_FLOAT, {.dbl
> = 1.0/3}, 0, 100 },
> {"dbl", "set double", OFFSET(dbl), AV_OPT_TYPE_DOUBLE, {.dbl
> = 1.0/3}, 0, 100 },
> @@ -1949,6 +1990,29 @@ int main(void)
> }
> }
>
> + printf("\nTest av_opt_serialize()\n");
> + {
> + TestContext test_ctx = { 0 };
> + char *buf;
> + test_ctx.class = &test_class;
> +
> + av_log_set_level(AV_LOG_QUIET);
> +
> + av_opt_set_defaults(&test_ctx);
> + if (av_opt_serialize(&test_ctx, 0, 0, &buf, '=', ',') >= 0) {
> + printf("%s\n", buf);
> + av_opt_free(&test_ctx);
> + memset(&test_ctx, 0, sizeof(test_ctx));
> + test_ctx.class = &test_class;
> + av_set_options_string(&test_ctx, buf, "=", ",");
> + av_free(buf);
> + if (av_opt_serialize(&test_ctx, 0, 0, &buf, '=', ',') >= 0) {
> + printf("%s\n", buf);
> + av_free(buf);
> + }
> + }
> + }
> +
> printf("\nTesting av_set_options_string()\n");
> {
> TestContext test_ctx = { 0 };
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 5158067..5fa7706 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -869,6 +869,26 @@ int av_opt_is_set_to_default(void *obj, const AVOption
> *o);
> */
> int av_opt_is_set_to_default_by_name(void *obj, const char *name, int
> search_flags);
>
> +
> +#define AV_OPT_SERIALIZE_SKIP_DEFAULTS 0x00000001 ///< Serialize options
> that are not set to default values only.
> +
> +/**
> + * Serialize object's options.
> + *
> + * Create a string containing object's serialized options.
> + * Such string may be passed back to av_opt_set_from_string() in order to
> restore option values.
> + *
> + * @param[in] obj AVClass object to serialize
> + * @param[in] opt_flags serialize options with all the specified flags
> set (AV_OPT_FLAG)
> + * @param[in] flags combination of AV_OPT_SERIALIZE_* flags
> + * @param[out] buffer Pointer to buffer that will be allocated with
> string containg serialized options.
> + * Buffer must be freed by the caller when is no
> longer needed.
nit: pointer
> + * @param[in] key_val_sep character used to separate key from value
> + * @param[in] pairs_sep character used to separate two pairs from each
> other
> + * @return >= 0 on success, negative on error
> + */
> +int av_opt_serialize(void *obj, int opt_flags, int flags, char **buffer,
> + const char key_val_sep, const char pairs_sep);
> /**
> * @}
> */
> diff --git a/tests/ref/fate/opt b/tests/ref/fate/opt
> index 7953ce8..16f3387 100644
> --- a/tests/ref/fate/opt
> +++ b/tests/ref/fate/opt
> @@ -34,6 +34,8 @@ name: duration default:0 error:
> name: color default:0 error:
> name: cl default:0 error:
> name: bin default:0 error:
> +name: bin1 default:1 error:
> +name: bin2 default:1 error:
> name: num64 default:0 error:
> name: flt default:0 error:
> name: dbl default:0 error:
> @@ -53,10 +55,16 @@ name: duration default:1 error:
> name: color default:1 error:
> name: cl default:1 error:
> name: bin default:1 error:
> +name: bin1 default:1 error:
> +name: bin2 default:1 error:
> name: num64 default:1 error:
> name: flt default:1 error:
> name: dbl default:1 error:
>
> +Test av_opt_serialize()
> +num=0,toggle=1,rational=1/1,string=default,flags=0x00000001,size=200x300,pix_fmt=0bgr,sample_fmt=s16,video_rate=25/1,duration=0:00:00.001000,color=0xffc0cbff,cl=0x137,bin=62696E00,bin1=,bin2=,num64=1,flt=0.333333,dbl=0.333333
> +num=0,toggle=1,rational=1/1,string=default,flags=0x00000001,size=200x300,pix_fmt=0bgr,sample_fmt=s16,video_rate=25/1,duration=0:00:00.001000,color=0xffc0cbff,cl=0x137,bin=62696E00,bin1=,bin2=,num64=1,flt=0.333333,dbl=0.333333
LGTM otherwise.
--
FFmpeg = Faithless and Formidable Multimedia Power Elected Game
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
