From: kaliko <efrim@azylum.org>
Date: Sun, 15 Dec 2013 10:41:29 +0000 (+0100)
Subject: Some refactoring around player
X-Git-Tag: mpd-sima/0.12.0pr2~9
X-Git-Url: https://git.kaliko.me/?a=commitdiff_plain;h=8ce90ac3;p=mpd-sima.git

Some refactoring around player

Move out of lastfm plugin what was player logic (album search
heuristic mainly)
---

diff --git a/launch b/launch
index c7ff8e6..39fac64 100755
--- a/launch
+++ b/launch
@@ -3,7 +3,7 @@
 """Sima
 """
 
-# standart library import
+# standard library import
 import logging
 import sys
 
diff --git a/sima/client.py b/sima/client.py
index 80c16e5..9724357 100644
--- a/sima/client.py
+++ b/sima/client.py
@@ -5,12 +5,11 @@ This client is built above python-musicpd a fork of python-mpd
 """
 #  pylint: disable=C0111
 
-# standart library import
-
+# standard library import
 from difflib import get_close_matches
 from select import select
 
-# third parties componants
+# third parties components
 try:
     from musicpd import (MPDClient, MPDError, CommandError)
 except ImportError as err:
@@ -32,6 +31,7 @@ class PlayerCommandError(PlayerError):
 
 PlayerUnHandledError = MPDError  # pylint: disable=C0103
 
+
 class PlayerClient(Player):
     """MPC Client
     From python-musicpd:
@@ -53,17 +53,22 @@ class PlayerClient(Player):
         self._mpd = host, port, password
         self._client = MPDClient()
         self._client.iterate = True
+        self._cache = None
 
     def __getattr__(self, attr):
         command = attr
         wrapper = self._execute
         return lambda *args: wrapper(command, args)
 
+    def __del__(self):
+        """Avoid hanging sockets"""
+        self.disconnect()
+
     def _execute(self, command, args):
         self._write_command(command, args)
         return self._client_wrapper()
 
-    def _write_command(self, command, args=[]):
+    def _write_command(self, command, args=None):
         self._comm = command
         self._args = list()
         for arg in args:
@@ -105,13 +110,26 @@ class PlayerClient(Player):
             return False
         return (self.current.id != old_curr.id)  # pylint: disable=no-member
 
+    def _flush_cache(self):
+        """
+        Both flushes and instantiates _cache
+        """
+        if isinstance(self._cache, dict):
+            self.log.info('Player: Flushing cache!')
+        else:
+            self.log.info('Player: Initialising cache!')
+        self._cache = {
+                'artists': None,
+                }
+        self._cache['artists'] = frozenset(self._client.list('artist'))
+
     def find_track(self, artist, title=None):
         #return getattr(self, 'find')('artist', artist, 'title', title)
         if title:
             return self.find('artist', artist, 'title', title)
         return self.find('artist', artist)
 
-    def fuzzy_find(self, art):
+    def fuzzy_find_artist(self, art):
         """
         Controls presence of artist in music library.
         Crosschecking artist names with SimaStr objects / difflib / levenshtein
@@ -123,14 +141,13 @@ class PlayerClient(Player):
         """
         matching_artists = list()
         artist = SimaStr(art)
-        all_artists = self.list('artist')
 
         # Check against the actual string in artist list
-        if artist.orig in all_artists:
+        if artist.orig in self.artists:
             self.log.debug('found exact match for "%s"' % artist)
             return [artist]
         # Then proceed with fuzzy matching if got nothing
-        match = get_close_matches(artist.orig, all_artists, 50, 0.73)
+        match = get_close_matches(artist.orig, self.artists, 50, 0.73)
         if not match:
             return []
         self.log.debug('found close match for "%s": %s' %
@@ -172,13 +189,20 @@ class PlayerClient(Player):
 
     def find_albums(self, artist):
         """
-        Special wrapper around album search:
-        Album lookup is made through AlbumArtist/Album instead of Artist/Album
+        Fetch all albums for "AlbumArtist" == artist
+        Filter albums returned for "artist" == artist since MPD returns any
+               album containing at least a single track for artist
         """
-        alb_art_search = self.list('album', 'albumartist', artist,)
-        if alb_art_search:
-            return alb_art_search
-        return self.list('album', 'artist', artist)
+        albums = set()
+        albums.update(self.list('album', 'albumartist', artist))
+        for album in self.list('album', 'artist', artist):
+            arts = set([trk.artist for trk in self.find('album', album)])
+            if len(arts) < 2:
+                albums.add(album)
+            else:
+                self.log.debug('"{0}" probably not an album of "{1}"'.format(
+                             album, artist) + '({0})'.format('/'.join(arts)))
+        return albums
 
     def monitor(self):
         curr = self.current
@@ -188,6 +212,8 @@ class PlayerClient(Player):
             ret = self._client.fetch_idle()
             if self.__skipped_track(curr):
                 ret.append('skipped')
+            if 'database' in ret:
+                self._flush_cache()
             return ret
         except (MPDError, IOError) as err:
             raise PlayerError("Couldn't init idle: %s" % err)
@@ -200,6 +226,10 @@ class PlayerClient(Player):
         object"""
         self._client.add(track.file)
 
+    @property
+    def artists(self):
+        return self._cache.get('artists')
+
     @property
     def state(self):
         return str(self._client.status().get('state'))
@@ -266,6 +296,7 @@ class PlayerClient(Player):
                 raise PlayerError('Could connect to "%s", '
                                   'but command "%s" not available' %
                                   (host, nddcmd))
+        self._flush_cache()
 
     def disconnect(self):
         # Try to tell MPD we're closing the connection first
diff --git a/sima/lib/player.py b/sima/lib/player.py
index ea84a64..751e464 100644
--- a/sima/lib/player.py
+++ b/sima/lib/player.py
@@ -3,7 +3,7 @@
 # TODO:
 # Add decorator to filter through history?
 
-# standart library import
+# standard library import
 import logging
 
 # local import
@@ -16,6 +16,14 @@ class Player(object):
 
     When querying player music library for tracks, Player instance *must* return
     Track objects (usually a list of them)
+
+    Player instance should expose the following immutable attributes:
+        * artists
+        * state
+        * current
+        * queue
+        * playlist
+        *
     """
 
     def __init__(self):
@@ -65,10 +73,10 @@ class Player(object):
         """
         raise NotImplementedError
 
-    def fuzzy_find(self, artist):
+    def fuzzy_find_artist(self, artist):
         """
         Find artists based on a fuzzy search in the media library
-            >>> bea = player.fuzzy_find('beatles')
+            >>> bea = player.fuzzy_find_artist('beatles')
             >>> print(bea)
             >>> ['The Beatles']
 
diff --git a/sima/plugins/contrib/placeholder.py b/sima/plugins/contrib/placeholder.py
index 665668d..60e34b5 100644
--- a/sima/plugins/contrib/placeholder.py
+++ b/sima/plugins/contrib/placeholder.py
@@ -2,10 +2,10 @@
 """Crops playlist
 """
 
-# standart library import
+# standard library import
 #from select import select
 
-# third parties componants
+# third parties components
 
 # local import
 from sima.lib.plugin import Plugin
diff --git a/sima/plugins/internal/crop.py b/sima/plugins/internal/crop.py
index d4df0c3..a38b45d 100644
--- a/sima/plugins/internal/crop.py
+++ b/sima/plugins/internal/crop.py
@@ -2,10 +2,10 @@
 """Crops playlist
 """
 
-# standart library import
+# standard library import
 #from select import select
 
-# third parties componants
+# third parties components
 
 # local import
 from ...lib.plugin import Plugin
diff --git a/sima/plugins/internal/lastfm.py b/sima/plugins/internal/lastfm.py
index 03cea6c..1b9ae41 100644
--- a/sima/plugins/internal/lastfm.py
+++ b/sima/plugins/internal/lastfm.py
@@ -3,14 +3,14 @@
 Fetching similar artists from last.fm web services
 """
 
-# standart library import
+# standard library import
 import random
 
 from collections import deque
 from itertools import dropwhile
 from hashlib import md5
 
-# third parties componants
+# third parties components
 
 # local import
 from ...lib.plugin import Plugin
@@ -96,11 +96,9 @@ class Lastfm(Plugin):
         else:
             self.log.info('Lastfm: Initialising cache!')
         self._cache = {
-                'artists': None,
                 'asearch': dict(),
                 'tsearch': dict(),
                 }
-        self._cache['artists'] = frozenset(self.player.list('artist'))
 
     def _cleanup_cache(self):
         """Avoid bloated cache
@@ -184,10 +182,10 @@ class Lastfm(Plugin):
         Look in player library for availability of similar artists in
         similarities
         """
-        dynamic = int(self.plugin_conf.get('dynamic'))
+        dynamic = self.plugin_conf.getint('dynamic')
         if dynamic <= 0:
             dynamic = 100
-        similarity = int(self.plugin_conf.get('similarity'))
+        similarity = self.plugin_conf.getint('similarity')
         results = list()
         similarities.reverse()
         while (len(results) < dynamic
@@ -195,7 +193,7 @@ class Lastfm(Plugin):
             art_pop, match = similarities.pop()
             if match < similarity:
                 break
-            results.extend(self.player.fuzzy_find(art_pop))
+            results.extend(self.player.fuzzy_find_artist(art_pop))
         results and self.log.debug('Similarity: %d%%' % match) # pylint: disable=w0106
         return results
 
@@ -231,7 +229,7 @@ class Lastfm(Plugin):
         depth = 0
         current = self.player.current
         extra_arts = list()
-        while depth < int(self.plugin_conf.get('depth')):
+        while depth < self.plugin_conf.getint('depth'):
             if len(history) == 0:
                 break
             trk = history.popleft()
@@ -282,29 +280,6 @@ class Lastfm(Plugin):
         # artist first.
         return self._get_artists_list_reorg(ret)
 
-    def _detects_var_artists_album(self, album, artist):
-        """Detects either an album is a "Various Artists" or a
-        single artist release."""
-        art_first_track = None
-        for track in self.player.find_album(artist, album):
-            if not art_first_track:  # set artist for the first track
-                art_first_track = track.artist
-            alb_art = track.albumartist
-            #  Special heuristic used when AlbumArtist is available
-            if (alb_art):
-                if artist == alb_art:
-                    # When album artist field is similar to the artist we're
-                    # looking an album for, the album is considered good to
-                    # queue
-                    return False
-                else:
-                    self.log.debug(track)
-                    self.log.debug('album art says "%s", looking for "%s",'
-                                   ' not queueing this album' %
-                                   (alb_art, artist))
-                    return True
-        return False
-
     def _get_album_history(self, artist=None):
         """Retrieve album history"""
         duration = self.daemon_conf.getint('sima', 'history_duration')
@@ -318,10 +293,10 @@ class Lastfm(Plugin):
         """
         self.to_add = list()
         nb_album_add = 0
-        target_album_to_add = int(self.plugin_conf.get('album_to_add'))
+        target_album_to_add = self.plugin_conf.getint('album_to_add')
         for artist in artists:
             self.log.info('Looking for an album to add for "%s"...' % artist)
-            albums = set(self.player.find_albums(artist))
+            albums = self.player.find_albums(artist)
             # albums yet in history for this artist
             albums_yet_in_hist = albums & self._get_album_history(artist=artist)
             albums_not_in_hist = list(albums - albums_yet_in_hist)
@@ -332,9 +307,7 @@ class Lastfm(Plugin):
             album_to_queue = str()
             random.shuffle(albums_not_in_hist)
             for album in albums_not_in_hist:
-                tracks = self.player.find('album', album)
-                if self._detects_var_artists_album(album, artist):
-                    continue
+                tracks = self.player.find_album(artist, album)
                 if tracks and self.sdb.get_bl_album(tracks[0], add_not=True):
                     self.log.info('Blacklisted album: "%s"' % album)
                     self.log.debug('using track: "%s"' % tracks[0])
@@ -361,7 +334,7 @@ class Lastfm(Plugin):
         """Get some tracks for track queue mode
         """
         artists = self.get_local_similar_artists()
-        nbtracks_target = int(self.plugin_conf.get('track_to_add'))
+        nbtracks_target = self.plugin_conf.getint('track_to_add')
         for artist in artists:
             self.log.debug('Trying to find titles to add for "{}"'.format(
                            artist))
diff --git a/sima/plugins/internal/randomfallback.py b/sima/plugins/internal/randomfallback.py
index f2eaac4..07201c0 100644
--- a/sima/plugins/internal/randomfallback.py
+++ b/sima/plugins/internal/randomfallback.py
@@ -3,10 +3,10 @@
 Fetching similar artists from last.fm web services
 """
 
-# standart library import
+# standard library import
 import random
 
-# third parties componants
+# third parties components
 
 # local import
 from ...lib.plugin import Plugin