Image list cleanup

picard/album.py

@@ -63,6 +63,7 @@
     N_,
     gettext as _,
 )
+from picard.item import MetadataItem
 from picard.mbjson import (
     medium_to_metadata,
     release_group_to_metadata,
@@ -89,15 +90,8 @@
     format_time,
     mbid_validate,
 )
-from picard.util.imagelist import (
-    add_metadata_images,
-    remove_metadata_images,
-    update_metadata_images,
-)
 from picard.util.textencoding import asciipunct
 
-from picard.ui.item import Item
-
 
 RECORDING_QUERY_LIMIT = 100
 
@@ -131,15 +125,11 @@ class ParseResult(IntEnum):
     MISSING_TRACK_RELS = 2
 
 
-class Album(DataObject, Item):
-
-    metadata_images_changed = QtCore.pyqtSignal()
+class Album(DataObject, MetadataItem):
 
     def __init__(self, album_id, discid=None):
         DataObject.__init__(self, album_id)
         self.tagger = QtCore.QCoreApplication.instance()
-        self.metadata = Metadata()
-        self.orig_metadata = Metadata()
         self.tracks = []
         self.loaded = False
         self.load_task = None
@@ -156,7 +146,7 @@ def __init__(self, album_id, discid=None):
         self.unmatched_files.metadata_images_changed.connect(self.update_metadata_images)
         self.status = AlbumStatus.NONE
         self._album_artists = []
-        self.update_metadata_images_enabled = True
+        self.update_children_metadata_attrs = {'metadata', 'orig_metadata'}
 
     def __repr__(self):
         return '<Album %s %r>' % (self.id, self.metadata['album'])
@@ -170,9 +160,6 @@ def iterfiles(self, save=False):
     def iter_correctly_matched_tracks(self):
         yield from (track for track in self.tracks if track.num_linked_files == 1)
 
-    def enable_update_metadata_images(self, enabled):
-        self.update_metadata_images_enabled = enabled
-
     def append_album_artist(self, album_artist_id):
         """Append artist id to the list of album artists
         and return an AlbumArtist instance"""
@@ -656,13 +643,13 @@ def add_file(self, track, file, new_album=True):
         self._files_count += 1
         if new_album:
             self.update(update_tracks=False)
-            add_metadata_images(self, [file])
+            self.add_metadata_images_from_children([file])
 
     def remove_file(self, track, file, new_album=True):
         self._files_count -= 1
         if new_album:
             self.update(update_tracks=False)
-            remove_metadata_images(self, [file])
+            self.remove_metadata_images_from_children([file])
 
     @staticmethod
     def _match_files(files, tracks, unmatched_files, threshold=0):
@@ -859,8 +846,8 @@ def update_metadata_images(self):
         if not self.update_metadata_images_enabled:
             return
 
-        if update_metadata_images(self):
-            self.update(False)
+        if self.update_metadata_images_from_children():
+            self.update(update_tracks=False)
             self.metadata_images_changed.emit()
 
     def keep_original_images(self):
@@ -872,6 +859,12 @@ def keep_original_images(self):
         self.enable_update_metadata_images(True)
         self.update_metadata_images()
 
+    def children_metadata_items(self):
+        for track in self.tracks:
+            yield track
+            yield from track.files
+        yield from self.unmatched_files.files
+
 
 class NatAlbum(Album):
 
@@ -891,7 +884,7 @@ def update(self, update_tracks=True, update_selection=True):
             for file in track.files:
                 track.update_file_metadata(file)
         self.enable_update_metadata_images(True)
-        super().update(update_tracks, update_selection)
+        super().update(update_tracks=update_tracks, update_selection=update_selection)
 
     def _finalize_loading(self, error):
         self.update()

picard/cluster.py

@@ -50,27 +50,19 @@
     N_,
     gettext as _,
 )
-from picard.metadata import (
-    Metadata,
-    SimMatchRelease,
+from picard.item import (
+    FileListItem,
+    Item,
 )
+from picard.metadata import SimMatchRelease
 from picard.track import Track
 from picard.util import (
     album_artist_from_path,
     find_best_match,
     format_time,
 )
-from picard.util.imagelist import (
-    add_metadata_images,
-    remove_metadata_images,
-    update_metadata_images,
-)
 
 from picard.ui.enums import MainAction
-from picard.ui.item import (
-    FileListItem,
-    Item,
-)
 
 
 # Weights for different elements when comparing a cluster to a release
@@ -87,17 +79,13 @@
 
 class FileList(QtCore.QObject, FileListItem):
 
-    metadata_images_changed = QtCore.pyqtSignal()
-
     def __init__(self, files=None):
         QtCore.QObject.__init__(self)
         FileListItem.__init__(self, files)
-        self.metadata = Metadata()
-        self.orig_metadata = Metadata()
         if self.files and self.can_show_coverart:
             for file in self.files:
                 file.metadata_images_changed.connect(self.update_metadata_images)
-            update_metadata_images(self)
+            self.update_metadata_images_from_children()
 
     def iterfiles(self, save=False):
         yield from self.files
@@ -142,9 +130,9 @@ def album(self):
     def _update_related_album(self, added_files=None, removed_files=None):
         if self.related_album:
             if added_files:
-                add_metadata_images(self.related_album, added_files)
+                self.related_album.add_metadata_images_from_children(added_files)
             if removed_files:
-                remove_metadata_images(self.related_album, removed_files)
+                self.related_album.remove_metadata_images_from_children(removed_files)
             self.related_album.update()
 
     def add_files(self, files, new_album=True):
@@ -161,7 +149,7 @@ def add_files(self, files, new_album=True):
         self.files.extend(added_files)
         self.update(signal=False)
         if self.can_show_coverart:
-            add_metadata_images(self, added_files)
+            self.add_metadata_images_from_children(added_files)
         self.item.add_files(added_files)
         if new_album:
             self._update_related_album(added_files=added_files)
@@ -177,7 +165,7 @@ def remove_file(self, file, new_album=True):
         self.item.remove_file(file)
         if self.can_show_coverart:
             file.metadata_images_changed.disconnect(self.update_metadata_images)
-            remove_metadata_images(self, [file])
+            self.remove_metadata_images_from_children([file])
         if new_album:
             self._update_related_album(removed_files=[file])
         self.tagger.window.set_processing(False)

picard/file.py

@@ -75,6 +75,7 @@
     N_,
     gettext as _,
 )
+from picard.item import MetadataItem
 from picard.metadata import (
     Metadata,
     SimMatchTrack,
@@ -111,8 +112,6 @@
     PRESERVED_TAGS,
 )
 
-from picard.ui.item import Item
-
 
 FILE_COMPARISON_WEIGHTS = {
     'album': 5,
@@ -136,9 +135,7 @@ class FileErrorType(Enum):
     PARSER = auto()
 
 
-class File(QtCore.QObject, Item):
-
-    metadata_images_changed = QtCore.pyqtSignal()
+class File(QtCore.QObject, MetadataItem):
 
     NAME = None
 
@@ -173,9 +170,6 @@ def __init__(self, filename):
         self.state = File.PENDING
         self.error_type = FileErrorType.UNKNOWN
 
-        self.orig_metadata = Metadata()
-        self.metadata = Metadata()
-
         self.similarity = 1.0
         self.parent = None
 

picard/ui/item.py → picard/item.py

@@ -25,9 +25,11 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
+from PyQt6 import QtCore
+
 from picard import log
 from picard.i18n import ngettext
-from picard.util.imagelist import update_metadata_images
+from picard.metadata import Metadata
 
 
 class Item(object):
@@ -155,22 +157,23 @@ def cover_art_description_detailed(self):
                             number_of_images) % number_of_images
 
 
-class FileListItem(Item):
+class MetadataItem(Item):
+    metadata_images_changed = QtCore.pyqtSignal()
 
-    def __init__(self, files=None):
+    def __init__(self):
         super().__init__()
-        self.files = files or []
+        self.metadata = Metadata()
+        self.orig_metadata = Metadata()
         self.update_metadata_images_enabled = True
-
-    def iterfiles(self, save=False):
-        yield from self.files
+        self.update_children_metadata_attrs = {}
+        self.iter_children_items_metadata_ignore_attrs = {}
 
     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:
-            if update_metadata_images(self):
+            if self.update_metadata_images_from_children():
                 self.metadata_images_changed.emit()
 
     def keep_original_images(self):
@@ -180,3 +183,110 @@ def keep_original_images(self):
                 file.keep_original_images()
         self.enable_update_metadata_images(True)
         self.update_metadata_images()
+
+    def children_metadata_items(self):
+        """Yield MetadataItems that are children of the current object"""
+
+    def iter_children_items_metadata(self, metadata_attr):
+        for s in self.children_metadata_items():
+            if metadata_attr in s.iter_children_items_metadata_ignore_attrs:
+                continue
+            yield getattr(s, metadata_attr)
+
+    @staticmethod
+    def get_sources_metadata_images(sources_metadata):
+        images = set()
+        for s in sources_metadata:
+            images = images.union(s.images)
+        return images
+
+    def remove_metadata_images_from_children(self, removed_sources):
+        """Remove the images in the metadata of `removed_sources` from the metadata.
+
+        Args:
+            removed_sources: List of child objects (`Track` or `File`) which's metadata images should be removed from
+        """
+        changed = False
+
+        for metadata_attr in self.update_children_metadata_attrs:
+            removed_images = self.get_sources_metadata_images(getattr(s, metadata_attr) for s in removed_sources)
+            sources_metadata = list(self.iter_children_items_metadata(metadata_attr))
+            metadata = getattr(self, metadata_attr)
+            changed |= metadata.remove_images(sources_metadata, removed_images)
+
+        return changed
+
+    def add_metadata_images_from_children(self, added_sources):
+        """Add the images in the metadata of `added_sources` to the metadata.
+
+        Args:
+            added_sources: List of child objects (`Track` or `File`) which's metadata images should be added to current object
+        """
+        changed = False
+
+        for metadata_attr in self.update_children_metadata_attrs:
+            added_images = self.get_sources_metadata_images(getattr(s, metadata_attr) for s in added_sources)
+            metadata = getattr(self, metadata_attr)
+            changed |= metadata.add_images(added_images)
+
+        return changed
+
+    def update_metadata_images_from_children(self):
+        """Update the metadata images of the current object based on its children.
+
+        Based on the type of the current object, this will update `self.metadata.images` to
+        represent the metadata images of all children (`Track` or `File` objects).
+
+        This method will iterate over all children and completely rebuild
+        `self.metadata.images`. Whenever possible the more specific functions
+        `add_metadata_images_from_children` or `remove_metadata_images_from_children` should be used.
+
+        Returns:
+            bool: True, if images where changed, False otherwise
+        """
+        from picard.util.imagelist import ImageList
+
+        class ImageListState:
+            def __init__(self):
+                self.images = {}
+                self.has_common_images = True
+                self.first_obj = True
+
+            def process_images(self, src_obj_metadata):
+                src_dict = src_obj_metadata.images.hash_dict()
+                prev_len = len(self.images)
+                self.images.update(src_dict)
+                if len(self.images) != prev_len:
+                    if not self.first_obj:
+                        self.has_common_images = False
+                if self.first_obj:
+                    self.first_obj = False
+
+        changed = False
+
+        for metadata_attr in self.update_children_metadata_attrs:
+            state = ImageListState()
+            for src_obj_metadata in self.iter_children_items_metadata(metadata_attr):
+                state.process_images(src_obj_metadata)
+
+            updated_images = ImageList(state.images.values())
+            metadata = getattr(self, metadata_attr)
+            changed |= set(updated_images.hash_dict()) != set(metadata.images.hash_dict())
+            metadata.images = updated_images
+            metadata.has_common_images = state.has_common_images
+
+        return changed
+
+
+class FileListItem(MetadataItem):
+
+    def __init__(self, files=None):
+        super().__init__()
+        self.files = files or []
+        self.update_children_metadata_attrs = {'metadata', 'orig_metadata'}
+
+    def iterfiles(self, save=False):
+        yield from self.files
+
+    def children_metadata_items(self):
+        yield from self.files