Bug 140980 - last played track does not get counted and rated
Summary: last played track does not get counted and rated
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 1.4-SVN
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 144486 144671 148271 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-01-31 23:28 UTC by Sander Zwier
Modified: 2008-09-01 16:49 UTC (History)
4 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sander Zwier 2007-01-31 23:28:20 UTC
Version:           1.4-SVN (using KDE Devel)
Installed from:    Compiled sources
Compiler:          gcc (GCC) 4.1.2 20061215 (prerelease) 
OS:                Linux

1. Start playing a track
2. Select the "stop after current track" option

Amarok stops after the track and the playlist marker advances properly, but the "Last Played" column still says "Never" and also the Score remains unaltered. Type of playlist doesn't matter (be it a "Dynamic" or normal playlist).

This is with SVN r628758.
Comment 1 Sander Zwier 2007-01-31 23:43:12 UTC
Actually, when a playlist finishes normally (so without the "stop after current track" function) the track does get counted and scored properly.
Comment 2 richlv 2007-02-01 09:36:28 UTC
damn. confirmed in revision 628947 :)
Comment 3 Lorenz Röhrl 2007-02-14 11:59:08 UTC
yup, i also can confirm this
Comment 4 Kevin Funk 2007-04-21 15:06:21 UTC
*** Bug 144486 has been marked as a duplicate of this bug. ***
Comment 5 Kevin Funk 2007-04-25 18:15:21 UTC
*** Bug 144671 has been marked as a duplicate of this bug. ***
Comment 6 Jeff Mitchell 2007-04-28 23:40:50 UTC
Can anyone confirm whether this happens if a playlist ends normally, or if it's only when using "Stop after current track"?
Comment 7 Jeff Mitchell 2007-04-28 23:56:02 UTC
Heh, also, I believe that it has something to do with this comment in the code:

    //TODO bummer why'd I do it this way? it should _not_ be in play!
    //let Amarok know that the previous track is no longer playing

:-)

Still looking though...
Comment 8 Jeff Mitchell 2007-04-30 20:40:18 UTC
SVN commit 659747 by mitchell:

zOMG, the bug that wouldn't die...took forever to figure this one out.  Fixed, though.

BUG: 140980


 M  +6 -2      collectiondb.cpp  
 M  +12 -0     enginecontroller.cpp  
 M  +3 -1      enginecontroller.h  
 M  +3 -5      playlist.cpp  


--- branches/stable/extragear/multimedia/amarok/src/collectiondb.cpp #659746:659747
@@ -4842,8 +4842,12 @@
     // Don't update statistics if song has been played for less than 15 seconds
     // if ( finalPosition < 15000 ) return;
 
-    const KURL url = EngineController::instance()->bundle().url();
-    debug() << "track ended: " << url.url() << endl;
+    //below check is necessary because if stop after current track is selected,
+    //the url's path will be empty, so check the previous URL for the path that
+    //had just played
+    const KURL url = EngineController::instance()->bundle().url().path().isEmpty() ?
+                            EngineController::instance()->previousURL() :
+                            EngineController::instance()->bundle().url();
     PodcastEpisodeBundle peb;
     if( getPodcastEpisodeBundle( url.url(), &peb ) )
     {
--- branches/stable/extragear/multimedia/amarok/src/enginecontroller.cpp #659746:659747
@@ -323,6 +323,7 @@
 
 void EngineController::next( bool forceNext ) //SLOT
 {
+    m_previousUrl = m_bundle.url();
     m_isTiming = false;
     emit orderNext(forceNext);
 }
@@ -426,6 +427,10 @@
     {
         //assign bundle now so that it is available when the engine
         //emits stateChanged( Playing )
+        if( !m_bundle.url().path().isEmpty() ) //wasn't playing before
+            m_previousUrl = m_bundle.url();
+        else
+            m_previousUrl = bundle.url();
         m_bundle = bundle;
 
         if( m_engine->play( offset ) )
@@ -640,6 +645,7 @@
 
     m_lastMetadata << bundle;
 
+    m_previousUrl = m_bundle.url();
     m_bundle = bundle;
     m_lastPositionOffset = m_positionOffset;
     if( m_lastFm )
@@ -649,6 +655,12 @@
     newMetaDataNotify( m_bundle, false /* not a new track */ );
 }
 
+void EngineController::currentTrackMetaDataChanged( const MetaBundle& bundle )
+{
+    m_previousUrl = m_bundle.url();
+    m_bundle = bundle;
+    newMetaDataNotify( bundle, false /* no track change */ );
+}
 
 //////////////////////////////////////////////////////////////////////////////////////////
 // PRIVATE SLOTS
--- branches/stable/extragear/multimedia/amarok/src/enginecontroller.h #659746:659747
@@ -55,6 +55,7 @@
 
     uint trackLength() const { return m_bundle.length() * 1000; }
     const MetaBundle &bundle() const;
+    KURL previousURL() const { return m_previousUrl; }
     KURL playingURL() const { return bundle().url(); }
 
     void restoreSession();
@@ -94,7 +95,7 @@
     void playlistChanged() { m_engine->playlistChanged(); }
 
     void slotStreamMetaData( const MetaBundle &bundle );
-    void currentTrackMetaDataChanged( const MetaBundle& bundle ) { m_bundle = bundle; newMetaDataNotify( bundle, false /* no track change */ ); }
+    void currentTrackMetaDataChanged( const MetaBundle& bundle );
 
 signals:
     void orderPrevious();
@@ -124,6 +125,7 @@
     EngineBase*     m_engine;
     EngineBase*     m_voidEngine;
     MetaBundle      m_bundle;
+    KURL            m_previousUrl;
     BundleList      m_lastMetadata;
     long            m_delayTime;
     int             m_muteVolume;
--- branches/stable/extragear/multimedia/amarok/src/playlist.cpp #659746:659747
@@ -940,8 +940,8 @@
             m_dynamicDirt = false;
         }
 
-        setStopAfterMode(DoNotStop);
         EngineController::instance()->stop();
+        setStopAfterMode( DoNotStop );
 
         if( !AmarokConfig::randomMode() ) {
             item = MyIt::nextVisible( item );
@@ -1415,7 +1415,8 @@
         prev_stopafter->update();
 }
 
-void Playlist::setStopAfterItem( PlaylistItem *item ) {
+void Playlist::setStopAfterItem( PlaylistItem *item )
+{
     if( !item ) {
         setStopAfterMode( DoNotStop );
         return;
@@ -2057,9 +2058,6 @@
 
             PlaylistItem::setPixmapChanged();
 
-            if( stopAfterMode() == StopAfterCurrent )
-                setStopAfterMode( DoNotStop );
-
             //reset glow state
             slotGlowTimer();
         }
Comment 9 Sander Zwier 2007-05-01 11:51:10 UTC
I confirm this is fixed. Thanks Jeff!
Comment 10 Kevin Funk 2007-07-27 20:56:38 UTC
*** Bug 148271 has been marked as a duplicate of this bug. ***
Comment 11 missive 2008-09-01 16:49:07 UTC
I think this bug has come back from the dead...

It's kind of difficult from the comments to determine which version should have the fix. That would be a nice addition to this bug tracker -- when a bug is fixed, you should be able to show which version will have the fix so that people can tell.

Anyhow, from the duplicate bug 148271 it looks like it was fixed in 1.4.6 but I still see the bug in 1.4.9.1