Merge lp:~mhr3/unity-lens-music/rb-proper-album-handling into lp:unity-lens-music

[Michal Hruby]

Rhythmbox scope: Implement proper album handling

Clicking on music lens's rhythmbox album results plays only random song from the album instead of all songs from the album.

This branch implements proper album handling for rhythmbox, which was missing in the original merge when the scope was merged before beta2.

No tests, as this would require rhythmbox's database and we don't have the infrastructure setup for that. But FWIW this only builds on top of the data structures already present /and tested/ in the scope.

[Alex Launi (alexlauni)]

+1. Looks fine, couldn't test it. RB doesn't work on my machine.

review: Approve (code-only)

[Gord Allott (gordallott)]

+1, seems okay here

review: Approve

=== modified file 'src/rhythmbox-collection.vala'
--- src/rhythmbox-collection.vala	2012-04-23 12:34:57 +0000
+++ src/rhythmbox-collection.vala	2012-04-25 17:47:20 +0000
@@ -43,10 +43,12 @@
 
     class RhythmboxCollection : Object
     {
+        const string UNKNOWN_ALBUM = "Unknown";
 
         SequenceModel all_tracks;
         ModelTag<int> album_art_tag;
         FilterModel tracks_by_play_count;
+        HashTable<string, GenericArray<ModelIter>> album_to_tracks_map;
 
         TDB.Database album_art_tdb;
         FileMonitor tdb_monitor;
@@ -153,6 +155,9 @@
               case "album-artist":
                 if (current_data >= 0) current_data = -1;
                 break;
+              case "hidden":
+                if (processing_song) processing_song = false;
+                break;
               case "entry":
                 if (processing_song && current_track != null)
                 {
@@ -212,6 +217,9 @@
                                  "s", "s", "i", "i", "i");
           assert (all_tracks.get_schema ().length == Columns.N_COLUMNS);
           album_art_tag = new ModelTag<int> (all_tracks);
+          album_to_tracks_map =
+            new HashTable<string, GenericArray<ModelIter>> (str_hash,
+                                                            str_equal);
 
           var filter = Dee.Filter.new_sort ((row1, row2) =>
           {
@@ -333,6 +341,22 @@
             return null;
         }
 
+        public SList<unowned string> get_album_tracks (string album_key)
+        {
+          SList<unowned string> results = new SList<unowned string> ();
+
+          var iter_arr = album_to_tracks_map[album_key];
+          if (iter_arr != null)
+          {
+            for (int i = iter_arr.length - 1; i >= 0; i--)
+            {
+              results.prepend (all_tracks.get_string (iter_arr[i], Columns.URI));
+            }
+          }
+
+          return results;
+        }
+
         private Variant cached_variant_for_string (string? input)
         {
           unowned string text = input != null ? input : "";
@@ -436,6 +460,7 @@
           all_tracks.clear ();
           initialize_index ();
           current_album_art_tag = 0;
+          album_to_tracks_map.remove_all ();
 
           var parser = new XmlParser ();
           parser.track_info_ready.connect ((track) =>
@@ -448,7 +473,17 @@
               track.artwork_path = "audio-x-generic";
 
             prepare_row_buffer (track);
-            all_tracks.append_row (row_buffer);
+            var iter = all_tracks.append_row (row_buffer);
+
+            if (track.album == UNKNOWN_ALBUM) return;
+            var album_key = "%s - %s".printf (track.album, track.album_artist != null ? track.album_artist : track.artist);
+            var arr = album_to_tracks_map[album_key];
+            if (arr == null)
+            {
+              arr = new GenericArray<ModelIter> ();
+              album_to_tracks_map[album_key] = arr;
+            }
+            arr.add (iter);
           });
 
           var file = File.new_for_path (path);
@@ -468,10 +503,28 @@
           {
             warning ("Error while parsing rhythmbox DB: %s", err.message);
           }
+
+          GLib.List<unowned string> all_albums = album_to_tracks_map.get_keys ();
+          foreach (unowned string s in all_albums)
+          {
+            album_to_tracks_map[s].sort_with_data ((ref a, ref b) =>
+            {
+              var trackno1 = all_tracks.get_int32 (a, Columns.TRACK_NUMBER);
+              var trackno2 = all_tracks.get_int32 (b, Columns.TRACK_NUMBER);
+
+              return trackno1 - trackno2;
+            });
+          }
+        }
+
+        private enum ResultType
+        {
+          ALBUM,
+          SONG
         }
 
         private void add_result (Model results_model, Model model,
-                                 ModelIter iter, Columns title_col,
+                                 ModelIter iter, ResultType result_type,
                                  uint category_id)
         {
           // check for updated album art
@@ -503,13 +556,35 @@
             album_art_tag[model, iter] = current_album_art_tag;
           }
 
-          results_model.append (model.get_string (iter, Columns.URI),
+          var title_col = result_type == ResultType.SONG ?
+            Columns.TITLE : Columns.ALBUM;
+          unowned string title = model.get_string (iter, title_col);
+          var uri = model.get_string (iter, Columns.URI);
+          var dnd_uri = model.get_string (iter, Columns.URI);
+          if (result_type == ResultType.ALBUM)
+          {
+            if (title == UNKNOWN_ALBUM) return;
+            unowned string artist = model.get_string (iter,
+                                                      Columns.ALBUM_ARTIST);
+            if (artist == "")
+              artist = model.get_string (iter, Columns.ARTIST);
+            var album_key = "%s - %s".printf (title, artist);
+            StringBuilder sb = new StringBuilder ();
+            foreach (unowned string track_uri in get_album_tracks (album_key))
+            {
+              sb.append_printf ("%s\r\n", track_uri);
+            }
+            dnd_uri = (owned) sb.str;
+            uri = "album://%s".printf (album_key);
+          }
+
+          results_model.append (uri,
                                 model.get_string (iter, Columns.ARTWORK),
                                 category_id,
                                 model.get_string (iter, Columns.MIMETYPE),
-                                model.get_string (iter, title_col),
+                                title,
                                 model.get_string (iter, Columns.ARTIST),
-                                model.get_string (iter, Columns.URI));
+                                dnd_uri);
         }
 
         public void search (LensSearch search, 
@@ -528,6 +603,14 @@
             get_decade_filter (filters, out min_year, out max_year);
             var active_genres = get_genre_filter (filters);
 
+            // we need this to be able to sort the albums properly
+            var helper_model = search.results_model;
+            if (category_override >= 0)
+            {
+              helper_model = new Dee.SequenceModel ();
+              helper_model.set_schema_full (search.results_model.get_schema ());
+            }
+
             if (empty_search)
             {
                 // display a couple of most played songs
@@ -568,14 +651,14 @@
                             category_override : Category.ALBUMS;
                         
                         add_result (search.results_model, model, iter,
-                                    Columns.ALBUM, category_id);
+                                    ResultType.ALBUM, category_id);
                     }
 
                     category_id = category_override >= 0 ?
                         category_override : Category.SONGS;
 
-                    add_result (search.results_model, model, iter,
-                                Columns.TITLE, category_id);
+                    add_result (helper_model, model, iter,
+                                ResultType.SONG, category_id);
 
                     num_results++;
                     if (max_results >= 0 && num_results >= max_results) break;
@@ -660,20 +743,33 @@
                         category_override : Category.ALBUMS;
 
                     add_result (search.results_model, model, model_iter,
-                                Columns.ALBUM, category_id);
+                                ResultType.ALBUM, category_id);
                 }
 
                 category_id = category_override >= 0 ?
                     category_override : Category.SONGS;
 
-                add_result (search.results_model, model, model_iter,
-                            Columns.TITLE, category_id);
+                add_result (helper_model, model, model_iter,
+                            ResultType.SONG, category_id);
 
                 num_results++;
                 if (max_results >= 0 && num_results >= max_results) break;
 
                 seq_iter = seq_iter.next ();
             }
+
+            if (helper_model == search.results_model) return;
+
+            // we need to do this because the dash doesn't care about position
+            // of a newly added rows in the model - it just appends it
+            var iter = helper_model.get_first_iter ();
+            var last = helper_model.get_last_iter ();
+            while (iter != last)
+            {
+              var row = helper_model.get_row (iter);
+              search.results_model.append_row (row);
+              iter = helper_model.next (iter);
+            }
         }
 
         private void get_decade_filter (GLib.List<FilterParser> filters,

=== modified file 'src/rhythmbox-scope.vala'
--- src/rhythmbox-scope.vala	2012-04-23 12:34:57 +0000
+++ src/rhythmbox-scope.vala	2012-04-25 17:47:20 +0000
@@ -45,35 +45,58 @@
     protected override int num_results_global_search { get { return 20; } }
     protected override int num_results_lens_search { get { return 50; } }
 
+    private async void prepare_rhythmbox_play_queue (string[] main_exec)
+    {
+      Thread.create<void> (() =>
+      {
+        try
+        {
+          Process.spawn_command_line_sync ("rhythmbox-client --pause");
+          Process.spawn_command_line_sync ("rhythmbox-client --clear-queue");
+          Process.spawn_sync (null, main_exec, null, SpawnFlags.SEARCH_PATH,
+                              null, null);
+          Process.spawn_command_line_sync ("rhythmbox-client --next");
+          // make sure we're playing
+          Process.spawn_command_line_sync ("rhythmbox-client --play");
+        }
+        catch (Error err)
+        {
+          warning ("%s", err.message);
+        }
+        Idle.add (prepare_rhythmbox_play_queue.callback);
+      }, false);
+      yield;
+    }
     /**
      * Tells banshee to play the selected uri(s)
      */
     public Unity.ActivationResponse activate (string uri)
     {
-      string[] exec = {"rhythmbox-client", "--play-uri"};
+      string[] exec = {"rhythmbox-client"};
 
-      try {
+      try 
+      {
         if (Uri.parse_scheme (uri) == "album")
-          {
-            debug (@"searching for tracks for $uri");
-            string[] split = uri.split ("/");
-            string artist = split[2];
-            string title = split[3];
-        
-            Album album = new Album ();
-            album.artist = artist;
-            album.title = title;
-            // FIXME there must be a better way..
-//            foreach (string track in collection.get_track_uris (album))
-//              exec += track;
-          }
+        {
+            // this will be a bit crazy, let's do it in a separate thread
+            exec += "--enqueue";
+            string album_key = uri.substring (8);
+            foreach (unowned string track_uri in collection.get_album_tracks (album_key))
+            {
+                exec += track_uri;
+            }
+            exec += null;
+            prepare_rhythmbox_play_queue.begin (exec);
+            return new Unity.ActivationResponse (Unity.HandledType.HIDE_DASH);
+        }
         else
-          {
+        {
+            exec += "--play-uri";
             exec += uri;
-          }
-    
+        }
+
         exec += null;
-    
+
         debug ("Spawning rb '%s'", string.joinv (" ", exec));
         Process.spawn_async (null, 
                  exec,