[gnome-music/wip/jfelder/untangle-mpris-playertoolbar] mpris: Untangle MPRIS and playertoolbar

[Jean Felder <jfelder src gnome org>]

commit 6cfc8a975b0bb7e1fa58138034e53c9547292f5a
Author: Jean Felder <jfelder src gnome org>
Date:   Wed Apr 10 19:45:27 2019 +0200

    mpris: Untangle MPRIS and playertoolbar
    
    MPRIS is tied with PlayerToolbar because the thumbnail updating logic
    takes place in the CoverStack from PlayerToolbar.
    In fact, on every song change the following thing happens:
    * PlayerToolbar looks up for the song's thumbnail (via its
      cover_stack) and emits a "thumbnail-updated" signal once the
      operation is finished.
    * MPRIS sends the song's metadata without the thumbnail url (it has
      not been updated yet). A bit later, once the "thumbnail-updated"
      signal is received, the metadata are sent again with the correct
      thumbnail url.
    This can result in a glitter on some MPRIS clients: the client is
    updated with an empty thumbnail, and then, very quickly, with the
    correct one.
    
    Fix that issue by factoring out the thumbnail cache lookup and
    directly looking up the song thumbnail from MPRIS. The disadvantage of
    this solution is that the lookup_art_file_from_cache function is
    called two times on a song change. One call from PlayerToolbar and the
    other one from mpris. However, this function call is less costy than
    sending two times the song's metadata.
    
    Related: #43, #172
    Closes: #101

 gnomemusic/albumartcache.py         | 42 +++++++++++++++++++------------------
 gnomemusic/gstplayer.py             |  1 -
 gnomemusic/mpris.py                 | 27 +++++++++---------------
 gnomemusic/widgets/playertoolbar.py | 11 +---------
 4 files changed, 33 insertions(+), 48 deletions(-)
---
diff --git a/gnomemusic/albumartcache.py b/gnomemusic/albumartcache.py
index 01561f7e..8ffdb618 100644
--- a/gnomemusic/albumartcache.py
+++ b/gnomemusic/albumartcache.py
@@ -29,6 +29,7 @@ import os
 
 import cairo
 import gi
+gi.require_version('GstPbutils', '1.0')
 gi.require_version('GstTag', '1.0')
 gi.require_version('MediaArt', '2.0')
 from gi.repository import (Gdk, GdkPixbuf, Gio, GLib, GObject, Gtk, MediaArt,
@@ -42,6 +43,25 @@ import gnomemusic.utils as utils
 logger = logging.getLogger(__name__)
 
 
+@log
+def lookup_art_file_from_cache(media):
+    """Lookup MediaArt cache art of an album or song.
+
+    :param Grl.Media media: song or album
+    :returns: a cache file
+    :rtype: Gio.File
+    """
+    album = utils.get_album_title(media)
+    artist = utils.get_artist_name(media)
+
+    success, thumb_file = MediaArt.get_file(artist, album, "album")
+    if (not success
+            or not thumb_file.query_exists()):
+        return None
+
+    return thumb_file
+
+
 @log
 def _make_icon_frame(icon_surface, art_size=None, scale=1, default_icon=False):
     border = 3
@@ -213,7 +233,6 @@ class Art(GObject.GObject):
             pixbuf, self._scale, None)
         surface = _make_icon_frame(surface, self._size, self._scale)
         self._surface = surface
-        self._set_grilo_thumbnail_path()
 
         self.emit('finished')
 
@@ -284,18 +303,6 @@ class Art(GObject.GObject):
 
         return False
 
-    def _set_grilo_thumbnail_path(self):
-        # TODO: This sets the thumbnail path for the Grilo Media object
-        # to be used by MPRIS. However, calling this by default for
-        # every cache hit is unnecessary.
-        album = utils.get_album_title(self._media)
-        artist = utils.get_artist_name(self._media)
-
-        success, thumb_file = MediaArt.get_file(artist, album, "album")
-        if success:
-            self._media.set_thumbnail(
-                GLib.filename_to_uri(thumb_file.get_path(), None))
-
     @GObject.Property
     def surface(self):
         if self._surface is None:
@@ -339,13 +346,8 @@ class Cache(GObject.GObject):
 
         :param Grl.Media media: The media object to search art for
         """
-        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 = lookup_art_file_from_cache(media)
+        if thumb_file:
             thumb_file.read_async(
                 GLib.PRIORITY_LOW, None, self._open_stream, None)
             return
diff --git a/gnomemusic/gstplayer.py b/gnomemusic/gstplayer.py
index 16435c15..42bf8906 100644
--- a/gnomemusic/gstplayer.py
+++ b/gnomemusic/gstplayer.py
@@ -29,7 +29,6 @@ from gettext import gettext as _, ngettext
 import gi
 gi.require_version('Gst', '1.0')
 gi.require_version('GstAudio', '1.0')
-gi.require_version('GstPbutils', '1.0')
 from gi.repository import Gtk, Gio, GObject, Gst, GstAudio, GstPbutils
 
 from gnomemusic import log
diff --git a/gnomemusic/mpris.py b/gnomemusic/mpris.py
index 6639c080..d37647ad 100644
--- a/gnomemusic/mpris.py
+++ b/gnomemusic/mpris.py
@@ -23,6 +23,7 @@
 # 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 gnomemusic.albumartcache import lookup_art_file_from_cache
 from gnomemusic.gstplayer import Playback
 from gnomemusic.player import PlayerPlaylist, RepeatMode
 from gnomemusic.grilo import grilo
@@ -229,9 +230,6 @@ class MediaPlayer2Service(Server):
         self.player.connect('seek-finished', self._on_seek_finished)
         self.player.connect(
             'playlist-changed', self._on_player_playlist_changed)
-        self.player_toolbar = app.get_active_window()._player_toolbar
-        self.player_toolbar.connect(
-            'thumbnail-updated', self._on_thumbnail_updated)
         playlists = Playlists.get_default()
         playlists.connect('playlist-created', self._on_playlists_count_changed)
         playlists.connect('playlist-deleted', self._on_playlists_count_changed)
@@ -330,12 +328,15 @@ class MediaPlayer2Service(Server):
         except:
             pass
 
-        try:
-            artUrl = media.get_thumbnail()
-            assert artUrl is not None
-            metadata['mpris:artUrl'] = GLib.Variant('s', artUrl)
-        except:
-            pass
+        art_url = media.get_thumbnail()
+        if not art_url:
+            thumb_file = lookup_art_file_from_cache(media)
+            if thumb_file:
+                art_url = GLib.filename_to_uri(thumb_file.get_path())
+                media.set_thumbnail(art_url)
+
+        if art_url:
+            metadata['mpris:artUrl'] = GLib.Variant('s', art_url)
 
         return metadata
 
@@ -458,14 +459,6 @@ class MediaPlayer2Service(Server):
                                },
                                [])
 
-    @log
-    def _on_thumbnail_updated(self, player, data=None):
-        self.PropertiesChanged(MediaPlayer2Service.MEDIA_PLAYER2_PLAYER_IFACE,
-                               {
-                                   'Metadata': GLib.Variant('a{sv}', self._get_metadata()),
-                               },
-                               [])
-
     @log
     def _on_player_state_changed(self, klass, args):
         self.PropertiesChanged(MediaPlayer2Service.MEDIA_PLAYER2_PLAYER_IFACE,
diff --git a/gnomemusic/widgets/playertoolbar.py b/gnomemusic/widgets/playertoolbar.py
index 7c0b69ca..fd4916d4 100644
--- a/gnomemusic/widgets/playertoolbar.py
+++ b/gnomemusic/widgets/playertoolbar.py
@@ -23,7 +23,7 @@
 # delete this exception statement from your version.
 
 from gettext import gettext as _
-from gi.repository import GObject, Gtk
+from gi.repository import Gtk
 
 from gnomemusic import log
 from gnomemusic.albumartcache import Art
@@ -42,10 +42,6 @@ class PlayerToolbar(Gtk.ActionBar):
     Contains the ui of playing a song with Music.
     """
 
-    __gsignals__ = {
-        'thumbnail-updated': (GObject.SignalFlags.RUN_FIRST, None, ()),
-    }
-
     __gtype_name__ = 'PlayerToolbar'
 
     _artist_label = Gtk.Template.Child()
@@ -74,7 +70,6 @@ class PlayerToolbar(Gtk.ActionBar):
         self._progress_scale.props.player = self._player
 
         self._cover_stack.props.size = Art.Size.XSMALL
-        self._cover_stack.connect('updated', self._on_cover_stack_updated)
 
         self._tooltip = TwoLineTip()
 
@@ -92,10 +87,6 @@ class PlayerToolbar(Gtk.ActionBar):
         seconds = int(progress_scale.get_value() / 60)
         self._progress_time_label.set_label(utils.seconds_to_string(seconds))
 
-    @log
-    def _on_cover_stack_updated(self, klass):
-        self.emit('thumbnail-updated')
-
     @Gtk.Template.Callback()
     @log
     def _on_prev_button_clicked(self, button):