[FFmpeg-devel] [PATCH] metadata conversion API

Michael Niedermayer michaelni
Sat Feb 28 20:22:41 CET 2009


On Sat, Feb 28, 2009 at 12:07:11AM +0100, Aurelien Jacobs wrote:
> Michael Niedermayer wrote:
> 
> > On Thu, Feb 26, 2009 at 11:14:15PM +0100, Aurelien Jacobs wrote:
> > > Michael Niedermayer wrote:
> > > 
> > > > On Thu, Feb 26, 2009 at 02:49:01AM +0100, Aurelien Jacobs wrote:
> > > > > Michael Niedermayer wrote:
> > > > > 
> > > > > > On Thu, Feb 26, 2009 at 02:13:51AM +0100, Aurelien Jacobs wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > There is one last and important issue I want to address with the new
> > > > > > > metadata API.
> > > > > > > Old API allowed client apps and muxers to get a few select well known
> > > > > > > tags (title, author...). With the new API, there is no simple way to
> > > > > > > do that, right now. For example, if you demux an ASF file, and want to
> > > > > > > get the name of the album, av_metadata_get(..., "album", ...) won't
> > > > > > > give you any results, because ASF stores this information in a tag
> > > > > > > named "AlbumTitle". There are lots of examples with various demuxers,
> > > > > > > even for simple common tags. This also prevent correct remuxing between
> > > > > > > different containers.
> > > > > > > 
> > > > > > > One simple solution would be to force any demuxers to export their tags
> > > > > > > respecting a well defined list of known common key names (for all tags
> > > > > > > that have a corresponding common name). But this have issues which makes
> > > > > > > this impractical. It looses information. The client app can't retrieve
> > > > > > > the actual key names used in the original file. And this would prevent
> > > > > > > lossless remuxing in the same container.
> > > > > > > 
> > > > > > > So I built up another solution. The principle is to allows (de)muxers
> > > > > > > to associate a conversion table to a AVMetadata. This table describe
> > > > > > > the correspondence between the native format key names and the generic
> > > > > > > common key names. Then with this conversion table,
> > > > > > > av_metadata_get(..., "album", ...) is able to get the native tag
> > > > > > > corresponding the the generic "album" key. And a muxer is able to
> > > > > > > convert a whole AVMetadata to it's own native format.
> > > > > > > 
> > > > > > > First patch implements this conversion table API.
> > > > > > > Second patch just shows a simple example of usage of this API (yes
> > > > > > > I know it duplicates the ff_asf_metadata_conv table and I don't
> > > > > > > intend to apply it as is, it's only a simple incomplete and unclean
> > > > > > > example for now).
> > > > > > > 
> > > > > > > Also note the the av_metadata_get_conv() function which may look useless
> > > > > > > in this patch set, is in fact required for applications such as ffmpeg,
> > > > > > > to "copy" the conv table from input format ctx to output format ctx.
> > > > > > > 
> > > > > > > And finally, here is a concrete example of what this API allows. A simple
> > > > > > > stream copy of those Matroska tags:
> > > > > > >   LEAD_PERFORMER: Tori Amos
> > > > > > >   ALBUM: Under the Pink
> > > > > > > generates the following ASF tags:
> > > > > > >   AlbumArtist: Tori Amos
> > > > > > >   AlbumTitle: Under the Pink
> > > > > > 
> > > > > > IMHO The correct way to implement such conversion table is
> > > > > > by adding it to AVInputFormat & AVOutputFormat
> > > > > > and adding a simple av_metadata_conv() that takes the input metadata and
> > > > > > both tables to produce outpt metadata.
> > > > > > Thi convertion, which is just a single line of code would be called by the
> > > > > > user app.
> > > > > > muxers would only store exactly the metadata tags given to them and demuxers
> > > > > > would only read exactly the metadata tags, convertion would be entirely
> > > > > > outside of them.
> > > > > 
> > > > > I've thought about this possibility too. But then, do you think we should also
> > > > > add a conversion table to AVStream, AVChapter, etc... ?
> > > > > We may need to have a conversion table to handle their metadata in a different
> > > > > way than the main metadata...
> > > > 
> > > > I think the table should be opaque to the user app (for the momemt, the details
> > > > can be exported later when they have stabilized)
> > > 
> > > OK.
> > > 
> > > > and could contain AVStream/AVChapter information, it could even contain more
> > > > complex translation info, i mean theres no guarantee for a 1:1 mapping
> > > > for example one format might store per stream authors, while the other might
> > > > support only per file authors the convertion code could together with these
> > > > tables convert between these 2.
> > > 
> > > Good idea. Well, for now, I've kept the simplest possible version (ie. with
> > > no AVStream/AVChapter information, nor anything else fancy). But we can
> > > improve this later, in any possible way, as long as the tables are opaque.
> > > 
> > > New patches attached. They are significantly simpler than original versions,
> > > and work as well.
> > > 
> > > Aurel
> > > Index: libavformat/metadata.c
> > > ===================================================================
> > > --- libavformat/metadata.c	(r?vision 17619)
> > > +++ libavformat/metadata.c	(copie de travail)
> > > @@ -18,6 +18,7 @@
> > >   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > >   */
> > >  
> > > +#include <strings.h>
> > >  #include "metadata.h"
> > >  
> > >  AVMetadataTag *
> > > @@ -76,6 +77,35 @@
> > >      return 0;
> > >  }
> > >  
> > 
> > > +void av_metadata_conv(AVMetadata **dst, const AVMetadataConv *d_conv,
> > > +                      AVMetadata  *src, const AVMetadataConv *s_conv)
> > > +{
> > > +    const AVMetadataConv *s_conv1 = s_conv, *d_conv1 = d_conv, *sc, *dc;
> > > +    AVMetadataTag *mtag = NULL;
> > > +    const char *key, *key2;
> > > +
> > > +    while((mtag=av_metadata_get(src, "", mtag, AV_METADATA_IGNORE_SUFFIX))) {
> > > +        key = key2 = mtag->key;
> > > +        if (s_conv != d_conv) {
> > > +            if (!s_conv)
> > > +                s_conv1 = (const AVMetadataConv[2]){{key,key}};
> > > +            for (sc=s_conv1; sc->native; sc++)
> > > +                if (!strcasecmp(key, sc->native)) {
> > > +                    key2 = sc->generic;
> > > +                    break;
> > > +                }
> > > +            if (!d_conv)
> > > +                d_conv1 = (const AVMetadataConv[2]){{key2,key2}};
> > > +            for (dc=d_conv1; dc->native; dc++)
> > > +                if (!strcasecmp(key2, dc->generic)) {
> > > +                    key = dc->native;
> > > +                    break;
> > > +                }
> > 
> > maybe bsearch() could be used to do the look up in the 2 tables, though
> > it makes no sense ATM considering the table size so thats just a request
> > for a //TODO comment
> 
> OK. Added.
> 
> > [...]
> > 
> > > @@ -96,6 +97,13 @@
> > >  int av_metadata_set(AVMetadata **pm, const char *key, const char *value);
> > >  
> > >  /**
> > > + * Convert a metadata set from src to dst according to their associated
> > > + * d_conv and s_conv conversion tables.
> > > + */
> > > +void av_metadata_conv(AVMetadata **dst, const AVMetadataConv *d_conv,
> > > +                      AVMetadata  *src, const AVMetadataConv *s_conv);
> > > +
> > > +/**
> > 
> > i was wondering if it might be better to pass AVFormatContexts in that, i mean
> > for moving things betweem AVStream and the main metadata ...
> 
> Indeed, passing a AVFormatContext allows doing some more advanced conversions.
> I don't know if those advanced possibilities would ever be used, but it's
> probably worth providing the most versatile API.
> It has some pros and cons:
>   + more versatile and future proof
>   + allows converting all the various metadata in a AVFormatContext with a
>     single function call
>   - requires one more memcpy of the metadata when stream copying (but speed
>     shouldn't matter here)


>   - requires AVFormatContext to be defined before the metadata API

-void av_metadata_conv(AVFormatContext *ctx, const AVMetadataConv *d_conv,
+void av_metadata_conv(struct AVFormatContext *ctx, const AVMetadataConv *d_conv,
                                             const AVMetadataConv *s_conv);
[...]

> +void av_metadata_conv(AVFormatContext *ctx, const AVMetadataConv *d_conv,
> +                                            const AVMetadataConv *s_conv)
> +{
> +    int i;
> +    metadata_conv(&ctx->metadata, d_conv, s_conv);
> +    for (i=0; i<ctx->nb_streams ; i++)
> +        metadata_conv(&ctx->streams [i]->metadata, d_conv, s_conv);
> +    for (i=0; i<ctx->nb_chapters; i++)
> +        metadata_conv(&ctx->chapters[i]->metadata, d_conv, s_conv);
> +    for (i=0; i<ctx->nb_programs; i++)
> +        metadata_conv(&ctx->programs[i]->metadata, d_conv, s_conv);
> +}

this in place convertion has a disadvatage,
its rather easy to entangle itself in more complex convertions
i think 2 AVFormatContext and src and dst would be easier

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090228/5d012f58/attachment.pgp>



More information about the ffmpeg-devel mailing list