[FFmpeg-devel] [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types

[Kevin Wheatley]

On Fri, Sep 4, 2015 at 4:18 PM, wm4 <nfxjfg at googlemail.com> wrote:
> On Fri, 4 Sep 2015 14:38:54 +0100
> Kevin Wheatley <kevin.j.wheatley at gmail.com> wrote:
>
>> Hi,
>>
>> as part of adding support for non-string data types to .mov metadata,
>> I wondered about adding the following helper functions for storing
>> numeric types into an AVDictionary.
>>
>> upfront I'll say I'm not 100% happy with the float32 and float64 named
>> variants (vs float and double) as there is no clear preference in
>> other areas of the code that I could see.
>
> I don't understand it either. Why not just use float and double,
> instead of obfuscating the names further? (It'd be a difference if this
> function e.g. serialized the floats to binary - but it literally just
> passes them as-is to libc. Having the C data type in the argument seems
> advantageous.
>
> Also, what's the use in having a float function at all?
>
> I'd call av_dict_set_uint av_dict_set_uint64.

I'll explain my POV...

Essentially these helpers do the same as the existing
av_dict_set_int() does for int64_t so I've essentially just copied
that and substituted the required conversions from <type> to char
array. The floating point is needed to correctly decode those formats.

Using the bit length in the name of the floating point variation of
the functions is because they will only correctly produce the textual
representation sufficient to transform back into the same binary
representation based on the bit lengths of the floating point
representation (they assume IEEE single and double precision in the
lengths of the formatting options and thus buffer size).

Mentally I was in the middle of the Apple QuickTime metadata
representations where they also use the floatXX terminology - not a
great excuse I know :-)


Re Nicholas' comments on the

    flags &= ~AV_DICT_DONT_STRDUP_VAL;

I was simply replicating the existing form of the function av_dict_set_int().

As to portability in the tests - yes I agree - happy for somebody to
point me towards a good solution for that.


My alternative to these would have been to embed the formatting in the
libavformat/mov.c code which obviously would have less impact.

Kevin