[FFmpeg-devel] [PATCH] use new metadata API in mov (de)muxer

[Baptiste Coudurier]

Hi,

Aurelien Jacobs wrote:
> Aurelien Jacobs wrote:
> 
>> Baptiste Coudurier wrote:
>>
>>> Hi,
>>>
>>> On 2/14/2009 5:13 PM, Aurelien Jacobs wrote:
>>>> Aurelien Jacobs wrote:
>>>>
>>>>> Baptiste Coudurier wrote:
>>>>>
>>>>>> Hi Aurelien,
>>>>>>
>>>>>> Aurelien Jacobs wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Attached patch converts the mov muxer and demuxer to the new
>>>>>>> metadata API. It properly (de)mux lang when available. Note
>>>>>>> that current muxer code set lang to english when lang is
>>>>>>> unknown. I left this behavior unchanged, especially because I
>>>>>>> don't know what numeric language code to use for unknown
>>>>>>> language.
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>> -    case MKTAG(0xa9,'n','a','m'): -        str = c->fc->title;
>>>>>>> size = sizeof(c->fc->title); break; +    case
>>>>>>> MKTAG(0xa9,'n','a','m'): key = "title";     break; +    case
>>>>>>> MKTAG(0xa9,'a','u','t'): case MKTAG(0xa9,'A','R','T'): -
>>>>>>> case MKTAG(0xa9,'w','r','t'): -        str = c->fc->author;
>>>>>>> size = sizeof(c->fc->author); break; -    case
>>>>>>> MKTAG(0xa9,'c','p','y'): -        str = c->fc->copyright; size
>>>>>>> = sizeof(c->fc->copyright); break; +    case
>>>>>>> MKTAG(0xa9,'w','r','t'): key = "author";    break; +    case
>>>>>>> MKTAG(0xa9,'c','p','y'): key = "copyright"; break; case
>>>>>>> MKTAG(0xa9,'c','m','t'): -    case MKTAG(0xa9,'i','n','f'): -
>>>>>>> str = c->fc->comment; size = sizeof(c->fc->comment); break; -
>>>>>>> case MKTAG(0xa9,'a','l','b'): -        str = c->fc->album; size
>>>>>>> = sizeof(c->fc->album); break; +    case
>>>>>>> MKTAG(0xa9,'i','n','f'): key = "comment";   break; +    case
>>>>>>> MKTAG(0xa9,'a','l','b'): key = "album";     break; +    case
>>>>>>> MKTAG(0xa9,'d','a','y'): key = "year";      break; +    case
>>>>>>> MKTAG(0xa9,'g','e','n'): key = "genre";     break; +    case
>>>>>>> MKTAG(0xa9,'t','o','o'): +    case MKTAG(0xa9,'e','n','c'): key
>>>>>>> = "muxer";     break;
>>>>>> I saw that in vorbis demuxer you exported metadata "as is", but
>>>>>> here you apply some generic metadata.
>>>>>>
>>>>>> 1) Shouldn't we also export "as is" everything in "udta" atom
>>>>>> coded in a somewhat standard way (itunes, 3gp, mov) ? This is
>>>>>> what I would call "generic" and user could retrieve it if
>>>>>> wanted.
>>>>> That would indeed be more generic, but from my understanding, the 
>>>>> metadata key is supposed to be a user understandable string. A end 
>>>>> user application should display the straight key string. mov atom
>>>>> identifier don't really qualify as user readable string... BTW:
>>>>> does mov have a small limited set of well know supported metadata
>>>>> atom, or is anyone allowed to set any random metadata at will ?
>>>> Well. I still think my patch is the right way to do it. For formats
>>>> which don't have free text keys, the demuxer should export the small
>>>> set of allowed tags each with a comprehensible text key.
>>> Well, I'm still not happy with the language mechanism, but oh well...
>>>
>>>> So finally, here is the demuxer part of the patch, split, but 
>>>> otherwise unchanged. First patch switch to the new API. Second patch
>>>> adds support for a few more tags.
>>> I also think that before adding more tags we should clean the code by
>>> using an array.
>> OK. Applied the demuxer patch. I will send a patch to use an array for
>> keys/atom.
> 
> Here is a patch using an array for keys/atom.

Hummm I didn't realize it would require messing with mov_read_default,
this is a bit messy, I'd really prefer to isolate metadata handling in
its own read_udta generic function I think.

So you can just add the new tags, sorry.

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