fergalmoran/picard
1
0
Fork 0
mirror of https://github.com/fergalmoran/picard.git synced 2026-03-30 00:55:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

PICARD-1783: Fix deletion of comment, performer and lyrics tags

Browse Source 
This adds tests for deleting comment, performer and lyrics tags and fixes various issues with this in ID3, MP4, Vorbis and APEv2.
This commit is contained in:
Philipp Wolfer
2020-03-03 17:09:57 +01:00
parent 60d9827fa6
commit 57c8fe209a
6 changed files with 84 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
picard/formats/apev2.py
View File

@@ -169,7 +169,7 @@ class APEv2File(File):
if len(disc) > 1:
metadata["totaldiscs"] = disc[1]
value = disc[0]
elif name == 'performer' or name == 'comment':
elif name in ('performer', 'comment'):
if value.endswith(')'):
start = value.rfind(' (')
if start > 0:
@@ -236,14 +236,17 @@ class APEv2File(File):
"""Remove the tags from the file that were deleted in the UI"""
for tag in metadata.deleted_tags:
real_name = self._get_tag_name(tag)
if (real_name in ('Lyrics', 'Comment', 'Performer')
and ':' in tag and not tag.endswith(':')):
tag_type = re.compile(r"\(%s\)$" % tag.split(':', 1)[1])
existing_tags = tags.get(real_name)
if existing_tags:
for item in existing_tags:
if tag_type.search(item):
tags.get(real_name).remove(item)
if real_name in ('Lyrics', 'Comment', 'Performer'):
parts = tag.split(':', 1)
if len(parts) == 2:
tag_type_regex = r"\(%s\)$" % parts[1]
else:
tag_type_regex = r"[^)]$"
existing_tags = tags.get(real_name, [])
for item in existing_tags:
if re.search(tag_type_regex, item):
existing_tags.remove(item)
tags[real_name] = existing_tags
elif tag in ('totaltracks', 'totaldiscs'):
tagstr = real_name.lower() + 'number'
if tagstr in metadata:

2
picard/formats/id3.py
View File

@@ -547,7 +547,7 @@ class ID3File(File):
for people in frame.people:
if people[0] == role:
frame.people.remove(people)
elif name.startswith('comment:'):
elif name.startswith('comment:') or name == 'comment':
(lang, desc) = parse_comment_tag(name)
if desc.lower()[:4] != 'itun':
for key, frame in list(tags.items()):

8
picard/formats/mp4.py
View File

@@ -280,8 +280,7 @@ class MP4File(File):
tags[name] = values
elif name == "musicip_fingerprint":
tags["----:com.apple.iTunes:fingerprint"] = [b"MusicMagic Fingerprint%s" % v.encode('ascii') for v in values]
elif self.supports_tag(name) and name not in ('tracknumber',
'totaltracks', 'discnumber', 'totaldiscs'):
elif self.supports_tag(name) and name not in self.__other_supported_tags:
values = [v.encode("utf-8") for v in values]
name = self.__casemap.get(name, name)
tags['----:com.apple.iTunes:' + name] = values
@@ -351,8 +350,9 @@ class MP4File(File):
return "trkn"
elif name in ("discnumber", "totaldiscs"):
return "disk"
else:
return None
elif self.supports_tag(name) and name not in self.__other_supported_tags:
name = self.__casemap.get(name, name)
return '----:com.apple.iTunes:' + name
def _info(self, metadata, file):
super()._info(metadata, file)

16
picard/formats/vorbis.py
View File

@@ -4,7 +4,7 @@
#
# Copyright (C) 2006-2008, 2012 Lukáš Lalinský
# Copyright (C) 2008 Hendrik van Antwerpen
# Copyright (C) 2008-2010, 2014-2015, 2018-2019 Philipp Wolfer
# Copyright (C) 2008-2010, 2014-2015, 2018-2020 Philipp Wolfer
# Copyright (C) 2012-2013 Michael Wiencek
# Copyright (C) 2012-2014 Wieland Hoffmann
# Copyright (C) 2013 Calvin Walton
@@ -316,10 +316,16 @@ class VCommentFile(File):
real_name = self._get_tag_name(tag)
if real_name and real_name in tags:
if real_name in ('performer', 'comment'):
tag_type = r"\(%s\)" % tag.split(':', 1)[1]
for item in tags.get(real_name):
if re.search(tag_type, item):
tags.get(real_name).remove(item)
parts = tag.split(':', 1)
if len(parts) == 2:
tag_type_regex = r"\(%s\)$" % parts[1]
else:
tag_type_regex = r"[^)]$"
existing_tags = tags.get(real_name)
for item in existing_tags:
if re.search(tag_type_regex, item):
existing_tags.remove(item)
tags[real_name] = existing_tags
else:
if tag in ('totaldiscs', 'totaltracks'):
# both tag and real_name are to be deleted in this case

48
test/formats/common.py
View File

@@ -149,6 +149,7 @@ TAGS = {
'originaldate': '1980-01-20',
'originalyear': '1980',
'originalfilename': 'Foo',
'performer': 'Foo',
'performer:guest vocal': 'Foo',
'podcast': '1',
'podcasturl': 'Foo',
@@ -330,14 +331,51 @@ class CommonTests:
self.assertNotIn('~rating', new_metadata.keys())
@skipUnlessTestfile
def test_delete_lyrics_tags(self):
for key in ('lyrics', 'lyrics:'):
metadata = Metadata(self.tags)
def test_delete_tags_with_empty_description(self):
for key in ('lyrics', 'lyrics:', 'comment', 'comment:', 'performer', 'performer:'):
name = key.rstrip(':')
name_with_description = name + ':foo'
if not self.format.supports_tag(name):
continue
metadata = Metadata()
metadata[name] = 'bar'
metadata[name_with_description] = 'other'
original_metadata = save_and_load_metadata(self.filename, metadata)
self.assertIn('lyrics', original_metadata)
self.assertIn(name, original_metadata)
del metadata[key]
new_metadata = save_and_load_metadata(self.filename, metadata)
self.assertNotIn('lyrics', new_metadata)
self.assertNotIn(name, new_metadata)
# Ensure the names with description did not get deleted
if name_with_description in original_metadata:
self.assertIn(name_with_description, new_metadata)
@skipUnlessTestfile
def test_delete_tags_with_description(self):
for key in ('comment:foo', 'comment:de:foo', 'performer:foo', 'lyrics:foo'):
if not self.format.supports_tag(key):
continue
prefix = key.split(':')[0]
metadata = Metadata()
metadata[key] = 'bar'
metadata[prefix] = '(foo) bar'
original_metadata = save_and_load_metadata(self.filename, metadata)
if not key in original_metadata and prefix in original_metadata:
continue # Skip if the type did not support saving this kind of tag
del metadata[key]
new_metadata = save_and_load_metadata(self.filename, metadata)
self.assertNotIn(key, new_metadata)
self.assertEqual('(foo) bar', new_metadata[prefix])
@skipUnlessTestfile
def test_delete_nonexistant_tags(self):
for key in ('title', 'foo', 'comment:foo', 'comment:de:foo', 'performer:foo', 'lyrics:foo'):
if not self.format.supports_tag(key):
continue
metadata = Metadata()
save_metadata(self.filename, metadata)
del metadata[key]
new_metadata = save_and_load_metadata(self.filename, metadata)
self.assertNotIn(key, new_metadata)
@skipUnlessTestfile
def test_delete_non_existant_tags(self):

14
test/formats/test_mp4.py
View File

@@ -2,7 +2,7 @@
#
# Picard, the next-generation MusicBrainz tagger
#
# Copyright (C) 2019 Philipp Wolfer
# Copyright (C) 2019-2020 Philipp Wolfer
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
@@ -24,11 +24,13 @@ import unittest
import mutagen
from picard.formats import ext_to_format
from picard.metadata import Metadata
from .common import (
CommonTests,
load_metadata,
load_raw,
save_and_load_metadata,
save_metadata,
save_raw,
skipUnlessTestfile,
@@ -93,6 +95,16 @@ class CommonMP4Tests:
self.assertEqual(1, len(raw_metadata['----:com.apple.iTunes:' + name]))
self.assertNotIn('----:com.apple.iTunes:' + name.upper(), raw_metadata)
@skipUnlessTestfile
def test_delete_freeform_tags(self):
metadata = Metadata()
metadata['foo'] = 'bar'
original_metadata = save_and_load_metadata(self.filename, metadata)
self.assertEqual('bar', original_metadata['foo'])
del metadata['foo']
new_metadata = save_and_load_metadata(self.filename, metadata)
self.assertNotIn('foo', new_metadata)
class M4ATest(CommonMP4Tests.MP4TestCase):
testfile = 'test.m4a'