From 6bdba0948fd1edbc761bf23e1bc5625a8a2fc4c4 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Sun, 4 Oct 2020 11:28:04 +0200 Subject: [PATCH] PICARD-1877: Support language for lyrics tags Lyrics tags will be set as lyrcis[:lang[:desc]], where lang can also be empty to default to 'xxx'. --- picard/formats/id3.py | 25 +++++++++++-------------- picard/util/tags.py | 35 ++++++++++++++++++++++++++++++++++- test/formats/common.py | 2 +- test/formats/test_id3.py | 19 +++++++++++++++++-- test/test_util_tags.py | 18 ++++++++++++++++++ 5 files changed, 81 insertions(+), 18 deletions(-) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 74f3ce9c1b..061aa7393d 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -61,7 +61,11 @@ encode_filename, sanitize_date, ) -from picard.util.tags import parse_comment_tag +from picard.util.tags import ( + create_lang_desc_tag, + parse_comment_tag, + parse_lang_desc_tag, +) id3.GRP1 = compatid3.GRP1 @@ -324,9 +328,7 @@ def _load(self, filename): for text in frame.text: metadata.add(name, text) elif frameid == 'USLT': - name = 'lyrics' - if frame.desc: - name += ':%s' % frame.desc + name = create_lang_desc_tag('lyrics', frame.lang, frame.desc) metadata.add(name, frame.text) elif frameid == 'UFID' and frame.owner == 'http://musicbrainz.org': metadata['musicbrainz_recordingid'] = frame.data.decode('ascii', 'ignore') @@ -444,12 +446,9 @@ def _save(self, filename, metadata): else: tags.add(id3.COMM(encoding=encoding, desc=desc, lang=lang, text=values)) elif name.startswith('lyrics:') or name == 'lyrics': - if ':' in name: - desc = name.split(':', 1)[1] - else: - desc = '' + (lang, desc) = parse_lang_desc_tag(name) for value in values: - tags.add(id3.USLT(encoding=encoding, desc=desc, text=value)) + tags.add(id3.USLT(encoding=encoding, desc=desc, lang=lang, text=value)) elif name in self._rtipl_roles: for value in values: tipl.people.append([self._rtipl_roles[name], value]) @@ -556,12 +555,10 @@ def _remove_deleted_tags(self, metadata, tags): and frame.lang == lang): del tags[key] elif name.startswith('lyrics:') or name == 'lyrics': - if ':' in name: - desc = name.split(':', 1)[1] - else: - desc = '' + (lang, desc) = parse_lang_desc_tag(name) for key, frame in list(tags.items()): - if frame.FrameID == 'USLT' and frame.desc == desc: + if (frame.FrameID == 'USLT' and frame.desc == desc + and frame.lang == lang): del tags[key] elif name in self._rtipl_roles: role = self._rtipl_roles[name] diff --git a/picard/util/tags.py b/picard/util/tags.py index 0f426f184d..d01db602cb 100644 --- a/picard/util/tags.py +++ b/picard/util/tags.py @@ -146,7 +146,7 @@ def display_tag_name(name): def parse_comment_tag(name): # noqa: E302 """ Parses a tag name like "comment:XXX:desc", where XXX is the language. - If language is not set ("comment:desc") "eng" is assumed as default. + If language is not set ("comment:desc") default_language is used. Returns a (lang, desc) tuple. """ try: @@ -159,3 +159,36 @@ def parse_comment_tag(name): # noqa: E302 lang = match.group(1) desc = desc[4:] return (lang, desc) + + +RE_LYRICS_TAG = re.compile('^(?P[^:]+)(?::(?P[a-zA-Z]{3})?)?(?::(?P.*))?$') +def parse_lang_desc_tag(name, default_language='xxx'): # noqa: E302 + """ + Parses a tag name like with a language and description such as "lyrics:XXX:desc". + + Language must be a 3 letter language code as defined in ISO639-2, with the special + value 'xxx' being used if the language is not specified. If language is not set ("lyrics" + or "lyrics::desc") default_language is used. + Language and description are optional. + + Returns a (lang, desc) tuple. + """ + match = RE_LYRICS_TAG.match(name) + lang = default_language + desc = '' + if match: + lang = match.group('lang') or default_language + desc = match.group('desc') or '' + return (lang, desc) + + +def create_lang_desc_tag(name, language='xxx', description='', default_language='xxx'): + name_parts = [name, ] + if language and language.lower() != default_language: + name_parts.append(language) + elif description: + # Add empty language part if description is also set + name_parts.append('') + if description: + name_parts.append(description) + return ':'.join(name_parts) diff --git a/test/formats/common.py b/test/formats/common.py index 52134c3f8d..16066937da 100644 --- a/test/formats/common.py +++ b/test/formats/common.py @@ -356,7 +356,7 @@ def test_delete_tags_with_empty_description(self): @skipUnlessTestfile def test_delete_tags_with_description(self): for key in ( - 'comment:foo', 'comment:de:foo', 'performer:foo', 'lyrics:foo', + 'comment:foo', 'comment:deu:foo', 'performer:foo', 'lyrics::foo', 'lyrics:jpn:foo' 'comment:a*', 'comment:a[', 'performer:(x)', 'performer: Ä é ' ): if not self.format.supports_tag(key): diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index 6b5291ea18..63b35eb682 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -333,11 +333,26 @@ def test_ci_tags_preserve_case(self): self.assertEqual(1, len(raw_metadata['TXXX:Replaygain_Album_Peak'].text)) self.assertNotIn('TXXX:REPLAYGAIN_ALBUM_PEAK', raw_metadata) + @skipUnlessTestfile + def test_lyrics_with_language(self): + metadata = Metadata({'lyrics:jpn': 'bar'}) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + self.assertIn('lyrics:jpn', loaded_metadata) + self.assertEqual(metadata['lyrics:jpn'], loaded_metadata['lyrics:jpn']) + @skipUnlessTestfile def test_lyrics_with_description(self): - metadata = Metadata({'lyrics:foo': 'bar'}) + metadata = Metadata({'lyrics::foo': 'bar'}) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + self.assertIn('lyrics::foo', loaded_metadata) + self.assertEqual(metadata['lyrics::foo'], loaded_metadata['lyrics::foo']) + + @skipUnlessTestfile + def test_lyrics_with_language_and_description(self): + metadata = Metadata({'lyrics:fra:foo': 'bar'}) loaded_metadata = save_and_load_metadata(self.filename, metadata) - self.assertEqual(metadata['lyrics:foo'], loaded_metadata['lyrics:foo']) + self.assertIn('lyrics:fra:foo', loaded_metadata) + self.assertEqual(metadata['lyrics:fra:foo'], loaded_metadata['lyrics:fra:foo']) @skipUnlessTestfile def test_invalid_track_and_discnumber(self): diff --git a/test/test_util_tags.py b/test/test_util_tags.py index 9a53915997..e648abf7a4 100644 --- a/test/test_util_tags.py +++ b/test/test_util_tags.py @@ -22,8 +22,10 @@ from test.picardtestcase import PicardTestCase from picard.util.tags import ( + create_lang_desc_tag, display_tag_name, parse_comment_tag, + parse_lang_desc_tag, ) @@ -37,3 +39,19 @@ def test_parse_comment_tag(self): self.assertEqual(('XXX', 'foo'), parse_comment_tag('comment:XXX:foo')) self.assertEqual(('eng', 'foo'), parse_comment_tag('comment:foo')) self.assertEqual(('eng', ''), parse_comment_tag('comment')) + + def test_parse_lang_desc_tag(self): + self.assertEqual(('jpn', ''), parse_lang_desc_tag('lyrics:jpn')) + self.assertEqual(('jpn', ''), parse_lang_desc_tag('lyrics:jpn:')) + self.assertEqual(('jpn', 'foo'), parse_lang_desc_tag('lyrics:jpn:foo')) + self.assertEqual(('xxx', 'foo'), parse_lang_desc_tag('lyrics::foo')) + self.assertEqual(('xxx', 'notalanguage:foo'), parse_lang_desc_tag('lyrics:notalanguage:foo')) + + def test_create_lang_desc_tag(self): + self.assertEqual('comment', create_lang_desc_tag('comment')) + self.assertEqual('comment:eng', create_lang_desc_tag('comment', language='eng')) + self.assertEqual('comment::foo', create_lang_desc_tag('comment', description='foo')) + self.assertEqual('lyrics:jpn:foo', create_lang_desc_tag( + 'lyrics', language='jpn', description='foo')) + self.assertEqual('lyrics::foo', create_lang_desc_tag( + 'lyrics', language='jpn', description='foo', default_language='jpn'))