diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 7e0a1419d..1a5dd5ea8 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -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': diff --git a/test/formats/common.py b/test/formats/common.py index 899ef7ed4..fc1d01f33 100644 --- a/test/formats/common.py +++ b/test/formats/common.py @@ -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): diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index a8e7b8fc8..c3bbb0435 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -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()