[FFmpeg-devel] [PATCH 2/9] lavu/opt: introduce av_opt_serialize()
Stefano Sabatini
stefasab at gmail.com
Wed Nov 12 17:45:56 CET 2014
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.
> Is there any unescape function available?
av_get_token()
Probably we should update documentation.
> From e2f15237517381bb1a7a6613ab80196cd5ae92d6 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki2 at 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.luki2 at gmail.com>
> ---
> libavutil/opt.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> libavutil/opt.h | 17 +++++++++++++++
> tests/ref/fate/opt | 8 +++++++
> 3 files changed, 89 insertions(+)
>
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 1381cc9..1d8e4b4 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 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). I don't know if we should also add a parameter for choosing
the escaping algorithm, probably not.
> +{
> + 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.
> + av_bprint_finalize(&bprint, NULL);
> + return ret;
> + }
This will print alias options as well. This was my solution:
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 && val) {
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_AUTO, 0);
av_freep(&val);
}
prev_opt = opt;
}
}
> + 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;
> +}
> +
> #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..4669c55 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -870,6 +870,23 @@ 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);
>
> /**
> + * Serialize object's options.
> + *
> + * Create string containing object's serialized options.
Create _a_ string ...?
> + * Such string may be passed back to av_set_options_string() in order to restore option values.
av_set_options_string() => av_opt_set_from_string() (the former should
be probably rewritten using the latter and deprecated, since it's redundant)
s/restore/reset or set
> + *
> + * @param[in] obj AVClass object to check option on.
> + * @param[in] opt_flags Serialize only options with all the specified flags set (AV_OPT_FLAG).
> + * @param[in] skip_defaults When set to non-zero options that are set to their default will not be serialized.
> + * @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.
> + * @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.
nit: please do not capitalize and terminate with a dot incomplete
sentences, like this:
* @param[in] obj AVClass object to check option on
* @param[in] opt_flags flags used to mark options to serialize. It will only serialize ...
[...]
* @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
[...]
--
FFmpeg = Freak and Foolish Multipurpose Proud Elastic Gladiator
More information about the ffmpeg-devel
mailing list