[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