[FFmpeg-devel] [PATCH][RFC] add a list of generic tag names

Aurelien Jacobs aurel
Wed Oct 21 00:35:36 CEST 2009


On Tue, Oct 20, 2009 at 03:31:23PM +0200, Anton Khirnov wrote:
> On Sun, Oct 18, 2009 at 05:44:44PM +0200, Aurelien Jacobs wrote:
> >
> > Also I think all of those tags needs a proper description. Just listing
> > a tag name without describing its purpose is mostly useless.
> > 
> I tried to think of something, but failed to find a non-redundant
> description of genre :) Suggestions for it and other tags welcome.

Indeed, this one is quite self-explanatory.
Not sure if this is true but maybe you can specify something about the
format of the content, like:
  you may prefer using id3v1 predefined genre name for best compatibility

> > 
> > Your patch is also missing some conversion from "year" to "date", at
> > least in mov.c, oggparsevorbis.c and id3v1.c.
> done. There are still some uses of 'year' in movenc.c, but i'm not
> familiar with the specs so I leave fixing it to its maintainer.
> > You will also need to add a conversion table for apetag.c (used in ape
> > and mpc) with the following conversions:
> >   "Artist" => author
> >   "Year"   => date
> 
> Added in a separate patch. however mpc seems to use both ape and id3v2
> tags so i'm not sure which table to use. maybe we should let (de)muxers
> to choose the conversion table (or more than one?) at runtime. that
> would also be useful for different versions of id3v2 for example.

Indeed, a runtime set table would be better in this situation.
Without runtime table, the only solution I see for now is to build a table
which contains a concatenation of both ape and id3v2 tables. This is quite
ugly and duplication...

> > Many demuxers are using the tag "muxer" to store the name of the
> > encoding software. Maybe those should be converted to "encoder".
> > 
> Many? i only found it in mov.c

Hum... I was pretty sure they were many of them but I can't find any now.
I must have dreamed about it... sorry.

> diff --git a/libavformat/asf.c b/libavformat/asf.c
> index cf01e07..799bbef 100644
> --- a/libavformat/asf.c
> +++ b/libavformat/asf.c
> @@ -129,12 +129,11 @@ const ff_asf_guid ff_asf_digital_signature = {
>  };
>  
>  const AVMetadataConv ff_asf_metadata_conv[] = {
> -    { "AlbumArtist", "artist"    },
>      { "AlbumTitle" , "album"     },
>      { "Author"     , "author"    },
>      { "Genre"      , "genre"     },
>      { "Copyright"  , "copyright" },
>      { "TrackNumber", "track"     },
> -    { "Year"       , "year"      },
> +    { "Year"       , "date"      },
>      { 0 }
>  };

OK

> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 19914b6..804c04d 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -99,7 +99,29 @@ int av_metadata_set(AVMetadata **pm, const char *key, const char *value);
>  
>  /**
>   * Converts all the metadata sets from ctx according to the source and
> - * destination conversion tables.
> + * destination conversion tables. If one of the tables is NULL, then
> + * tags are converted to/from ffmpeg generic tag names. Currently defined
> + * generic tag names are:
> + * album        -- name of the set this work belongs to
> + * author       -- main author of the work
> + * comment      -- any additional description of the file
> + * composer     -- artist who composed the work, if different from author
> + * copyright    -- name of copyright holder
> + * date         -- date when the work was created, preferably in ISO 8601
> + * disc         -- number of a subset, e.g. disc in a multi-disc collection
> + * encoder      -- name/settings of the software/hardware that produced the file
> + * encodedby    -- person who created the file
> + * filename     -- original name of the file
> + * genre        -- <self-evident>
> + * language     -- main language in which the work is performed
> + * performer    -- artist who performed the work, if different from author.
> + *                 e.g for 'Also sprach Zarathustra', author would be 'Richard
> + *                 Strauss' and performer 'London Philharmonic Orchestra'.
> + * publisher    -- name of the label/publisher
> + * title        -- name of the work
> + * track        -- number of this work in the set, can be in form current/total
> + * Each tag type can also have a -sort variant, which contains a version of the
> + * tag that should be used for sorting.
>   * @param d_conv destination tags format conversion table
>   * @param s_conv source tags format conversion table
>   */

OK

> diff --git a/libavformat/id3v1.c b/libavformat/id3v1.c
> index cf5cb95..f11f819 100644
> --- a/libavformat/id3v1.c
> +++ b/libavformat/id3v1.c
> @@ -189,7 +189,7 @@ static int parse_tag(AVFormatContext *s, const uint8_t *buf)
>      get_string(s, "title",   buf +  3, 30);
>      get_string(s, "author",  buf + 33, 30);
>      get_string(s, "album",   buf + 63, 30);
> -    get_string(s, "year",    buf + 93,  4);
> +    get_string(s, "date",    buf + 93,  4);
>      get_string(s, "comment", buf + 97, 30);
>      if (buf[125] == 0 && buf[126] != 0) {
>          snprintf(str, sizeof(str), "%d", buf[126]);

OK

> diff --git a/libavformat/matroska.c b/libavformat/matroska.c
> index f101a00..7a1d5b4 100644
> --- a/libavformat/matroska.c
> +++ b/libavformat/matroska.c
> @@ -92,8 +92,9 @@ const CodecMime ff_mkv_mime_tags[] = {
>  };
>  
>  const AVMetadataConv ff_mkv_metadata_conv[] = {
> -    { "ARTIST"        , "artist" },
> -    { "LEAD_PERFORMER", "artist" },
> +    { "ARTIST"        , "author" },
> +    { "LEAD_PERFORMER", "performer" },
> +    { "name"          , "title"  },
>      { "PART_NUMBER"   , "track"  },
>      { 0 }
>  };

"name" is not a standard matroska tag, thus it has nothing to do
in this table.

> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 688169a..69ecd91 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1342,7 +1342,7 @@ static int matroska_read_header(AVFormatContext *s, AVFormatParameters *ap)
>          st->start_time = 0;
>          if (strcmp(track->language, "und"))
>              av_metadata_set(&st->metadata, "language", track->language);
> -        av_metadata_set(&st->metadata, "description", track->name);
> +        av_metadata_set(&st->metadata, "name", track->name);

Just use "title" directly.

> diff --git a/libavformat/metadata_compat.c b/libavformat/metadata_compat.c
> index b05fb04..9869a17 100644
> --- a/libavformat/metadata_compat.c
> +++ b/libavformat/metadata_compat.c
> @@ -45,8 +45,11 @@ static const struct {
>      { "creator",         SIZE_OFFSET(author)    },
>      { "written_by",      SIZE_OFFSET(author)    },
>      { "lead_performer",  SIZE_OFFSET(author)    },
> +    { "composer",        SIZE_OFFSET(author)    },
> +    { "performer",       SIZE_OFFSET(author)    },
>      { "description",     SIZE_OFFSET(comment)   },
>      { "albumtitle",      SIZE_OFFSET(album)     },
> +    { "date",            SIZE_OFFSET(year)      },
>      { "date_written",    SIZE_OFFSET(year)      },
>      { "date_released",   SIZE_OFFSET(year)      },
>      { "tracknumber",     SIZE_OFFSET(track)     },

OK

> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index fdf921c..75260f4 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -112,10 +112,10 @@ static int mov_read_udta_string(MOVContext *c, ByteIOContext *pb, MOVAtom atom)
>      case MKTAG(0xa9,'c','m','t'):
>      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,'d','a','y'): key = "date";      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;
> +    case MKTAG(0xa9,'e','n','c'): key = "encoder";     break;
                                                     ^^^^^
alignment

> diff --git a/libavformat/mp3.c b/libavformat/mp3.c
> index 5b5e06d..e788d9f 100644
> --- a/libavformat/mp3.c
> +++ b/libavformat/mp3.c
> @@ -210,7 +210,7 @@ static int id3v1_create_tag(AVFormatContext *s, uint8_t *buf)
>      count += id3v1_set_string(s, "title",   buf +  3, 30);
>      count += id3v1_set_string(s, "author",  buf + 33, 30);
>      count += id3v1_set_string(s, "album",   buf + 63, 30);
> -    count += id3v1_set_string(s, "year",    buf + 93,  4);
> +    count += id3v1_set_string(s, "date",    buf + 93,  4);
>      count += id3v1_set_string(s, "comment", buf + 97, 30);
>      if ((tag = av_metadata_get(s->metadata, "track", NULL, 0))) {
>          buf[125] = 0;

OK

> diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c
> index fd23cb0..80ab215 100644
> --- a/libavformat/oggparsevorbis.c
> +++ b/libavformat/oggparsevorbis.c
> @@ -37,7 +37,6 @@
>   */
>  const AVMetadataConv ff_vorbiscomment_metadata_conv[] = {
>      { "ARTIST"     , "author" },
> -    { "DATE"       , "year"   },
>      { "TRACKNUMBER", "track"  },
>      { 0 }

OK

> From 196e436eb30bc8ca5d09ca7a1363da478c8dec66 Mon Sep 17 00:00:00 2001
> From: Anton Khirnov <wyskas at gmail.com>
> Date: Tue, 20 Oct 2009 15:18:45 +0200
> Subject: [PATCH 2/2] apetag: Add metadata conversion table and use it in ape and wv.
> 
> ---
>  libavformat/ape.c    |    3 ++-
>  libavformat/apetag.c |    8 +++++++-
>  libavformat/apetag.h |    3 +++
>  libavformat/wv.c     |    1 +
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/ape.c b/libavformat/ape.c
> index cc2e657..8fdbd3a 100644
> --- a/libavformat/ape.c
> +++ b/libavformat/ape.c
> @@ -400,5 +400,6 @@ AVInputFormat ape_demuxer = {
>      ape_read_packet,
>      ape_read_close,
>      ape_read_seek,
> -    .extensions = "ape,apl,mac"
> +    .extensions    = "ape,apl,mac",
> +    .metadata_conv = ff_ape_metadata_conv,
>  };

unrelated cosmetic

> diff --git a/libavformat/apetag.c b/libavformat/apetag.c
> index 262270c..21af5cd 100644
> --- a/libavformat/apetag.c
> +++ b/libavformat/apetag.c
> @@ -21,7 +21,7 @@
>   */
>  
>  #include "libavutil/intreadwrite.h"
> -#include "avformat.h"
> +#include "apetag.h"
>  
>  #define ENABLE_DEBUG 0
>  
> @@ -108,3 +108,9 @@ void ff_ape_parse_tag(AVFormatContext *s)
>      for (i=0; i<fields; i++)
>          if (ape_tag_read_field(s) < 0) break;
>  }
> +
> +const AVMetadataConv ff_ape_metadata_conv[] = {
> +    { "Artist", "author" },
> +    { "Year",   "date"   },
> +    { 0 }
> +};
> diff --git a/libavformat/apetag.h b/libavformat/apetag.h
> index 8aaef68..b5741c0 100644
> --- a/libavformat/apetag.h
> +++ b/libavformat/apetag.h
> @@ -24,10 +24,13 @@
>  #define AVFORMAT_APETAG_H
>  
>  #include "avformat.h"
> +#include "metadata.h"
>  
>  /**
>   * Read and parse an APE tag
>   */
>  void ff_ape_parse_tag(AVFormatContext *s);
>  
> +extern const AVMetadataConv ff_ape_metadata_conv[];
> +
>  #endif /* AVFORMAT_ID3V2_H */

OK

> diff --git a/libavformat/wv.c b/libavformat/wv.c
> index ffb2b35..8dadb07 100644
> --- a/libavformat/wv.c
> +++ b/libavformat/wv.c
> @@ -230,4 +230,5 @@ AVInputFormat wv_demuxer = {
>      wv_read_packet,
>      NULL,
>      wv_read_seek,
> +    .metadata_conv = ff_ape_metadata_conv,
>  };

Not perfect but probably OK for now.

Aurel



More information about the ffmpeg-devel mailing list