GNOME.org
 

[gnome-music/wip/mschraal/coverart: 1/4] albumartcache: cleanup of art lookup

commit fbd67732e6e476d117daa3ed62e1b570bb492cc8
Author: Marinus Schraal <mschraal src gnome org>
Date:   Fri Aug 26 16:41:50 2016 +0200

    albumartcache: cleanup of art lookup
    
    Reflow & cleanup the albumart retrieval, so it is not as arcane as it
    looked before.

 gnomemusic/albumartcache.py |  288 +++++++++++++++++++++++--------------------
 gnomemusic/grilo.py         |    6 +-
 gnomemusic/player.py        |    4 +-
 gnomemusic/view.py          |   19 ++--
 gnomemusic/widgets.py       |   11 +-
 5 files changed, 174 insertions(+), 154 deletions(-)
---
diff --git a/gnomemusic/albumartcache.py b/gnomemusic/albumartcache.py
index 89430df..1aba258 100644
--- a/gnomemusic/albumartcache.py
+++ b/gnomemusic/albumartcache.py
@@ -40,6 +40,7 @@ from gi.repository import Gdk, GdkPixbuf, Gio, GLib, GObject, Gtk, MediaArt
 
 from gnomemusic import log
 from gnomemusic.grilo import grilo
+import gnomemusic.utils as utils
 
 
 logger = logging.getLogger(__name__)
@@ -146,17 +147,16 @@ class DefaultIcon(GObject.GObject):
 
 
 class AlbumArtCache(GObject.GObject):
-    instance = None
+    """Album art retrieval class
+
+    On basis of a given media item looks up album art locally and if
+    not found remotely.
+    """
+    _instance = None
     blacklist = {}
 
     def __repr__(self):
-        return '<AlbumArt>'
-
-    @classmethod
-    def get_default(cls):
-        if not cls.instance:
-            cls.instance = AlbumArtCache()
-        return cls.instance
+        return '<AlbumArtCache>'
 
     @staticmethod
     def get_media_title(media, escaped=False):
@@ -186,157 +186,181 @@ class AlbumArtCache(GObject.GObject):
     @log
     def __init__(self):
         GObject.GObject.__init__(self)
-        try:
-            self.cacheDir = os.path.join(GLib.get_user_cache_dir(), 'media-art')
-            if not os.path.exists(self.cacheDir):
-                Gio.file_new_for_path(self.cacheDir).make_directory(None)
-        except Exception as e:
-            logger.warn("Error: %s", e)
 
-        self.default_icon = DefaultIcon()
+        self.cache_dir = os.path.join(GLib.get_user_cache_dir(), 'media-art')
+        if not os.path.exists(self.cache_dir):
+            try:
+                Gio.file_new_for_path(self.cache_dir).make_directory(None)
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)
+                return
 
     @log
-    def lookup(self, item, width, height, callback, itr, artist, album, first=True):
-        if artist in self.blacklist and album in self.blacklist[artist]:
-            self.finish(item, None, None, callback, itr, width, height)
-            return
-
-        try:
-            [success, thumb_file] = MediaArt.get_file(artist, album, "album")
+    def lookup(self, item, width, height, callback, itr):
+        """Find art for the given item
+
+        :param item: Grilo media item
+        :param int width: Width of the icon to return
+        :param int height: Height of the icon to return
+        :param callback: Callback function when retrieved
+        :param itr: Iter to return with callback
+        """
+        self._lookup_local(item, callback, itr, width, height)
 
-            if success == False:
-                self.finish(item, None, None, callback, itr, width, height)
+    @log
+    def _lookup_local(self, item, callback, itr, width, height):
+        """Checks if there is already a local art file, if not calls
+        the remote lookup function"""
+        album = self.get_media_title(item)
+        artist = utils.get_artist_name(item)
+
+        def stream_open(thumb_file, result, arguments):
+            try:
+                stream = thumb_file.read_finish(result)
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)
+                do_callback(None)
                 return
 
-            if not thumb_file.query_exists():
-                if first:
-                    self.cached_thumb_not_found(item, width, height, thumb_file.get_path(), callback, itr, 
artist, album)
-                else:
-                    self.finish(item, None, None, callback, itr, width, height)
-                return
+            GdkPixbuf.Pixbuf.new_from_stream_async(stream, None,
+                                                   pixbuf_loaded, None)
 
-            stream = thumb_file.read_async(GLib.PRIORITY_LOW, None, self.stream_open,
-                                           [item, width, height, thumb_file, callback, itr, artist, album])
-        except Exception as e:
-            logger.warn("Error: %s, %s", e.__class__, e)
+        def pixbuf_loaded(stream, result, data):
+            try:
+                pixbuf = GdkPixbuf.Pixbuf.new_from_stream_finish(result)
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)
+                do_callback(None)
+                return
 
-    @log
-    def stream_open(self, thumb_file, result, arguments):
-        (item, width, height, thumb_file, callback, itr, artist, album) = arguments
+            do_callback(pixbuf)
+            return
 
-        try:
-            width = width or -1
-            height = height or -1
-            stream = thumb_file.read_finish (result)
-            GdkPixbuf.Pixbuf.new_from_stream_at_scale_async(stream, width, height, True, None, 
self.pixbuf_loaded,
-                                                            [item, width, height, thumb_file, callback, itr, 
artist, album])
-        except Exception as e:
-            logger.warn("Error: %s, %s", e.__class__, e)
-            self.finish(item, None, None, callback, itr, width, height)
+        def do_callback(pixbuf):
+            if not pixbuf:
+                pixbuf = DefaultIcon().get(width, height,
+                                           DefaultIcon.Type.music)
+            else:
+                pixbuf = pixbuf.scale_simple(width, height,
+                                             GdkPixbuf.InterpType.HYPER)
+                pixbuf = _make_icon_frame(pixbuf)
 
-    @log
-    def pixbuf_loaded(self, stream, result, arguments):
-        (item, width, height, thumb_file, callback, itr, artist, album) = arguments
+            GLib.idle_add(callback, pixbuf, None, itr)
+            return
 
-        try:
-            pixbuf = GdkPixbuf.Pixbuf.new_from_stream_finish (result)
-            self.finish(item, _make_icon_frame(pixbuf), thumb_file.get_path(), callback, itr, width, height, 
artist, album)
-        except Exception as e:
-            logger.warn("Error: %s, %s", e.__class__, e)
-            self.finish(item, None, None, callback, itr, width, height)
+        [success, thumb_file] = MediaArt.get_file(artist, album, "album")
 
-    @log
-    def finish(self, item, pixbuf, path, callback, itr, width=-1, height=-1, artist=None, album=None):
-        if (pixbuf is None and artist is not None):
-            # Blacklist artist-album combination
-            if artist not in self.blacklist:
-                self.blacklist[artist] = []
-            self.blacklist[artist].append(album)
+        if (success
+                and thumb_file.query_exists()):
+            thumb_file.read_async(GLib.PRIORITY_LOW,
+                                  None,
+                                  stream_open,
+                                  None)
+            return
 
-        if pixbuf is None:
-            pixbuf = self.default_icon.get(width, height, DefaultIcon.Type.music)
+        stripped_album = MediaArt.strip_invalid_entities(album)
+        if (artist in self.blacklist
+                and stripped_album in self.blacklist[artist]):
+            do_callback(None)
+            return
 
-        try:
-            if path:
-                item.set_thumbnail(GLib.filename_to_uri(path, None))
-            GLib.idle_add(callback, pixbuf, path, itr)
-        except Exception as e:
-            logger.warn("Error: %s", e)
+        self._lookup_remote(item, callback, itr, width, height)
 
     @log
-    def cached_thumb_not_found(self, item, width, height, path, callback, itr, artist, album):
-        try:
-            uri = item.get_thumbnail()
-            if uri is None:
-                grilo.get_album_art_for_item(item, self.album_art_for_item_callback,
-                                             (item, width, height, path, callback, itr, artist, album))
-                return
-
-            self.download_thumb(item, width, height, path, callback, itr, artist, album, uri)
-        except Exception as e:
-            logger.warn("Error: %s", e)
-            self.finish(item, None, None, callback, itr, width, height, artist, album)
+    def _lookup_remote(self, item, callback, itr, width, height):
+        """Lookup remote art
 
-    @log
-    def album_art_for_item_callback(self, source, param, item, count, data, error):
-        old_item, width, height, path, callback, itr, artist, album = data
-        try:
-            if item is None:
+        Lookup remote art through Grilo and if found copy locally
+        """
+        album = self.get_media_title(item)
+        artist = utils.get_artist_name(item)
+
+        @log
+        def delete_cb(src, result, data):
+            try:
+                src.delete_finish(result)
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)
+
+        @log
+        def splice_cb(src, result, data):
+            tmp_file, iostream = data
+
+            try:
+                src.splice_finish(result)
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)
+                art_retrieved(False)
                 return
 
-            uri = item.get_thumbnail()
-            if uri is None:
-                logger.warn("can't find artwork for album '%s' by %s", album, artist)
-                self.finish(item, None, None, callback, itr, width, height, artist, album)
+            success, cache_path = MediaArt.get_path(artist, album, "album")
+            try:
+                # FIXME: I/O blocking
+                MediaArt.file_to_jpeg(tmp_file.get_path(), cache_path)
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)
+                art_retrieved(False)
                 return
-            self.download_thumb(item, width, height, path, callback, itr, artist, album, uri)
-        except Exception as e:
-            logger.warn("Error: %s", e)
-            self.finish(item, None, None, callback, itr, width, height, artist, album)
 
-    @log
-    def download_thumb(self, item, width, height, thumb_file, callback, itr, artist, album, uri):
-        src = Gio.File.new_for_uri(uri)
-        src.read_async(GLib.PRIORITY_LOW, None, self.open_remote_thumb,
-                       [item, width, height, thumb_file, callback, itr, artist, album])
+            art_retrieved(True)
 
-    @log
-    def open_remote_thumb(self, src, result, arguments):
-        (item, width, height, thumb_file, callback, itr, artist, album) = arguments
-        dest = Gio.File.new_for_path(thumb_file)
+            tmp_file.delete_async(GLib.PRIORITY_LOW,
+                                  None,
+                                  delete_cb,
+                                  None)
 
-        try:
-            istream = src.read_finish(result)
-            dest.replace_async(None, False, Gio.FileCreateFlags.REPLACE_DESTINATION,
-                               GLib.PRIORITY_LOW, None, self.open_local_thumb,
-                               [item, width, height, thumb_file, callback, itr, artist, album, istream])
-        except Exception as e:
-            logger.warn("Error: %s", e)
-            self.finish(item, None, None, callback, itr, width, height, artist, album)
+        @log
+        def async_read_cb(src, result, data):
+            try:
+                istream = src.read_finish(result)
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)
+                art_retrieved(False)
+                return
 
-    @log
-    def open_local_thumb(self, dest, result, arguments):
-        (item, width, height, thumb_file, callback, itr, artist, album, istream) = arguments
+            try:
+                [tmp_file, iostream] = Gio.File.new_tmp()
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)
+                art_retrieved(False)
+                return
 
-        try:
-            ostream = dest.replace_finish(result)
+            ostream = iostream.get_output_stream()
+            # FIXME: Passing the iostream here, otherwise it gets
+            # closed. PyGI specific issue?
             ostream.splice_async(istream,
                                  Gio.OutputStreamSpliceFlags.CLOSE_SOURCE |
                                  Gio.OutputStreamSpliceFlags.CLOSE_TARGET,
-                                 GLib.PRIORITY_LOW, None,
-                                 self.copy_finished,
-                                 [item, width, height, thumb_file, callback, itr, artist, album])
-        except Exception as e:
-            logger.warn("Error: %s", e)
-            self.finish(item, None, None, callback, itr, width, height, artist, album)
+                                 GLib.PRIORITY_LOW,
+                                 None,
+                                 splice_cb,
+                                 [tmp_file, iostream])
+
+        @log
+        def album_art_for_item_cb(source, param, item, count, error):
+            if error:
+                logger.warn("Grilo error %s", error)
+                art_retrieved(False)
+                return
 
-    @log
-    def copy_finished(self, ostream, result, arguments):
-        (item, width, height, thumb_file, callback, itr, artist, album) = arguments
+            thumb_uri = item.get_thumbnail()
 
-        try:
-            ostream.splice_finish(result)
-            self.lookup(item, width, height, callback, itr, artist, album, False)
-        except Exception as e:
-            logger.warn("Error: %s", e)
-            self.finish(item, None, None, callback, itr, width, height, artist, album)
+            if thumb_uri is None:
+                art_retrieved(False)
+                return
+
+            src = Gio.File.new_for_uri(thumb_uri)
+            src.read_async(GLib.PRIORITY_LOW, None, async_read_cb, None)
+
+        @log
+        def art_retrieved(result):
+            if not result:
+                if artist not in self.blacklist:
+                    self.blacklist[artist] = []
+
+                album_stripped = MediaArt.strip_invalid_entities(album)
+                self.blacklist[artist].append(album_stripped)
+
+            self.lookup(item, width, height, callback, itr)
+
+        grilo.get_album_art_for_item(item, album_art_for_item_cb)
diff --git a/gnomemusic/grilo.py b/gnomemusic/grilo.py
index 120466c..74cd4f7 100644
--- a/gnomemusic/grilo.py
+++ b/gnomemusic/grilo.py
@@ -305,10 +305,9 @@ class Grilo(GObject.GObject):
                                 _multiple_search_callback, data)
 
     @log
-    def get_album_art_for_item(self, item, callback, data=None):
+    def get_album_art_for_item(self, item, callback):
         item_id = item.get_id()
 
-        query = None
         if item.is_audio():
             query = Query.get_album_for_song_id(item_id)
         else:
@@ -317,7 +316,8 @@ class Grilo(GObject.GObject):
         options = self.full_options.copy()
         options.set_count(1)
 
-        self.search_source.query(query, self.METADATA_THUMBNAIL_KEYS, options, callback, data)
+        self.search_source.query(query, self.METADATA_THUMBNAIL_KEYS, options,
+                                 callback)
 
     @log
     def get_playlist_with_id(self, playlist_id, callback):
diff --git a/gnomemusic/player.py b/gnomemusic/player.py
index cb30ea9..fff3787 100644
--- a/gnomemusic/player.py
+++ b/gnomemusic/player.py
@@ -110,7 +110,7 @@ class Player(GObject.GObject):
         self.currentTrack = None
         self.currentTrackUri = None
         self._lastState = Gst.State.PAUSED
-        self.cache = AlbumArtCache.get_default()
+        self.cache = AlbumArtCache()
         self._no_artwork_icon = DefaultIcon().get(ART_SIZE,
                                                   ART_SIZE,
                                                   DefaultIcon.Type.music)
@@ -610,7 +610,7 @@ class Player(GObject.GObject):
 
         self.coverImg.set_from_pixbuf(self._no_artwork_icon)
         self.cache.lookup(
-            media, ART_SIZE, ART_SIZE, self._on_cache_lookup, None, artist, album)
+            media, ART_SIZE, ART_SIZE, self._on_cache_lookup, None)
 
         self._currentTitle = AlbumArtCache.get_media_title(media)
         self.titleLabel.set_label(self._currentTitle)
diff --git a/gnomemusic/view.py b/gnomemusic/view.py
index d83aa41..e511d0c 100644
--- a/gnomemusic/view.py
+++ b/gnomemusic/view.py
@@ -129,7 +129,7 @@ class ViewContainer(Gtk.Stack):
         self.show_all()
         self.view.hide()
         self._items = []
-        self.cache = AlbumArtCache.get_default()
+        self.cache = AlbumArtCache()
         self._loading_icon = DefaultIcon().get(self._iconWidth,
                                                self._iconHeight,
                                                DefaultIcon.Type.loading)
@@ -242,7 +242,6 @@ class ViewContainer(Gtk.Stack):
         self._offset += 1
         artist = utils.get_artist_name(item)
         title = AlbumArtCache.get_media_title(item)
-        # item.set_title(title)
 
         _iter = self.model.append(None)
         self.model.set(_iter,
@@ -250,8 +249,8 @@ class ViewContainer(Gtk.Stack):
                        [str(item.get_id()), '', title,
                         artist, self._loading_icon, item,
                         0, False])
-        self.cache.lookup(item, self._iconWidth, self._iconHeight, self._on_lookup_ready,
-                          _iter, artist, title)
+        self.cache.lookup(item, self._iconWidth, self._iconHeight,
+                          self._on_lookup_ready, _iter)
 
     @log
     def _on_lookup_ready(self, icon, path, _iter):
@@ -518,8 +517,8 @@ class Albums(ViewContainer):
         child.add(builder.get_object('main_box'))
         child.show()
 
-        self.cache.lookup(item, self._iconWidth, self._iconHeight, self._on_lookup_ready,
-                          child, artist, title)
+        self.cache.lookup(item, self._iconWidth, self._iconHeight,
+                          self._on_lookup_ready, child)
 
         return child
 
@@ -1703,8 +1702,8 @@ class Search(ViewContainer):
                 [0, 2, 3, 4, 5, 9, 11],
                 [str(item.get_id()), title, artist,
                  self._loading_icon, item, 2, category])
-            self.cache.lookup(item, self._iconWidth, self._iconHeight, self._on_lookup_ready,
-                              _iter, artist, title)
+            self.cache.lookup(item, self._iconWidth, self._iconHeight,
+                              self._on_lookup_ready, _iter)
         elif category == 'song':
             _iter = self.model.insert_with_values(
                 self.head_iters[group], -1,
@@ -1718,8 +1717,8 @@ class Search(ViewContainer):
                     [0, 2, 4, 5, 9, 11],
                     [str(item.get_id()), artist,
                      self._loading_icon, item, 2, category])
-                self.cache.lookup(item, self._iconWidth, self._iconHeight, self._on_lookup_ready,
-                                  _iter, artist, title)
+                self.cache.lookup(item, self._iconWidth, self._iconHeight,
+                                  self._on_lookup_ready, _iter)
                 self._artists[artist.casefold()] = {'iter': _iter, 'albums': []}
 
             self._artists[artist.casefold()]['albums'].append(item)
diff --git a/gnomemusic/widgets.py b/gnomemusic/widgets.py
index e4edf4b..5700892 100644
--- a/gnomemusic/widgets.py
+++ b/gnomemusic/widgets.py
@@ -45,7 +45,6 @@ import gnomemusic.utils as utils
 
 logger = logging.getLogger(__name__)
 
-ALBUM_ART_CACHE = AlbumArtCache.get_default()
 NOW_PLAYING_ICON_NAME = 'media-playback-start-symbolic'
 ERROR_ICON_NAME = 'dialog-error-symbolic'
 
@@ -138,6 +137,7 @@ class AlbumWidget(Gtk.EventBox):
         :param parent_view: The view this widget is part of
         """
         Gtk.EventBox.__init__(self)
+        self._cache = AlbumArtCache()
         self._player = player
         self._iter_to_clean = None
 
@@ -290,10 +290,8 @@ class AlbumWidget(Gtk.EventBox):
         self.selection_toolbar = selection_toolbar
         self._header_bar = header_bar
         self._album = album
-        real_artist = utils.get_artist_name(item)
         self._ui.get_object('cover').set_from_pixbuf(self._loading_icon)
-        ALBUM_ART_CACHE.lookup(item, 256, 256, self._on_look_up, None,
-                               real_artist, album)
+        self._cache.lookup(item, 256, 256, self._on_look_up, None)
         self._duration = 0
         self._create_model()
         GLib.idle_add(grilo.populate_album_songs, item, self.add_item)
@@ -635,6 +633,7 @@ class ArtistAlbumWidget(Gtk.Box):
     @log
     def __init__(self, artist, album, player, model, header_bar, selectionModeAllowed):
         Gtk.Box.__init__(self, orientation=Gtk.Orientation.HORIZONTAL)
+        self._cache = AlbumArtCache()
         self.player = player
         self.album = album
         self.artist = artist
@@ -714,9 +713,7 @@ class ArtistAlbumWidget(Gtk.Box):
 
     @log
     def _update_album_art(self):
-        artist = utils.get_artist_name(self.album)
-        ALBUM_ART_CACHE.lookup(self.album, 128, 128, self._get_album_cover,
-                               None, artist, self.album.get_title())
+        self._cache.lookup(self.album, 128, 128, self._get_album_cover, None)
 
     @log
     def _get_album_cover(self, pixbuf, path, data=None):
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]