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

[Justin Ruggles]

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?

-Justin