From c07168f7341c770fd75c3d544e221d0154df2d6a Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Thu, 5 Sep 2019 23:09:45 +0200 Subject: [PATCH] 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):