Merge with Upstream

1 files changed, 39 insertions, 39 deletions

diff --git a/src/mpris2-controller.vala b/src/mpris2-controller.vala
index 2975066..8eeac08 100644
--- a/src/mpris2-controller.vala
+++ b/src/mpris2-controller.vala
@@ -19,10 +19,6 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 using Dbusmenu;
 using Transport;
 
-/*
- This class will entirely replace mpris-controller.vala hence why there is no
- point in trying to get encorporate both into the same object model. 
- */
 public class Mpris2Controller : GLib.Object
 {
   public MprisRoot mpris2_root {get; construct;}    
@@ -60,12 +56,22 @@ public class Mpris2Controller : GLib.Object
       critical("Problems connecting to the session bus - %s", e.message);
     }
   }
-
+  /*
+   * property_changed_cb
+   * Called when a property changed signal is emitted from any of mpris
+   * objects on the bus.
+   * Note that the signal will be received by each instance for each player
+   * and at that moment there is no way to know what player that signal 
+   * came from therefore it is necessary to query each relevant property
+   * to update the respective dbusmenuitem property inorder to keep the UI in sync
+   * Please also note due to some race condition in the depths of gdbus
+   * a timeout is needed between receiving the prop update and query the respective property.
+   * This can be seen at various points below.
+   */
   public void property_changed_cb ( string interface_source,
                                     HashTable<string, Variant?> changed_properties,
                                     string[] invalid )
   {
-    //debug("properties-changed for interface %s and owner %s", interface_source, this.owner.dbus_name);
     if ( changed_properties == null ||
         interface_source.has_prefix ( MPRIS_PREFIX ) == false ){
       warning("Property-changed hash is null or this is an interface that doesn't concern us");
@@ -73,31 +79,18 @@ public class Mpris2Controller : GLib.Object
     }
     Variant? play_v = changed_properties.lookup("PlaybackStatus");
     if(play_v != null){
-      // Race condition sometimes appears with the playback status 
-      // 200ms timeout ensures we have the correct playback status at all times.
       string state = this.player.PlaybackStatus;
-      //debug("in the property update and the playback status = %s and update = %s", state, (string)play_v);
       Timeout.add ( 200, ensure_correct_playback_status );
       Transport.State p = (Transport.State)this.determine_play_state(state);
       (this.owner.custom_items[PlayerController.widget_order.TRANSPORT] as TransportMenuitem).change_play_state(p);
     }
     Variant? meta_v = changed_properties.lookup("Metadata");
-    if(meta_v != null){
-      GLib.HashTable<string, Variant?> changed_updates = clean_metadata();      
-      PlayerItem metadata = this.owner.custom_items[PlayerController.widget_order.METADATA];
-      metadata.reset (MetadataMenuitem.relevant_attributes_for_ui());
-      metadata.update ( changed_updates, 
-                        MetadataMenuitem.relevant_attributes_for_ui());
-      MetadataMenuitem md = this.owner.custom_items[PlayerController.widget_order.METADATA] as MetadataMenuitem;      
-      bool collapsing = !metadata.populated(MetadataMenuitem.relevant_attributes_for_ui());
-      md.should_collapse(collapsing);
-      
-      debug ("Should metadata collapse %s", collapsing.to_string());
+    if(meta_v != null)
+    {
+      Timeout.add ( 200, ensure_correct_metadata );
     }
     Variant? playlist_v = changed_properties.lookup("ActivePlaylist");
     if ( playlist_v != null && this.owner.use_playlists == true ){
-      // Once again A GDBus race condition, the property_changed signal is sent
-      // before the value is set on the respective property.
       Timeout.add (300, this.fetch_active_playlist);
     }
     Variant? playlist_count_v = changed_properties.lookup("PlaylistCount");
@@ -116,9 +109,23 @@ public class Mpris2Controller : GLib.Object
       md.alter_label (this.mpris2_root.Identity);
     }
   }
-                                      
-  private bool ensure_correct_playback_status(){
-    //debug("TEST playback status = %s", this.player.PlaybackStatus);
+         
+  private bool ensure_correct_metadata ()
+  {
+    GLib.HashTable<string, Variant?> changed_updates = clean_metadata();      
+    PlayerItem metadata = this.owner.custom_items[PlayerController.widget_order.METADATA];
+    metadata.reset (MetadataMenuitem.relevant_attributes_for_ui());
+    metadata.update ( changed_updates, 
+                      MetadataMenuitem.relevant_attributes_for_ui());
+    MetadataMenuitem md = this.owner.custom_items[PlayerController.widget_order.METADATA] as MetadataMenuitem;      
+    bool collapsing = !metadata.populated(MetadataMenuitem.relevant_attributes_for_ui());
+    md.should_collapse(collapsing);
+    
+    return false;
+  }
+  
+  private bool ensure_correct_playback_status()
+  {
     Transport.State p = (Transport.State)this.determine_play_state(this.player.PlaybackStatus);
     (this.owner.custom_items[PlayerController.widget_order.TRANSPORT] as TransportMenuitem).change_play_state(p);
     return false;
@@ -130,19 +137,16 @@ public class Mpris2Controller : GLib.Object
     
     Variant? artist_v = this.player.Metadata.lookup("xesam:artist");
     if(artist_v != null){
-      Variant? v_artists = this.player.Metadata.lookup("xesam:artist");
-      //debug("artists is of type %s", v_artists.get_type_string ());
       string display_artists;
-      if(v_artists.get_type_string() == "s"){
-        //debug("SPOTIFY is that you ?");
-        display_artists = v_artists.get_string();
+      // Accomodate Spotify (should return 'as' and not 's') 
+      if(artist_v.get_type_string() == "s"){
+        display_artists = artist_v.get_string();
       }
       else{
-       string[] artists = v_artists.dup_strv();
+        string[] artists = artist_v.dup_strv();
         display_artists = string.joinv(", ", artists);
       }
       changed_updates.replace("xesam:artist", display_artists);
-      //debug("artist : %s", (string)changed_updates.lookup("xesam:artist"));
     }
     return changed_updates;
   }
@@ -181,7 +185,6 @@ public class Mpris2Controller : GLib.Object
 
   public void transport_update(Transport.Action command)
   {
-    //debug("transport_event input = %i", (int)command);
     if(command == Transport.Action.PLAY_PAUSE){
       this.player.PlayPause.begin();              
     }
@@ -192,11 +195,9 @@ public class Mpris2Controller : GLib.Object
       this.player.Next.begin();
     }
     else if(command == Transport.Action.REWIND){
-     //debug("transport_event rewind = %i", (int)command);
      this.player.Seek.begin(-500000);
     }
     else if(command == Transport.Action.FORWIND){
-     //debug("transport_event input = %i", (int)command);
      this.player.Seek.begin(400000);
     }
   }
@@ -230,12 +231,10 @@ public class Mpris2Controller : GLib.Object
                                                               false);
     }
     catch (IOError e){
-      //debug("Could not fetch playlists because %s", e.message);
       return;
     }
     
     if( current_playlists != null ){
-      //debug( "Size of the playlist array = %i", current_playlists.length );
       PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
       playlists_item.update(current_playlists);
     }
@@ -248,7 +247,8 @@ public class Mpris2Controller : GLib.Object
   private bool fetch_active_playlist()
   {    
     if (this.playlists.ActivePlaylist.valid == false){
-      //debug(" We don't have an active playlist");
+      // TODO 
+      // What happens here ?
     }    
     PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
     playlists_item.active_playlist_update ( this.playlists.ActivePlaylist.details );
@@ -261,7 +261,7 @@ public class Mpris2Controller : GLib.Object
       this.playlists.ActivatePlaylist.begin(path);
     }
     catch(IOError e){
-      //debug("Could not activate playlist %s because %s", (string)path, e.message);
+      warning ("Could not activate playlist %s because %s", (string)path, e.message);
     }
   }
 }