diff --git a/picard/file.py b/picard/file.py index 6d422f9dc..4d05e4afd 100644 --- a/picard/file.py +++ b/picard/file.py @@ -16,7 +16,7 @@ # Copyright (C) 2012-2014 Wieland Hoffmann # Copyright (C) 2013 Calvin Walton # Copyright (C) 2013-2014 Ionuț Ciocîrlan -# Copyright (C) 2013-2014, 2017 Sophist-UK +# Copyright (C) 2013-2014, 2017, 2021 Sophist-UK # Copyright (C) 2013-2014, 2017-2019 Laurent Monin # Copyright (C) 2016 Rahul Raturi # Copyright (C) 2016 Ville Skyttä @@ -156,9 +156,9 @@ class File(QtCore.QObject, Item): def __repr__(self): return '<%s %r>' % (type(self).__name__, self.base_filename) - @property - def new_metadata(self): - return self.metadata + # pylint: disable=no-self-use + def format_specific_metadata(self, metadata, tag): + return metadata.getall(tag) def load(self, callback): thread.run_task( @@ -370,14 +370,14 @@ class File(QtCore.QObject, Item): temp_info = {} for info in FILE_INFO_TAGS: temp_info[info] = self.orig_metadata[info] - images_changed = self.orig_metadata.images != self.new_metadata.images + images_changed = self.orig_metadata.images != self.metadata.images # Data is copied from New to Original because New may be # a subclass to handle id3v23 config = get_config() if config.setting["clear_existing_tags"]: - self.orig_metadata.copy(self.new_metadata) + self.orig_metadata.copy(self.metadata) else: - self.orig_metadata.update(self.new_metadata) + self.orig_metadata.update(self.metadata) # After saving deleted tags should no longer be marked deleted self.metadata.clear_deleted() self.orig_metadata.clear_deleted() @@ -630,22 +630,21 @@ class File(QtCore.QObject, Item): return self.similarity == 1.0 and self.state == File.NORMAL def update(self, signal=True): - new_metadata = self.new_metadata - names = set(new_metadata.keys()) - names.update(self.orig_metadata.keys()) + metadata = self.metadata + names = set(metadata) | set(self.orig_metadata) config = get_config() clear_existing_tags = config.setting["clear_existing_tags"] ignored_tags = config.setting["compare_ignore_tags"] for name in names: if (not name.startswith('~') and self.supports_tag(name) and name not in ignored_tags): - new_values = new_metadata.getall(name) + new_values = metadata.getall(name) if not (new_values or clear_existing_tags - or name in new_metadata.deleted_tags): + or name in metadata.deleted_tags): continue orig_values = self.orig_metadata.getall(name) if orig_values != new_values: - self.similarity = self.orig_metadata.compare(new_metadata, ignored_tags) + self.similarity = self.orig_metadata.compare(metadata, ignored_tags) if self.state == File.NORMAL: self.state = File.CHANGED break diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 130896aa9..75fc19ad1 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -10,7 +10,7 @@ # Copyright (C) 2011-2014 Wieland Hoffmann # Copyright (C) 2013 Calvin Walton # Copyright (C) 2013-2014, 2017-2019 Laurent Monin -# Copyright (C) 2013-2015, 2017 Sophist-UK +# Copyright (C) 2013-2015, 2017, 2021 Sophist-UK # Copyright (C) 2015 Frederik “Freso” S. Olesen # Copyright (C) 2016 Christoph Reiter # Copyright (C) 2016-2018 Sambhav Kothari @@ -650,36 +650,29 @@ class ID3File(File): tags.update_to_v24() tags.save(filename, v2_version=4, v1=v1) - @property - def new_metadata(self): + def format_specific_metadata(self, metadata, tag): config = get_config() if not config.setting["write_id3v23"]: - return self.metadata - - copy = Metadata() - copy.copy(self.metadata) + return super().format_specific_metadata(metadata, tag) join_with = config.setting["id3v23_join_with"] - copy.multi_valued_joiner = join_with - for name, values in copy.rawitems(): - # ID3v23 can only save TDOR dates in YYYY format. Mutagen cannot - # handle ID3v23 dates which are YYYY-MM rather than YYYY or - # YYYY-MM-DD. - if name == "originaldate": - values = [v[:4] for v in values] - elif name == "date": - values = [(v[:4] if len(v) < 10 else v) for v in values] + values = metadata.getall(tag) + if not values: + return values - # If this is a multi-valued field, then it needs to be flattened, - # unless it's TIPL or TMCL which can still be multi-valued. - if (len(values) > 1 and name not in ID3File._rtipl_roles - and not name.startswith("performer:")): - values = [join_with.join(values)] + if tag == "originaldate": + values = [v[:4] for v in values] + elif tag == "date": + values = [(v[:4] if len(v) < 10 else v) for v in values] - copy[name] = values + # If this is a multi-valued field, then it needs to be flattened, + # unless it's TIPL or TMCL which can still be multi-valued. + if (len(values) > 1 and tag not in ID3File._rtipl_roles + and not tag.startswith("performer:")): + values = [join_with.join(values)] - return copy + return values class MP3File(ID3File): diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 6a6e63936..b95b568e0 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -6,7 +6,7 @@ # Copyright (C) 2006-2007, 2012 Lukáš Lalinský # Copyright (C) 2011-2014 Michael Wiencek # Copyright (C) 2012 Nikolai Prokoschenko -# Copyright (C) 2013-2014 Sophist-UK +# Copyright (C) 2013-2014, 2021 Sophist-UK # Copyright (C) 2013-2014, 2017-2019 Laurent Monin # Copyright (C) 2015 Ohm Patel # Copyright (C) 2015 Wieland Hoffmann @@ -586,13 +586,13 @@ class MetadataBox(QtWidgets.QTableWidget): top_tags_set = set(top_tags) for file in files: - new_metadata = file.new_metadata + new_metadata = file.metadata orig_metadata = file.orig_metadata tags = set(list(new_metadata.keys()) + list(orig_metadata.keys())) for name in filter(lambda x: not x.startswith("~") and file.supports_tag(x), tags): - new_values = new_metadata.getall(name) - orig_values = orig_metadata.getall(name) + new_values = file.format_specific_metadata(new_metadata, name) + orig_values = file.format_specific_metadata(orig_metadata, name) if not clear_existing_tags and not new_values: new_values = list(orig_values or [""]) diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index 8e065a355..862bf9bfa 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -3,7 +3,7 @@ # Picard, the next-generation MusicBrainz tagger # # Copyright (C) 2019 Laurent Monin -# Copyright (C) 2019-2020 Philipp Wolfer +# Copyright (C) 2019-2021 Philipp Wolfer # Copyright (C) 2019 Zenara Daley # # This program is free software; you can redistribute it and/or @@ -638,3 +638,33 @@ class Id3UtilTest(PicardTestCase): class Mp3CoverArtTest(CommonCoverArtTests.CoverArtTestCase): testfile = 'test.mp3' + + +class ID3FileTest(PicardTestCase): + + def setUp(self): + super().setUp() + self.file = id3.ID3File('somepath/somefile.mp3') + config.setting['write_id3v23'] = False + config.setting['id3v23_join_with'] = ' / ' + self.file.metadata['artist'] = ['foo', 'bar'] + self.file.metadata['originaldate'] = '2020-04-01' + self.file.metadata['date'] = '2021-04-01' + + def test_format_specific_metadata_v24(self): + metadata = self.file.metadata + for name, values in metadata.rawitems(): + self.assertEqual(values, self.file.format_specific_metadata(metadata, name)) + + def test_format_specific_metadata_v23(self): + config.setting['write_id3v23'] = True + metadata = self.file.metadata + self.assertEqual(['foo / bar'], self.file.format_specific_metadata(metadata, 'artist')) + self.assertEqual(['2020'], self.file.format_specific_metadata(metadata, 'originaldate')) + self.assertEqual(['2021-04-01'], self.file.format_specific_metadata(metadata, 'date')) + + def test_format_specific_metadata_v23_incomplete_date(self): + config.setting['write_id3v23'] = True + metadata = self.file.metadata + metadata['date'] = '2021-04' + self.assertEqual(['2021'], self.file.format_specific_metadata(metadata, 'date')) diff --git a/test/test_file.py b/test/test_file.py index 5cc37c4fa..625fa8529 100644 --- a/test/test_file.py +++ b/test/test_file.py @@ -2,7 +2,7 @@ # # Picard, the next-generation MusicBrainz tagger # -# Copyright (C) 2018-2020 Philipp Wolfer +# Copyright (C) 2018-2021 Philipp Wolfer # Copyright (C) 2019-2020 Laurent Monin # # This program is free software; you can redistribute it and/or @@ -87,6 +87,11 @@ class DataObjectTest(PicardTestCase): self.assertEqual(None, self.file.acoustid_fingerprint) self.assertEqual(0, self.file.acoustid_length) + def format_specific_metadata(self): + values = ['foo', 'bar'] + self.file.metadata['test'] = values + self.assertEqual(values, self.file.format_specific_metadata(self.file.metadata, 'test')) + class TestPreserveTimes(PicardTestCase):