[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