[FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

Michael Niedermayer michaelni at gmx.at
Fri Jan 27 04:03:03 EET 2017


On Thu, Jan 26, 2017 at 02:29:21PM +0100, Paul Arzelier wrote:
> Alright, attached is the last version (I hope!)
> 
> Paul
> 
> 
> Le 26/01/2017 à 13:43, wm4 a écrit :
> >On Thu, 26 Jan 2017 13:32:08 +0100
> >Paul Arzelier <paul.arzelier at free.fr> wrote:
> >
> >> From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
> >>From: Polochon-street <polochonstreet at gmx.fr>
> >>Date: Thu, 26 Jan 2017 13:25:22 +0100
> >>Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis
> >>  comments)
> >>Originally-by: Ben Boeckel <mathstuf at gmail.com>
> >>---
> >>  libavformat/utils.c | 17 ++++++++++++++++-
> >>  1 file changed, 16 insertions(+), 1 deletion(-)
> >>
> >Patch looks generally great to me now.
> >
> >>diff --git a/libavformat/utils.c b/libavformat/utils.c
> >>index d5dfca7dec..ce24244135 100644
> >>--- a/libavformat/utils.c
> >>+++ b/libavformat/utils.c
> >>@@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
> >>      AVFormatContext *s = *ps;
> >>      int i, ret = 0;
> >>      AVDictionary *tmp = NULL;
> >>+    AVDictionary *id3_meta = NULL;
> >>      ID3v2ExtraMeta *id3v2_extra_meta = NULL;
> >>      if (!s && !(s = avformat_alloc_context()))
> >>@@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
> >>      /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
> >>      if (s->pb)
> >>-        ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, 0);
> >>+        ff_id3v2_read_dict(s->pb, &id3_meta, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta);
> >>      if (!(s->flags&AVFMT_FLAG_PRIV_OPT) && s->iformat->read_header)
> >>          if ((ret = s->iformat->read_header(s)) < 0)
> >>              goto fail;
> >>+    if (!s->metadata) {
> >>+        av_dict_copy(&s->metadata, id3_meta, AV_DICT_DONT_OVERWRITE);
> >>+        av_dict_free(&id3_meta);
> >Strictly speaking, this line is redundant, but it doesn't harm either.
> >If you feel like it you could remove it, but it's not necessary.
> >
> >>+    }
> >>+    else if (id3_meta) {
> >If you make another patch, you could merge the } with the next line too
> >(I'm not sure, but I think that's preferred coding-style wise).
> >
> >>+        int level = AV_LOG_WARNING;
> >>+        if (s->error_recognition & AV_EF_COMPLIANT)
> >>+            level = AV_LOG_ERROR;
> >>+        av_log(s, level, "Discarding ID3 tag because more suitable tags were found.\n");
> >>+        if (s->error_recognition & AV_EF_EXPLODE)
> >>+            return AVERROR_INVALIDDATA;
> >>+    }
> >>+
> >>      if (id3v2_extra_meta) {
> >>          if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
> >>              !strcmp(s->iformat->name, "tta")) {
> >>@@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
> >>  fail:
> >>      ff_id3v2_free_extra_meta(&id3v2_extra_meta);
> >>      av_dict_free(&tmp);
> >>+   av_dict_free(&id3_meta);
> >>      if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO))
> >>          avio_closep(&s->pb);
> >>      avformat_free_context(s);
> >_______________________________________________
> >ffmpeg-devel mailing list
> >ffmpeg-devel at ffmpeg.org
> >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> -- 
> Paul ARZELIER
> Élève ingénieur à l'École Centrale de Lille
> 2014-2017
> 

>  utils.c |   17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 477d4f87058a098464e2c1dbfe7e7ff806d2c85f  0001-Ignore-ID3-tags-if-other-tags-are-present-e.g.-vorbi.patch
> From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
> From: Polochon-street <polochonstreet at gmx.fr>
> Date: Thu, 26 Jan 2017 13:25:22 +0100
> Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis 
>  comments)
> Originally-by: Ben Boeckel <mathstuf at gmail.com>
> ---
>  libavformat/utils.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index d5dfca7dec..ce24244135 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
>      AVFormatContext *s = *ps;
>      int i, ret = 0;
>      AVDictionary *tmp = NULL;
> +    AVDictionary *id3_meta = NULL;
>      ID3v2ExtraMeta *id3v2_extra_meta = NULL;
>  
>      if (!s && !(s = avformat_alloc_context()))
> @@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
>  
>      /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
>      if (s->pb)
> -        ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, 0);
> +        ff_id3v2_read_dict(s->pb, &id3_meta, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta);
>  
>      if (!(s->flags&AVFMT_FLAG_PRIV_OPT) && s->iformat->read_header)
>          if ((ret = s->iformat->read_header(s)) < 0)
>              goto fail;
>  
> +    if (!s->metadata) {
> +        s->metadata = id3_meta;
> +        id3_meta = NULL;
> +    } else if (id3_meta) {
> +        int level = AV_LOG_WARNING;
> +        if (s->error_recognition & AV_EF_COMPLIANT)
> +            level = AV_LOG_ERROR;
> +        av_log(s, level, "Discarding ID3 tag because more suitable tags were found.\n");
> +        av_dict_free(&id3_meta);
> +        if (s->error_recognition & AV_EF_EXPLODE)
> +            return AVERROR_INVALIDDATA;
> +    }
> +
>      if (id3v2_extra_meta) {
>          if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
>              !strcmp(s->iformat->name, "tta")) {
> @@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
>  fail:
>      ff_id3v2_free_extra_meta(&id3v2_extra_meta);
>      av_dict_free(&tmp);
> +    av_dict_free(&id3_meta);
>      if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO))
>          avio_closep(&s->pb);
>      avformat_free_context(s);

This causes some metadata to be lost
for example
~/tickets/4003/mp3_demuxer_EOI.mp3
genre           : Ethno / Thrash Métal
changes to
genre           : Other

It also seems to do something to the character encoding of metadata
in some files

and it results in multiple additional seeks during probe/analysis
for example:

./ffplay-new -v 99 /home/michael/tickets//3327/sample.mp3
[mp3 @ 0x7effbc000920] After avformat_find_stream_info() pos: 2526661 bytes read:2556032 seeks:2 frames:50
vs. before:
[mp3 @ 0x7fa1c4000920] After avformat_find_stream_info() pos: 2526661 bytes read:2555904 seeks:0 frames:50

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170127/2ec1568c/attachment.sig>


More information about the ffmpeg-devel mailing list