[FFmpeg-cvslog] r16966 - trunk/libavformat/aiff.c

[Baptiste Coudurier]

Aurelien Jacobs wrote:
> Baptiste Coudurier wrote:
> 
>> Aurelien Jacobs wrote:
>>> Baptiste Coudurier wrote:
>>>
>>>> aurel wrote:
>>>>> Author: aurel
>>>>> Date: Tue Feb  3 00:43:12 2009
>>>>> New Revision: 16966
>>>>>
>>>>> Log:
>>>>> use new metadata API in aiff demuxer
>>>>>
>>>>> Modified:
>>>>>    trunk/libavformat/aiff.c
>>>>>
>>>>> Modified: trunk/libavformat/aiff.c
>>>>> ==============================================================================
>>>>> --- trunk/libavformat/aiff.c	Tue Feb  3 00:37:03 2009	(r16965)
>>>>> +++ trunk/libavformat/aiff.c	Tue Feb  3 00:43:12 2009	(r16966)
>>>>> @@ -79,9 +79,10 @@ static int get_tag(ByteIOContext *pb, ui
>>>>>  }
>>>>>  
>>>>>  /* Metadata string read */
>>>>> -static void get_meta(ByteIOContext *pb, char * str, int strsize, int size)
>>>>> +static void get_meta(AVFormatContext *s, const char *key, int size)
>>>>>  {
>>>>> -    int res = get_buffer(pb, (uint8_t*)str, FFMIN(strsize-1, size));
>>>>> +    uint8_t str[1024];
>>>>> +    int res = get_buffer(s->pb, str, FFMIN(sizeof(str)-1, size));
>>>>>      if (res < 0)
>>>>>          return;
>>>> Does the new API have the 1024 limitation ? I hope not.
>>> Hopefully not !
>>>
>>>> Therefore everything must be read and it's up the compat layer to
>>>> truncate it to 1024.
>>> Everything ? size is a 32 byte value read directly from the bitstream,
>>> so I would rather not try to av_malloc(size).
>>> So IMO some sanity check must be applied. I though 1024 would be plenty
>>> enough. How much do you think the limitation should be ?
>> You have a "comment" field, what if quicktime reads it all ?
> 
> If quicktime wants to read a full 1GB comment field, that is it's own
> problem. I don't think FFmpeg should do this...

Well you can get users complaining...
Though I also think it's not reasonable.

>> What if I want to write my lyrics in there ? :>
> 
> Then you would probably need a bit more than 1024 bytes...
> If you think it's useful, I will av_malloc() the buffer,
> but I still need to set some limit. Would a 1MB limit
> be reasonable enough ?

To be honest, I don't really know, I guess 1MB is ok, I'd like to get
others opinion also.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org