[FFmpeg-devel] [PATCH] metadata conversion API

Michael Niedermayer michaelni
Fri Feb 27 03:09:07 CET 2009


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

[...]

> @@ -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 ...



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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- 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/20090227/5813c9c9/attachment.pgp>



More information about the ffmpeg-devel mailing list