[gnome-music/wip/mschraal/artrework: 7/7] albumartcache: Rewrite lookup



commit 2b7e4d96c7544855998c2fe9cf8cc9ecf9b6547d
Author: Marinus Schraal <mschraal gnome org>
Date:   Sat Jan 6 02:13:08 2018 +0100

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

 gnomemusic/albumartcache.py    | 548 +++++++++++++++++++++--------------------
 gnomemusic/views/searchview.py |   5 +-
 2 files changed, 290 insertions(+), 263 deletions(-)
---
diff --git a/gnomemusic/albumartcache.py b/gnomemusic/albumartcache.py
index 489ba9a..7479f96 100644
--- a/gnomemusic/albumartcache.py
+++ b/gnomemusic/albumartcache.py
@@ -46,40 +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
 def _make_icon_frame(pixbuf, art_size=None, scale=1):
@@ -204,12 +204,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 +234,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 +247,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()
+
+        embedded_art = EmbeddedArt()
+        embedded_art.connect('found', self._embedded_art_found)
+        embedded_art.connect('not-available', self._embedded_art_not_available)
+
+        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_not_available(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._surface = DefaultIcon(scale).get(DefaultIcon.Type.loading, size)
+        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')
+
+    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)
 
-        cache = AlbumArtCache(scale)
+    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)
 
-        iter_ = None
-        cache.lookup(self._media, self._size, self._callback, iter_)
+        if artist in self._blacklist:
+            if album_stripped in self._blacklist[artist]:
+                return True
+
+        return False
 
     @GObject.Property
     @log
@@ -271,170 +364,128 @@ 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()
 
-        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
+    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")
 
-        Gst.init(None)
-        self._discoverer = GstPbutils.Discoverer.new(Gst.SECOND)
-        self._discoverer.connect('discovered', self._discovered_cb)
-        self._discoverer.start()
+        if (success
+                and thumb_file.query_exists()):
+            thumb_file.read_async(
+                GLib.PRIORITY_LOW, None, self._open_stream, None)
+            return
 
-        self._discoverer_items = {}
+        self.emit('miss')
 
-        self._media_art = None
+    def _open_stream(self, thumb_file, result, arguments):
         try:
-            self._media_art = MediaArt.Process.new()
+            stream = thumb_file.read_finish(result)
         except Exception as err:
             logger.warn("Error: %s, %s", err.__class__, err)
+            stream.close_async(
+                GLib.PRIORITY_LOW, None, self._close_stream, None)
+            self.emit('miss')
+            return
 
-    @log
-    def lookup(self, item, art_size, callback, itr):
-        """Find art for the given item
-
-        :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)
+        GdkPixbuf.Pixbuf.new_from_stream_async(
+            stream, None, self._pixbuf_loaded, None)
 
-    @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 _pixbuf_loaded(self, stream, result, data):
+        try:
+            pixbuf = GdkPixbuf.Pixbuf.new_from_stream_finish(result)
+        except Exception as err:
+            logger.warn("Error: %s, %s", err.__class__, err)
+            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
+        stream.close_async(GLib.PRIORITY_LOW, None, self._close_stream, None)
+        self.emit('hit', pixbuf)
 
-            GdkPixbuf.Pixbuf.new_from_stream_async(stream,
-                                                   None,
-                                                   pixbuf_loaded,
-                                                   None)
+    def _close_stream(self, stream, result, data):
+        stream.close_finish(result)
+        # TODO: Try except
 
-        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
 
-            do_callback(pixbuf)
-            return
+class EmbeddedArt(GObject.GObject):
 
-        def do_callback(pixbuf):
+    __gsignals__ = {
+        'found': (GObject.SignalFlags.RUN_FIRST, None, ()),
+        'not-available': (
+            GObject.SignalFlags.RUN_FIRST, None, ()
+        )
+    }
 
-            # Lookup finished, decrease the counter
-            LookupQueue.pop()
+    def __init__(self):
+        super().__init__()
 
-            if not pixbuf:
-                surface = DefaultIcon(self._scale).get(DefaultIcon.Type.music,
-                                                       art_size)
-            else:
-                surface = _make_icon_frame(pixbuf, art_size, self._scale)
+        try:
+            Gst.init(None)
+            self._discoverer = GstPbutils.Discoverer.new(Gst.SECOND)
+            #self._discoverer.connect('discovered', self._discovered)
+            #self._discoverer.start()
+        except Exception as err:
+            # report
+            return
 
-                # Sets the thumbnail location for MPRIS to use.
-                item.set_thumbnail(GLib.filename_to_uri(thumb_file.get_path(),
-                                                        None))
+        self._path = None
 
-            GLib.idle_add(callback, surface, itr)
-            return
 
-        success, thumb_file = MediaArt.get_file(artist, album, "album")
+    def query(self, media):
+        album = utils.get_album_title(media)
+        artist = utils.get_artist_name(media)
 
-        if (success
-                and thumb_file.query_exists()):
-            thumb_file.read_async(GLib.PRIORITY_LOW,
-                                  None,
-                                  stream_open,
-                                  None)
+        success, path = MediaArt.get_path(artist, album, "album")
+        if not success:
+            self.emit('not-available')
+            #self._discoverer.stop()
             return
 
-        stripped_album = MediaArt.strip_invalid_entities(album)
-        if (artist in self.blacklist
-                and stripped_album in self.blacklist[artist]):
-            do_callback(None)
+        self._path = path
+        try:
+            info_ = self._discoverer.discover_uri(media.get_url())
+        except Exception as err:
+            self.emit('not-available')
+            #self._discoverer.stop()
             return
+        self._discovered(info_)
 
-        # 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)
-
-    @log
-    def _discovered_cb(self, discoverer, info, error):
-        item, art_size, callback, itr, cache_path = \
-            self._discoverer_items[info.get_uri()]
-
-        album = utils.get_album_title(item)
-        artist = utils.get_artist_name(item)
+    #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('not-available')
+        #    return
+
+        # album = utils.get_album_title(self._item)
+        # artist = utils.get_artist_name(self._item)
 
         while True:
             success, sample = tags.get_sample_index(Gst.TAG_IMAGE, index)
@@ -442,8 +493,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 +507,118 @@ 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)
 
-        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('not-available')
+        #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 _async_delete_callback(self, src, result, data):
+        try:
+            src.delete_finish(result)
+        except Exception as err:
+            logger.warn("Error: %s, %s", err.__class__, err)
 
-            try:
-                src.splice_finish(result)
-            except Exception as err:
-                logger.warn("Error: %s, %s", err.__class__, err)
-                art_retrieved(False)
-                return
+    @log
+    def _async_splice_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 Exception as err:
+            logger.warn("Error: %s, %s", err.__class__, err)
+            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 Exception as err:
+            logger.warn("Error: %s, %s", err.__class__, err)
+            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._async_delete_callback, None)
 
-            thumb_uri = item.get_thumbnail()
+    @log
+    def _read_async_callback(self, src, result, data):
+        try:
+            istream = src.read_finish(result)
+        except Exception as err:
+            logger.warn("Error: %s, %s", err.__class__, err)
+            self.emit('unavailable')
+            return
 
-            if thumb_uri is None:
-                art_retrieved(False)
-                return
+        try:
+            [tmp_file, iostream] = Gio.File.new_tmp()
+        except Exception as err:
+            logger.warn("Error: %s, %s", err.__class__, err)
+            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._async_splice_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 %s", 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/views/searchview.py b/gnomemusic/views/searchview.py
index 87c53fa..78902fe 100644
--- a/gnomemusic/views/searchview.py
+++ b/gnomemusic/views/searchview.py
@@ -267,8 +267,9 @@ class SearchView(BaseView):
                     'albums': []
                 }
             self._artists[artist.casefold()]['albums'].append(item)
-        art.iter = _iter
-        art.connect('finished', self._retrieval_finished)
+        if _iter is not None:
+            art.iter = _iter
+            art.connect('finished', self._retrieval_finished)
 
         if self.model.iter_n_children(self._head_iters[group]) == 1:
             path = self.model.get_path(self._head_iters[group])


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