From patchwork Thu Jan 26 11:55:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Arzelier X-Patchwork-Id: 2325 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.89.21 with SMTP id n21csp142603vsb; Thu, 26 Jan 2017 03:55:04 -0800 (PST) X-Received: by 10.28.9.148 with SMTP id 142mr25711569wmj.24.1485431704654; Thu, 26 Jan 2017 03:55:04 -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 m131si2767912wmd.60.2017.01.26.03.55.03; Thu, 26 Jan 2017 03:55:04 -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 1905268A8DB; Thu, 26 Jan 2017 13:55:00 +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 8BF8D68A4F7 for ; Thu, 26 Jan 2017 13:54:53 +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 E764E5FFBA for ; Thu, 26 Jan 2017 12:54:52 +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> From: Paul Arzelier Message-ID: <1f383fee-feb5-c43d-ef0d-46bf1cb227fe@free.fr> Date: Thu, 26 Jan 2017 12:55:15 +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: <20170126112457.0ed024e9@debian> X-Content-Filtered-By: Mailman/MimeDel 2.1.20 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" You're right, I made a patch for libavformat/utils.c instead. I modified a bit Ben's version and kept only the "spec-compliant flac files do not support ID3 tags" warning, I hope it's okay with you. Paul Le 26/01/2017 à 11:24, wm4 a écrit : > On Thu, 26 Jan 2017 11:08:45 +0100 > Paul Arzelier wrote: > >> From bb5dc73c42ad2e0bffd7e38fac18c6f01ec05ab3 Mon Sep 17 00:00:00 2001 >> From: Paul Arzelier >> Date: Thu, 26 Jan 2017 11:04:44 +0100 >> Subject: [PATCH] Fixed behavior when id3 tags were found on FLAC files >> Originally-by: Ben Boeckel >> --- >> libavformat/flacdec.c | 30 ++++++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c >> index 66baba5922..869936621b 100644 >> --- a/libavformat/flacdec.c >> +++ b/libavformat/flacdec.c >> @@ -48,6 +48,7 @@ static int flac_read_header(AVFormatContext *s) >> int ret, metadata_last=0, metadata_type, metadata_size, found_streaminfo=0; >> uint8_t header[4]; >> uint8_t *buffer=NULL; >> + int has_id3 = 0; >> FLACDecContext *flac = s->priv_data; >> AVStream *st = avformat_new_stream(s, NULL); >> if (!st) >> @@ -63,6 +64,22 @@ static int flac_read_header(AVFormatContext *s) >> return 0; >> } >> >> + if (av_dict_count(s->metadata)) { >> + /* XXX: Is there a better way to parse this out? ID3 parsing is done >> + * all the way out in avformat_open_input. */ >> + has_id3 = 1; >> + } >> + >> + if (has_id3) { >> + int level = AV_LOG_WARNING; >> + if (s->error_recognition & AV_EF_COMPLIANT) >> + level = AV_LOG_ERROR; >> + av_log(s, level, "Spec-compliant flac files do not support ID3 tags.\n"); >> + if (s->error_recognition & AV_EF_EXPLODE) >> + return AVERROR_INVALIDDATA; >> + } >> + >> + >> /* process metadata blocks */ >> while (!avio_feof(s->pb) && !metadata_last) { >> if (avio_read(s->pb, header, 4) != 4) >> @@ -173,8 +190,16 @@ static int flac_read_header(AVFormatContext *s) >> } >> /* process supported blocks other than STREAMINFO */ >> if (metadata_type == FLAC_METADATA_TYPE_VORBIS_COMMENT) { >> + AVDictionary *other_meta = NULL; >> AVDictionaryEntry *chmask; >> >> + if (has_id3) { >> + av_log(s, AV_LOG_VERBOSE, "FLAC found with ID3 and vorbis tags; ignoring ID3 tags also provided by vorbis.\n"); >> + >> + other_meta = s->metadata; >> + s->metadata = NULL; >> + } >> + >> ret = ff_vorbis_comment(s, &s->metadata, buffer, metadata_size, 1); >> if (ret < 0) { >> av_log(s, AV_LOG_WARNING, "error parsing VorbisComment metadata\n"); >> @@ -182,6 +207,11 @@ static int flac_read_header(AVFormatContext *s) >> s->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED; >> } >> >> + if (other_meta) { >> + av_dict_copy(&s->metadata, other_meta, AV_DICT_DONT_OVERWRITE); >> + av_dict_free(&other_meta); >> + } >> + >> /* parse the channels mask if present */ >> chmask = av_dict_get(s->metadata, "WAVEFORMATEXTENSIBLE_CHANNEL_MASK", NULL, 0); >> if (chmask) { > I feel like there should be a more general solution. > > ID3v2 tags are read by common code, and such tags are supported by > basically all file formats in libavformat due to the way it's > implemented. > > This fixes it for FLAC only. Are we going to duplicate this for every > format? > > I suggest reading id3 tags into a separate dict instead, and then > copying it in libavformat/utils.c after read_header if the demuxer > hasn't set any tags. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel From a3dc6068fb06722aacea56365f948afdb8df841f Mon Sep 17 00:00:00 2001 From: Paul Arzelier Date: Thu, 26 Jan 2017 12:51:33 +0100 Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis comments) Originally-by: Ben Boeckel --- libavformat/utils.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/libavformat/utils.c b/libavformat/utils.c index d5dfca7dec..bbe5c1ff1c 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); + } + else if (id3_meta) { + int level = AV_LOG_WARNING; + if (s->error_recognition & AV_EF_COMPLIANT) + level = AV_LOG_ERROR; + av_log(s, level, "Spec-compliant flac files do not support ID3 tags.\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")) { -- 2.11.0