From 5b53af16df934e2c9d2c19e5395c7a1126cc125a Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 2 Sep 2019 19:57:52 +0200 Subject: [PATCH 1/8] PICARD-1586: Added basic tag mapping for ReplayGain tags --- picard/formats/apev2.py | 7 +++++++ picard/formats/asf.py | 7 +++++++ picard/formats/id3.py | 7 +++++++ picard/formats/mp4.py | 7 +++++++ picard/util/tags.py | 7 +++++++ test/formats/common.py | 18 ++++++++++++++++++ 6 files changed, 53 insertions(+) diff --git a/picard/formats/apev2.py b/picard/formats/apev2.py index 5c8db4c17..cadec49d0 100644 --- a/picard/formats/apev2.py +++ b/picard/formats/apev2.py @@ -97,6 +97,13 @@ class APEv2File(File): "musicbrainz_trackid": "musicbrainz_recordingid", "musicbrainz_releasetrackid": "musicbrainz_trackid", "Original Artist": "originalartist", + "REPLAYGAIN_ALBUM_GAIN": "replaygain_album_gain", + "REPLAYGAIN_ALBUM_PEAK": "replaygain_album_peak", + "REPLAYGAIN_ALBUM_RANGE": "replaygain_album_range", + "REPLAYGAIN_TRACK_GAIN": "replaygain_track_gain", + "REPLAYGAIN_TRACK_PEAK": "replaygain_track_peak", + "REPLAYGAIN_TRACK_RANGE": "replaygain_track_range", + "REPLAYGAIN_REFERENCE_LOUDNESS": "replaygain_reference_loudness", } __rtranslate = dict([(v, k) for k, v in __translate.items()]) diff --git a/picard/formats/asf.py b/picard/formats/asf.py index d65f0be82..0f65eaeec 100644 --- a/picard/formats/asf.py +++ b/picard/formats/asf.py @@ -158,6 +158,13 @@ class ASFFile(File): 'artists': 'WM/ARTISTS', 'work': 'WM/Work', 'website': 'WM/AuthorURL', + 'replaygain_album_gain': 'REPLAYGAIN_ALBUM_GAIN', + 'replaygain_album_peak': 'REPLAYGAIN_ALBUM_PEAK', + 'replaygain_album_range': 'REPLAYGAIN_ALBUM_RANGE', + 'replaygain_track_gain': 'REPLAYGAIN_TRACK_GAIN', + 'replaygain_track_peak': 'REPLAYGAIN_TRACK_PEAK', + 'replaygain_track_range': 'REPLAYGAIN_TRACK_RANGE', + 'replaygain_reference_loudness': 'REPLAYGAIN_REFERENCE_LOUDNESS', } __RTRANS = dict([(b, a) for a, b in __TRANS.items()]) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 8709b4bf6..80703eaa6 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -173,6 +173,13 @@ class ID3File(File): 'WORK': 'work', 'Writer': 'writer', 'SHOWMOVEMENT': 'showmovement', + 'REPLAYGAIN_ALBUM_GAIN': 'replaygain_album_gain', + 'REPLAYGAIN_ALBUM_PEAK': 'replaygain_album_peak', + 'REPLAYGAIN_ALBUM_RANGE': 'replaygain_album_range', + 'REPLAYGAIN_TRACK_GAIN': 'replaygain_track_gain', + 'REPLAYGAIN_TRACK_PEAK': 'replaygain_track_peak', + 'REPLAYGAIN_TRACK_RANGE': 'replaygain_track_range', + 'REPLAYGAIN_REFERENCE_LOUDNESS': 'replaygain_reference_loudness', } __rtranslate_freetext = dict([(v, k) for k, v in __translate_freetext.items()]) __translate_freetext['writer'] = 'writer' # For backward compatibility of case diff --git a/picard/formats/mp4.py b/picard/formats/mp4.py index b73008ec4..6f7636855 100644 --- a/picard/formats/mp4.py +++ b/picard/formats/mp4.py @@ -121,6 +121,13 @@ class MP4File(File): "----:com.apple.iTunes:ARTISTS": "artists", "----:com.apple.iTunes:WORK": "work", "----:com.apple.iTunes:initialkey": "key", + "----:com.apple.iTunes:REPLAYGAIN_ALBUM_GAIN": "replaygain_album_gain", + "----:com.apple.iTunes:REPLAYGAIN_ALBUM_PEAK": "replaygain_album_peak", + "----:com.apple.iTunes:REPLAYGAIN_ALBUM_RANGE": "replaygain_album_range", + "----:com.apple.iTunes:REPLAYGAIN_TRACK_GAIN": "replaygain_track_gain", + "----:com.apple.iTunes:REPLAYGAIN_TRACK_PEAK": "replaygain_track_peak", + "----:com.apple.iTunes:REPLAYGAIN_TRACK_RANGE": "replaygain_track_range", + "----:com.apple.iTunes:REPLAYGAIN_REFERENCE_LOUDNESS": "replaygain_reference_loudness", } __r_freeform_tags = dict([(v, k) for k, v in __freeform_tags.items()]) diff --git a/picard/util/tags.py b/picard/util/tags.py index 07546bf4d..7d4d55b77 100644 --- a/picard/util/tags.py +++ b/picard/util/tags.py @@ -96,6 +96,13 @@ TAG_NAMES = { 'musicbrainz_originalartistid': N_('MusicBrainz Original Artist Id'), 'originalalbum': N_('Original Album'), 'musicbrainz_originalalbumid': N_('MusicBrainz Original Release Id'), + 'replaygain_album_gain': N_('ReplayGain Album Gain'), + 'replaygain_album_peak': N_('ReplayGain Album Peak'), + 'replaygain_album_range': N_('ReplayGain Album Range'), + 'replaygain_track_gain': N_('ReplayGain Track Gain'), + 'replaygain_track_peak': N_('ReplayGain Track Peak'), + 'replaygain_track_range': N_('ReplayGain Track Range'), + 'replaygain_reference_loudness': N_('ReplayGain Reference Loudness'), } PRESERVED_TAGS = [ diff --git a/test/formats/common.py b/test/formats/common.py index 0a0c63231..31edbf2f4 100644 --- a/test/formats/common.py +++ b/test/formats/common.py @@ -130,6 +130,16 @@ TAGS = { 'work': 'Foo' } +REPLAYGAIN_TAGS = { + 'replaygain_album_gain': '-6.48 dB', + 'replaygain_album_peak': '0.978475', + 'replaygain_album_range': '7.84 dB', + 'replaygain_track_gain': '-6.16 dB', + 'replaygain_track_peak': '0.976991', + 'replaygain_track_range': '8.22 dB', + 'replaygain_reference_loudness': '-18.00 LUFS', +} + def skipUnlessTestfile(func): def _decorator(self, *args, **kwargs): @@ -190,6 +200,7 @@ class CommonTests: def setUp(self): super().setUp() self.tags = TAGS.copy() + self.replaygain_tags = REPLAYGAIN_TAGS.copy() self.setup_tags() def setup_tags(self): @@ -213,6 +224,13 @@ class CommonTests: for (key, value) in self.tags.items(): self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + @skipUnlessTestfile + def test_replaygain_tags(self): + metadata = Metadata(self.replaygain_tags) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + for (key, value) in self.replaygain_tags.items(): + self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + @skipUnlessTestfile def test_save_does_not_modify_metadata(self): tags = dict(self.tags) From 9ab4aefc4905046b2bfe2494537b7d6a28d50dd6 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 2 Sep 2019 21:24:18 +0200 Subject: [PATCH 2/8] PICARD-1586: Support Opus R128_*_GAIN tags --- picard/formats/vorbis.py | 6 ++++++ picard/util/tags.py | 2 ++ test/formats/test_vorbis.py | 22 ++++++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/picard/formats/vorbis.py b/picard/formats/vorbis.py index be37ee9ac..b0db97bf2 100644 --- a/picard/formats/vorbis.py +++ b/picard/formats/vorbis.py @@ -360,6 +360,12 @@ class OggOpusFile(VCommentFile): NAME = "Ogg Opus" _File = mutagen.oggopus.OggOpus + @classmethod + def supports_tag(cls, name): + if name.startswith('replaygain_'): + return False + return VCommentFile.supports_tag(name) + def OggAudioFile(filename): """Generic Ogg audio file.""" diff --git a/picard/util/tags.py b/picard/util/tags.py index 7d4d55b77..c0d4ccc23 100644 --- a/picard/util/tags.py +++ b/picard/util/tags.py @@ -103,6 +103,8 @@ TAG_NAMES = { 'replaygain_track_peak': N_('ReplayGain Track Peak'), 'replaygain_track_range': N_('ReplayGain Track Range'), 'replaygain_reference_loudness': N_('ReplayGain Reference Loudness'), + 'r128_album_gain': N_('R128 Album Gain'), + 'r128_track_gain': N_('R128 Track Gain'), } PRESERVED_TAGS = [ diff --git a/test/formats/test_vorbis.py b/test/formats/test_vorbis.py index be0b4c209..34cd05cd0 100644 --- a/test/formats/test_vorbis.py +++ b/test/formats/test_vorbis.py @@ -9,8 +9,10 @@ from picard import ( ) from picard.coverart.image import CoverArtImage from picard.formats import vorbis +from picard.metadata import Metadata from .common import ( + REPLAYGAIN_TAGS, TAGS, CommonTests, load_metadata, @@ -103,6 +105,26 @@ class OggOpusTest(CommonVorbisTests.VorbisTestCase): '~channels': '2', } + @skipUnlessTestfile + def test_replaygain_tags(self): + # The normal replaygain tags are not supported by Opus + tags = REPLAYGAIN_TAGS.copy() + metadata = Metadata(tags) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + for (key, value) in tags.items(): + self.assertEqual(loaded_metadata[key], '', '%s: %r != %r' % (key, loaded_metadata[key], '')) + + @skipUnlessTestfile + def test_r128_replaygain_tags(self): + tags = { + 'r128_album_gain': '-2857', + 'r128_track_gain': '-2857', + } + metadata = Metadata(tags) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + for (key, value) in tags.items(): + self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + class VorbisUtilTest(PicardTestCase): def test_sanitize_key(self): From a4990746a757e1f54edc79af966133e138ab8b0d Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 2 Sep 2019 21:46:49 +0200 Subject: [PATCH 3/8] PICARD-1586: Support R128_* tags only for Opus Test other formats for not supporting it. --- picard/formats/apev2.py | 2 ++ picard/formats/id3.py | 2 +- picard/formats/vorbis.py | 4 +++- test/formats/common.py | 28 ++++++++++++++++------------ test/formats/test_apev2.py | 5 +++++ test/formats/test_id3.py | 2 ++ test/formats/test_vorbis.py | 20 +++++++++++--------- 7 files changed, 40 insertions(+), 23 deletions(-) diff --git a/picard/formats/apev2.py b/picard/formats/apev2.py index cadec49d0..0245bd1c9 100644 --- a/picard/formats/apev2.py +++ b/picard/formats/apev2.py @@ -55,6 +55,8 @@ UNSUPPORTED_TAGS = [ 'podcasturl', 'show', 'showsort', + 'r128_album_gain', + 'r128_track_gain', ] diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 80703eaa6..0ab73ec07 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -549,7 +549,7 @@ class ID3File(File): @classmethod def supports_tag(cls, name): - unsupported_tags = {} + unsupported_tags = ['r128_album_gain', 'r128_track_gain'] return ((name and not name.startswith("~") and name not in unsupported_tags) or name in ("~rating", "~length") or name.startswith("~id3")) diff --git a/picard/formats/vorbis.py b/picard/formats/vorbis.py index b0db97bf2..9ccf71c5f 100644 --- a/picard/formats/vorbis.py +++ b/picard/formats/vorbis.py @@ -304,7 +304,7 @@ class VCommentFile(File): @classmethod def supports_tag(cls, name): - unsupported_tags = {} + unsupported_tags = ['r128_album_gain', 'r128_track_gain'] return (bool(name) and name not in unsupported_tags and is_valid_key(name)) @@ -364,6 +364,8 @@ class OggOpusFile(VCommentFile): def supports_tag(cls, name): if name.startswith('replaygain_'): return False + elif name.startswith('r128_'): + return True return VCommentFile.supports_tag(name) diff --git a/test/formats/common.py b/test/formats/common.py index 31edbf2f4..cd7e2444f 100644 --- a/test/formats/common.py +++ b/test/formats/common.py @@ -201,6 +201,7 @@ class CommonTests: super().setUp() self.tags = TAGS.copy() self.replaygain_tags = REPLAYGAIN_TAGS.copy() + self.unsupported_tags = {} self.setup_tags() def setup_tags(self): @@ -219,17 +220,11 @@ class CommonTests: @skipUnlessTestfile def test_simple_tags(self): - metadata = Metadata(self.tags) - loaded_metadata = save_and_load_metadata(self.filename, metadata) - for (key, value) in self.tags.items(): - self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + self._test_supported_tags(self.tags) @skipUnlessTestfile def test_replaygain_tags(self): - metadata = Metadata(self.replaygain_tags) - loaded_metadata = save_and_load_metadata(self.filename, metadata) - for (key, value) in self.replaygain_tags.items(): - self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + self._test_supported_tags(self.replaygain_tags) @skipUnlessTestfile def test_save_does_not_modify_metadata(self): @@ -243,10 +238,7 @@ class CommonTests: @skipUnlessTestfile def test_unsupported_tags(self): - metadata = Metadata(self.unsupported_tags) - loaded_metadata = save_and_load_metadata(self.filename, metadata) - for tag in self.unsupported_tags: - self.assertTrue(tag not in loaded_metadata, '%s: %r != None' % (tag, loaded_metadata[tag])) + self._test_unsupported_tags(self.unsupported_tags) @skipUnlessTestfile def test_preserve_unchanged_tags(self): @@ -346,3 +338,15 @@ class CommonTests: self.assertEqual(f._fixed_splitext(f.EXTENSIONS[0]), ('', f.EXTENSIONS[0])) self.assertEqual(f._fixed_splitext('.test'), os.path.splitext('.test')) self.assertNotEqual(f._fixed_splitext(f.EXTENSIONS[0]), os.path.splitext(f.EXTENSIONS[0])) + + def _test_supported_tags(self, tags): + metadata = Metadata(tags) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + for (key, value) in tags.items(): + self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + + def _test_unsupported_tags(self, tags): + metadata = Metadata(tags) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + for tag in tags: + self.assertTrue(tag not in loaded_metadata, '%s: %r != None' % (tag, loaded_metadata[tag])) diff --git a/test/formats/test_apev2.py b/test/formats/test_apev2.py index 3d81b28c5..37e36d017 100644 --- a/test/formats/test_apev2.py +++ b/test/formats/test_apev2.py @@ -36,6 +36,11 @@ SUPPORTED_TAGS = list(set(TAGS.keys()) - set(apev2.UNSUPPORTED_TAGS)) class CommonApeTests: class ApeTestCase(CommonTests.TagFormatsTestCase): + def setup_tags(self): + super().setup_tags() + self.unsupported_tags['r128_album_gain'] = '-2857' + self.unsupported_tags['r128_track_gain'] = '-2857' + def test_supports_tags(self): supports_tag = self.format.supports_tag for key in VALID_KEYS + SUPPORTED_TAGS: diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index 45edee1c7..48da7ab80 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -28,6 +28,8 @@ class CommonId3Tests: self.set_tags({ 'originaldate': '1980' }) + self.unsupported_tags['r128_album_gain'] = '-2857' + self.unsupported_tags['r128_track_gain'] = '-2857' @skipUnlessTestfile def test_id3_freeform_delete(self): diff --git a/test/formats/test_vorbis.py b/test/formats/test_vorbis.py index 34cd05cd0..a98dccb0b 100644 --- a/test/formats/test_vorbis.py +++ b/test/formats/test_vorbis.py @@ -9,7 +9,6 @@ from picard import ( ) from picard.coverart.image import CoverArtImage from picard.formats import vorbis -from picard.metadata import Metadata from .common import ( REPLAYGAIN_TAGS, @@ -58,6 +57,15 @@ class CommonVorbisTests: for key in INVALID_KEYS + ['']: self.assertFalse(supports_tag(key), '%r should be unsupported' % key) + @skipUnlessTestfile + def test_r128_replaygain_tags(self): + # Vorbis files other then Opus must not support the r128_* tags + tags = { + 'r128_album_gain': '-2857', + 'r128_track_gain': '-2857', + } + self._test_unsupported_tags(tags) + class FLACTest(CommonVorbisTests.VorbisTestCase): testfile = 'test.flac' @@ -109,10 +117,7 @@ class OggOpusTest(CommonVorbisTests.VorbisTestCase): def test_replaygain_tags(self): # The normal replaygain tags are not supported by Opus tags = REPLAYGAIN_TAGS.copy() - metadata = Metadata(tags) - loaded_metadata = save_and_load_metadata(self.filename, metadata) - for (key, value) in tags.items(): - self.assertEqual(loaded_metadata[key], '', '%s: %r != %r' % (key, loaded_metadata[key], '')) + self._test_unsupported_tags(tags) @skipUnlessTestfile def test_r128_replaygain_tags(self): @@ -120,10 +125,7 @@ class OggOpusTest(CommonVorbisTests.VorbisTestCase): 'r128_album_gain': '-2857', 'r128_track_gain': '-2857', } - metadata = Metadata(tags) - loaded_metadata = save_and_load_metadata(self.filename, metadata) - for (key, value) in tags.items(): - self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + self._test_supported_tags(tags) class VorbisUtilTest(PicardTestCase): From ec777be2cc17ebd880460dec0412f0df8bd1c434 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 2 Sep 2019 23:03:11 +0200 Subject: [PATCH 4/8] PICARD-1586: ReplayGain tags case-insensitive for ID3 --- picard/formats/id3.py | 33 +++++++++++++++++++-------- picard/formats/mutagenext/__init__.py | 26 +++++++++++++++++++++ test/formats/common.py | 7 ++++++ test/formats/test_id3.py | 18 +++++++++++++++ test/formats/test_mutagenext.py | 17 ++++++++++++++ 5 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 test/formats/test_mutagenext.py diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 0ab73ec07..a08323ae0 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -37,7 +37,10 @@ from picard.coverart.image import ( TagCoverArtImage, ) from picard.file import File -from picard.formats.mutagenext import compatid3 +from picard.formats.mutagenext import ( + compatid3, + delall_ci, +) from picard.metadata import Metadata from picard.util import ( encode_filename, @@ -173,17 +176,22 @@ class ID3File(File): 'WORK': 'work', 'Writer': 'writer', 'SHOWMOVEMENT': 'showmovement', - 'REPLAYGAIN_ALBUM_GAIN': 'replaygain_album_gain', - 'REPLAYGAIN_ALBUM_PEAK': 'replaygain_album_peak', - 'REPLAYGAIN_ALBUM_RANGE': 'replaygain_album_range', - 'REPLAYGAIN_TRACK_GAIN': 'replaygain_track_gain', - 'REPLAYGAIN_TRACK_PEAK': 'replaygain_track_peak', - 'REPLAYGAIN_TRACK_RANGE': 'replaygain_track_range', - 'REPLAYGAIN_REFERENCE_LOUDNESS': 'replaygain_reference_loudness', } __rtranslate_freetext = dict([(v, k) for k, v in __translate_freetext.items()]) __translate_freetext['writer'] = 'writer' # For backward compatibility of case + # Freetext fields that are loaded case-insensitive + __rtranslate_freetext_ci = { + 'replaygain_album_gain': 'REPLAYGAIN_ALBUM_GAIN', + 'replaygain_album_peak': 'REPLAYGAIN_ALBUM_PEAK', + 'replaygain_album_range': 'REPLAYGAIN_ALBUM_RANGE', + 'replaygain_track_gain': 'REPLAYGAIN_TRACK_GAIN', + 'replaygain_track_peak': 'REPLAYGAIN_TRACK_PEAK', + 'replaygain_track_range': 'REPLAYGAIN_TRACK_RANGE', + 'replaygain_reference_loudness': 'REPLAYGAIN_REFERENCE_LOUDNESS', + } + __translate_freetext_ci = dict([(b.lower(), a) for a, b in __rtranslate_freetext_ci.items()]) + # Obsolete tag names which will still be loaded, but will get renamed on saving __rename_freetext = { 'Artists': 'ARTISTS', @@ -263,9 +271,12 @@ class ID3File(File): metadata.add('performer:%s' % role, name) elif frameid == 'TXXX': name = frame.desc + name_lower = name.lower() if name in self.__rename_freetext: name = self.__rename_freetext[name] - if name in self.__translate_freetext: + if name_lower in self.__translate_freetext_ci: + name = self.__translate_freetext_ci[name_lower] + elif name in self.__translate_freetext: name = self.__translate_freetext[name] elif ((name in self.__rtranslate) != (name in self.__rtranslate_freetext)): @@ -458,6 +469,10 @@ class ID3File(File): tags.delall('XSOP') elif frameid == 'TSO2': tags.delall('TXXX:ALBUMARTISTSORT') + elif name in self.__rtranslate_freetext_ci: + description = self.__rtranslate_freetext_ci[name] + delall_ci(tags, 'TXXX:' + description) + tags.add(self.build_TXXX(encoding, description, values)) elif name in self.__rtranslate_freetext: description = self.__rtranslate_freetext[name] if description in self.__rrename_freetext: diff --git a/picard/formats/mutagenext/__init__.py b/picard/formats/mutagenext/__init__.py index e69de29bb..98ac8136c 100644 --- a/picard/formats/mutagenext/__init__.py +++ b/picard/formats/mutagenext/__init__.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# +# Picard, the next-generation MusicBrainz tagger +# Copyright (C) 2019 Philipp Wolfer +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + +def delall_ci(tags, key): + """Delete all tags with given key, case-insensitive""" + key = key.lower() + for k in list(tags.keys()): + if k.lower() == key: + del tags[k] diff --git a/test/formats/common.py b/test/formats/common.py index cd7e2444f..cdf9fa604 100644 --- a/test/formats/common.py +++ b/test/formats/common.py @@ -55,6 +55,13 @@ def load_raw(filename): return mutagen.File(filename) +def save_raw(filename, tags): + file = load_raw(filename) + for k, v in tags.items(): + file[k] = v + file.save() + + TAGS = { 'albumartist': 'Foo', 'albumartistsort': 'Foo', diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index 48da7ab80..f39c4acfc 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -1,5 +1,7 @@ import os.path +import mutagen + from test.picardtestcase import PicardTestCase from picard import config @@ -12,6 +14,7 @@ from .common import ( load_raw, save_and_load_metadata, save_metadata, + save_raw, skipUnlessTestfile, ) from .coverart import CommonCoverArtTests @@ -193,6 +196,21 @@ class CommonId3Tests: config.setting['write_id3v23'] = True self.test_preserve_unchanged_tags() + @skipUnlessTestfile + def test_replaygain_tags_case_insensitive(self): + tags = mutagen.id3.ID3Tags() + tags.add(mutagen.id3.TXXX(desc='replaygain_album_gain', text='-6.48 dB')) + tags.add(mutagen.id3.TXXX(desc='Replaygain_Album_Peak', text='0.978475')) + tags.add(mutagen.id3.TXXX(desc='replaygain_album_range', text='7.84 dB')) + tags.add(mutagen.id3.TXXX(desc='replaygain_track_gain', text='-6.16 dB')) + tags.add(mutagen.id3.TXXX(desc='REPLAYGAIN_track_peak', text='0.976991')) + tags.add(mutagen.id3.TXXX(desc='REPLAYGAIN_TRACK_RANGE', text='8.22 dB')) + tags.add(mutagen.id3.TXXX(desc='replaygain_reference_loudness', text='-18.00 LUFS')) + save_raw(self.filename, tags) + loaded_metadata = load_metadata(self.filename) + for (key, value) in self.replaygain_tags.items(): + self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + class MP3Test(CommonId3Tests.Id3TestCase): testfile = 'test.mp3' diff --git a/test/formats/test_mutagenext.py b/test/formats/test_mutagenext.py new file mode 100644 index 000000000..73f966579 --- /dev/null +++ b/test/formats/test_mutagenext.py @@ -0,0 +1,17 @@ +# -*- coding: utf-8 -*- +from test.picardtestcase import PicardTestCase + +from picard.formats import mutagenext + + +class MutagenExtTest(PicardTestCase): + + def test_delall_ci(self): + tags = { + 'TAGNAME:ABC': 'a', + 'tagname:abc': 'a', + 'TagName:Abc': 'a', + 'OtherTag': 'a' + } + mutagenext.delall_ci(tags, 'tagname:Abc') + self.assertEqual({'OtherTag': 'a'}, tags) From 9d0e1232c6235df61d26fa9a4784bb5a87124ebb Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Tue, 3 Sep 2019 09:02:06 +0200 Subject: [PATCH 5/8] PICARD-1586: ReplayGain tags case-insensitive for ASF and MP4 --- picard/formats/asf.py | 28 ++++++++++++++++++++++------ picard/formats/mp4.py | 33 ++++++++++++++++++++++++++------- test/formats/common.py | 22 +++++++++++++++++++++- test/formats/test_asf.py | 27 ++++++++++++++++++++++++--- test/formats/test_mp4.py | 21 +++++++++++++++++++++ 5 files changed, 114 insertions(+), 17 deletions(-) diff --git a/picard/formats/asf.py b/picard/formats/asf.py index 0f65eaeec..230da70dc 100644 --- a/picard/formats/asf.py +++ b/picard/formats/asf.py @@ -37,6 +37,7 @@ from picard.formats.id3 import ( image_type_as_id3_num, types_from_id3, ) +from picard.formats.mutagenext import delall_ci from picard.metadata import Metadata from picard.util import encode_filename @@ -158,6 +159,11 @@ class ASFFile(File): 'artists': 'WM/ARTISTS', 'work': 'WM/Work', 'website': 'WM/AuthorURL', + } + __RTRANS = dict([(b, a) for a, b in __TRANS.items()]) + + # Tags to load case insensitive + __TRANS_CI = { 'replaygain_album_gain': 'REPLAYGAIN_ALBUM_GAIN', 'replaygain_album_peak': 'REPLAYGAIN_ALBUM_PEAK', 'replaygain_album_range': 'REPLAYGAIN_ALBUM_RANGE', @@ -166,10 +172,11 @@ class ASFFile(File): 'replaygain_track_range': 'REPLAYGAIN_TRACK_RANGE', 'replaygain_reference_loudness': 'REPLAYGAIN_REFERENCE_LOUDNESS', } - __RTRANS = dict([(b, a) for a, b in __TRANS.items()]) + __RTRANS_CI = dict([(b.lower(), a) for a, b in __TRANS_CI.items()]) def _load(self, filename): log.debug("Loading file %r", filename) + self.__casemap = {} file = ASF(encode_filename(filename)) metadata = Metadata() for name, values in file.tags.items(): @@ -191,8 +198,6 @@ class ASFFile(File): else: metadata.images.append(coverartimage) - continue - elif name not in self.__RTRANS: continue elif name == 'WM/SharedUserRating': # Rating in WMA ranges from 0 to 99, normalize this to the range 0 to 5 @@ -202,7 +207,13 @@ class ASFFile(File): if len(disc) > 1: metadata["totaldiscs"] = disc[1] values[0] = disc[0] - name = self.__RTRANS[name] + name_lower = name.lower() + if name in self.__RTRANS: + name = self.__RTRANS[name] + elif name_lower in self.__RTRANS_CI: + name = self.__RTRANS_CI[name_lower] + else: + continue values = [str(value) for value in values if value] if values: metadata[name] = values @@ -231,9 +242,13 @@ class ASFFile(File): values = [int(values[0]) * 99 // (config.setting['rating_steps'] - 1)] elif name == 'discnumber' and 'totaldiscs' in metadata: values = ['%s/%s' % (metadata['discnumber'], metadata['totaldiscs'])] - if name not in self.__TRANS: + if name in self.__TRANS: + name = self.__TRANS[name] + elif name in self.__TRANS_CI: + name = self.__TRANS_CI[name] + delall_ci(tags, name) + else: continue - name = self.__TRANS[name] tags[name] = values self._remove_deleted_tags(metadata, tags) @@ -250,6 +265,7 @@ class ASFFile(File): @classmethod def supports_tag(cls, name): return (name in cls.__TRANS + or name in cls.__TRANS_CI or name in ('~rating', '~length', 'totaldiscs') or name.startswith('lyrics')) diff --git a/picard/formats/mp4.py b/picard/formats/mp4.py index 6f7636855..1f3be5605 100644 --- a/picard/formats/mp4.py +++ b/picard/formats/mp4.py @@ -31,6 +31,7 @@ from picard.coverart.image import ( TagCoverArtImage, ) from picard.file import File +from picard.formats.mutagenext import delall_ci from picard.metadata import Metadata from picard.util import encode_filename @@ -121,25 +122,32 @@ class MP4File(File): "----:com.apple.iTunes:ARTISTS": "artists", "----:com.apple.iTunes:WORK": "work", "----:com.apple.iTunes:initialkey": "key", - "----:com.apple.iTunes:REPLAYGAIN_ALBUM_GAIN": "replaygain_album_gain", - "----:com.apple.iTunes:REPLAYGAIN_ALBUM_PEAK": "replaygain_album_peak", - "----:com.apple.iTunes:REPLAYGAIN_ALBUM_RANGE": "replaygain_album_range", - "----:com.apple.iTunes:REPLAYGAIN_TRACK_GAIN": "replaygain_track_gain", - "----:com.apple.iTunes:REPLAYGAIN_TRACK_PEAK": "replaygain_track_peak", - "----:com.apple.iTunes:REPLAYGAIN_TRACK_RANGE": "replaygain_track_range", - "----:com.apple.iTunes:REPLAYGAIN_REFERENCE_LOUDNESS": "replaygain_reference_loudness", } __r_freeform_tags = dict([(v, k) for k, v in __freeform_tags.items()]) + # Tags to load case insensitive + __r_freeform_tags_ci = { + "replaygain_album_gain": "----:com.apple.iTunes:REPLAYGAIN_ALBUM_GAIN", + "replaygain_album_peak": "----:com.apple.iTunes:REPLAYGAIN_ALBUM_PEAK", + "replaygain_album_range": "----:com.apple.iTunes:REPLAYGAIN_ALBUM_RANGE", + "replaygain_track_gain": "----:com.apple.iTunes:REPLAYGAIN_TRACK_GAIN", + "replaygain_track_peak": "----:com.apple.iTunes:REPLAYGAIN_TRACK_PEAK", + "replaygain_track_range": "----:com.apple.iTunes:REPLAYGAIN_TRACK_RANGE", + "replaygain_reference_loudness": "----:com.apple.iTunes:REPLAYGAIN_REFERENCE_LOUDNESS", + } + __freeform_tags_ci = dict([(b.lower(), a) for a, b in __r_freeform_tags_ci.items()]) + __other_supported_tags = ("discnumber", "tracknumber", "totaldiscs", "totaltracks") def _load(self, filename): log.debug("Loading file %r", filename) + self.__casemap = {} file = MP4(encode_filename(filename)) tags = file.tags or {} metadata = Metadata() for name, values in tags.items(): + name_lower = name.lower() if name in self.__text_tags: for value in values: metadata.add(self.__text_tags[name], value) @@ -152,6 +160,10 @@ class MP4File(File): for value in values: value = value.decode("utf-8", "replace").strip("\x00") metadata.add(self.__freeform_tags[name], value) + elif name_lower in self.__freeform_tags_ci: + for value in values: + value = value.decode("utf-8", "replace").strip("\x00") + metadata.add(self.__freeform_tags_ci[name_lower], value) elif name == "----:com.apple.iTunes:fingerprint": for value in values: value = value.decode("utf-8", "replace").strip("\x00") @@ -208,6 +220,10 @@ class MP4File(File): elif name in self.__r_freeform_tags: values = [v.encode("utf-8") for v in values] tags[self.__r_freeform_tags[name]] = values + elif name in self.__r_freeform_tags_ci: + values = [v.encode("utf-8") for v in values] + delall_ci(tags, self.__r_freeform_tags_ci[name]) + tags[self.__r_freeform_tags_ci[name]] = values elif name == "musicip_fingerprint": tags["----:com.apple.iTunes:fingerprint"] = [b"MusicMagic Fingerprint%s" % v.encode('ascii') for v in values] @@ -251,6 +267,7 @@ class MP4File(File): return (name in cls.__r_text_tags or name in cls.__r_bool_tags or name in cls.__r_freeform_tags + or name in cls.__r_freeform_tags_ci or name in cls.__r_int_tags or name in cls.__other_supported_tags or name.startswith('lyrics:') @@ -267,6 +284,8 @@ class MP4File(File): return self.__r_int_tags[name] elif name in self.__r_freeform_tags: return self.__r_freeform_tags[name] + elif name in self.__r_freeform_tags_ci: + return self.__r_freeform_tags_ci[name] elif name == "musicip_fingerprint": return "----:com.apple.iTunes:fingerprint" elif name in ("tracknumber", "totaltracks"): diff --git a/test/formats/common.py b/test/formats/common.py index cdf9fa604..6eb3a7fff 100644 --- a/test/formats/common.py +++ b/test/formats/common.py @@ -11,6 +11,7 @@ from test.picardtestcase import PicardTestCase from picard import config import picard.formats from picard.formats import ext_to_format +from picard.formats.mutagenext.tak import TAK from picard.metadata import Metadata @@ -52,7 +53,10 @@ def save_and_load_metadata(filename, metadata): def load_raw(filename): - return mutagen.File(filename) + file = mutagen.File(filename) + if file is None: + file = mutagen.File(filename, [TAK]) + return file def save_raw(filename, tags): @@ -233,6 +237,22 @@ class CommonTests: def test_replaygain_tags(self): self._test_supported_tags(self.replaygain_tags) + @skipUnlessTestfile + def test_replaygain_tags_case_insensitive(self): + tags = { + 'replaygain_album_gain': '-6.48 dB', + 'Replaygain_Album_Peak': '0.978475', + 'replaygain_album_range': '7.84 dB', + 'replaygain_track_gain': '-6.16 dB', + 'replaygain_track_peak': '0.976991', + 'replaygain_track_range': '8.22 dB', + 'replaygain_reference_loudness': '-18.00 LUFS', + } + save_raw(self.filename, tags) + loaded_metadata = load_metadata(self.filename) + for (key, value) in self.replaygain_tags.items(): + self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + @skipUnlessTestfile def test_save_does_not_modify_metadata(self): tags = dict(self.tags) diff --git a/test/formats/test_asf.py b/test/formats/test_asf.py index 93ae2cdc6..0fe185092 100644 --- a/test/formats/test_asf.py +++ b/test/formats/test_asf.py @@ -3,13 +3,34 @@ from test.picardtestcase import ( create_fake_png, ) -from picard.formats import asf +from picard.formats import ( + asf, + ext_to_format, +) from .common import CommonTests from .coverart import CommonCoverArtTests -class ASFTest(CommonTests.TagFormatsTestCase): +# prevent unittest to run tests in those classes +class CommonAsfTests: + + class AsfTestCase(CommonTests.TagFormatsTestCase): + + def test_supports_tag(self): + fmt = ext_to_format(self.testfile_ext[1:]) + self.assertTrue(fmt.supports_tag('copyright')) + self.assertTrue(fmt.supports_tag('compilation')) + self.assertTrue(fmt.supports_tag('bpm')) + self.assertTrue(fmt.supports_tag('djmixer')) + self.assertTrue(fmt.supports_tag('discnumber')) + self.assertTrue(fmt.supports_tag('lyrics:lead')) + self.assertTrue(fmt.supports_tag('~length')) + for tag in self.replaygain_tags.keys(): + self.assertTrue(fmt.supports_tag(tag)) + + +class ASFTest(CommonAsfTests.AsfTestCase): testfile = 'test.asf' supports_ratings = True expected_info = { @@ -20,7 +41,7 @@ class ASFTest(CommonTests.TagFormatsTestCase): } -class WMATest(CommonTests.TagFormatsTestCase): +class WMATest(CommonAsfTests.AsfTestCase): testfile = 'test.wma' supports_ratings = True expected_info = { diff --git a/test/formats/test_mp4.py b/test/formats/test_mp4.py index eaeaf5915..e21e4a439 100644 --- a/test/formats/test_mp4.py +++ b/test/formats/test_mp4.py @@ -1,8 +1,12 @@ +import mutagen + from picard.formats import ext_to_format from .common import ( CommonTests, load_metadata, + save_raw, + skipUnlessTestfile, ) from .coverart import CommonCoverArtTests @@ -27,11 +31,28 @@ class MP4Test(CommonTests.TagFormatsTestCase): self.assertTrue(fmt.supports_tag('discnumber')) self.assertTrue(fmt.supports_tag('lyrics:lead')) self.assertTrue(fmt.supports_tag('~length')) + for tag in self.replaygain_tags.keys(): + self.assertTrue(fmt.supports_tag(tag)) def test_format(self): metadata = load_metadata(self.filename) self.assertIn('AAC LC', metadata['~format']) + @skipUnlessTestfile + def test_replaygain_tags_case_insensitive(self): + tags = mutagen.mp4.MP4Tags() + tags['----:com.apple.iTunes:replaygain_album_gain'] = [b'-6.48 dB'] + tags['----:com.apple.iTunes:Replaygain_Album_Peak'] = [b'0.978475'] + tags['----:com.apple.iTunes:replaygain_album_range'] = [b'7.84 dB'] + tags['----:com.apple.iTunes:replaygain_track_gain'] = [b'-6.16 dB'] + tags['----:com.apple.iTunes:REPLAYGAIN_track_peak'] = [b'0.976991'] + tags['----:com.apple.iTunes:REPLAYGAIN_TRACK_RANGE'] = [b'8.22 dB'] + tags['----:com.apple.iTunes:replaygain_reference_loudness'] = [b'-18.00 LUFS'] + save_raw(self.filename, tags) + loaded_metadata = load_metadata(self.filename) + for (key, value) in self.replaygain_tags.items(): + self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + class Mp4CoverArtTest(CommonCoverArtTests.CoverArtTestCase): testfile = 'test.m4a' From a7faed5ccaa599bea965ea0b8006885d71ea9cc9 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Wed, 4 Sep 2019 19:33:21 +0200 Subject: [PATCH 6/8] PICARD-1586: Test proper deletion of case insensitive tags --- test/formats/test_asf.py | 22 +++++++++++++++++++++- test/formats/test_id3.py | 12 ++++++++++++ test/formats/test_mp4.py | 14 ++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/test/formats/test_asf.py b/test/formats/test_asf.py index 0fe185092..333016dd5 100644 --- a/test/formats/test_asf.py +++ b/test/formats/test_asf.py @@ -8,7 +8,14 @@ from picard.formats import ( ext_to_format, ) -from .common import CommonTests +from .common import ( + CommonTests, + load_metadata, + load_raw, + save_metadata, + save_raw, + skipUnlessTestfile, +) from .coverart import CommonCoverArtTests @@ -29,6 +36,19 @@ class CommonAsfTests: for tag in self.replaygain_tags.keys(): self.assertTrue(fmt.supports_tag(tag)) + @skipUnlessTestfile + def test_replaygain_tags_not_duplicated(self): + # Ensure values are not duplicated on repeated save + tags = { + 'Replaygain_Album_Peak': '-6.48 dB' + } + save_raw(self.filename, tags) + loaded_metadata = load_metadata(self.filename) + save_metadata(self.filename, loaded_metadata) + raw_metadata = load_raw(self.filename) + self.assertFalse('Replaygain_Album_Peak' in raw_metadata) + self.assertTrue('REPLAYGAIN_ALBUM_PEAK' in raw_metadata) + class ASFTest(CommonAsfTests.AsfTestCase): testfile = 'test.asf' diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index f39c4acfc..4d904942c 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -211,6 +211,18 @@ class CommonId3Tests: for (key, value) in self.replaygain_tags.items(): self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + @skipUnlessTestfile + def test_replaygain_tags_not_duplicated(self): + # Ensure values are not duplicated on repeated save + tags = mutagen.id3.ID3Tags() + tags.add(mutagen.id3.TXXX(desc='Replaygain_Album_Peak', text='0.978475')) + save_raw(self.filename, tags) + loaded_metadata = load_metadata(self.filename) + save_metadata(self.filename, loaded_metadata) + raw_metadata = load_raw(self.filename) + self.assertFalse('TXXX:Replaygain_Album_Peak' in raw_metadata) + self.assertTrue('TXXX:REPLAYGAIN_ALBUM_PEAK' in raw_metadata) + class MP3Test(CommonId3Tests.Id3TestCase): testfile = 'test.mp3' diff --git a/test/formats/test_mp4.py b/test/formats/test_mp4.py index e21e4a439..9c3df04c6 100644 --- a/test/formats/test_mp4.py +++ b/test/formats/test_mp4.py @@ -5,6 +5,8 @@ from picard.formats import ext_to_format from .common import ( CommonTests, load_metadata, + load_raw, + save_metadata, save_raw, skipUnlessTestfile, ) @@ -53,6 +55,18 @@ class MP4Test(CommonTests.TagFormatsTestCase): for (key, value) in self.replaygain_tags.items(): self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) + @skipUnlessTestfile + def test_replaygain_tags_not_duplicated(self): + # Ensure values are not duplicated on repeated save + tags = mutagen.mp4.MP4Tags() + tags['----:com.apple.iTunes:Replaygain_Album_Peak'] = [b'-6.48 dB'] + save_raw(self.filename, tags) + loaded_metadata = load_metadata(self.filename) + save_metadata(self.filename, loaded_metadata) + raw_metadata = load_raw(self.filename) + self.assertFalse('----:com.apple.iTunes:Replaygain_Album_Peak' in raw_metadata) + self.assertTrue('----:com.apple.iTunes:REPLAYGAIN_ALBUM_PEAK' in raw_metadata) + class Mp4CoverArtTest(CommonCoverArtTests.CoverArtTestCase): testfile = 'test.m4a' From c07168f7341c770fd75c3d544e221d0154df2d6a Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Thu, 5 Sep 2019 23:09:45 +0200 Subject: [PATCH 7/8] PICARD-1586: Preserve case for ReplayGain tags in ID3, MP4, ASF --- picard/formats/asf.py | 11 ++++++++++- picard/formats/id3.py | 13 +++++++++++-- picard/formats/mp4.py | 14 ++++++++++++-- test/formats/common.py | 2 ++ test/formats/test_asf.py | 12 ++++++++---- test/formats/test_id3.py | 14 ++++++++++---- test/formats/test_mp4.py | 14 ++++++++++---- 7 files changed, 63 insertions(+), 17 deletions(-) diff --git a/picard/formats/asf.py b/picard/formats/asf.py index 230da70dc..78a57a772 100644 --- a/picard/formats/asf.py +++ b/picard/formats/asf.py @@ -174,6 +174,10 @@ class ASFFile(File): } __RTRANS_CI = dict([(b.lower(), a) for a, b in __TRANS_CI.items()]) + def __init__(self, filename): + super().__init__(filename) + self.__casemap = {} + def _load(self, filename): log.debug("Loading file %r", filename) self.__casemap = {} @@ -211,7 +215,9 @@ class ASFFile(File): if name in self.__RTRANS: name = self.__RTRANS[name] elif name_lower in self.__RTRANS_CI: + orig_name = name name = self.__RTRANS_CI[name_lower] + self.__casemap[name] = orig_name else: continue values = [str(value) for value in values if value] @@ -245,7 +251,10 @@ class ASFFile(File): if name in self.__TRANS: name = self.__TRANS[name] elif name in self.__TRANS_CI: - name = self.__TRANS_CI[name] + if name in self.__casemap: + name = self.__casemap[name] + else: + name = self.__TRANS_CI[name] delall_ci(tags, name) else: continue diff --git a/picard/formats/id3.py b/picard/formats/id3.py index a08323ae0..84f42eeb3 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -217,6 +217,10 @@ class ID3File(File): 'MVIN': re.compile(r'^(?P\d+)(?:/(?P\d+))?$') } + def __init__(self, filename): + super().__init__(filename) + self.__casemap = {} + def build_TXXX(self, encoding, desc, values): """Construct and return a TXXX frame.""" # This is here so that plugins can customize the behavior of TXXX @@ -229,6 +233,7 @@ class ID3File(File): def _load(self, filename): log.debug("Loading file %r", filename) + self.__casemap = {} file = self._get_file(encode_filename(filename)) tags = file.tags or {} # upgrade custom 2.3 frames to 2.4 @@ -275,7 +280,9 @@ class ID3File(File): if name in self.__rename_freetext: name = self.__rename_freetext[name] if name_lower in self.__translate_freetext_ci: + orig_name = name name = self.__translate_freetext_ci[name_lower] + self.__casemap[name] = orig_name elif name in self.__translate_freetext: name = self.__translate_freetext[name] elif ((name in self.__rtranslate) @@ -388,7 +395,6 @@ class ID3File(File): tmcl = mutagen.id3.TMCL(encoding=encoding, people=[]) tipl = mutagen.id3.TIPL(encoding=encoding, people=[]) - for name, values in metadata.rawitems(): values = [id3text(v, encoding) for v in values] name = id3text(name, encoding) @@ -470,7 +476,10 @@ class ID3File(File): elif frameid == 'TSO2': tags.delall('TXXX:ALBUMARTISTSORT') elif name in self.__rtranslate_freetext_ci: - description = self.__rtranslate_freetext_ci[name] + if name in self.__casemap: + description = self.__casemap[name] + else: + description = self.__rtranslate_freetext_ci[name] delall_ci(tags, 'TXXX:' + description) tags.add(self.build_TXXX(encoding, description, values)) elif name in self.__rtranslate_freetext: diff --git a/picard/formats/mp4.py b/picard/formats/mp4.py index 1f3be5605..de78da6a3 100644 --- a/picard/formats/mp4.py +++ b/picard/formats/mp4.py @@ -140,6 +140,10 @@ class MP4File(File): __other_supported_tags = ("discnumber", "tracknumber", "totaldiscs", "totaltracks") + def __init__(self, filename): + super().__init__(filename) + self.__casemap = {} + def _load(self, filename): log.debug("Loading file %r", filename) self.__casemap = {} @@ -163,7 +167,9 @@ class MP4File(File): elif name_lower in self.__freeform_tags_ci: for value in values: value = value.decode("utf-8", "replace").strip("\x00") - metadata.add(self.__freeform_tags_ci[name_lower], value) + tag_name = self.__freeform_tags_ci[name_lower] + metadata.add(tag_name, value) + self.__casemap[tag_name] = name elif name == "----:com.apple.iTunes:fingerprint": for value in values: value = value.decode("utf-8", "replace").strip("\x00") @@ -223,7 +229,11 @@ class MP4File(File): elif name in self.__r_freeform_tags_ci: values = [v.encode("utf-8") for v in values] delall_ci(tags, self.__r_freeform_tags_ci[name]) - tags[self.__r_freeform_tags_ci[name]] = values + if name in self.__casemap: + name = self.__casemap[name] + else: + name = self.__r_freeform_tags_ci[name] + tags[name] = values elif name == "musicip_fingerprint": tags["----:com.apple.iTunes:fingerprint"] = [b"MusicMagic Fingerprint%s" % v.encode('ascii') for v in values] diff --git a/test/formats/common.py b/test/formats/common.py index 6eb3a7fff..334cf6637 100644 --- a/test/formats/common.py +++ b/test/formats/common.py @@ -35,6 +35,8 @@ settings = { def save_metadata(filename, metadata): f = picard.formats.open_(filename) + loaded_metadata = f._load(filename) + f._copy_loaded_metadata(loaded_metadata) f._save(filename, metadata) diff --git a/test/formats/test_asf.py b/test/formats/test_asf.py index 333016dd5..d32c41cbf 100644 --- a/test/formats/test_asf.py +++ b/test/formats/test_asf.py @@ -37,17 +37,21 @@ class CommonAsfTests: self.assertTrue(fmt.supports_tag(tag)) @skipUnlessTestfile - def test_replaygain_tags_not_duplicated(self): - # Ensure values are not duplicated on repeated save + def test_ci_tags_preserve_case(self): + # Ensure values are not duplicated on repeated save and are saved + # case preserving. tags = { 'Replaygain_Album_Peak': '-6.48 dB' } save_raw(self.filename, tags) loaded_metadata = load_metadata(self.filename) + loaded_metadata['replaygain_album_peak'] = '1.0' save_metadata(self.filename, loaded_metadata) raw_metadata = load_raw(self.filename) - self.assertFalse('Replaygain_Album_Peak' in raw_metadata) - self.assertTrue('REPLAYGAIN_ALBUM_PEAK' in raw_metadata) + self.assertIn('Replaygain_Album_Peak', raw_metadata) + self.assertEqual(raw_metadata['Replaygain_Album_Peak'][0], loaded_metadata['replaygain_album_peak']) + self.assertEqual(1, len(raw_metadata['Replaygain_Album_Peak'])) + self.assertNotIn('REPLAYGAIN_ALBUM_PEAK', raw_metadata) class ASFTest(CommonAsfTests.AsfTestCase): diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index 4d904942c..911250021 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -212,16 +212,22 @@ class CommonId3Tests: self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) @skipUnlessTestfile - def test_replaygain_tags_not_duplicated(self): - # Ensure values are not duplicated on repeated save + def test_ci_tags_preserve_case(self): + # Ensure values are not duplicated on repeated save and are saved + # case preserving. tags = mutagen.id3.ID3Tags() tags.add(mutagen.id3.TXXX(desc='Replaygain_Album_Peak', text='0.978475')) save_raw(self.filename, tags) loaded_metadata = load_metadata(self.filename) + loaded_metadata['replaygain_album_peak'] = '1.0' save_metadata(self.filename, loaded_metadata) raw_metadata = load_raw(self.filename) - self.assertFalse('TXXX:Replaygain_Album_Peak' in raw_metadata) - self.assertTrue('TXXX:REPLAYGAIN_ALBUM_PEAK' in raw_metadata) + self.assertIn('TXXX:Replaygain_Album_Peak', raw_metadata) + self.assertEqual( + raw_metadata['TXXX:Replaygain_Album_Peak'].text[0], + loaded_metadata['replaygain_album_peak']) + self.assertEqual(1, len(raw_metadata['TXXX:Replaygain_Album_Peak'].text)) + self.assertNotIn('TXXX:REPLAYGAIN_ALBUM_PEAK', raw_metadata) class MP3Test(CommonId3Tests.Id3TestCase): diff --git a/test/formats/test_mp4.py b/test/formats/test_mp4.py index 9c3df04c6..386ee3bb2 100644 --- a/test/formats/test_mp4.py +++ b/test/formats/test_mp4.py @@ -56,16 +56,22 @@ class MP4Test(CommonTests.TagFormatsTestCase): self.assertEqual(loaded_metadata[key], value, '%s: %r != %r' % (key, loaded_metadata[key], value)) @skipUnlessTestfile - def test_replaygain_tags_not_duplicated(self): - # Ensure values are not duplicated on repeated save + def test_ci_tags_preserve_case(self): + # Ensure values are not duplicated on repeated save and are saved + # case preserving. tags = mutagen.mp4.MP4Tags() tags['----:com.apple.iTunes:Replaygain_Album_Peak'] = [b'-6.48 dB'] save_raw(self.filename, tags) loaded_metadata = load_metadata(self.filename) + loaded_metadata['replaygain_album_peak'] = '1.0' save_metadata(self.filename, loaded_metadata) raw_metadata = load_raw(self.filename) - self.assertFalse('----:com.apple.iTunes:Replaygain_Album_Peak' in raw_metadata) - self.assertTrue('----:com.apple.iTunes:REPLAYGAIN_ALBUM_PEAK' in raw_metadata) + self.assertIn('----:com.apple.iTunes:Replaygain_Album_Peak', raw_metadata) + self.assertEqual( + raw_metadata['----:com.apple.iTunes:Replaygain_Album_Peak'][0].decode('utf-8'), + loaded_metadata['replaygain_album_peak']) + self.assertEqual(1, len(raw_metadata['----:com.apple.iTunes:Replaygain_Album_Peak'])) + self.assertNotIn('----:com.apple.iTunes:REPLAYGAIN_ALBUM_PEAK', raw_metadata) class Mp4CoverArtTest(CommonCoverArtTests.CoverArtTestCase): From feec4cb2991681b1d0fdc9e6fccd4e59ba72c297 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Fri, 6 Sep 2019 07:52:53 +0200 Subject: [PATCH 8/8] Do not use file as variable name --- test/formats/common.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/formats/common.py b/test/formats/common.py index 334cf6637..af46e277d 100644 --- a/test/formats/common.py +++ b/test/formats/common.py @@ -55,17 +55,17 @@ def save_and_load_metadata(filename, metadata): def load_raw(filename): - file = mutagen.File(filename) - if file is None: - file = mutagen.File(filename, [TAK]) - return file + f = mutagen.File(filename) + if f is None: + f = mutagen.File(filename, [TAK]) + return f def save_raw(filename, tags): - file = load_raw(filename) + f = load_raw(filename) for k, v in tags.items(): - file[k] = v - file.save() + f[k] = v + f.save() TAGS = {