[banshee] PlayerEngine: workaround dupe about-to-finish signals (bgo#679938)

[Andrés Aragoneses <aaragoneses src gnome org>]

commit 60f3bebe1f77f78fa0a6b674878f26a2f0cfce03
Author: Andrés G. Aragoneses <knocte gmail com>
Date:   Sat Feb 1 14:07:41 2014 +0100

    PlayerEngine: workaround dupe about-to-finish signals (bgo#679938)
    
    Banshee codebase was making the reasonable assumption that
    GStreamer was sending one (and only one) about-to-finish
    signal for each play of each track, however this is not the
    case, and this issue has been reported as bug BGO#722769.
    
    Gapless feature was relying certainly on this assumption, and
    therefore it was causing subtle unintended behaviours, such as:
    
    - bgo#679938: Repeat always ON (tested by Ismael Olea)
    - bgo#674293: Stop when finished is ignored (very likely)
    - and possibly more...
    
    To fix this we need to prevent the event RequestNextTrack to
    be fired twice, so we put a timestamp every time current_track
    is assigned, store that timestamp in the native and managed
    PlayerEngines, and compare it, to prevent the OnAboutToFinish()
    method to run twice for the same play.

 .../Banshee.GStreamer/PlayerEngine.cs              |   11 +++++
 .../Banshee.GStreamerSharp/PlayerEngine.cs         |   11 +++++
 .../Banshee.MediaEngine/PlayerEngine.cs            |    8 ++++
 .../Banshee.Services/Banshee.MediaEngine/Tests.cs  |   40 ++++++++++++++++++++
 4 files changed, 70 insertions(+), 0 deletions(-)
---
diff --git a/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs 
b/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs
index 31766e1..6a058c0 100644
--- a/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs
+++ b/src/Backends/Banshee.GStreamer/Banshee.GStreamer/PlayerEngine.cs
@@ -431,8 +431,19 @@ namespace Banshee.GStreamer
             }
         }
 
+        private System.DateTime about_to_finish_time_stamp;
+
         private void OnAboutToFinish (IntPtr player)
         {
+            // HACK: ugly workaround for GStreamer's bug http://bugzilla.gnome.org/722769
+            // long story short, AboutToFinish signal firing twice for the same play of the same track
+            // causes problems when Gapless Enabled because of RequestNextTrack event being fired twice
+            if (about_to_finish_time_stamp == CurrentTrackTimeStamp) {
+                return;
+            }
+            about_to_finish_time_stamp = CurrentTrackTimeStamp;
+
+
             // This is needed to make Shuffle-by-* work.
             // Shuffle-by-* uses the LastPlayed field to determine what track in the grouping to play next.
             // Therefore, we need to update this before requesting the next track.
diff --git a/src/Backends/Banshee.GStreamerSharp/Banshee.GStreamerSharp/PlayerEngine.cs 
b/src/Backends/Banshee.GStreamerSharp/Banshee.GStreamerSharp/PlayerEngine.cs
index ea51892..b24a41f 100644
--- a/src/Backends/Banshee.GStreamerSharp/Banshee.GStreamerSharp/PlayerEngine.cs
+++ b/src/Backends/Banshee.GStreamerSharp/Banshee.GStreamerSharp/PlayerEngine.cs
@@ -396,8 +396,19 @@ namespace Banshee.GStreamerSharp
             OnEventChanged (PlayerEvent.PrepareVideoWindow);
         }
 
+        private System.DateTime about_to_finish_time_stamp;
+
         void OnAboutToFinish (object o, GLib.SignalArgs args)
         {
+            // HACK: ugly workaround for GStreamer's bug http://bugzilla.gnome.org/722769
+            // long story short, AboutToFinish signal firing twice for the same play of the same track
+            // causes problems when Gapless Enabled because of RequestNextTrack event being fired twice
+            if (about_to_finish_time_stamp == CurrentTrackTimeStamp) {
+                return;
+            }
+            about_to_finish_time_stamp = CurrentTrackTimeStamp;
+
+
             // This is needed to make Shuffle-by-* work.
             // Shuffle-by-* uses the LastPlayed field to determine what track in the grouping to play next.
             // Therefore, we need to update this before requesting the next track.
diff --git a/src/Core/Banshee.Services/Banshee.MediaEngine/PlayerEngine.cs 
b/src/Core/Banshee.Services/Banshee.MediaEngine/PlayerEngine.cs
index d66f8ea..acc5a2a 100644
--- a/src/Core/Banshee.Services/Banshee.MediaEngine/PlayerEngine.cs
+++ b/src/Core/Banshee.Services/Banshee.MediaEngine/PlayerEngine.cs
@@ -50,6 +50,11 @@ namespace Banshee.MediaEngine
         private PlayerState current_state = PlayerState.NotReady;
         private PlayerState last_state = PlayerState.NotReady;
 
+        public DateTime CurrentTrackTimeStamp {
+            get;
+            private set;
+        }
+
         // will be changed to PlayerState.Idle after going to PlayerState.Ready
         private PlayerState idle_state = PlayerState.NotReady;
 
@@ -67,6 +72,7 @@ namespace Banshee.MediaEngine
 
         public void Reset ()
         {
+            CurrentTrackTimeStamp = DateTime.Now;
             current_track = null;
             OnStateChanged (idle_state);
         }
@@ -136,6 +142,7 @@ namespace Banshee.MediaEngine
             }
 
             try {
+                CurrentTrackTimeStamp = DateTime.Now;
                 current_track = track;
                 OnStateChanged (PlayerState.Loading);
                 OpenUri (uri, track.HasAttribute (TrackMediaAttributes.VideoStream) || track is 
UnknownTrackInfo);
@@ -217,6 +224,7 @@ namespace Banshee.MediaEngine
             if (args.Event == PlayerEvent.StartOfStream && pending_track != null) {
                 Log.DebugFormat ("OnEventChanged called with StartOfStream.  Replacing current_track with 
pending_track: \"{0}\"",
                                  pending_track.DisplayTrackTitle);
+                CurrentTrackTimeStamp = DateTime.Now;
                 current_track = pending_track;
                 pending_track = null;
             }
diff --git a/src/Core/Banshee.Services/Banshee.MediaEngine/Tests.cs 
b/src/Core/Banshee.Services/Banshee.MediaEngine/Tests.cs
index 7820c8b..b7b8fe7 100644
--- a/src/Core/Banshee.Services/Banshee.MediaEngine/Tests.cs
+++ b/src/Core/Banshee.Services/Banshee.MediaEngine/Tests.cs
@@ -128,6 +128,42 @@ namespace Banshee.MediaEngine
             Assert.AreEqual (a_valid_track, actual_filename);
         }
 
+        private void StopEngine ()
+        {
+            service.SetNextTrack ((TrackInfo)null);
+            service.Play ();
+        }
+
+        [Test]
+        // make sure https://bugzilla.gnome.org/show_bug.cgi?id=722769 doesn't happen, or that our 
workaround works
+        public void TestThatRequestNextTrackEventsAreNotDuplicated ()
+        {
+            WaitUntil (PlayerState.Idle, StopEngine);
+
+            var a_valid_track1 = "A_boy.ogg";
+            var a_valid_uri1 = new SafeUri (Paths.Combine (TestsDir, "data", a_valid_track1));
+
+            var a_valid_track2 = "A_girl.ogg";
+            var a_valid_uri2 = new SafeUri (Paths.Combine (TestsDir, "data", a_valid_track2));
+
+            var ignore_events_func = new Func<PlayerState?, PlayerEvent?, bool> ((s, e) =>
+                e != null && (e.Value == PlayerEvent.Volume));
+
+            WaitFor (ignore_events_func,
+                () => {
+                    service.Open (a_valid_uri1);
+                    service.Play ();
+                },
+                PlayerState.Loading,
+                PlayerState.Loaded,
+                PlayerEvent.StartOfStream,
+                PlayerState.Playing);
+
+            WaitFor (ignore_events_func,
+                PlayerEvent.RequestNextTrack, // <- this is the key, notice that we expect only one, not two
+                PlayerState.Idle);
+        }
+
         private void LoadAndPlay (string filename)
         {
             track_intercepts = 0;
@@ -406,7 +442,11 @@ namespace Banshee.MediaEngine
             });
             service.ConnectEvent (handler);
 
+            bool return_early = (state != null && service.CurrentState.Equals (state.Value));
             action ();
+            if (return_early) {
+                return;
+            }
 
             const int seconds = 3;
             int count = 0;