gooder gapless track switching

src/playbin.vala

@@ -3,7 +3,8 @@ class Playbin : Object {
     // lets us access the about-to-finish signal
     private dynamic Gst.Element playbin;
 
-    private SourceFunc? async_done_callback;
+    // used to prevent stale seeks
+    private uint64 stream_counter = 0;
 
     // cubic: recommended for media player volume sliders?
     public double volume {
@@ -20,10 +21,11 @@ class Playbin : Object {
         set { this.playbin.mute = value; }
     }
 
+    private bool seeking = false;
     public int64 position { get; private set; }
     public int64 duration { get; private set; }
 
-    public signal void stream_started ();
+    public signal void stream_started (string uri);
     public signal void stream_over ();
 
     private void source_setup (Gst.Element playbin, dynamic Gst.Element source) {
@@ -39,12 +41,14 @@ class Playbin : Object {
 
         // regularly update position
         Timeout.add (500, () => {
+            if (!this.seeking) {
                 int64 new_position;
                 if (this.playbin.query_position (Gst.Format.TIME, out new_position)) {
                     this.position = new_position < this.duration ? new_position : this.duration;
                 } else {
                     this.position = 0;
                 }
+            }
 
             // keep rerunning
             return true;
@@ -67,122 +71,69 @@ class Playbin : Object {
             warning ("gst playbin bus warning: %s", err.message);
         });
 
-        bus.message["async-done"].connect ((message) => {
+        bus.message["state-changed"].connect ((message) => {
-            assert (this.async_done_callback != null);
+            if (message.src != this.playbin) return;
-            var cb = (owned) this.async_done_callback;
+        
-            assert (this.async_done_callback == null); // sanity check
+            Gst.State old_state;
-            cb ();
+            Gst.State new_state;
+            message.parse_state_changed (out old_state, out new_state, null);
+
+            switch (new_state) {
+            case Gst.State.NULL:
+                break;
+
+            case Gst.State.READY:
+                break;
+
+            case Gst.State.PAUSED:
+                break;
+
+            case Gst.State.PLAYING:
+                if (this.queued_seek_position < 0) {
+                    this.seeking = false;
+                } else if (this.queued_seek_counter == this.stream_counter) {
+                    assert (this.playbin.seek_simple (Gst.Format.TIME, Gst.SeekFlags.FLUSH, this.queued_seek_position));
+                }
+                this.queued_seek_position = -1;
+                break;
+            }
         });
 
         bus.message["stream-start"].connect ((message) => {
+            this.stream_counter += 1;
+
             int64 new_duration;
             assert (this.playbin.query_duration (Gst.Format.TIME, out new_duration));
             this.duration = new_duration;
 
-            string? next_uri = null;
+            this.stream_started ((string) this.playbin.current_uri);
-
-            this.next_uri_lock.lock ();
-            next_uri = this.next_uri;
-
-            if (next_uri != (string) this.playbin.current_uri) {
-                this.next_uri_lock.unlock ();
-                // WHOOPS! didn't actually switch to the track the play queue wanted
-                // we can still fix this though
-                assert (next_uri != null);
-                this.playbin.set_state (Gst.State.READY);
-                this.playbin.uri = next_uri;
-                this.playbin.set_state (Gst.State.PLAYING);
-                // no one will ever know
-            } else {
-                this.next_uri = null;
-                this.next_uri_lock.unlock ();
-
-                this.stream_started ();
-            }
         });
         
         bus.message["eos"].connect ((message) => {
-            string next_uri;
-
-            this.next_uri_lock.lock ();
-            next_uri = this.next_uri;
-            this.next_uri_lock.unlock ();
-
-            if (next_uri == null) {
-                // no next track was arranged, we're done
             this.stream_over ();
-            }
         });
     }
 
-    private async void set_state (Gst.State state) {
+    private uint64 queued_seek_counter;
-        assert (this.async_done_callback == null);
+    private int64 queued_seek_position = -1;
 
-        switch (this.playbin.set_state (state)) {
+    public void seek (int64 position) {
-        case Gst.StateChangeReturn.SUCCESS:
+        if (this.playbin.seek_simple (Gst.Format.TIME, Gst.SeekFlags.FLUSH/* | Gst.SeekFlags.KEY_UNIT*/, position)) {
-            break;
+            this.seeking = true;
-
+            this.position = position;
-        case Gst.StateChangeReturn.ASYNC:
+        } else {
-            this.async_done_callback = this.set_state.callback;
+            // queue this seek
-            yield;
+            this.queued_seek_counter = this.stream_counter;
-            break;
+            this.queued_seek_position = position;
-
-        default:
-            assert (false);
-            break;
         }
     }
 
-    public async bool seek (int64 position) {
+    public void play_now (string uri) {
-        // don't actually seek if an operation is pending
+        this.playbin.set_state (Gst.State.READY);
-        if (this.async_done_callback != null) {
-            return false;
-        }
-
-        // ASSUMPTION: this can only work asynchronously
-        // (will wait for an ASYNC_DONE in the bus)
-        assert (this.playbin.seek_simple (Gst.Format.TIME, Gst.SeekFlags.FLUSH | Gst.SeekFlags.KEY_UNIT, position));
-        this.async_done_callback = this.seek.callback;
-        yield;
-
-        return true;
-    }
-
-    private string? play_now_queued;
-    // returns true if the "play now" request wasn't overriden by a later call
-    public async bool play_now (string uri) {
-        if (this.async_done_callback != null) {
-            // an operation was already pending
-            // last writer wins here
-            this.play_now_queued = uri;
-            // idle spinning is probably fine
-            while (this.async_done_callback != null) {
-                Idle.add (this.play_now.callback);
-                yield;
-                // only keep spinning if we can still win
-                if (this.play_now_queued != uri) {
-                    return false;
-                }
-            }
-            this.play_now_queued = null;
-        }
-
-        // pretend this track was locked in by about-to-finish before
-        this.next_uri_lock.lock ();
-        this.next_uri = uri;
-        this.next_uri_lock.unlock ();
-
-        yield this.set_state (Gst.State.READY);
         this.playbin.uri = uri;
-        yield this.set_state (Gst.State.PLAYING);
+        this.playbin.set_state (Gst.State.PLAYING);
 
-        if (this.play_now_queued != null) {
+        this.set_next_uri (uri);
-            // another "play now" was queued while we were busy
-            // defer to it
-            return false;
-        }
-
-        return true;
     }
 
     Mutex next_uri_lock;

src/ui/play_queue.vala

@@ -55,8 +55,15 @@ public class Ui.PlayQueue : Adw.NavigationPage {
         }
     }
 
-    internal void on_stream_start (Playbin playbin) {
+    internal void on_stream_start (Playbin playbin, string uri) {
         Song song = (Song) this.songs.get_item (this.next_stream_index);
+        if (public_api.stream_uri (song.id) != uri) {
+            // prerolled track wasnt actually next one!
+            // this can happen if it was deleted from the play queue after about-to-finish was signalled
+            // gapless playback is ruined anyway, go wild
+            this.pick_song (this.next_stream_index);
+            return;
+        }
         this.now_playing (song);
 
         this.ignore_selection = true;
@@ -91,6 +98,16 @@ public class Ui.PlayQueue : Adw.NavigationPage {
         button.clicked.connect (() => {
             this.songs.remove (cell.position);
             this.can_clear_all = this.songs.get_n_items() > 0;
+
+            if (cell.position == this.next_stream_index) {
+                // we just deleted the track that was to be prerolled next
+                // replace it
+                this.play_next ((Song?) this.songs.get_item (cell.position));
+            } else if (cell.position+1 == this.next_stream_index) {
+                // conversely, we just deleted the currently playing track
+                // redo
+                this.pick_song (cell.position);
+            }
         });
     }
 }

src/ui/window.vala

@@ -91,7 +91,7 @@ class Ui.Window : Adw.ApplicationWindow {
             });
 
             this.play_queue.play_now.connect ((song) => {
-                playbin.play_now.begin (api.stream_uri (song.id));
+                playbin.play_now (api.stream_uri (song.id));
             });
             this.play_queue.play_next.connect ((song) => {
                 if (song == null) {
@@ -154,11 +154,11 @@ class Ui.Window : Adw.ApplicationWindow {
     // same timeout logic as https://code.videolan.org/videolan/npapi-vlc/blob/6eae0ffb9cbaf8f6e04423de2ff38daabdf7cae3/npapi/vlcplugin_gtk.cpp#L312
     private uint seek_timeout_id = 0;
     [GtkCallback] private bool on_play_position_seek (Gtk.Range range, Gtk.ScrollType scroll_type, double value) {
+        this.position = (int64) range.adjustment.value;
         if (this.seek_timeout_id == 0) {
             this.seek_timeout_id = Timeout.add (500, () => {
-                playbin.seek.begin ((int64) range.adjustment.value, (obj, res) => {
+                playbin.seek((int64) range.adjustment.value);
                 this.seek_timeout_id = 0;
-                });
                 return false;
             });
         }
@@ -208,7 +208,7 @@ class Ui.Window : Adw.ApplicationWindow {
         int64 new_position = position - (int64)10 * 1000 * 1000000;
         if (new_position < 0) new_position = 0;
         this.position = new_position;
-        this.playbin.seek.begin (new_position);
+        this.playbin.seek (new_position);
     }
 
     [GtkCallback] private void seek_forward () {
@@ -216,6 +216,6 @@ class Ui.Window : Adw.ApplicationWindow {
         int64 new_position = position + (int64)10 * 1000 * 1000000;
         if (new_position > this.playbin.duration) new_position = this.playbin.duration;
         this.position = new_position;
-        this.playbin.seek.begin (new_position);
+        this.playbin.seek (new_position);
     }
 }