[FFmpeg-devel] [PATCH 2/9] lavu/opt: introduce av_opt_serialize()

[Lukasz Marek]

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.


>> +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. 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavu-opt-introduce-av_opt_serialize.patch
Type: text/x-patch
Size: 7041 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141112/fe1d7fdf/attachment.bin>