[FFmpeg-devel] [PATCH] Add writing of vorbis comments to flac files

[Baptiste Coudurier]

On 3/14/10 11:24 AM, Justin Ruggles wrote:
> Aurelien Jacobs wrote:
>
>> On Sat, Mar 13, 2010 at 06:24:30PM +0100, James Darnley wrote:
>>> On 13 March 2010 02:28, James Darnley<james.darnley at gmail.com>  wrote:
>>>> On 13 March 2010 01:00, Aurelien Jacobs<aurel at gnuage.org>  wrote:
>>>>> On Fri, Mar 12, 2010 at 02:19:54PM +0100, James Darnley wrote:
>>>>>> On 12 March 2010 13:44, Aurelien Jacobs<aurel at gnuage.org>  wrote:
>>>>>>> You are not allowed to access to the content of AVMetadata *m by
>>>>>>> yourself. You must not use anything else than what's described in
>>>>>>> Public Metadata API (avformat.h).
>>>>>>> Basically you should use av_metadata_get() to iterate over all the
>>>>>>> elements.
>>>>>>> Something like this should do the trick:
>>>>>>>
>>>>>>>   AVMetadataTag *t = NULL;
>>>>>>>   while ((t = av_metadata_get(m, "", t, AV_METADATA_IGNORE_SUFFIX))) {
>>>>>>>      /* do wathever you want with 't' */
>>>>>>>   }
>>>>>>>
>>>>>>> Aurel
>>>>>> Ew.  Why isn't there a cleaner method for getting the next tag?
>>>>> What kind of cleaner method would you expect ?
>>>>> Are you simply thinking about something like this :
>>>>>
>>>>>   #define av_metadata_next(m, t) \
>>>>>           av_metadata_get(m, "", t, AV_METADATA_IGNORE_SUFFIX)
>>>>>
>>>>> I thougt about something like this while we were designing the API,
>>>>> but it would only hide the real API... I'm not sure that it's a good
>>>>> idea.
>>>>>
>>>>> Aurel
>>>> Yeah, something like that.  This is used in 13 other places.  Another
>>>> thing I discovered when making changes is that if I can't use m->count
>>>> I need to count the number of tags over the while loop.  Sounds
>>>> redundant to me.  Is making this value public (somehow) such a bad
>>>> idea?
>>>>
>>> Changed patches attached.
>>
>> The metadata API usage looks OK now.
>
> So patch 1 looks good.
>
> Baptiste, does patch 2 look ok for the ogg muxer?

Looks good.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org