On Fri, Sep 4, 2015 at 4:18 PM, wm4 <nfx...@googlemail.com> wrote: > On Fri, 4 Sep 2015 14:38:54 +0100 > Kevin Wheatley <kevin.j.wheat...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel