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

[Aurelien Jacobs]

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...

> 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 ?

Aurel