[gnome-music/wip/mschraal/artrework: 3/3] albumartcache: Rewrite AlbumArtCache



commit 6fb3fcdaf4499d2f2305cf7287d091db89ffe007
Author: Marinus Schraal <mschraal gnome org>
Date:   Sun Jan 14 18:14:37 2018 +0100

    albumartcache: Rewrite AlbumArtCache
    
    Remove the old AlbumArtCache in favour of a modular approach where the
    local Cache, EmbeddedArt & RemoteArt retrieval all have their own class.
    
    Give these classes their own limited lookup queue to not hammer the
    system so hard on start-up.

 gnomemusic/albumartcache.py | 554 +++++++++++++++++++++++---------------------
 1 file changed, 296 insertions(+), 258 deletions(-)
---
diff --git a/gnomemusic/albumartcache.py b/gnomemusic/albumartcache.py
index 489ba9a..e8fcdb1 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
@@ -204,12 +205,18 @@ class DefaultIcon(GObject.GObject):
 
         return self._cache[(icon_type, art_size)]
 
+
 class Art(GObject.GObject):
 
     __gsignals__ = {
         'finished': (GObject.SignalFlags.RUN_FIRST, None, ())
     }
 
+    _blacklist = {}
+    _cache_queue = Queue()
+    _embedded_queue = Queue()
+    _remote_queue = Queue()
+
     class Size(Enum):
         """Enum for icon sizes"""
         XSMALL = (34, 34)
@@ -228,6 +235,8 @@ class Art(GObject.GObject):
         super().__init__()
 
         self._iter = None
+        self._embedded = False
+        self._remote = False
 
         # FIXME: A pixbuf instead of an image means this art is
         # requested by a treeview.
@@ -239,20 +248,105 @@ class Art(GObject.GObject):
 
         self._size = size
         self._media = media
+        self._media_url = media.get_url()
 
         self._image.set_property("width-request", size.width)
         self._image.set_property("height-request", size.height)
 
-        scale = self._image.get_scale_factor()
+        self._scale = self._image.get_scale_factor()
+
+        self._surface = DefaultIcon(self._scale).get(
+            DefaultIcon.Type.loading, self._size)
+
+        self._image.set_from_surface(self._surface)
+
+        if self._in_blacklist():
+            self._no_art_available()
+            return
+
+        cache = Cache()
+        cache.connect('miss', self._cache_miss)
+        cache.connect('hit', self._cache_hit)
+        if self._cache_queue.push(cache.query, self._media):
+            cache.query(self._media)
+
+    def _cache_miss(self, klass):
+        self._cache_queue.pop()
 
-        self._surface = DefaultIcon(scale).get(DefaultIcon.Type.loading, size)
+        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._image.set_from_surface(self._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(self._scale).get(
+            DefaultIcon.Type.music, self._size)
+
+        self._image.set_from_surface(self._surface)
+        self.emit('finished')
 
-        cache = AlbumArtCache(scale)
+    def _add_to_blacklist(self):
+        album = utils.get_album_title(self._media)
+        artist = utils.get_artist_name(self._media)
 
-        iter_ = None
-        cache.lookup(self._media, self._size, self._callback, iter_)
+        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
@@ -271,170 +365,138 @@ class Art(GObject.GObject):
     def iter(self, iter_):
         self._iter = iter_
 
-    @log
-    def _callback(self, surface, __):
-        self._surface = surface
-        self._image.set_from_surface(self._surface)
+# 1. libmediaart
+# 2  embedded -> libmediaart
+# 3  remote -> libmediaart
 
-        self.emit('finished')
 
+class Cache(GObject.GObject):
 
-class AlbumArtCache(GObject.GObject):
-    """Album art retrieval class
-
-    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
-
-    def __repr__(self):
-        return '<AlbumArtCache>'
+    __gsignals__ = {
+        'miss': (GObject.SignalFlags.RUN_FIRST, None, ()),
+        'hit': (GObject.SignalFlags.RUN_FIRST, None, (GObject.GObject, ))
+    }
 
-    @log
-    def __init__(self, scale=1):
+    def __init__(self):
         super().__init__()
 
-        self._scale = scale
+        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 Exception as err:
-                logger.warn("Error: %s, %s", err.__class__, err)
+            except GLib.Error as error:
+                logger.warn(
+                    "Error: {}, {}".format(error.__class__, error.message))
                 return
 
-        Gst.init(None)
-        self._discoverer = GstPbutils.Discoverer.new(Gst.SECOND)
-        self._discoverer.connect('discovered', self._discovered_cb)
-        self._discoverer.start()
+    def query(self, media):
+        album = utils.get_album_title(media)
+        artist = utils.get_artist_name(media)
 
-        self._discoverer_items = {}
-
-        self._media_art = None
-        try:
-            self._media_art = MediaArt.Process.new()
-        except Exception as err:
-            logger.warn("Error: %s, %s", err.__class__, err)
+        success, thumb_file = MediaArt.get_file(artist, album, "album")
 
-    @log
-    def lookup(self, item, art_size, callback, itr):
-        """Find art for the given item
+        if (success
+                and thumb_file.query_exists()):
+            thumb_file.read_async(
+                GLib.PRIORITY_LOW, None, self._open_stream, None)
+            return
 
-        :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)
+        self.emit('miss')
 
-    @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 _open_stream(self, thumb_file, result, arguments):
+        try:
+            stream = thumb_file.read_finish(result)
+        except GLib.Error as error:
+            logger.warn(
+                "Error: {}, {}".format(error.__class__, error.message))
+            stream.close_async(
+                GLib.PRIORITY_LOW, None, self._close_stream, None)
+            self.emit('miss')
+            return
 
-        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
+        GdkPixbuf.Pixbuf.new_from_stream_async(
+            stream, None, self._pixbuf_loaded, None)
 
-            GdkPixbuf.Pixbuf.new_from_stream_async(stream,
-                                                   None,
-                                                   pixbuf_loaded,
-                                                   None)
+    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.__class__, error.message))
+            stream.close_async(
+                GLib.PRIORITY_LOW, None, self._close_stream, None)
+            self.emit('miss')
+            return
 
-        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
+        stream.close_async(GLib.PRIORITY_LOW, None, self._close_stream, None)
+        self.emit('hit', pixbuf)
 
-            do_callback(pixbuf)
-            return
+    def _close_stream(self, stream, result, data):
+        stream.close_finish(result)
+        # TODO: Try except
 
-        def do_callback(pixbuf):
 
-            # Lookup finished, decrease the counter
-            LookupQueue.pop()
+class EmbeddedArt(GObject.GObject):
 
-            if not pixbuf:
-                surface = DefaultIcon(self._scale).get(DefaultIcon.Type.music,
-                                                       art_size)
-            else:
-                surface = _make_icon_frame(pixbuf, art_size, self._scale)
+    __gsignals__ = {
+        'found': (GObject.SignalFlags.RUN_FIRST, None, ()),
+        'unavailable': (
+            GObject.SignalFlags.RUN_FIRST, None, ()
+        )
+    }
 
-                # Sets the thumbnail location for MPRIS to use.
-                item.set_thumbnail(GLib.filename_to_uri(thumb_file.get_path(),
-                                                        None))
+    def __init__(self):
+        super().__init__()
 
-            GLib.idle_add(callback, surface, itr)
+        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.__class__, error.message))
             return
 
-        success, thumb_file = MediaArt.get_file(artist, album, "album")
+        self._path = None
 
-        if (success
-                and thumb_file.query_exists()):
-            thumb_file.read_async(GLib.PRIORITY_LOW,
-                                  None,
-                                  stream_open,
-                                  None)
-            return
+    def query(self, media):
+        album = utils.get_album_title(media)
+        artist = utils.get_artist_name(media)
 
-        stripped_album = MediaArt.strip_invalid_entities(album)
-        if (artist in self.blacklist
-                and stripped_album in self.blacklist[artist]):
-            do_callback(None)
+        success, path = MediaArt.get_path(artist, album, "album")
+        if not success:
+            self.emit('unavailable')
+            # self._discoverer.stop()
             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()
-
-        self._lookup_embedded(item, art_size, callback, itr)
+        self._path = path
+        try:
+            info_ = self._discoverer.discover_uri(media.get_url())
+        except GLib.Error as error:
+            logger.warn("Error: {}, {}".format(error.__class__, error.message))
+            self.emit('unavailable')
+            # self._discoverer.stop()
+            return
 
-    @log
-    def _discovered_cb(self, discoverer, info, error):
-        item, art_size, callback, itr, cache_path = \
-            self._discoverer_items[info.get_uri()]
+        self._discovered(info_)
 
-        album = utils.get_album_title(item)
-        artist = utils.get_artist_name(item)
+    # FIXME: async is triggering a bug.
+    # def _discovered(self, discoverer, info, error):
+    def _discovered(self, info):
         tags = info.get_tags()
         index = 0
 
-        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, art_size, callback, itr)
-
         # 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)
@@ -442,8 +504,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:
@@ -456,143 +518,119 @@ 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(error.__class__, error.message))
 
-        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)
-                return
-        except Exception as err:
-            logger.warn("Trying to process misc albumart: %s, %s",
-                        err.__class__, err)
+        self.emit('unavailable')
+        # self._discoverer.stop()
 
-        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.__class__, 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.__class__, 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.__class__, 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.__class__, 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.__class__, 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)


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