[FFmpeg-devel] AVDictionary for binary tags

[Dmitry Kostjuchenko]

Hi Reimar!

> The smallest issues is a type of char * simply isn't appropriate for 
> binary data since it is unpredictable whether it is signed or unsigned

For binary data it does not matter to my view, including sign because it is 
anyway casted to needed type or to void * or copied further and strict 
aligning rules will not be broken either. So it is ok, to my view, and 
preserving char * preserves the normal use of AVDictionary.

> Because it breaks both API and ABI.

Does it really break ABI for apps using older version of FFmpeg? 
AVDictionaryEntry got 1 new member and new allocation will be 8 bytes 
bigger. Old clients will not know about it and will read/write key/value 
members only. Binary data is identified by new keys which are not expected 
by old clients, for example, 'cover_jpeg'. Even if client, somehow parses 
all entries of AVDictionary (this is design error to my view) and tries to 
read value as string which in fact is binary data then crashing shall not 
happen as 0 byte is very likely in binary data and client side strlen will 
stop somewhen (the probability to read somewhere long ahead still exists I 
agree).

> However what if someone tries to put some 0-sized binary data?

I had initial proposal for binary data being used by AVDictionary and it can 
be improved for other situations if accepted for further development. One of 
solutions would be using av_dict_set with empty string "" as value. Client 
expects meaningful data which is identified by key value. So, once needed 
key is found and bsize == 0 and value is not NULL then it is 0-sized binary 
data.

> What if someone tries to add 6GB large binary data?

In case with MOV format I processed cover art tag. I doubt it will ever be 
bigger than 1-2 mb. In this sense bsize member can even be uint32_t but to 
avoid control of overflaw it is easier by implementation to have int64_t .

Best regards,
Dmitry.

----- Original Message ----- 
From: "Reimar Döffinger" <Reimar.Doeffinger at gmx.de>

> Because it breaks both API and ABI.
> The smallest issues is a type of char * simply isn't appropriate
> for binary data since it is unpredictable whether it is signed
> or unsigned (admittedly, in the light of UTF-8 it was always a
> bad choice).
> Applications that are not updated to support it (thus no longer
> working with older FFmpeg) cannot use it.
> Worse, the way you implemented it a non-updated application encountering
> a binary key might just crash.
> Then it also complicates the code a lot, and is likely to add a good
> deal of new bugs.
> For example it seems you decided to use bsize=0 to indicate a string.
> However what if someone tries to put some 0-sized binary data?
> What if someone tries to add 6GB large binary data? Or exactly 4 GB?
> No, this would not work "well" at all with base64-encoded data, but
> at least those issues aren't almost certain to cause a crash or similar.