From patchwork Sat Jan 28 16:28:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Arzelier X-Patchwork-Id: 2350 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.89.21 with SMTP id n21csp744233vsb; Sat, 28 Jan 2017 08:28:21 -0800 (PST) X-Received: by 10.28.148.76 with SMTP id w73mr7867963wmd.43.1485620901702; Sat, 28 Jan 2017 08:28:21 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id n107si10035576wrb.302.2017.01.28.08.28.20; Sat, 28 Jan 2017 08:28:21 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3853768A4A8; Sat, 28 Jan 2017 18:28:16 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from smtp5-g21.free.fr (smtp5-g21.free.fr [212.27.42.5]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4536868A2FA for ; Sat, 28 Jan 2017 18:28:09 +0200 (EET) Received: from [192.168.1.47] (unknown [93.163.38.190]) (Authenticated sender: paul.arzelier) by smtp5-g21.free.fr (Postfix) with ESMTPSA id 39E0F5FF9A for ; Sat, 28 Jan 2017 17:28:09 +0100 (CET) To: ffmpeg-devel@ffmpeg.org References: <76132ba8-af6a-ce81-87ce-5e4703103425@free.fr> <9f5d1b0d-0336-4a56-e7d6-baea3688ff04@free.fr> <20170126011234.GJ4698@nb4> <20170126112457.0ed024e9@debian> <1f383fee-feb5-c43d-ef0d-46bf1cb227fe@free.fr> <20170126131047.5169000e@debian> <20170126134326.2b997bee@debian> <6f6bf9cf-145e-a087-b633-79baa096ed3c@free.fr> <20170127020303.GW4698@nb4> <20170127070222.58a42b32@debian> <20170127174240.14c22f4b@debian> From: Paul Arzelier Message-ID: Date: Sat, 28 Jan 2017 17:28:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <20170127174240.14c22f4b@debian> Subject: Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Hopefuly, everything is okay right now :) Le 27/01/2017 à 17:42, wm4 a écrit : > On Fri, 27 Jan 2017 17:32:08 +0100 > Paul Arzelier wrote: > >> From 227d7d1f4b5665f824900cf2b14863621180dd1c Mon Sep 17 00:00:00 2001 >> From: Polochon-street >> Date: Fri, 27 Jan 2017 16:49:41 +0100 >> Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis) >> Originally-by: Ben Boeckel >> --- >> libavformat/internal.h | 5 +++++ >> libavformat/mp3dec.c | 5 +++++ >> libavformat/options.c | 1 + >> libavformat/utils.c | 16 +++++++++++++++- >> 4 files changed, 26 insertions(+), 1 deletion(-) >> > Looks largely ok to me, just a few minor nits. > >> diff --git a/libavformat/internal.h b/libavformat/internal.h >> index 9d614fb12c..63a1724cfa 100644 >> --- a/libavformat/internal.h >> +++ b/libavformat/internal.h >> @@ -140,6 +140,11 @@ struct AVFormatInternal { >> * Whether or not avformat_init_output fully initialized streams >> */ >> int streams_initialized; >> + >> + /** >> + * ID3v2 tag useful for MP3 demuxing > The tag is used for many file formats (unfortunately), not just mp3. So > I'm not sure if the comment is correct. > >> + */ >> + AVDictionary *id3v2_meta; >> }; >> >> struct AVStreamInternal { >> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c >> index 099ca57d24..8540e45e4f 100644 >> --- a/libavformat/mp3dec.c >> +++ b/libavformat/mp3dec.c >> @@ -349,6 +349,11 @@ static int mp3_read_header(AVFormatContext *s) >> int ret; >> int i; >> >> + if (s->internal->id3v2_meta) >> + s->metadata = s->internal->id3v2_meta; > Slightly odd that it checks for id3v2_meta being NULL. Seems redundant? > >> + >> + s->internal->id3v2_meta = NULL; >> + >> st = avformat_new_stream(s, NULL); >> if (!st) >> return AVERROR(ENOMEM); >> diff --git a/libavformat/options.c b/libavformat/options.c >> index 25a506eef8..d8aca472c5 100644 >> --- a/libavformat/options.c >> +++ b/libavformat/options.c >> @@ -144,6 +144,7 @@ AVFormatContext *avformat_alloc_context(void) >> ic->internal->offset = AV_NOPTS_VALUE; >> ic->internal->raw_packet_buffer_remaining_size = RAW_PACKET_BUFFER_SIZE; >> ic->internal->shortest_end = AV_NOPTS_VALUE; >> + ic->internal->id3v2_meta = NULL; > Not necessary. All fields are initialized to 0 by default, because the > struct is allocated by av_mallocz(). > >> >> return ic; >> } >> diff --git a/libavformat/utils.c b/libavformat/utils.c >> index d5dfca7dec..b44d688213 100644 >> --- a/libavformat/utils.c >> +++ b/libavformat/utils.c >> @@ -588,12 +588,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, &s->internal->id3v2_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 = s->internal->id3v2_meta; >> + s->internal->id3v2_meta = NULL; >> + } else if (s->internal->id3v2_meta) { >> + int level = AV_LOG_WARNING; >> + if (s->error_recognition & AV_EF_COMPLIANT) >> + level = AV_LOG_ERROR; >> + av_log(s, level, "Discarding ID3 tags because more suitable tags were found.\n"); >> + av_dict_free(&s->internal->id3v2_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 +640,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(&s->internal->id3v2_meta); >> if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO)) >> avio_closep(&s->pb); >> avformat_free_context(s); > You could free the tag in avformat_free_context(), which might be > slightly simpler (don't need to care about success vs. error path), but > it doesn't actually matter. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel From 08de8e059393f4f6b412d482aa3e045f6a6b06c7 Mon Sep 17 00:00:00 2001 From: Paul Arzelier Date: Sat, 28 Jan 2017 17:25:27 +0100 Subject: [PATCH] Ignore ID3v2 tags if other tags are present e.g. vorbis Originally-by: Ben Boeckel --- libavformat/internal.h | 5 +++++ libavformat/mp3dec.c | 3 +++ libavformat/utils.c | 17 ++++++++++++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/libavformat/internal.h b/libavformat/internal.h index 9d614fb12c..63a1724cfa 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -140,6 +140,11 @@ struct AVFormatInternal { * Whether or not avformat_init_output fully initialized streams */ int streams_initialized; + + /** + * ID3v2 tag useful for MP3 demuxing + */ + AVDictionary *id3v2_meta; }; struct AVStreamInternal { diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c index 099ca57d24..b45a066686 100644 --- a/libavformat/mp3dec.c +++ b/libavformat/mp3dec.c @@ -349,6 +349,9 @@ static int mp3_read_header(AVFormatContext *s) int ret; int i; + s->metadata = s->internal->id3v2_meta; + s->internal->id3v2_meta = NULL; + st = avformat_new_stream(s, NULL); if (!st) return AVERROR(ENOMEM); diff --git a/libavformat/utils.c b/libavformat/utils.c index d5dfca7dec..0711310792 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -588,12 +588,26 @@ 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, &s->internal->id3v2_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 = s->internal->id3v2_meta; + s->internal->id3v2_meta = NULL; + } else if (s->internal->id3v2_meta) { + int level = AV_LOG_WARNING; + if (s->error_recognition & AV_EF_COMPLIANT) + level = AV_LOG_ERROR; + av_log(s, level, "Discarding ID3 tags because more suitable tags were found.\n"); + av_dict_free(&s->internal->id3v2_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")) { @@ -4178,6 +4192,7 @@ void avformat_free_context(AVFormatContext *s) } av_freep(&s->chapters); av_dict_free(&s->metadata); + av_dict_free(&s->internal->id3v2_meta); av_freep(&s->streams); av_freep(&s->internal); flush_packet_queue(s); -- 2.11.0