[banshee/gapless-ng] [Gapless] Work around BGO #602437

[Christopher James Halse Rogers <chrishr src gnome org>]

commit aa52de225310563f0e0f1db2866c972993ff4997
Author: Christopher James Halse Rogers <chrishr src gnome org>
Date:   Sun Mar 7 22:02:22 2010 +1100

    [Gapless] Work around BGO #602437
    
    Playbin2 currently behaves incorrectly when trying to transition gaplessly to or from
    a stream audio and video, resulting in the A/V sync being significantly out.
    
    Don't send about-to-finish when the current track has a video stream; instead let
    the track play out to EOS.
    
    If the new track has a video stream and we're in a gapless transition, drop
    back to READY and do a non-gapless transition

 libbanshee/banshee-player-pipeline.c               |   48 ++++++++++++++++++--
 libbanshee/banshee-player-private.h                |    1 +
 libbanshee/banshee-player.c                        |    4 ++
 .../Banshee.GStreamer/PlayerEngine.cs              |   12 +++++-
 4 files changed, 60 insertions(+), 5 deletions(-)
---
diff --git a/libbanshee/banshee-player-pipeline.c b/libbanshee/banshee-player-pipeline.c
index 5903ec9..b466469 100644
--- a/libbanshee/banshee-player-pipeline.c
+++ b/libbanshee/banshee-player-pipeline.c
@@ -38,6 +38,15 @@
 // Private Functions
 // ---------------------------------------------------------------------------
 
+static gboolean
+bp_stream_has_video (GstElement *playbin)
+{
+    int n_video;
+    g_object_get (G_OBJECT (playbin), "n-video", &n_video, NULL);
+    return n_video > 0;
+}
+
+
 static void
 bp_pipeline_process_tag (const GstTagList *tag_list, const gchar *tag_name, BansheePlayer *player)
 {
@@ -59,13 +68,36 @@ bp_pipeline_process_tag (const GstTagList *tag_list, const gchar *tag_name, Bans
 }
 
 static gboolean
-bp_next_track_starting (gpointer player)
+bp_next_track_starting (BansheePlayer *player)
 {
     g_return_val_if_fail (IS_BANSHEE_PLAYER (player), FALSE);
+    g_return_val_if_fail (GST_IS_ELEMENT (player->playbin), FALSE);
 
-    bp_debug ("[gapless] Triggering track-change signal");
-    if (((BansheePlayer *)player)->next_track_starting_cb != NULL) {
-        ((BansheePlayer *)player)->next_track_starting_cb (player);
+    // Work around BGO #602437 - gapless transition between tracks with 
+    // video streams results in broken behaviour - most obviously, huge A/V
+    // sync issues.
+    gboolean has_video = bp_stream_has_video (player->playbin);
+    if (player->in_gapless_transition && has_video) {
+        gchar *uri;
+    
+        bp_debug ("[Gapless]: Aborting gapless transition to stream with video.");
+        bp_debug ("[Gapless]: Triggering normal track change.");
+        g_object_get (G_OBJECT (player->playbin), "uri", &uri, NULL);
+        gst_element_set_state (player->playbin, GST_STATE_READY);
+        
+        g_object_set (G_OBJECT (player->playbin), "uri", uri, NULL);
+        gst_element_set_state (player->playbin, GST_STATE_PLAYING);
+        g_free (uri);
+        player->in_gapless_transition = FALSE;
+        // The transition to playing will happen asynchronously, and will trigger
+        // a second track-starting message.  Stop processing this one.
+        return FALSE;
+    }
+    player->in_gapless_transition = FALSE;
+
+    if (player->next_track_starting_cb != NULL) {
+        bp_debug ("[gapless] Triggering track-change signal");
+        player->next_track_starting_cb (player);
     }
     return FALSE;
 }
@@ -187,8 +219,16 @@ bp_pipeline_bus_callback (GstBus *bus, GstMessage *message, gpointer userdata)
 static void bp_about_to_finish_callback (GstElement *playbin, BansheePlayer *player)
 {
     g_return_if_fail (IS_BANSHEE_PLAYER (player));
+    g_return_if_fail (GST_IS_ELEMENT (playbin));
+
+    if (bp_stream_has_video (playbin)) {
+        bp_debug ("[Gapless]: Not attempting gapless transition from stream with video");
+        return;
+    }
 
     if (player->about_to_finish_cb != NULL) {
+        player->in_gapless_transition = TRUE;
+
         bp_debug ("[Gapless] Requesting next track");
         player->about_to_finish_cb (player);
     }
diff --git a/libbanshee/banshee-player-private.h b/libbanshee/banshee-player-private.h
index 20a2d1c..1edd375 100644
--- a/libbanshee/banshee-player-private.h
+++ b/libbanshee/banshee-player-private.h
@@ -121,6 +121,7 @@ struct BansheePlayer {
     guint iterate_timeout_id;
     gboolean buffering;
     gchar *cdda_device;
+    gboolean in_gapless_transition;
     
     // Video State
     BpVideoDisplayContextType video_display_context_type;
diff --git a/libbanshee/banshee-player.c b/libbanshee/banshee-player.c
index 157494d..fe47eb5 100644
--- a/libbanshee/banshee-player.c
+++ b/libbanshee/banshee-player.c
@@ -163,6 +163,8 @@ bp_open (BansheePlayer *player, const gchar *uri)
     // Pass the request off to playbin
     g_object_set (G_OBJECT (player->playbin), "uri", uri, NULL);
     
+    player->in_gapless_transition = FALSE;
+    
     return TRUE;
 }
 
@@ -181,6 +183,8 @@ bp_stop (BansheePlayer *player, gboolean nullstate)
     bp_debug ("bp_stop: setting state to %s",
         state == GST_STATE_NULL ? "GST_STATE_NULL" : "GST_STATE_PAUSED");
     
+    player->in_gapless_transition = FALSE;
+    
     bp_pipeline_set_state (player, state);
 }
 
diff --git a/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs b/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs
index 38d4f2b..f37c975 100644
--- a/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs
+++ b/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs
@@ -90,6 +90,7 @@ namespace Banshee.GStreamer
         private BansheePlayerAboutToFinishCallback about_to_finish_callback;
 #endif
 
+        private bool next_track_pending;
         private SafeUri pending_uri;
 
         private bool buffering_finished;
@@ -238,6 +239,7 @@ namespace Banshee.GStreamer
 
         public override void SetNextTrackUri (SafeUri uri)
         {
+            next_track_pending = false;
             if (next_track_set.WaitOne (0)) {
                 // We're not waiting for the next track to be set.
                 // This means that we've missed the window for gapless.
@@ -284,7 +286,7 @@ namespace Banshee.GStreamer
         {
             Close (false);
             OnEventChanged (PlayerEvent.EndOfStream);
-            if (!GaplessEnabled) {
+            if (!next_track_pending) {
                 OnEventChanged (PlayerEvent.RequestNextTrack);
             } else if (pending_uri != null) {
                 Log.Warning ("[Gapless] EOS signalled while waiting for next track.");
@@ -294,6 +296,13 @@ namespace Banshee.GStreamer
                 OpenUri (pending_uri);
                 Play ();
                 pending_uri = null;
+            } else {
+                // This should be unreachable - the RequestNextTrack event is delegated to the main thread
+                // and so blocks the bus callback from delivering the EOS message.
+                //
+                // Playback should continue as normal from here, when the RequestNextTrack message gets handled.
+                Log.Warning ("[Gapless] EndOfStream message recieved before the next track has been set");
+                Log.Warning ("[Gapless] If this happens frequently, please file a bug");
             }
         }
 
@@ -319,6 +328,7 @@ namespace Banshee.GStreamer
 
             next_track_set.Reset ();
             pending_uri = null;
+            next_track_pending = true;
 
             OnEventChanged (PlayerEvent.RequestNextTrack);
             // Gapless playback with Playbin2 requires that the about-to-finish callback does not return until