diff --git a/picard/album.py b/picard/album.py index dc821a3b9..f69903375 100644 --- a/picard/album.py +++ b/picard/album.py @@ -718,9 +718,9 @@ class Album(DataObject, Item): if not self.update_metadata_images_enabled: return - update_metadata_images(self) - self.update(False) - self.metadata_images_changed.emit() + if update_metadata_images(self): + self.update(False) + self.metadata_images_changed.emit() def keep_original_images(self): self.enable_update_metadata_images(False) diff --git a/picard/cluster.py b/picard/cluster.py index 6280f0396..a08929a49 100644 --- a/picard/cluster.py +++ b/picard/cluster.py @@ -102,8 +102,8 @@ class FileList(QtCore.QObject, Item): def update_metadata_images(self): if self.update_metadata_images_enabled and self.can_show_coverart: - update_metadata_images(self) - self.metadata_images_changed.emit() + if update_metadata_images(self): + self.metadata_images_changed.emit() def keep_original_images(self): self.enable_update_metadata_images(False) @@ -379,13 +379,6 @@ class Cluster(FileList): yield album_name, artist_name, (files[i] for i in album) - def enable_update_metadata_images(self, enabled): - self.update_metadata_images_enabled = enabled - - def update_metadata_images(self): - if self.update_metadata_images_enabled and self.can_show_coverart: - update_metadata_images(self) - class UnclusteredFiles(Cluster): diff --git a/picard/file.py b/picard/file.py index 715545c0a..aee06cea6 100644 --- a/picard/file.py +++ b/picard/file.py @@ -251,6 +251,7 @@ class File(QtCore.QObject, Item): if tag in preserved_tags or tag in PRESERVED_TAGS: saved_metadata[tag] = values deleted_tags = self.metadata.deleted_tags + images_changed = self.metadata.images != metadata.images self.metadata.copy(metadata) for info in FILE_INFO_TAGS: metadata[info] = self.orig_metadata[info] @@ -262,12 +263,14 @@ class File(QtCore.QObject, Item): if acoustid and "acoustid_id" not in metadata.deleted_tags: self.metadata["acoustid_id"] = acoustid - self.metadata_images_changed.emit() + if images_changed: + self.metadata_images_changed.emit() def keep_original_images(self): - self.metadata.images = self.orig_metadata.images.copy() - self.update(signal=False) - self.metadata_images_changed.emit() + if self.metadata.images != self.orig_metadata.images: + self.metadata.images = self.orig_metadata.images.copy() + self.update(signal=False) + self.metadata_images_changed.emit() def has_error(self): return self.state == File.ERROR @@ -365,6 +368,7 @@ 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 # Data is copied from New to Original because New may be # a subclass to handle id3v23 if config.setting["clear_existing_tags"]: @@ -381,7 +385,8 @@ class File(QtCore.QObject, Item): self.clear_errors() self.clear_pending() self._add_path_to_metadata(self.orig_metadata) - self.metadata_images_changed.emit() + if images_changed: + self.metadata_images_changed.emit() # run post save hook run_file_post_save_processors(self) diff --git a/picard/track.py b/picard/track.py index db0d62cdd..f7c894d9f 100644 --- a/picard/track.py +++ b/picard/track.py @@ -196,8 +196,8 @@ class Track(DataObject, FileList): return self.files.remove(file) self.num_linked_files -= 1 - file.copy_metadata(file.orig_metadata, preserve_deleted=False) file.metadata_images_changed.disconnect(self.update_metadata_images) + file.copy_metadata(file.orig_metadata, preserve_deleted=False) self.album._remove_file(self, file) remove_metadata_images(self, [file]) run_file_post_removal_from_track_processors(self, file) diff --git a/picard/util/imagelist.py b/picard/util/imagelist.py index a485690e3..c503136bf 100644 --- a/picard/util/imagelist.py +++ b/picard/util/imagelist.py @@ -53,6 +53,8 @@ class ImageList(MutableSequence): return sorted(self, key=lambda image: image.normalized_types()) def __eq__(self, other): + if len(self) != len(other): + return False return self._sorted() == other._sorted() def copy(self): @@ -124,17 +126,24 @@ def _process_images(state, src_obj): def _update_state(obj, state): + changed = False for src_obj in state.sources: _process_images(state, src_obj) if state.update_new_metadata: - obj.metadata.images = ImageList(state.new_images) + updated_images = ImageList(state.new_images) + changed |= updated_images != obj.metadata.images + obj.metadata.images = updated_images obj.metadata.has_common_images = state.has_common_new_images if state.update_orig_metadata: - obj.orig_metadata.images = ImageList(state.orig_images) + updated_images = ImageList(state.orig_images) + changed |= updated_images != obj.orig_metadata.images + obj.orig_metadata.images = updated_images obj.orig_metadata.has_common_images = state.has_common_orig_images + return changed + # TODO: use functools.singledispatch when py3 is supported def _get_state(obj): @@ -188,8 +197,10 @@ def update_metadata_images(obj): Args: obj: A `Cluster`, `Album` or `Track` object with `metadata` property + Returns: + bool: True, if images where changed, False otherwise """ - _update_state(obj, _get_state(obj)) + return _update_state(obj, _get_state(obj)) def _add_images(metadata, added_images): diff --git a/test/test_imagelist.py b/test/test_imagelist.py index 4b0230184..cd375519e 100644 --- a/test/test_imagelist.py +++ b/test/test_imagelist.py @@ -67,44 +67,44 @@ class UpdateMetadataImagesTest(PicardTestCase): def test_update_cluster_images(self): cluster = Cluster('Test') cluster.files = list(self.test_files) - update_metadata_images(cluster) + self.assertTrue(update_metadata_images(cluster)) self.assertEqual(set(self.test_images), set(cluster.metadata.images)) self.assertFalse(cluster.metadata.has_common_images) cluster.files.remove(self.test_files[2]) - update_metadata_images(cluster) + self.assertFalse(update_metadata_images(cluster)) self.assertEqual(set(self.test_images), set(cluster.metadata.images)) self.assertFalse(cluster.metadata.has_common_images) cluster.files.remove(self.test_files[0]) - update_metadata_images(cluster) + self.assertTrue(update_metadata_images(cluster)) self.assertEqual(set(self.test_images[1:]), set(cluster.metadata.images)) self.assertTrue(cluster.metadata.has_common_images) cluster.files.append(self.test_files[2]) - update_metadata_images(cluster) + self.assertFalse(update_metadata_images(cluster)) self.assertEqual(set(self.test_images[1:]), set(cluster.metadata.images)) self.assertTrue(cluster.metadata.has_common_images) def test_update_track_images(self): track = Track('00000000-0000-0000-0000-000000000000') track.files = list(self.test_files) - update_metadata_images(track) + self.assertTrue(update_metadata_images(track)) self.assertEqual(set(self.test_images), set(track.orig_metadata.images)) self.assertFalse(track.orig_metadata.has_common_images) track.files.remove(self.test_files[2]) - update_metadata_images(track) + self.assertFalse(update_metadata_images(track)) self.assertEqual(set(self.test_images), set(track.orig_metadata.images)) self.assertFalse(track.orig_metadata.has_common_images) track.files.remove(self.test_files[0]) - update_metadata_images(track) + self.assertTrue(update_metadata_images(track)) self.assertEqual(set(self.test_images[1:]), set(track.orig_metadata.images)) self.assertTrue(track.orig_metadata.has_common_images) track.files.append(self.test_files[2]) - update_metadata_images(track) + self.assertFalse(update_metadata_images(track)) self.assertEqual(set(self.test_images[1:]), set(track.orig_metadata.images)) self.assertTrue(track.orig_metadata.has_common_images) @@ -116,23 +116,23 @@ class UpdateMetadataImagesTest(PicardTestCase): track2.files.append(self.test_files[1]) album.tracks = [track1, track2] album.unmatched_files.files.append(self.test_files[2]) - update_metadata_images(album) + self.assertTrue(update_metadata_images(album)) self.assertEqual(set(self.test_images), set(album.orig_metadata.images)) self.assertFalse(album.orig_metadata.has_common_images) album.tracks.remove(track2) - update_metadata_images(album) + self.assertFalse(update_metadata_images(album)) self.assertEqual(set(self.test_images), set(album.orig_metadata.images)) self.assertFalse(album.orig_metadata.has_common_images) # album.unmatched_files.files.remove(self.test_files[2]) album.tracks.remove(track1) - update_metadata_images(album) + self.assertTrue(update_metadata_images(album)) self.assertEqual(set(self.test_images[1:]), set(album.orig_metadata.images)) self.assertTrue(album.orig_metadata.has_common_images) album.tracks.append(track2) - update_metadata_images(album) + self.assertFalse(update_metadata_images(album)) self.assertEqual(set(self.test_images[1:]), set(album.orig_metadata.images)) self.assertTrue(album.orig_metadata.has_common_images)