Bug 122514 - pause in xfade using xine, causes the next song to continue playing
Summary: pause in xfade using xine, causes the next song to continue playing
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-02-22 21:40 UTC by Scott M. Likens
Modified: 2006-09-19 17:42 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Patch solving the issue (3.40 KB, patch)
2006-08-29 15:02 UTC, Markus Kaufhold
Details
Patch solving the issue (3.40 KB, patch)
2006-08-29 15:37 UTC, Markus Kaufhold
Details
Patch solving the issue (3.99 KB, patch)
2006-08-29 15:41 UTC, Markus Kaufhold
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Scott M. Likens 2006-02-22 21:40:04 UTC
Version:           1.4-SVN (as of 2days ago) (using KDE KDE 3.5.1)
Installed from:    Gentoo Packages
Compiler:          gcc version 3.4.5 (Gentoo 3.4.5, ssp-3.4.5-1.0, pie-8.7.9) 
OS:                Linux

While playing a song, and the song nears the end and the xfade starts, you hit pause, and it will pause the xfade.  Right? Right.

Then shortly after the fade would end, the next song starts playing, and the pause button is still stuck.

Can't unpause, or pause, because the pause button is still depressed.

Stop only works :(

using Xine 1.1.1-r4

Qt: 3.3.5
KDE: 3.5.1
kde-config: WARNING: KLocale: trying to look up "" in catalog. Fix the program
kde-config: 1.0

Not a whole lot here, to reproduce.

Use the xine engine, play, while the xfade starts, hit pause, and shortly afterwards you'll find the next song playing.

Thanks.
Comment 1 Christie 2006-03-11 01:50:10 UTC
I can confirm this with the xine engine.
It looks like the second track is recognised as having started playing at the beginning of the xfade, resulting in two concurrent playback threads - if pause is hit in this xfade period, the fade-out of the first track is what is actually paused, while the fade-in and playback of the second track continues. 
This seems to confuse the button state, so that the pause state is still on for the first inactive track.
Hitting pause after this doesn't seem to do anything useful.
Expected behaviour would be to pause playback of all tracks.
Comment 2 SlashDevDsp 2006-04-22 09:44:30 UTC
I can also confirm this with xine and alsa output with crossfade enabled with the latest svn (22/04/06). I hope this one gets fixed.
Comment 3 Markus Kaufhold 2006-08-29 15:01:39 UTC
Attached a patch solving the issue

Note:
On some places in XineEngine the call to xine_get_param( m_stream, XINE_PARAM_SPEED ) is checked against 0
0 is defined in the xine header as XINE_SPEED_PAUSE.
I've replaced the 0 check with XINE_SPEED_PAUSE, as that is more safe.
The value of XINE_SPEED_PAUSE is xine internal, and could change in the future.
Comment 4 Markus Kaufhold 2006-08-29 15:02:38 UTC
Created attachment 17549 [details]
Patch solving the issue
Comment 5 Markus Kaufhold 2006-08-29 15:37:29 UTC
Created attachment 17550 [details]
Patch solving the issue

Ooops,
In case of stopping the current track or quitting amarok in the xfade pause
state, the fade thread thread should go on, otherwise we end up in an endless
loop.
Patch corrected
Comment 6 Markus Kaufhold 2006-08-29 15:41:00 UTC
Created attachment 17551 [details]
Patch solving the issue

Sorry,
I've forgot to save the patch in the editor.
Now the correct patch should have been appended
Comment 7 Mark Kretschmann 2006-09-19 17:42:33 UTC
SVN commit 586431 by markey:

This patch by Markus Kaufhold <M.Kaufhold@gmx.de> solves the following problem:
"Pause during crossfade using xine causes the next song to continue playing."

Also it provides a number of safety code improvements.

BUG: 122514


 M  +31 -6     xine-engine.cpp  
 M  +3 -0      xine-engine.h  


--- trunk/extragear/multimedia/amarok/src/engine/xine/xine-engine.cpp #586430:586431
@@ -35,7 +35,6 @@
 
 #include <qapplication.h>
 #include <qdir.h>
-#include <qtimer.h>
 
 extern "C"
 {
@@ -93,6 +92,7 @@
     // Wait until the fader thread is done
     if( s_fader ) {
         m_stopFader = true;
+        s_fader->resume(); // safety call if the engine is in the pause state
         s_fader->wait();
         delete s_fader;
     }
@@ -349,6 +349,9 @@
 void
 XineEngine::stop()
 {
+    if( s_fader && s_fader->running() )
+        s_fader->resume(); // safety call if the engine is in the pause state
+
     if ( !m_stream )
        return;
     if( !m_fadeOutRunning || state() == Engine::Paused )
@@ -367,14 +370,20 @@
 void
 XineEngine::pause()
 {
-    if( xine_get_param( m_stream, XINE_PARAM_SPEED ) )
+    if( xine_get_param( m_stream, XINE_PARAM_SPEED ) != XINE_SPEED_PAUSE )
     {
+        if( s_fader && s_fader->running() )
+            s_fader->pause();
+
         xine_set_param( m_stream, XINE_PARAM_SPEED, XINE_SPEED_PAUSE ); 
         xine_set_param( m_stream, XINE_PARAM_AUDIO_CLOSE_DEVICE, 1);
         emit stateChanged( Engine::Paused );
 
     } else {
 
+        if( s_fader && s_fader->running() )
+            s_fader->resume();
+
         xine_set_param( m_stream, XINE_PARAM_SPEED, XINE_SPEED_NORMAL );
         emit stateChanged( Engine::Playing );
     }
@@ -388,7 +397,7 @@
 
     switch( xine_get_status( m_stream ) )
     {
-    case XINE_STATUS_PLAY: return xine_get_param( m_stream, XINE_PARAM_SPEED ) ? Engine::Playing : Engine::Paused;
+    case XINE_STATUS_PLAY: return xine_get_param( m_stream, XINE_PARAM_SPEED )  != XINE_SPEED_PAUSE ? Engine::Playing : Engine::Paused;
     case XINE_STATUS_IDLE: return Engine::Empty;
     case XINE_STATUS_STOP:
     default:               return m_url.isEmpty() ? Engine::Empty : Engine::Idle;
@@ -991,6 +1000,7 @@
    , m_port( engine->m_audioPort )
    , m_post( engine->m_post )
    , m_fadeLength( fadeMs )
+   , m_paused( false )
 {
     if( engine->makeNewStream() )
     {
@@ -1029,19 +1039,23 @@
     uint stepsCount = m_fadeLength < 1000 ? m_fadeLength / 10 : 100;
     uint stepSizeUs = (int)( 1000.0 * (float)m_fadeLength / (float)stepsCount );
 
-    QTime t;
-    t.start();
     float mix = 0.0;
+    float elapsedUs = 0.0;
     while ( mix < 1.0 )
     {
         // sleep a constant amount of time
         QThread::usleep( stepSizeUs );
 
+        if ( m_paused )
+        	continue;
+
+        elapsedUs += stepSizeUs;
+
         // get volume (amarok main * equalizer preamp)
         float vol = Engine::Base::makeVolumeLogarithmic( m_engine->m_volume ) * m_engine->m_preamp;
 
         // compute the mix factor as the percentage of time spent since fade begun
-        float mix = (float)t.elapsed() / (float)m_fadeLength;
+        float mix = (elapsedUs / 1000.0) / (float)m_fadeLength;
         if ( mix > 1.0 )
         {
             if ( m_increase )
@@ -1117,7 +1131,18 @@
     deleteLater();
 }
 
+void
+Fader::pause()
+{
+	m_paused = true;
+}
 
+void
+Fader::resume()
+{
+	m_paused = false;
+}
+
 bool XineEngine::metaDataForUrl(const KURL &url, Engine::SimpleMetaBundle &b)
 {
     bool result = false;
--- trunk/extragear/multimedia/amarok/src/engine/xine/xine-engine.h #586430:586431
@@ -105,12 +105,15 @@
     xine_audio_port_t  *m_port;
     xine_post_t        *m_post;
     uint               m_fadeLength;
+    bool               m_paused;
 
     virtual void run();
 
 public:
     Fader( XineEngine *, uint fadeLengthMs );
    ~Fader();
+   void pause();
+   void resume();
 };
 
 class OutFader : public QObject, public QThread