Version: 1.3-SVN (using KDE 3.3.2, Mandrake Linux Cooker i586 - Cooker) Compiler: gcc version 3.4.3 (Mandrakelinux 10.2 3.4.3-7mdk) OS: Linux (i686) release 2.6.11-6mdk If you select the current track to stop playing, but then clear the playlist and add new items, amarok will start playing the new playlist instead of stopping when its supposed to. This is bad if a song is playing which is not in your playlist and you select stop playing "after current track" from the new stop button, but instead it starts from the beginning of the playlist.
illisius, could you take a look at this please?
> illisius, could you take a look at this please? Certainly. This is because we just keep a pointer to the track to stop after -- which, after it has been removed from the playlist, it naturally can't point to any longer. I'll see if I can come up with a solution to this that doesn't suck.
I have reproduced this bug using the svn version (rev 537005). Moreover, I found another bug linked to this one that crash amaroK. How to reproduce : - select the current track to "Stop after current track" - select all the tracks in the playlist and remove them - add other files in the playlist (did not try with same files...) - wait for the current track to finish (or seek near the end) - according to the former bug, amaroK should not play other tracks, but it does play the first track of the new playlist - select "Stop after current track" --> amaroK crash I will enclose the full debug trace of amaroK, as the backtrace. I have taken an (humble) eye on the code, and as Gábor wrote, it's a problem with the pointer to the track to stop after. I may have a solution, but I don't know if it is a good design! IMHO, if the user remove the track to stop after from the playlist, then reset the "stop after" status. It is really "stop after *this* track", not "stop once the end of a track has been reached". In that case, I may have a patch that solve the bug...
Created attachment 15903 [details] Debug log and backtrace Backtrace at the bottom
Created attachment 15904 [details] Patch for the bug #111690 svn diff against rev 537005
crash still reproducable with svn 558575
SVN commit 582322 by aoliveira: Don't keep a m_stopAfterTrack pointing to a deleted item, as it would lead to crashes. Thanks for the patch, Stephane Deraco <steph.dev@gmail.com> CCBUG: 111690 M +7 -1 playlist.cpp --- trunk/extragear/multimedia/amarok/src/playlist.cpp #582321:582322 @@ -2222,6 +2222,10 @@ m_prevTracks.clear(); m_prevAlbums.clear(); + if (m_stopAfterTrack) { + m_stopAfterTrack = 0; + setStopAfterMode( DoNotStop ); + } const PLItemList prev = m_nextTracks; m_nextTracks.clear(); emit queueChanged( PLItemList(), prev ); @@ -4377,8 +4381,10 @@ } } - if( m_stopAfterTrack == item ) + if( m_stopAfterTrack == item ) { m_stopAfterTrack = 0; //to be safe + setStopAfterMode( DoNotStop ); + } //keep m_nextTracks queue synchronized if( m_nextTracks.removeRef( item ) && !multi )
The following errors spew out in the console during the time the gui is frozen while using helix. This is only an excerpt, there is probably about 1000 lines. X Error: BadIDChoice (invalid resource ID chosen for this connection) 14 Major opcode: 1 Minor opcode: 0 Resource id: 0x3200e22 X Error: BadIDChoice (invalid resource ID chosen for this connection) 14 Major opcode: 1 Minor opcode: 0 Resource id: 0x3200e23 X Error: BadIDChoice (invalid resource ID chosen for this connection) 14 Major opcode: 1 Minor opcode: 0 Resource id: 0x3200e24 X Error: BadIDChoice (invalid resource ID chosen for this connection) 14 Major opcode: 1 Minor opcode: 0 Resource id: 0x3200e25 X Error: BadIDChoice (invalid resource ID chosen for this connection) 14 Major opcode: 53 Minor opcode: 0 Resource id: 0x3200e26
Adding my support for this request / bug. It would be nice if AmaroK remembered that the user had selected "stop after current song", even if he then removed the song from the playlist. I sometimes clear and "pre-fill" the playlist with what suits my mood, even if I don't intend to continue listening right-away. I also practically always use "stop after current" rather than plain "stop", as I dislike cutting off a song in the middle.
SVN commit 616542 by aoliveira: Let Stop After Current work even when current track is removed from playlist. It's a bit confusing I think. Maybe we need some indication that it's going to stop (maybe a different icon on the section of the down arrow/repeat modes?) RFC BUG: 111690 M +45 -27 playlist.cpp M +3 -1 playlist.h --- trunk/extragear/multimedia/amarok/src/playlist.cpp #616541:616542 @@ -179,6 +179,7 @@ , m_undoCounter( 0 ) , m_dynamicMode( 0 ) , m_stopAfterTrack( 0 ) + , m_stopAfterMode( DoNotStop ) , m_showHelp( true ) , m_dynamicDirt( false ) , m_queueDirt( false ) @@ -939,7 +940,7 @@ { PlaylistItem *item = currentTrack(); - if( !m_visCount || ( m_currentTrack && m_stopAfterTrack == m_currentTrack ) ) + if( !m_visCount || stopAfterMode() == StopAfterCurrent ) { if( dynamicMode() && m_visCount ) { @@ -948,7 +949,7 @@ m_dynamicDirt = false; } - m_stopAfterTrack = 0; + setStopAfterMode(DoNotStop); EngineController::instance()->stop(); if( !AmarokConfig::randomMode() ) { @@ -1179,7 +1180,7 @@ upcomingTracks > dynamicMode()->upcomingCount(); //keep upcomingTracks requirement, this seems to break StopAfterCurrent - if( !dontAppend && m_stopAfterTrack != m_currentTrack ) + if( !dontAppend && stopAfterMode() != StopAfterCurrent ) { addDynamicModeTracks( 1 ); } @@ -1410,10 +1411,12 @@ { PlaylistItem *prev_stopafter = m_stopAfterTrack; - if( on ) - m_stopAfterTrack = m_currentTrack; - else - m_stopAfterTrack = 0; + if( on ) { + setStopAfterItem( m_currentTrack ); + } + else { + setStopAfterMode( DoNotStop ); + } if( m_stopAfterTrack ) m_stopAfterTrack->update(); @@ -1421,6 +1424,20 @@ prev_stopafter->update(); } +void Playlist::setStopAfterItem( PlaylistItem *item ) { + if( !item ) { + setStopAfterMode( DoNotStop ); + return; + } + else if( item == m_currentTrack ) + setStopAfterMode( StopAfterCurrent ); + else if( item == m_nextTracks.getLast() ) + setStopAfterMode( StopAfterQueue ); + else + setStopAfterMode( StopAfterQueue ); + m_stopAfterTrack = item; +} + void Playlist::toggleStopAfterCurrentItem() { PlaylistItem *item = currentItem(); @@ -1430,11 +1447,13 @@ return; PlaylistItem *prev_stopafter = m_stopAfterTrack; - if( m_stopAfterTrack == item ) + if( m_stopAfterTrack == item ) { m_stopAfterTrack = 0; + setStopAfterMode( DoNotStop ); + } else { - m_stopAfterTrack = item; + setStopAfterItem( item ); item->setSelected( false ); item->update(); } @@ -1451,12 +1470,12 @@ PlaylistItem *prev_stopafter = m_stopAfterTrack; if( m_stopAfterTrack == item ) { - m_stopAfterTrack = 0; + setStopAfterMode( DoNotStop ); Amarok::OSD::instance()->OSDWidget::show( i18n("Stop Playing After Track: Off") ); } else { - m_stopAfterTrack = item; + setStopAfterItem( item ); item->setSelected( false ); item->update(); Amarok::OSD::instance()->OSDWidget::show( i18n("Stop Playing After Track: On") ); @@ -1469,7 +1488,7 @@ void Playlist::setStopAfterMode( int mode ) { PlaylistItem *prevStopAfter = m_stopAfterTrack; - + m_stopAfterMode = mode; switch( mode ) { case DoNotStop: @@ -1489,18 +1508,14 @@ m_stopAfterTrack->update(); } -int Playlist::stopAfterMode() const +int Playlist::stopAfterMode() { - if( !m_stopAfterTrack ) - return DoNotStop; - else if( m_stopAfterTrack == m_currentTrack ) - return StopAfterCurrent; - else if( m_stopAfterTrack == m_nextTracks.getLast() ) - return StopAfterQueue; - else if( m_stopAfterTrack ) - return StopAfterOther; - else - return DoNotStop; + if ( m_stopAfterMode != DoNotStop + && m_stopAfterTrack && m_stopAfterTrack == m_currentTrack ) { + m_stopAfterMode = StopAfterCurrent; + } + + return m_stopAfterMode; } void Playlist::generateInfo() @@ -2051,8 +2066,8 @@ PlaylistItem::setPixmapChanged(); - if( m_stopAfterTrack == m_currentTrack ) - m_stopAfterTrack = 0; //we just stopped + if( stopAfterMode() == StopAfterCurrent ) + setStopAfterMode( DoNotStop ); //reset glow state slotGlowTimer(); @@ -2099,7 +2114,9 @@ if (m_stopAfterTrack) { m_stopAfterTrack = 0; - setStopAfterMode( DoNotStop ); + if ( stopAfterMode() != StopAfterCurrent ) { + setStopAfterMode( DoNotStop ); + } } const PLItemList prev = m_nextTracks; m_nextTracks.clear(); @@ -4314,7 +4331,8 @@ if( m_stopAfterTrack == item ) { m_stopAfterTrack = 0; //to be safe - setStopAfterMode( DoNotStop ); + if (stopAfterMode() != StopAfterCurrent) + setStopAfterMode( DoNotStop ); } //keep m_nextTracks queue synchronized --- trunk/extragear/multimedia/amarok/src/playlist.h #616541:616542 @@ -138,7 +138,7 @@ //call this every time you modifyDynamicMode(), otherwise you'll get memory leaks and/or crashes void finishedModifying( DynamicMode *mode ); - int stopAfterMode() const; + int stopAfterMode(); void addCustomMenuItem ( const QString &submenu, const QString &itemTitle ); void customMenuClicked ( int id ); @@ -230,6 +230,7 @@ void setFilter( const QString &filter ); void setFilterSlot( const QString &filter ); //uses a delay where applicable void setStopAfterCurrent( bool on ); + void setStopAfterItem( PlaylistItem *item ); void toggleStopAfterCurrentItem(); void toggleStopAfterCurrentTrack(); void setStopAfterMode( int mode ); @@ -388,6 +389,7 @@ DynamicMode *m_dynamicMode; KURL::List m_queueList; PlaylistItem *m_stopAfterTrack; + int m_stopAfterMode; bool m_showHelp; bool m_dynamicDirt; //So we don't call advanceDynamicTrack() on activate() bool m_queueDirt; //When queuing disabled items, we need to place the marker on the newly inserted item