From patchwork Wed Jan 25 19:33:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Arzelier X-Patchwork-Id: 2308 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.89.21 with SMTP id n21csp2360828vsb; Wed, 25 Jan 2017 11:33:43 -0800 (PST) X-Received: by 10.28.195.70 with SMTP id t67mr25010721wmf.98.1485372823130; Wed, 25 Jan 2017 11:33:43 -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 61si28077587wrg.108.2017.01.25.11.33.42; Wed, 25 Jan 2017 11:33:43 -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 0B62268A42A; Wed, 25 Jan 2017 21:33:39 +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 C02E468A3F6 for ; Wed, 25 Jan 2017 21:33:32 +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 B7F3A5FF93; Wed, 25 Jan 2017 20:33:29 +0100 (CET) References: <76132ba8-af6a-ce81-87ce-5e4703103425@free.fr> To: ffmpeg-devel@ffmpeg.org, mathstuf@gmail.com From: Paul Arzelier X-Forwarded-Message-Id: <76132ba8-af6a-ce81-87ce-5e4703103425@free.fr> Message-ID: <9f5d1b0d-0336-4a56-e7d6-baea3688ff04@free.fr> Date: Wed, 25 Jan 2017 20:33:53 +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: <76132ba8-af6a-ce81-87ce-5e4703103425@free.fr> Subject: [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" Hi all, Would it be possible to continue the discussion began there ? http://ffmpeg.org/pipermail/ffmpeg-devel/2015-February/168248.html The current behavior of FFmpeg when it comes to manage FLAC tags with both vorbis tags and ID3 tags is to append both, which leads to, for example, "Artist: Pink Floyd;Pink Floyd" in FFprobe. This is not a desired output: as the XIPH faq says about FLAC (https://xiph.org/flac/faq.html): "FLAC has it's own native tagging system which is identical to that of Vorbis. They are called alternately "FLAC tags" and "Vorbis comments". It is the only tagging system required and guaranteed to be supported by FLAC implementations. Out of convenience, the reference decoder knows how to skip ID3 tags so that they don't interfere with decoding. But you should not expect any tags beside FLAC tags to be supported in applications; some implementations may not even be able to decode a FLAC file with ID3 tags." This issue was also submitted here https://trac.ffmpeg.org/ticket/3799, but still discuted in the above thread. Ben Boeckel made the attached patch (which I edited a bit to fit FFmpeg last version), which fixes the issue. The problem that remained was that many other files could have irrelevant id3v2 tags, not only Vorbis/FLAC files, so another more global patch may be needed to fix that definitely. So, do you think Ben's patch could be merged "as-is", or does it only raise more questions? Regards, Paul From b10a32fecf05b17f6e824b662b7db07bff195d2d Mon Sep 17 00:00:00 2001 From: Paul Arzelier Date: Wed, 25 Jan 2017 18:55:52 +0100 Subject: [PATCH] Fixed behavior when id3 tags were found on FLAC files Originally-by: Ben Boeckel --- libavformat/flacdec.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c index 3060dc45fd..f8d4611398 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) { avio_read(s->pb, header, 4); @@ -172,8 +189,17 @@ 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"); @@ -181,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) { -- 2.11.0