[FFmpeg-cvslog] r17686 - trunk/libavformat/r3d.c

[Baptiste Coudurier]

Aurelien Jacobs wrote:
> Baptiste Coudurier wrote:
> 
>> Hi,
>>
>> On 3/1/2009 7:28 AM, aurel wrote:
>>> Author: aurel
>>> Date: Sun Mar  1 16:28:56 2009
>>> New Revision: 17686
>>>
>>> Log:
>>> use new metadata API in r3d demuxer
>>>
>>> Modified:
>>>    trunk/libavformat/r3d.c
>>>
>>> Modified: trunk/libavformat/r3d.c
>>> ==============================================================================
>>> --- trunk/libavformat/r3d.c	Sun Mar  1 15:56:27 2009	(r17685)
>>> +++ trunk/libavformat/r3d.c	Sun Mar  1 16:28:56 2009	(r17686)
>>> @@ -51,6 +51,7 @@ static int read_atom(AVFormatContext *s,
>>>  static int r3d_read_red1(AVFormatContext *s)
>>>  {
>>>      AVStream *st = av_new_stream(s, 0);
>>> +    char filename[258];
>>>      int tmp, tmp2;
>>>  
>>>      if (!st)
>>> @@ -92,12 +93,11 @@ static int r3d_read_red1(AVFormatContext
>>>          av_set_pts_info(ast, 32, 1, st->time_base.den);
>>>      }
>>>  
>>> -    st->filename = av_mallocz(258);
>>> -    if (!st->filename)
>>> -        return AVERROR(ENOMEM);
>>> -    get_buffer(s->pb, st->filename, 257);
>>> +    get_buffer(s->pb, filename, 257);
>>> +    filename[sizeof(filename)-1] = 0;
>>> +    av_metadata_set(&st->metadata, "filename", filename);
>>>  
>>> -    dprintf(s, "filename %s\n", st->filename);
>>> +    dprintf(s, "filename %s\n", filename);
>>>      dprintf(s, "resolution %dx%d\n", st->codec->width, st->codec->height);
>>>      dprintf(s, "timescale %d\n", st->time_base.den);
>>>      dprintf(s, "frame rate %d/%d\n",
>> Wouldn't it be more convenient to be able to av_metadata_set using
>> uint8_t *, avoiding memcpy/strdup ?
> 
> Could you be more specific ? I don't get what you mean. This is exactly
> how av_metadata_set() works right now.

Really ? I see av_strdup in av_metadata_set.

> Or maybe you mean using av_metadata_set() with a ByteIOContext* ?
> And if so, should it read a fixed amount of bytes (eventually adding
> trailing 0) or should it read until it find a 0 ? In the last case,
> it wouldn't avoid a memcpy. And in the first case, I think it would
> anyway handle only a very limited subset of the av_metadata_set()
> use case.
> 

I mean:

filename = av_malloc(257);
get_buffer(s->pb, filename, 257);
<set trailing 0>
av_metadata_set(&st->metadata, "filename", filename);

av_metadata_set won't memcpy nor strdup, it will use given buffer, this
avoids declaring a local buffer.

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