yet another playbin refactor

src/playbin.vala

@@ -35,48 +35,31 @@ public class Playbin : GLib.Object {
         }
     }
 
+    // invariant: equal to play queue length iff state is STOPPED
     public uint play_queue_position { get; private set; }
 
-    public signal void now_playing (Subsonic.Song now, Subsonic.Song? next);
+    // signalled when a new track is current
+    public signal void new_track ();
+    // signalled when the last track is over
     public signal void stopped ();
 
+    // set to false when manually switching tracks
+    private bool inc_position;
+
+    // these are mostly synced with mpv
     public double position { get; private set; default = 0.0; }
     public double duration { get; private set; default = 0.0; }
 
     public Subsonic.Client api { get; set; default = null; }
 
-    public ListStore play_queue { get; private set; }
+    public ListStore _play_queue;
+    public ListModel play_queue { get { return this._play_queue; } }
 
-    private void on_play_queue_items_changed (ListModel play_queue, uint position, uint removed, uint added) {
+    // try to prevent wait_event to be called twice
-        for (uint i = 0; i < removed; i += 1) {
+    private bool is_handling_event = false;
-            assert (this.mpv.command ({
-                "playlist-remove",
-                position.to_string (),
-            }) >= 0);
-        }
-
-        for (uint i = 0; i < added; i += 1) {
-            assert (this.mpv.command ({
-                "loadfile",
-                this.api.stream_uri (((Subsonic.Song) play_queue.get_item (position+i)).id),
-                "insert-at-play",
-                (position+i).to_string (),
-            }) >= 0);
-        }
-
-        if (this.play_queue_position == position && removed > 0) {
-            if (this.play_queue_position < this.play_queue.get_n_items ()) {
-                // edge case: new track plays, playlist-pos doesn't change, so now_playing never n gets triggered
-                this.now_playing (
-                    (Subsonic.Song) this.play_queue.get_item (this.play_queue_position),
-                    (Subsonic.Song?) this.play_queue.get_item (this.play_queue_position+1));
-            }
-        }
-    }
 
     public Playbin () {
-        this.play_queue = new ListStore (typeof (Subsonic.Song));
+        this._play_queue = new ListStore (typeof (Subsonic.Song));
-        this.play_queue.items_changed.connect (this.on_play_queue_items_changed);
 
         assert (this.mpv.initialize () >= 0);
         assert (this.mpv.set_property_string ("user-agent", Audrey.Const.user_agent) >= 0);
@@ -86,9 +69,13 @@ public class Playbin : GLib.Object {
         assert (this.mpv.observe_property (0, "time-pos", Mpv.Format.DOUBLE) >= 0);
         assert (this.mpv.observe_property (1, "duration", Mpv.Format.DOUBLE) >= 0);
         assert (this.mpv.observe_property (2, "playlist-pos", Mpv.Format.INT64) >= 0);
+        assert (this.mpv.observe_property (3, "pause", Mpv.Format.FLAG) >= 0);
 
         this.mpv.wakeup_callback = () => {
             Idle.add (() => {
+                if (this.is_handling_event) return false;
+                this.is_handling_event = true;
+
                 while (true) {
                     var event = this.mpv.wait_event (0.0);
                     if (event.event_id == Mpv.EventId.NONE) break;
@@ -116,18 +103,35 @@ public class Playbin : GLib.Object {
                             break;
 
                         case 2:
+                            // here as a sanity check
+                            // should always match our own play_queu_position/state
                             assert (data.name == "playlist-pos");
-                            if (data.parse_int64 () < 0) {
+                            int64 playlist_pos = data.parse_int64 ();
-                                debug ("playlist-pos is null, sending stopped event");
+                            if (playlist_pos < 0) {
-                                this.play_queue_position = this.play_queue.get_n_items ();
+                                if (this.state != PlaybinState.STOPPED) {
-                                this.state = PlaybinState.STOPPED;
+                                    error ("mpv has no current playlist entry, but we think it's index %u", this.play_queue_position);
-                                this.stopped ();
+                                }
+                                assert (this.play_queue_position == this.play_queue.get_n_items ());
                             } else {
-                                this.play_queue_position = (uint) data.parse_int64 ();
+                                if (this.state == PlaybinState.STOPPED) {
-                                debug (@"playlist-pos has been updated to $(this.play_queue_position)");
+                                    error ("mpv is at playlist entry %u, but we're stopped", (uint) playlist_pos);
-                                this.now_playing (
+                                }
-                                    (Subsonic.Song) this.play_queue.get_item (this.play_queue_position),
+                                if (this.play_queue_position != (uint) playlist_pos) {
-                                    (Subsonic.Song?) this.play_queue.get_item (this.play_queue_position+1));
+                                    error ("mpv is at playlist entry %u, but we think it's %u", (uint) playlist_pos, this.play_queue_position);
+                                }
+                            }
+                            break;
+
+                        case 3:
+                            // also here as a sanity check
+                            // should always match our own state
+                            assert (data.name == "pause");
+                            bool pause = data.parse_flag ();
+                            if (pause && this.state != PlaybinState.PAUSED) {
+                                error (@"mpv is paused, but we are @(this.state)");
+                            }
+                            if (!pause && this.state == PlaybinState.PAUSED) {
+                                error ("mpv is not paused, but we are paused");
                             }
                             break;
 
@@ -139,6 +143,12 @@ public class Playbin : GLib.Object {
 
                     case Mpv.EventId.START_FILE:
                         debug ("START_FILE received");
+                        if (this.inc_position) {
+                            this.play_queue_position += 1;
+                        } else {
+                            this.inc_position = true;
+                        }
+                        this.new_track ();
                         break;
 
                     case Mpv.EventId.END_FILE:
@@ -147,6 +157,15 @@ public class Playbin : GLib.Object {
                         if (data.error < 0) {
                             warning ("playback of track aborted: %s", data.error.to_string ());
                         }
+
+                        if (this.inc_position) {
+                            // reached the end
+                            this.play_queue_position += 1;
+                            assert (this.play_queue_position == this._play_queue.get_n_items ());
+                            this.state = PlaybinState.STOPPED;
+                            this.stopped ();
+                        }
+
                         break;
                       
                     default:
@@ -155,6 +174,7 @@ public class Playbin : GLib.Object {
                     }
                 }
             
+                this.is_handling_event = false;
                 return false;
             });
         };
@@ -171,6 +191,8 @@ public class Playbin : GLib.Object {
     {
         assert (this.mpv.command ({"playlist-play-index", position.to_string ()}) >= 0);
         this.state = PlaybinState.PLAYING;
+        this.play_queue_position = position;
+        this.inc_position = false;
     }
 
     public void pause () {
@@ -194,15 +216,55 @@ public class Playbin : GLib.Object {
         }
     }
 
-    public void next_track () {
+    public void go_to_next_track ()
-        assert (this.state != PlaybinState.STOPPED);
+            requires (this.state != PlaybinState.STOPPED)
-        this.state = PlaybinState.PLAYING;
+    {
+        if (this.play_queue_position+1 < this._play_queue.get_n_items ()) {
+            this.inc_position = false;
+            this.play_queue_position += 1;
             assert (this.mpv.command ({"playlist-next-playlist"}) >= 0);
+        } else {
+            warning ("tried to skip forward at end of play queue, ignoring");
+        }
     }
 
-    public void prev_track () {
+    public void go_to_prev_track ()
-        assert (this.state != PlaybinState.STOPPED);
+            requires (this.state != PlaybinState.STOPPED)
-        this.state = PlaybinState.PLAYING;
+    {
+        if (this.play_queue_position > 0) {
+            this.inc_position = false;
+            this.play_queue_position -= 1;
             assert (this.mpv.command ({"playlist-prev-playlist"}) >= 0);
+        } else {
+            warning ("tried to skip to prev track at start of play queue, ignoring");
+        }
+    }
+
+    public void remove_track (uint position)
+            requires (position < this._play_queue.get_n_items ())
+    {
+        assert (false); // TODO
+    }
+
+    public void clear () {
+        assert (this.mpv.command({"playlist-clear"}) >= 0);
+        if (this.state != PlaybinState.STOPPED) {
+            assert (this.mpv.command({"playlist-remove", "current"}) >= 0);
+        }
+        this.state = PlaybinState.STOPPED;
+        this._play_queue.remove_all ();
+        this.play_queue_position = 0;
+
+        this.stopped ();
+    }
+
+    public void append_track (Subsonic.Song song) {
+        assert (this.mpv.command({
+            "loadfile",
+            this.api.stream_uri (song.id),
+            "append",
+        }) >= 0);
+        if (this.state == STOPPED) this.play_queue_position += 1;
+        this._play_queue.append (song);
     }
 }

src/ui/play_queue.vala

@@ -28,7 +28,7 @@ class Ui.PlayQueueSong : Gtk.ListBoxRow {
 
         var remove = new SimpleAction ("remove", null);
         remove.activate.connect (() => {
-            this.playbin.play_queue.remove (this.displayed_position-1);
+            this.playbin.remove_track (this.displayed_position-1);
         });
         action_group.add_action (remove);
 
@@ -89,7 +89,7 @@ public class Ui.PlayQueue : Adw.NavigationPage {
     public bool can_clear_all { get; private set; }
 
     [GtkCallback] private void on_clear () {
-        this.playbin.play_queue.remove_all ();
+        this.playbin.clear ();
     }
 
     private void on_store_items_changed (GLib.ListModel store, uint position, uint removed, uint added) {

src/ui/playbar.vala

@@ -45,11 +45,11 @@ class Ui.Playbar : Gtk.Box {
     }
 
     [GtkCallback] private void on_skip_forward_clicked () {
-        this.playbin.next_track ();
+        this.playbin.go_to_next_track ();
     }
 
     [GtkCallback] private void on_skip_backward_clicked () {
-        this.playbin.prev_track ();
+        this.playbin.go_to_prev_track ();
     }
 
     [GtkCallback] private void seek_backward () {

src/ui/window.vala

@@ -33,6 +33,32 @@ class Ui.Window : Adw.ApplicationWindow {
         Object (application: app);
     }
 
+    private void now_playing (Subsonic.Song song) {
+        this.song = song;
+        // api.scrobble.begin (this.song.id); TODO
+
+        if (this.cancel_loading_art != null) {
+            this.cancel_loading_art.cancel ();
+        }
+        this.cancel_loading_art = new GLib.Cancellable ();
+
+        this.playing_cover_art = null; // TODO: preload next art somehow
+        this.cover_art_loading = true;
+
+        string song_id = this.song.id;
+        this.api.cover_art.begin (song_id, this.cancel_loading_art, (obj, res) => {
+            try {
+                this.playing_cover_art = Gdk.Texture.for_pixbuf (this.api.cover_art.end (res));
+                this.cover_art_loading = false;
+            } catch (Error e) {
+                if (!(e is IOError.CANCELLED)) {
+                    warning ("could not load cover for %s: %s", song_id, e.message);
+                    this.cover_art_loading = false;
+                }
+            }
+        });
+    }
+
     construct {
         // TODO: mpris
         // Bus.own_name (
@@ -58,30 +84,8 @@ class Ui.Window : Adw.ApplicationWindow {
 
         this.sidebar.select_row (this.sidebar.get_row_at_index (0));
 
-        this.playbin.now_playing.connect ((playbin, now, next) => {
+        this.playbin.new_track.connect (() => {
-            this.song = now;
+            this.now_playing (this.playbin.play_queue.get_item (this.playbin.play_queue_position) as Subsonic.Song);
-            api.scrobble.begin (this.song.id);
-
-            if (this.cancel_loading_art != null) {
-                this.cancel_loading_art.cancel ();
-            }
-            this.cancel_loading_art = new GLib.Cancellable ();
-
-            this.playing_cover_art = null; // TODO: preload next art somehow
-            this.cover_art_loading = true;
-
-            string song_id = this.song.id;
-            this.api.cover_art.begin (song_id, this.cancel_loading_art, (obj, res) => {
-                try {
-                    this.playing_cover_art = Gdk.Texture.for_pixbuf (this.api.cover_art.end (res));
-                    this.cover_art_loading = false;
-                } catch (Error e) {
-                    if (!(e is IOError.CANCELLED)) {
-                        warning ("could not load cover for %s: %s", song_id, e.message);
-                        this.cover_art_loading = false;
-                    }
-                }
-            });
         });
 
         this.playbin.stopped.connect (() => {
@@ -92,9 +96,9 @@ class Ui.Window : Adw.ApplicationWindow {
         this.shuffle_all_tracks.sensitive = true;
         this.shuffle_all_tracks.activated.connect (() => {
             this.shuffle_all_tracks.sensitive = false;
-            this.playbin.play_queue.remove_all ();
+            this.playbin.clear ();
             api.get_random_songs.begin (null, (song) => {
-                this.playbin.play_queue.append (song);
+                this.playbin.append_track (song);
             }, (obj, res) => {
                 try {
                     api.get_random_songs.end (res);

src/vapi/mpv.vapi

@@ -143,6 +143,12 @@ namespace Mpv {
         {
             return * (double *) data;
         }
+
+        public bool parse_flag ()
+                requires (format == Format.FLAG)
+        {
+            return (* (int *) data) == 1;
+        }
     }
 
     [CCode (cname = "mpv_event_end_file", destroy_function = "", has_type_id = false, has_copy_function = false)]