PICARD-1786: Fix ID3 deleting multiple TIPL items with same role

Added tests for deleting TIPL entries from ID3, fixed issue of multiple people with same role not getting removed.
This commit is contained in:
Philipp Wolfer
2020-03-06 15:41:54 +01:00
parent 80b1b9ea93
commit bb4c417d04
3 changed files with 44 additions and 21 deletions

View File

@@ -109,6 +109,14 @@ def types_from_id3(id3type):
return [image_type_from_id3_num(id3type)]
def _remove_people_with_role(tags, frames, role):
for key, frame in tags.items():
if frame.FrameID in frames:
for people in list(frame.people):
if people[0] == role:
frame.people.remove(people)
class ID3File(File):
"""Generic ID3-based file."""
@@ -542,11 +550,7 @@ class ID3File(File):
try:
if name.startswith('performer:'):
role = name.split(':', 1)[1]
for key, frame in tags.items():
if frame.FrameID in ('TMCL', 'TIPL', 'IPLS'):
for people in frame.people:
if people[0] == role:
frame.people.remove(people)
_remove_people_with_role(tags, ['TMCL', 'TIPL', 'IPLS'], role)
elif name.startswith('comment:') or name == 'comment':
(lang, desc) = parse_comment_tag(name)
if desc.lower()[:4] != 'itun':
@@ -564,11 +568,7 @@ class ID3File(File):
del tags[key]
elif name in self._rtipl_roles:
role = self._rtipl_roles[name]
for key, frame in tags.items():
if frame.FrameID in ('TIPL', 'IPLS'):
for people in frame.people:
if people[0] == role:
frame.people.remove(people)
_remove_people_with_role(tags, ['TIPL', 'IPLS'], role)
elif name == 'musicbrainz_recordingid':
for key, frame in list(tags.items()):
if frame.FrameID == 'UFID' and frame.owner == 'http://musicbrainz.org':

View File

@@ -404,18 +404,22 @@ class CommonTests:
@skipUnlessTestfile
def test_delete_performer(self):
if 'performer:guest vocal' in self.tags:
metadata = Metadata(self.tags)
metadata['performer:piano'] = 'Foo'
if not self.format.supports_tag('performer:'):
raise unittest.SkipTest('Tag "performer:" not supported for %s' % self.format.NAME)
metadata = Metadata({
'performer:piano': ['Piano1', 'Piano2'],
'performer:guitar': ['Guitar1'],
})
original_metadata = save_and_load_metadata(self.filename, metadata)
self.assertIn('Piano1', original_metadata.getall('performer:piano'))
self.assertIn('Piano2', original_metadata.getall('performer:piano'))
self.assertEqual(2, len(original_metadata.getall('performer:piano')))
self.assertEqual('Guitar1', original_metadata['performer:guitar'])
original_metadata = save_and_load_metadata(self.filename, metadata)
del metadata['performer:piano']
new_metadata = save_and_load_metadata(self.filename, metadata)
self.assertIn('performer:guest vocal', original_metadata)
self.assertIn('performer:guest vocal', new_metadata)
self.assertIn('performer:piano', original_metadata)
self.assertNotIn('performer:piano', new_metadata)
del metadata['performer:piano']
new_metadata = save_and_load_metadata(self.filename, metadata)
self.assertNotIn('performer:piano', new_metadata)
self.assertEqual('Guitar1', metadata['performer:guitar'])
@skipUnlessTestfile
def test_save_performer(self):

View File

@@ -311,6 +311,25 @@ class CommonId3Tests:
self.assertNotIn('TOWN', raw_metadata)
self.assertNotIn('TXXX:foo', raw_metadata)
@skipUnlessTestfile
def test_delete_tipl(self):
tags = mutagen.id3.ID3Tags()
tags.add(mutagen.id3.TIPL(people=[
['mix', 'mixer1'],
['mix', 'mixer2'],
['producer', 'producer1'],
]))
save_raw(self.filename, tags)
metadata = Metadata()
metadata.delete('mixer')
save_metadata(self.filename, metadata)
raw_metadata = load_raw(self.filename)
people = raw_metadata['TIPL'].people
self.assertIn(['producer', 'producer1'], people)
self.assertNotIn(['mix', 'mixer1'], people)
self.assertNotIn(['mix', 'mixer2'], people)
self.assertEqual(1, len(people))
@skipUnlessTestfile
def test_load_conflicting_txxx_tags(self):
tags = mutagen.id3.ID3Tags()