[gnome-music/wip/mschraal/artrework: 2/3] albumartcache: Full rewrite



commit 86db3c2f150f13dd2222ecd8e8d3da23e9bf5468
Author: Marinus Schraal <mschraal gnome org>
Date:   Fri Jan 5 13:54:49 2018 +0100

    albumartcache: Full rewrite
    
    The former artworkcache was a monolithic method filled with callbacks,
    hard to debug and comprehend. It also left quite a bit for the caller to
    take care of.
    
    The current design is such that Music has an Art object that is specific
    to one cairo.Surface or Gtk.Image as requested. The Art object takes
    care of retrieving the correct image and emits a signal (cairo.Surface)
    or updates the Gtk.Image when done.
    
    The lookup process itself is now clearly divided into several steps:
    1. libmediaart cache lookup
    2. local lookup
        1. tags (gstreamer)
        2. coverart in the directory (libmediaart)
    3. remote lookup through Grilo coverart providers
    
    The three main steps each have their own class and lookup queue, which
    limits the amount of concurrent lookups. That should limit the stress on
    the system while starting the application.
    
    Using cairo.Surface for Gtk.TreeView also allows for HiDPI art in
    SearchView.
    
    All of this combined leads to less art related code in the views.
    
    For simplicity and cleanliness, all art related calls have been removed
    from BaseView as they were unused and there is no plan to bring it back
    to BaseView.

 gnomemusic/albumartcache.py             | 668 +++++++++++++++++++-------------
 gnomemusic/player.py                    |  17 +-
 gnomemusic/views/albumsview.py          |  15 +-
 gnomemusic/views/baseview.py            |  25 +-
 gnomemusic/views/initialstateview.py    |   4 +-
 gnomemusic/views/searchview.py          |  85 ++--
 gnomemusic/widgets/albumwidget.py       |  23 +-
 gnomemusic/widgets/artistalbumwidget.py |  22 +-
 8 files changed, 477 insertions(+), 382 deletions(-)
---
diff --git a/gnomemusic/albumartcache.py b/gnomemusic/albumartcache.py
index f9954ca..2a1bca0 100644
--- a/gnomemusic/albumartcache.py
+++ b/gnomemusic/albumartcache.py
@@ -46,39 +46,40 @@ import gnomemusic.utils as utils
 logger = logging.getLogger(__name__)
 
 
-class LookupQueue(object):
-    """A queue for IO operations"""
+class Queue(GObject.GObject):
+    """An operations queue"""
 
-    _max_simultaneous_lookups = 12
-    _lookup_queue = []
-    _n_lookups = 0
+    def __init__(self):
+        super().__init__()
+
+        self._max_simultaneous_lookups = 10
+        self._lookup_queue = []
+        self._n_lookups = 0
 
-    @classmethod
     @log
-    def push(cls, cache, item, art_size, callback, itr):
+    def push(self, func, argument):
         """Push a lookup counter or queue the lookup if needed"""
 
         # If reached the limit, queue the operation.
-        if cls._n_lookups >= cls._max_simultaneous_lookups:
-            cls._lookup_queue.append((cache, item, art_size, callback, itr))
+        if self._n_lookups >= self._max_simultaneous_lookups:
+            self._lookup_queue.append((func, argument))
             return False
         else:
-            cls._n_lookups += 1
+            func(argument)
+            self._n_lookups += 1
             return True
 
-    @classmethod
     @log
-    def pop(cls):
+    def pop(self):
         """Pops a lookup counter and consume the lookup queue if needed"""
-
-        cls._n_lookups -= 1
+        self._n_lookups -= 1
 
         # An available lookup slot appeared! Let's continue looking up
         # artwork then.
-        if (cls._n_lookups < cls._max_simultaneous_lookups
-                and cls._lookup_queue):
-            (cache, item, art_size, callback, itr) = cls._lookup_queue.pop(0)
-            cache.lookup(item, art_size, callback, itr)
+        if (self._n_lookups < self._max_simultaneous_lookups
+                and self._lookup_queue):
+            (func, argument) = self._lookup_queue.pop(0)
+            func(argument)
 
 
 @log
@@ -136,20 +137,6 @@ def _make_icon_frame(pixbuf, art_size=None, scale=1):
     return surface
 
 
-class ArtSize(Enum):
-    """Enum for icon sizes"""
-    XSMALL = (34, 34)
-    SMALL = (48, 48)
-    MEDIUM = (128, 128)
-    LARGE = (256, 256)
-    XLARGE = (512, 512)
-
-    def __init__(self, width, height):
-        """Intialize width and height"""
-        self.width = width
-        self.height = height
-
-
 class DefaultIcon(GObject.GObject):
     """Provides the symbolic fallback and loading icons."""
 
@@ -164,13 +151,11 @@ class DefaultIcon(GObject.GObject):
         return '<DefaultIcon>'
 
     @log
-    def __init__(self, scale=1):
+    def __init__(self):
         super().__init__()
 
-        self._scale = scale
-
     @log
-    def _make_default_icon(self, icon_type, art_size=None):
+    def _make_default_icon(self, icon_type, art_size, scale):
         width = art_size.width * self._scale
         height = art_size.height * self._scale
 
@@ -200,181 +185,335 @@ class DefaultIcon(GObject.GObject):
         return icon_surface
 
     @log
-    def get(self, icon_type, art_size):
+    def get(self, icon_type, art_size, scale=1):
         """Returns the requested symbolic icon
 
-        Returns a GdkPixbuf of the requested symbolic icon
-        in the given size.
+        Returns a cairo surface of the requested symbolic icon in the
+        given size.
 
         :param enum icon_type: The DefaultIcon.Type of the icon
-        :param enum art_size: The ArtSize requested
+        :param enum art_size: The Art.Size requested
 
         :return: The symbolic icon
-        :rtype: GdkPixbuf
+        :rtype: cairo.Surface
         """
-        if (icon_type, art_size) not in self._cache.keys():
-            new_icon = self._make_default_icon(icon_type, art_size)
-            self._cache[(icon_type, art_size)] = new_icon
+        if (icon_type, art_size, scale) not in self._cache.keys():
+            new_icon = self._make_default_icon(icon_type, art_size, scale)
+            self._cache[(icon_type, art_size, scale)] = new_icon
 
-        return self._cache[(icon_type, art_size)]
+        return self._cache[(icon_type, art_size, scale)]
 
 
-class AlbumArtCache(GObject.GObject):
-    """Album art retrieval class
+class Art(GObject.GObject):
 
-    On basis of a given media item looks up album art in the following order:
-    1) already existing in cache
-    2) from embedded images
-    3) from local images
-    3) remotely
-    """
-    _instance = None
-    blacklist = {}
-    _scale = 1
+    __gsignals__ = {
+        'finished': (GObject.SignalFlags.RUN_FIRST, None, ())
+    }
 
-    def __repr__(self):
-        return '<AlbumArtCache>'
+    _blacklist = {}
+    _cache_queue = Queue()
+    _embedded_queue = Queue()
+    _remote_queue = Queue()
+
+    class Size(Enum):
+        """Enum for icon sizes"""
+        XSMALL = (34, 34)
+        SMALL = (48, 48)
+        MEDIUM = (128, 128)
+        LARGE = (256, 256)
+        XLARGE = (512, 512)
+
+        def __init__(self, width, height):
+            """Intialize width and height"""
+            self.width = width
+            self.height = height
 
     @log
-    def __init__(self, scale=1):
+    def __init__(self, size, media, scale=1):
         super().__init__()
 
+        self._size = size
+        self._media = media
+        self._media_url = self._media.get_url()
+        self._surface = None
         self._scale = scale
 
-        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):
+        if self._in_blacklist():
+            self._no_art_available()
+            return
 
-        Gst.init(None)
-        self._discoverer = GstPbutils.Discoverer.new(Gst.SECOND)
-        self._discoverer.connect('discovered', self._discovered_cb)
-        self._discoverer.start()
+        cache = Cache()
+        cache.connect('miss', self._cache_miss)
+        cache.connect('hit', self._cache_hit)
+        self._cache_queue.push(cache.query, self._media)
 
-        self._discoverer_items = {}
+    def _cache_miss(self, klass):
+        self._cache_queue.pop()
 
-        self._media_art = None
-        try:
-            self._media_art = MediaArt.Process.new()
-        except Exception as err:
-            logger.warn("Error: %s, %s", err.__class__, err)
+        embedded_art = EmbeddedArt()
+        embedded_art.connect('found', self._embedded_art_found)
+        embedded_art.connect('unavailable', self._embedded_art_unavailable)
+
+        self._embedded_queue.push(embedded_art.query, self._media)
+
+    def _cache_hit(self, klass, pixbuf):
+        self._cache_queue.pop()
+
+        surface = _make_icon_frame(pixbuf, self._size, self._scale)
+        self._surface = surface
+
+        self.emit('finished')
+
+    def _embedded_art_found(self, klass):
+        self._embedded_queue.pop()
+
+        cache = Cache()
+        cache.connect('miss', self._cache_miss)
+        cache.connect('hit', self._cache_hit)
+
+        self._cache_queue.push(cache.query, self._media)
+
+    def _embedded_art_unavailable(self, klass):
+        self._embedded_queue.pop()
+
+        remote_art = RemoteArt()
+        remote_art.connect('retrieved', self._remote_art_retrieved)
+        remote_art.connect('unavailable', self._remote_art_unavailable)
+
+        self._remote_queue.push(remote_art.query, self._media)
+
+    def _remote_art_retrieved(self, klass):
+        self._remote_queue.pop()
+
+        cache = Cache()
+        cache.connect('miss', self._cache_miss)
+        cache.connect('hit', self._cache_hit)
+
+        self._cache_queue.push(cache.query, self._media)
+
+    def _remote_art_unavailable(self, klass):
+        self._remote_queue.pop()
 
+        self._add_to_blacklist()
+        self._no_art_available()
+
+    def _no_art_available(self):
+        self._surface = DefaultIcon().get(
+            DefaultIcon.Type.music, self._size, self._scale)
+
+        self.emit('finished')
+
+    def _add_to_blacklist(self):
+        album = utils.get_album_title(self._media)
+        artist = utils.get_artist_name(self._media)
+
+        if artist not in self._blacklist:
+            self._blacklist[artist] = []
+
+        album_stripped = MediaArt.strip_invalid_entities(album)
+        self._blacklist[artist].append(album_stripped)
+
+    def _in_blacklist(self):
+        album = utils.get_album_title(self._media)
+        artist = utils.get_artist_name(self._media)
+        album_stripped = MediaArt.strip_invalid_entities(album)
+
+        if artist in self._blacklist:
+            if album_stripped in self._blacklist[artist]:
+                return True
+
+        return False
+
+    @GObject.Property
     @log
-    def lookup(self, item, art_size, callback, itr):
-        """Find art for the given item
+    def surface(self):
+        if self._surface is None:
+            self._surface = DefaultIcon().get(
+                DefaultIcon.Type.loading, self._size, self._scale)
 
-        :param item: Grilo media item
-        :param ArtSize art_size: Size of the icon
-        :param callback: Callback function when retrieved
-        :param itr: Iter to return with callback
-        """
-        if LookupQueue.push(self, item, art_size, callback, itr):
-            self._lookup_local(item, art_size, callback, itr)
+        return self._surface
+
+
+class ArtImage(Art):
+
+    def __init__(self, size, media):
+        super().__init__(size, media)
+
+        self._image = None
+
+    def _cache_hit(self, klass, pixbuf):
+        super()._cache_hit(klass, pixbuf)
+
+        self._image.set_from_surface(self._surface)
+
+    def _no_art_available(self):
+        super()._no_art_available()
 
+        self._image.set_from_surface(self._surface)
+
+    @GObject.Property
     @log
-    def _lookup_local(self, item, art_size, callback, itr):
-        """Checks if there is already a local art file, if not calls
-        the embedded lookup function"""
-        album = utils.get_album_title(item)
-        artist = utils.get_artist_name(item)
+    def image(self):
+        return self._image.set_from_surface(self._surface)
 
-        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
+    @image.setter
+    @log
+    def image(self, image):
+        self._image = image
 
-            GdkPixbuf.Pixbuf.new_from_stream_async(stream,
-                                                   None,
-                                                   pixbuf_loaded,
-                                                   None)
+        self._image.set_property("width-request", self._size.width)
+        self._image.set_property("height-request", self._size.height)
 
-        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
+        self._scale = self._image.get_scale_factor()
 
-            do_callback(pixbuf)
-            return
+        self._surface = DefaultIcon().get(
+            DefaultIcon.Type.loading, self._size, self._scale)
 
-        def do_callback(pixbuf):
+        self._image.set_from_surface(self._surface)
 
-            # Lookup finished, decrease the counter
-            LookupQueue.pop()
+        self.lookup()
 
-            if not pixbuf:
-                surface = DefaultIcon(self._scale).get(DefaultIcon.Type.music,
-                                                       art_size)
-            else:
-                surface = _make_icon_frame(pixbuf, art_size, self._scale)
 
-                # Sets the thumbnail location for MPRIS to use.
-                item.set_thumbnail(GLib.filename_to_uri(thumb_file.get_path(),
-                                                        None))
+# 1. libmediaart
+# 2  embedded -> libmediaart
+# 3  remote -> libmediaart
 
-            GLib.idle_add(callback, surface, itr)
-            return
+
+class Cache(GObject.GObject):
+
+    __gsignals__ = {
+        'miss': (GObject.SignalFlags.RUN_FIRST, None, ()),
+        'hit': (GObject.SignalFlags.RUN_FIRST, None, (GObject.GObject, ))
+    }
+
+    def __init__(self):
+        super().__init__()
+
+        self._media_art = MediaArt.Process.new()
+
+        # FIXME: async
+        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 GLib.Error as error:
+                logger.warn(
+                    "Error: {}, {}".format(error.domain, error.message))
+                return
+
+    def query(self, media):
+        album = utils.get_album_title(media)
+        artist = utils.get_artist_name(media)
 
         success, thumb_file = MediaArt.get_file(artist, album, "album")
 
         if (success
                 and thumb_file.query_exists()):
-            thumb_file.read_async(GLib.PRIORITY_LOW,
-                                  None,
-                                  stream_open,
-                                  None)
+            thumb_file.read_async(
+                GLib.PRIORITY_LOW, None, self._open_stream, None)
             return
 
-        stripped_album = MediaArt.strip_invalid_entities(album)
-        if (artist in self.blacklist
-                and stripped_album in self.blacklist[artist]):
-            do_callback(None)
+        self.emit('miss')
+
+    def _open_stream(self, thumb_file, result, arguments):
+        try:
+            stream = thumb_file.read_finish(result)
+        except GLib.Error as error:
+            logger.warn(
+                "Error: {}, {}".format(error.domain, error.message))
+            stream.close_async(
+                GLib.PRIORITY_LOW, None, self._close_stream, None)
+            self.emit('miss')
             return
 
-        # When we reach here because it fails to retrieve the artwork,
-        # do a long round trip (either through _lookup_embedded or
-        # _lookup_remote) and call self.lookup() again. Thus, decrease
-        # global lookup counter.
-        LookupQueue.pop()
+        GdkPixbuf.Pixbuf.new_from_stream_async(
+            stream, None, self._pixbuf_loaded, None)
 
-        self._lookup_embedded(item, art_size, callback, itr)
+    def _pixbuf_loaded(self, stream, result, data):
+        try:
+            pixbuf = GdkPixbuf.Pixbuf.new_from_stream_finish(result)
+        except GLib.Error as error:
+            logger.warn("Error: {}, {}".format(error.domain, error.message))
+            stream.close_async(
+                GLib.PRIORITY_LOW, None, self._close_stream, None)
+            self.emit('miss')
+            return
 
-    @log
-    def _discovered_cb(self, discoverer, info, error):
-        item, art_size, callback, itr, cache_path = \
-            self._discoverer_items[info.get_uri()]
+        stream.close_async(GLib.PRIORITY_LOW, None, self._close_stream, None)
+        self.emit('hit', pixbuf)
 
-        album = utils.get_album_title(item)
-        artist = utils.get_artist_name(item)
-        tags = info.get_tags()
-        index = 0
+    def _close_stream(self, stream, result, data):
+        stream.close_finish(result)
+        # TODO: Try except
+
+
+class EmbeddedArt(GObject.GObject):
+
+    __gsignals__ = {
+        'found': (GObject.SignalFlags.RUN_FIRST, None, ()),
+        'unavailable': (GObject.SignalFlags.RUN_FIRST, None, ())
+    }
+
+    def __init__(self):
+        super().__init__()
 
-        def art_retrieved(result):
-            if not result:
-                if artist not in self.blacklist:
-                    self.blacklist[artist] = []
+        try:
+            Gst.init(None)
+            self._discoverer = GstPbutils.Discoverer.new(Gst.SECOND)
+            # self._discoverer.connect('discovered', self._discovered)
+            # self._discoverer.start()
+        except GLib.Error as error:
+            logger.warn("Error: {}, {}".format(error.domain, error.message))
+            return
+
+        self._media_art = MediaArt.Process.new()
 
-                album_stripped = MediaArt.strip_invalid_entities(album)
-                self.blacklist[artist].append(album_stripped)
+        self._album = None
+        self._artist = None
+        self._media = None
+        self._path = None
 
-            self.lookup(item, art_size, callback, itr)
+    def query(self, media):
+        self._album = utils.get_album_title(media)
+        self._artist = utils.get_artist_name(media)
+        self._media = media
+
+        success, path = MediaArt.get_path(
+            self._artist, self._album, "album")
+        if not success:
+            self.emit('unavailable')
+            # self._discoverer.stop()
+            return
+
+        self._path = path
+        try:
+            info_ = self._discoverer.discover_uri(self._media.get_url())
+        except GLib.Error as error:
+            print("HERE")
+            logger.warn("Error: {}, {}".format(error.domain, error.message))
+            self.emit('unavailable')
+            # self._discoverer.stop()
+            return
+
+        self._discovered(info_)
+
+    # FIXME: async is triggering a bug.
+    # def _discovered(self, discoverer, info, error):
+    def _discovered(self, info):
+        tags = info.get_tags()
+        index = 0
 
         # FIXME: tags should not return as None, but it sometimes is.
         # So as a workaround until we figure out what is wrong check
         # for it.
         # https://bugzilla.gnome.org/show_bug.cgi?id=780980
-        if (error is not None
-                or tags is None):
-            art_retrieved(False)
-            return
+        # if (error is not None
+        #        or tags is None):
+        #    self._discoverer.stop()
+        #    self.emit('unavailable')
+        #    return
 
         while True:
             success, sample = tags.get_sample_index(Gst.TAG_IMAGE, index)
@@ -382,8 +521,8 @@ class AlbumArtCache(GObject.GObject):
                 break
             index += 1
             struct = sample.get_info()
-            success, image_type = struct.get_enum('image-type',
-                                                  GstTag.TagImageType)
+            success, image_type = struct.get_enum(
+                'image-type', GstTag.TagImageType)
             if not success:
                 continue
             if image_type != GstTag.TagImageType.FRONT_COVER:
@@ -396,143 +535,140 @@ class AlbumArtCache(GObject.GObject):
 
             try:
                 mime = sample.get_caps().get_structure(0).get_name()
-                MediaArt.buffer_to_jpeg(map_info.data, mime, cache_path)
-                art_retrieved(True)
+                MediaArt.buffer_to_jpeg(map_info.data, mime, self._path)
+                self.emit('found')
+                # self._discoverer.stop()
                 return
-            except Exception as err:
-                logger.warn("Error: %s, %s", err.__class__, err)
+            except GLib.Error as error:
+                logger.warn("Error: {}, {}".format(
+                    MediaArt.Error(error.code), error.message))
+
+        # self._discoverer.stop()
 
+        # Find local art in cover.jpeg files.
+        self._media_art.uri_async(
+            MediaArt.Type.ALBUM, MediaArt.ProcessFlags.NONE,
+            self._media.get_url(), self._artist, self._album,
+            GLib.PRIORITY_LOW, None, self._uri_async_cb, None)
+
+    def _uri_async_cb(self, src, result, data):
         try:
-            self._media_art.uri(MediaArt.Type.ALBUM,
-                                MediaArt.ProcessFlags.NONE, item.get_url(),
-                                artist, album, None)
-            if os.path.exists(cache_path):
-                art_retrieved(True)
+            success = self._media_art.uri_finish(result)
+            if success:
+                self.emit('found')
                 return
-        except Exception as err:
-            logger.warn("Trying to process misc albumart: %s, %s",
-                        err.__class__, err)
+            self.emit('unavailable')
+        except GLib.Error as error:
+            if MediaArt.Error(error.code) == MediaArt.Error.SYMLINK_FAILED:
+                # This error indicates that the coverart has already
+                # been linked by another concurrent lookup.
+                self.emit('found')
+            else:
+                logger.warning("Error: {}, {}".format(
+                    MediaArt.Error(error.code), error.message))
 
-        self._lookup_remote(item, art_size, callback, itr)
 
-    @log
-    def _lookup_embedded(self, item, art_size, callback, itr):
-        """Lookup embedded cover
+class RemoteArt(GObject.GObject):
 
-        Lookup embedded art through Gst.Discoverer. If found
-        copy locally and call _lookup_local to finish retrieving
-        suitable art, otherwise follow up with _lookup_remote.
-        """
-        album = utils.get_album_title(item)
-        artist = utils.get_artist_name(item)
+    __gsignals__ = {
+        'retrieved': (GObject.SignalFlags.RUN_FIRST, None, ()),
+        'unavailable': (GObject.SignalFlags.RUN_FIRST, None, ())
+    }
 
-        success, cache_path = MediaArt.get_path(artist, album, "album")
-        if not success:
-            self._lookup_remote(item, art_size, callback, itr)
+    @log
+    def __init__(self):
+        super().__init__()
 
-        self._discoverer_items[item.get_url()] = [item, art_size, callback,
-                                                  itr, cache_path]
-        self._discoverer.discover_uri_async(item.get_url())
+        self._artist = None
+        self._album = None
 
     @log
-    def _lookup_remote(self, item, art_size, callback, itr):
+    def query(self, media):
         """Lookup remote art
 
         Lookup remote art through Grilo and if found copy locally. Call
         _lookup_local to finish retrieving suitable art.
         """
-        album = utils.get_album_title(item)
-        artist = utils.get_artist_name(item)
+        self._album = utils.get_album_title(media)
+        self._artist = utils.get_artist_name(media)
 
-        @log
-        def delete_cb(src, result, data):
-            try:
-                src.delete_finish(result)
-            except Exception as err:
-                logger.warn("Error: %s, %s", err.__class__, err)
+        grilo.get_album_art_for_item(media, self._remote_album_art)
 
-        @log
-        def splice_cb(src, result, data):
-            tmp_file, iostream = data
+    @log
+    def _delete_async_callback(self, src, result, data):
+        try:
+            src.delete_finish(result)
+        except GLib.Error as error:
+            logger.warn("Error: {}, {}".format(error.domain, error.message))
 
-            try:
-                src.splice_finish(result)
-            except Exception as err:
-                logger.warn("Error: %s, %s", err.__class__, err)
-                art_retrieved(False)
-                return
+    @log
+    def _splice_async_callback(self, src, result, data):
+        tmp_file, iostream = data
 
-            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
+        try:
+            src.splice_finish(result)
+        except GLib.Error as error:
+            logger.warn("Error: {}, {}".format(error.domain, error.message))
+            self.emit('unavailable')
+            return
 
-            art_retrieved(True)
+        success, cache_path = MediaArt.get_path(
+            self._artist, self._album, "album")
 
-            tmp_file.delete_async(GLib.PRIORITY_LOW,
-                                  None,
-                                  delete_cb,
-                                  None)
+        if not success:
+            self.emit('unavailable')
+            return
 
-        @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
+        try:
+            # FIXME: I/O blocking
+            MediaArt.file_to_jpeg(tmp_file.get_path(), cache_path)
+        except GLib.Error as error:
+            logger.warn("Error: {}, {}".format(error.domain, error.message))
+            self.emit('unavailable')
+            return
 
-            try:
-                [tmp_file, iostream] = Gio.File.new_tmp()
-            except Exception as err:
-                logger.warn("Error: %s, %s", err.__class__, err)
-                art_retrieved(False)
-                return
+        self.emit('retrieved')
 
-            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,
-                                 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
+        tmp_file.delete_async(
+            GLib.PRIORITY_LOW, None, self._delete_async_callback, None)
 
-            thumb_uri = item.get_thumbnail()
+    @log
+    def _read_async_callback(self, src, result, data):
+        try:
+            istream = src.read_finish(result)
+        except GLib.Error as error:
+            logger.warn("Error: {}, {}".format(error.domain, error.message))
+            self.emit('unavailable')
+            return
 
-            if thumb_uri is None:
-                art_retrieved(False)
-                return
+        try:
+            [tmp_file, iostream] = Gio.File.new_tmp()
+        except GLib.Error as error:
+            logger.warn("Error: {}, {}".format(error.domain, error.message))
+            self.emit('unavailable')
+            return
 
-            src = Gio.File.new_for_uri(thumb_uri)
-            src.read_async(GLib.PRIORITY_LOW,
-                           None,
-                           async_read_cb,
-                           None)
+        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._splice_async_callback, [tmp_file, iostream])
 
-        @log
-        def art_retrieved(result):
-            if not result:
-                if artist not in self.blacklist:
-                    self.blacklist[artist] = []
+    @log
+    def _remote_album_art(self, source, param, item, count, error):
+        if error:
+            logger.warn("Grilo error {}".format(error))
+            self.emit('unavailable')
+            return
 
-                album_stripped = MediaArt.strip_invalid_entities(album)
-                self.blacklist[artist].append(album_stripped)
+        thumb_uri = item.get_thumbnail()
 
-            self.lookup(item, art_size, callback, itr)
+        if thumb_uri is None:
+            self.emit('unavailable')
+            return
 
-        grilo.get_album_art_for_item(item, album_art_for_item_cb)
+        src = Gio.File.new_for_uri(thumb_uri)
+        src.read_async(
+            GLib.PRIORITY_LOW, None, self._read_async_callback, None)
diff --git a/gnomemusic/player.py b/gnomemusic/player.py
index d8798d2..8bd0478 100644
--- a/gnomemusic/player.py
+++ b/gnomemusic/player.py
@@ -46,7 +46,7 @@ from gi.repository import Gtk, Gdk, GLib, Gio, GObject, Gst, GstAudio, GstPbutil
 from gettext import gettext as _, ngettext
 
 from gnomemusic import log
-from gnomemusic.albumartcache import AlbumArtCache, DefaultIcon, ArtSize
+from gnomemusic.albumartcache import Art, ArtImage
 from gnomemusic.grilo import grilo
 from gnomemusic.playlists import Playlists
 from gnomemusic.scrobbler import LastFmScrobbler
@@ -109,11 +109,6 @@ class Player(GObject.GObject):
         self.playlistField = None
         self.currentTrack = None
         self.currentTrackUri = None
-        scale = parent_window.get_scale_factor()
-        self.cache = AlbumArtCache(scale)
-        self._loading_icon_surface = DefaultIcon(scale).get(
-            DefaultIcon.Type.loading,
-            ArtSize.XSMALL)
         self._missingPluginMessages = []
 
         Gst.init(None)
@@ -584,8 +579,8 @@ class Player(GObject.GObject):
         artist = utils.get_artist_name(media)
         self.artistLabel.set_label(artist)
 
-        self.coverImg.set_from_surface(self._loading_icon_surface)
-        self.cache.lookup(media, ArtSize.XSMALL, self._on_cache_lookup, None)
+        art = ArtImage(Art.Size.XSMALL, media)
+        art.image = self._image
 
         title = utils.get_media_title(media)
         self.titleLabel.set_label(title)
@@ -639,7 +634,7 @@ class Player(GObject.GObject):
 
     @log
     def _on_cache_lookup(self, surface, data=None):
-        self.coverImg.set_from_surface(surface)
+        # FIXME: Need this for mpris
         self.emit('thumbnail-updated')
 
     @log
@@ -780,9 +775,7 @@ class Player(GObject.GObject):
         self.songTotalTimeLabel = self._ui.get_object('duration')
         self.titleLabel = self._ui.get_object('title')
         self.artistLabel = self._ui.get_object('artist')
-        self.coverImg = self._ui.get_object('cover')
-        self.coverImg.set_property("width-request", ArtSize.XSMALL.width)
-        self.coverImg.set_property("height-request", ArtSize.XSMALL.height)
+        self._image = self._ui.get_object('cover')
 
         self.duration = self._ui.get_object('duration')
         self.repeatBtnImage = self._ui.get_object('playlistRepeat')
diff --git a/gnomemusic/views/albumsview.py b/gnomemusic/views/albumsview.py
index bdd83a7..97e5ee2 100644
--- a/gnomemusic/views/albumsview.py
+++ b/gnomemusic/views/albumsview.py
@@ -26,7 +26,7 @@ from gettext import gettext as _
 from gi.repository import GLib, GObject, Gtk, Gdk
 
 from gnomemusic import log
-from gnomemusic.albumartcache import ArtSize
+from gnomemusic.albumartcache import Art, ArtImage
 from gnomemusic.grilo import grilo
 from gnomemusic.toolbar import ToolbarState
 from gnomemusic.views.baseview import BaseView
@@ -166,13 +166,6 @@ class AlbumsView(BaseView):
         child.title.set_label(title)
         child.subtitle.set_label(artist)
 
-        child.image.set_from_surface(self._loading_icon_surface)
-        # In the case of off-sized icons (eg. provided in the soundfile)
-        # keep the size request equal to all other icons to get proper
-        # alignment with GtkFlowBox.
-        child.image.set_property("width-request", ArtSize.MEDIUM.width)
-        child.image.set_property("height-request", ArtSize.MEDIUM.height)
-
         child.events.add_events(Gdk.EventMask.TOUCH_MASK)
 
         child.events.connect('button-release-event',
@@ -189,7 +182,8 @@ class AlbumsView(BaseView):
         child.add(builder.get_object('main_box'))
         child.show()
 
-        self._cache.lookup(item, ArtSize.MEDIUM, self._on_lookup_ready, child)
+        art = ArtImage(Art.Size.MEDIUM, item)
+        art.image = child.image
 
         return child
 
@@ -200,9 +194,6 @@ class AlbumsView(BaseView):
             if self.selection_mode:
                 child.check.set_active(True)
 
-    def _on_lookup_ready(self, icon, child):
-        child.image.set_from_surface(icon)
-
     @log
     def _on_child_toggled(self, check, pspec, child):
         if (check.get_active()
diff --git a/gnomemusic/views/baseview.py b/gnomemusic/views/baseview.py
index 7f5489c..f2f43cd 100644
--- a/gnomemusic/views/baseview.py
+++ b/gnomemusic/views/baseview.py
@@ -26,7 +26,6 @@ from gettext import gettext as _, ngettext
 from gi.repository import Gd, Gdk, GdkPixbuf, GObject, Gtk
 
 from gnomemusic import log
-from gnomemusic.albumartcache import AlbumArtCache, DefaultIcon, ArtSize
 from gnomemusic.grilo import grilo
 from gnomemusic.widgets.starhandlerwidget import StarHandlerWidget
 import gnomemusic.utils as utils
@@ -107,11 +106,6 @@ class BaseView(Gtk.Stack):
         self.show_all()
         self._view.hide()
 
-        scale = self.get_scale_factor()
-        self._cache = AlbumArtCache(scale)
-        self._loading_icon_surface = DefaultIcon(scale).get(
-            DefaultIcon.Type.loading, ArtSize.MEDIUM)
-
         self._init = False
         grilo.connect('ready', self._on_grilo_ready)
         self._header_bar.connect('selection-mode-changed',
@@ -213,6 +207,10 @@ class BaseView(Gtk.Stack):
     def populate(self):
         pass
 
+    @log
+    def _retrieval_finished(self, klass):
+        self.model[klass.iter][4] = klass.pixbuf
+
     @log
     def _add_item(self, source, param, item, remaining=0, data=None):
         if not item:
@@ -226,30 +224,17 @@ class BaseView(Gtk.Stack):
         title = utils.get_media_title(item)
 
         itr = self.model.append(None)
-        loading_icon = Gdk.pixbuf_get_from_surface(
-            self._loadin_icon_surface, 0, 0,
-            self._loading_icon_surface.get_width(),
-            self._loading_icon_surface.get_height())
 
-        self.model[itr][0, 1, 2, 3, 4, 5, 7, 9] = [
+        self.model[itr][0, 1, 2, 3, 5, 7, 9] = [
             str(item.get_id()),
             '',
             title,
             artist,
-            loading_icon,
             item,
             0,
             False
         ]
 
-    @log
-    def _on_lookup_ready(self, surface, itr):
-        if surface:
-            pixbuf = Gdk.pixbuf_get_from_surface(surface, 0, 0,
-                                                 surface.get_width(),
-                                                 surface.get_height())
-            self.model[itr][4] = pixbuf
-
     @log
     def _add_list_renderers(self):
         pass
diff --git a/gnomemusic/views/initialstateview.py b/gnomemusic/views/initialstateview.py
index d005537..e06d065 100644
--- a/gnomemusic/views/initialstateview.py
+++ b/gnomemusic/views/initialstateview.py
@@ -25,7 +25,7 @@
 from gettext import gettext as _
 
 from gnomemusic import log
-from gnomemusic.albumartcache import ArtSize
+from gnomemusic.albumartcache import Art
 from gnomemusic.views.emptyview import EmptyView
 
 
@@ -43,7 +43,7 @@ class InitialStateView(EmptyView):
         icon.set_margin_bottom(32)
         icon.set_opacity(1)
         icon.set_from_resource('/org/gnome/Music/initial-state.png')
-        icon.set_size_request(ArtSize.LARGE.width, ArtSize.LARGE.height)
+        icon.set_size_request(Art.Size.LARGE.width, Art.Size.LARGE.height)
 
         # Update label
         label = self.builder.get_object('label')
diff --git a/gnomemusic/views/searchview.py b/gnomemusic/views/searchview.py
index 592ed06..7f04d86 100644
--- a/gnomemusic/views/searchview.py
+++ b/gnomemusic/views/searchview.py
@@ -23,9 +23,9 @@
 # delete this exception statement from your version.
 
 from gettext import gettext as _
-from gi.repository import Gd, Gdk, GdkPixbuf, GObject, Grl, Gtk, Pango
+from gi.repository import Gd, GdkPixbuf, GObject, Grl, Gtk, Pango
 
-from gnomemusic.albumartcache import DefaultIcon, ArtSize
+from gnomemusic.albumartcache import Art
 from gnomemusic.grilo import grilo
 from gnomemusic import log
 from gnomemusic.player import DiscoveryStatus
@@ -54,13 +54,6 @@ class SearchView(BaseView):
     def __init__(self, window, player):
         super().__init__('search', None, window, Gd.MainViewType.LIST)
 
-        scale = self.get_scale_factor()
-        loading_icon_surface = DefaultIcon(scale).get(
-            DefaultIcon.Type.loading, ArtSize.SMALL)
-        self._loading_icon = Gdk.pixbuf_get_from_surface(
-            loading_icon_surface, 0, 0, loading_icon_surface.get_width(),
-            loading_icon_surface.get_height())
-
         self._add_list_renderers()
         self.player = player
         self._head_iters = [None, None, None, None]
@@ -194,6 +187,13 @@ class SearchView(BaseView):
         self._albums[key].songs.append(item)
         self._add_item(source, None, item, 0, [self.model, 'song'])
 
+    @log
+    def _retrieval_finished(self, klass, model, _iter):
+        if not model[_iter][13]:
+            return
+
+        model[_iter][13] = klass.surface
+
     @log
     def _add_item(self, source, param, item, remaining=0, data=None):
         if data is None:
@@ -238,17 +238,12 @@ class SearchView(BaseView):
         except:
             pass
 
-        # FIXME: HiDPI icon lookups return a surface that can't be
-        # scaled by GdkPixbuf, so it results in a * scale factor sized
-        # icon for the search view.
         _iter = None
         if category == 'album':
             _iter = self.model.insert_with_values(
-                self._head_iters[group], -1, [0, 2, 3, 4, 5, 9, 11],
-                [str(item.get_id()), title, artist, self._loading_icon, item,
-                 2, category])
-            self._cache.lookup(
-                item, ArtSize.SMALL, self._on_lookup_ready, _iter)
+                self._head_iters[group], -1, [0, 2, 3, 5, 9, 11],
+                [str(item.get_id()), title, artist, item, 2,
+                 category])
         elif category == 'song':
             # FIXME: source specific hack
             if source.get_id() != 'grl-tracker-source':
@@ -256,26 +251,31 @@ class SearchView(BaseView):
             else:
                 fav = item.get_favourite()
             _iter = self.model.insert_with_values(
-                self._head_iters[group], -1, [0, 2, 3, 4, 5, 9, 11],
-                [str(item.get_id()), title, artist, self._loading_icon, item,
-                 fav, category])
-            self._cache.lookup(
-                item, ArtSize.SMALL, self._on_lookup_ready, _iter)
+                self._head_iters[group], -1, [0, 2, 3, 5, 9, 11],
+                [str(item.get_id()), title, artist, item, fav,
+                 category])
         else:
             if not artist.casefold() in self._artists:
                 _iter = self.model.insert_with_values(
-                    self._head_iters[group], -1, [0, 2, 4, 5, 9, 11],
-                    [str(item.get_id()), artist, self._loading_icon, item, 2,
+                    self._head_iters[group], -1, [0, 2, 5, 9, 11],
+                    [str(item.get_id()), artist, item, 2,
                      category])
-                self._cache.lookup(
-                    item, ArtSize.SMALL, self._on_lookup_ready, _iter)
                 self._artists[artist.casefold()] = {
                     'iter': _iter,
                     'albums': []
                 }
-
             self._artists[artist.casefold()]['albums'].append(item)
 
+        # FIXME: Figure out why iter can be None here, seems illogical.
+        if _iter is not None:
+            scale = self._view.get_scale_factor()
+            art = Art(Art.Size.SMALL, item, scale)
+            self.model[_iter][13] = art.surface
+            art.connect(
+                'finished', self._retrieval_finished, self.model, _iter)
+            art.lookup()
+
+
         if self.model.iter_n_children(self._head_iters[group]) == 1:
             path = self.model.get_path(self._head_iters[group])
             path = self._filter_model.convert_child_path_to_path(path)
@@ -295,13 +295,31 @@ class SearchView(BaseView):
             title_renderer, self._on_list_widget_title_render, None)
         cols[0].add_attribute(title_renderer, 'text', 2)
 
-        self._star_handler.add_star_renderers(list_widget, cols[0])
+        # Add our own surface renderer, instead of the one provided by
+        # Gd. This avoids us having to set the model to a cairo.Surface
+        # which is currently not a working solution in pygobject.
+        # https://gitlab.gnome.org/GNOME/pygobject/issues/155
+        pixbuf_renderer = Gtk.CellRendererPixbuf(
+            xalign=0.5, yalign=0.5, xpad=12, ypad=2)
+        list_widget.add_renderer(
+            pixbuf_renderer, self._on_list_widget_pixbuf_renderer, None)
+        cols[0].add_attribute(pixbuf_renderer, 'surface', 13)
 
+        self._star_handler.add_star_renderers(list_widget, cols[0])
         cells = cols[0].get_cells()
         cols[0].reorder(cells[0], -1)
+        cols[0].reorder(cells[4], 1)
         cols[0].set_cell_data_func(
             cells[0], self._on_list_widget_selection_render, None)
 
+    @log
+    def _on_list_widget_pixbuf_renderer(self, col, cell, model, _iter, data):
+        if not model[_iter][13]:
+            return
+
+        cell.set_property("surface", model[_iter][13])
+
+    @log
     def _on_list_widget_selection_render(self, col, cell, model, _iter, data):
         if (self._view.get_selection_mode()
                 and model.iter_parent(_iter) is not None):
@@ -309,11 +327,13 @@ class SearchView(BaseView):
         else:
             cell.set_visible(False)
 
+    @log
     def _on_list_widget_title_render(self, col, cell, model, _iter, data):
         cells = col.get_cells()
-        cells[0].set_visible(model.iter_parent(_iter) is not None)
+        cells[0].set_visible(False)
         cells[1].set_visible(model.iter_parent(_iter) is not None)
-        cells[2].set_visible(model.iter_parent(_iter) is None)
+        cells[2].set_visible(model.iter_parent(_iter) is not None)
+        cells[3].set_visible(model.iter_parent(_iter) is None)
 
     @log
     def populate(self):
@@ -456,7 +476,7 @@ class SearchView(BaseView):
             GObject.TYPE_STRING,
             GObject.TYPE_STRING,    # item title or header text
             GObject.TYPE_STRING,    # artist for albums and songs
-            GdkPixbuf.Pixbuf,       # album art
+            GdkPixbuf.Pixbuf,       # Gd placeholder album art
             GObject.TYPE_OBJECT,    # item
             GObject.TYPE_BOOLEAN,
             GObject.TYPE_INT,
@@ -464,7 +484,8 @@ class SearchView(BaseView):
             GObject.TYPE_INT,
             GObject.TYPE_BOOLEAN,
             GObject.TYPE_STRING,    # type
-            GObject.TYPE_INT
+            GObject.TYPE_INT,
+            object                  # album art surface
         )
 
         self._filter_model = self.model.filter_new(None)
diff --git a/gnomemusic/widgets/albumwidget.py b/gnomemusic/widgets/albumwidget.py
index 5f6841c..97aaaa1 100644
--- a/gnomemusic/widgets/albumwidget.py
+++ b/gnomemusic/widgets/albumwidget.py
@@ -26,7 +26,7 @@ from gettext import gettext as _, ngettext
 from gi.repository import GdkPixbuf, GLib, GObject, Gtk
 
 from gnomemusic import log
-from gnomemusic.albumartcache import AlbumArtCache, DefaultIcon, ArtSize
+from gnomemusic.albumartcache import Art, ArtImage
 from gnomemusic.grilo import grilo
 from gnomemusic.widgets.disclistboxwidget import DiscBox, DiscListBox
 import gnomemusic.utils as utils
@@ -55,12 +55,6 @@ class AlbumWidget(Gtk.EventBox):
 
         self._songs = []
 
-        scale = self.get_scale_factor()
-        self._cache = AlbumArtCache(scale)
-        self._loading_icon_surface = DefaultIcon(scale).get(
-            DefaultIcon.Type.loading,
-            ArtSize.LARGE)
-
         self._player = player
         self._iter_to_clean = None
 
@@ -143,10 +137,9 @@ class AlbumWidget(Gtk.EventBox):
         self.selection_toolbar = selection_toolbar
         self._header_bar = header_bar
         self._album = album
-        self._builder.get_object('cover').set_from_surface(
-            self._loading_icon_surface)
-        self._cache.lookup(item, ArtSize.LARGE, self._on_lookup, None)
         self._duration = 0
+        art = ArtImage(Art.Size.LARGE, item)
+        art.image = self._builder.get_object('cover')
 
         GLib.idle_add(grilo.populate_album_songs, item, self.add_item)
         header_bar._select_button.connect(
@@ -289,16 +282,6 @@ class AlbumWidget(Gtk.EventBox):
 
             self.show_all()
 
-    @log
-    def _on_lookup(self, surface, data=None):
-        """Albumart retrieved callback.
-
-        :param surface: The Cairo surface retrieved
-        :param path: The filesystem location the pixbuf
-        :param data: User data
-        """
-        self._builder.get_object('cover').set_from_surface(surface)
-
     @log
     def _update_model(self, player, playlist, current_iter):
         """Player changed callback.
diff --git a/gnomemusic/widgets/artistalbumwidget.py b/gnomemusic/widgets/artistalbumwidget.py
index 71bc01b..a86eca2 100644
--- a/gnomemusic/widgets/artistalbumwidget.py
+++ b/gnomemusic/widgets/artistalbumwidget.py
@@ -22,10 +22,10 @@
 # code, but you are not obligated to do so.  If you do not wish to do so,
 # delete this exception statement from your version.
 
-from gi.repository import GLib, GObject, Gtk
+from gi.repository import GObject, Gtk
 
 from gnomemusic import log
-from gnomemusic.albumartcache import AlbumArtCache, DefaultIcon, ArtSize
+from gnomemusic.albumartcache import Art, ArtImage
 from gnomemusic.grilo import grilo
 from gnomemusic.widgets.disclistboxwidget import DiscBox
 import gnomemusic.utils as utils
@@ -48,11 +48,6 @@ class ArtistAlbumWidget(Gtk.Box):
 
         self._size_group = size_group
         self._cover_size_group = cover_size_group
-        scale = self.get_scale_factor()
-        self._cache = AlbumArtCache(scale)
-        self._loading_icon_surface = DefaultIcon(scale).get(
-            DefaultIcon.Type.loading,
-            ArtSize.MEDIUM)
 
         self._media = media
         self._player = player
@@ -72,7 +67,8 @@ class ArtistAlbumWidget(Gtk.Box):
         ui.add_from_resource('/org/gnome/Music/ArtistAlbumWidget.ui')
 
         self.cover = ui.get_object('cover')
-        self.cover.set_from_surface(self._loading_icon_surface)
+        art = ArtImage(Art.Size.MEDIUM, self._media)
+        art.image = self.cover
 
         self._disc_listbox = ui.get_object('disclistbox')
         self._disc_listbox.set_selection_mode_allowed(
@@ -93,7 +89,6 @@ class ArtistAlbumWidget(Gtk.Box):
 
         self.pack_start(ui.get_object('ArtistAlbumWidget'), True, True, 0)
 
-        GLib.idle_add(self._update_album_art)
         grilo.populate_album_songs(self._media, self._add_item)
 
     def create_disc_box(self, disc_nr, disc_songs):
@@ -159,15 +154,6 @@ class ArtistAlbumWidget(Gtk.Box):
         if remaining == 0:
             self.emit("songs-loaded")
 
-    @log
-    def _update_album_art(self):
-        self._cache.lookup(self._media, ArtSize.MEDIUM, self._get_album_cover,
-                           None)
-
-    @log
-    def _get_album_cover(self, surface, data=None):
-        self.cover.set_from_surface(surface)
-
     @log
     def _song_activated(self, widget, song_widget):
         if (not song_widget.can_be_played


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]