[FFmpeg-devel] [PATCH] Metadata

Aurelien Jacobs aurel
Sun Jan 4 19:49:18 CET 2009


Michael Niedermayer wrote:

> On Sun, Jan 04, 2009 at 06:41:00PM +0100, Aurelien Jacobs wrote:
> > Michael Niedermayer wrote:
> > 
> > > Hi
> > > 
> > > Attached patch adds generic metadata support to
> > > ffmpeg.c, and the avi muxer and demuxer
> > > The implementation should be pretty much as simple as possible.
> > > 
> > > Differences to aurels variant
> > > * flat metadata, no trees
> > 
> > This is OK for me.
> > I don't really care about trees. Anyway it's rarely used, so I simply won't
> > support it in mkv at first. It can always be improved later in the mkv
> > (de)muxer, flattening tags such as AUTHOR/ADDRESS.
> > 
> > > * fully abstracted, implementation can be changed with no effect on ABI/API
> > > * only a small set of muxers & demuxers are updated yet.
> > > 
> > > Comments welcome, especally from aurel.
> > 
> > First, several general questions/comments:
> > 
> > * why is metadata.{c,h} in libavcodec while it's only used in libavformat ?
> > I think the patch shouldn't touch libavcodec at all...
> 
> It was because i thought some idiot might put metadata at the codec level
> and we might want to extract that too. But i dont mind moving the files to
> lavf if you prefer? We can always move them back if it becomes needed ...

Indeed, I prefer seeing it in lavf as long as it's not needed in lavc.

> > > Index: libavcodec/avcodec.h
> > > ===================================================================
> > > --- libavcodec/avcodec.h	(revision 16302)
> > > +++ libavcodec/avcodec.h	(working copy)
> > > @@ -400,7 +400,50 @@
> > >   */
> > >  #define FF_MIN_BUFFER_SIZE 16384
> > > 
> > > +
> > > +/*
> > > + * public Metadata API.
> > > + * Important concepts, to keep in mind
> > > + * 1. keys are unique, there are never 2 tags with equal keys, this is also
> > > + *    meant semantically that is key=Author, key=Author2 is illegal as well.
> > > + *    All authors have to be placed in the same tag for the case of Authors.
> > 
> > I think it's impossible to enforce in demuxer supporting generic metadata.
> > How could a demuxer differentiate those 2 cases:
> >  - key=Author1, key=Author2
> >  - key=IPV4, key=IPV6
> > The demuxer has no idea of the semantic.
> 
> It wasnt meant to be enforced like that, rather that a demuxer shouldnt
> knowingly produce Author1 Author2 from a container that allowed multiple
> identical keys
> 
> new text:
>  * 1. keys are unique, there are never 2 tags with equal keys, this is also
>  *    meant semantically that is a demuxer should not knowingly produce
>  *    several keys that are litterally different but semantically identical,
>  *    like key=Author5, key=Author6.
>  *    All authors have to be placed in the same tag for the case of Authors.

OK. That's saner.

> > > + * 3. A tag whichs value is translated has the ISO 639 3-letter language code
> > > + *    with a '-' between appended. So for example Author-ger=Michael, Author-eng=Mike
> > 
> > No support for country code (ISO 3166-1) ?
> > Just a question, I personally don't care.
> 
> i can add it if anyone wants ...
> We also can easily add it later, i had no special reason to omit it, it
> was just that i didnt immedeatly saw any real use case for it so i thought
> simpler is better when it can be added later ...

Let's forget about it for now. Keep things simple and we will add it
later if the need arise.

> > Overall, this patch looks like a sane basic implementation.
> > It misses a few important things (see below) but the core API
> > could probably already be commited (with no version bump, no
> > (de)muxer change and no ffmpeg/ffplay change) so that we can
> > improve it.
> 
> will commt after reading through the patch again (it alraedy passed
> my tests)

Great.

Aurel




More information about the ffmpeg-devel mailing list