----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105610/#review16636 -----------------------------------------------------------
Ship it! I see no issues. Didn't test, but I assume you did extensively anyway. Now go and roll 2.6 RC1 tarball or it will never happen! - Bart Cerneels On July 24, 2012, 4:07 p.m., Matěj Laitl wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105610/ > ----------------------------------------------------------- > > (Updated July 24, 2012, 4:07 p.m.) > > > Review request for Amarok and Bart Cerneels. > > > Description > ------- > > EngineController: don't do serious work in slotAboutToFinish() > > ...because slotAboutToFinish() may be called twice (or not at all) per > track by some Phonon backends (hi, vlc) - increase play count rather in > slotNewTrackPlaying() or in slotFinished(). This also needs to change > how m_currentTrack is handled, because slotNewTrackPlaying() needs to > have the old one in m_currentTrack. > > Also, PlaylistActions::requestNextTrack() is changed to be a read-only > method that shouldn't change playlist state especially when there is no > next track. PlaylistActions::reflectPlaybackFinished() is introduced to > do the thing and is called from EngineController::slotFinished(), which > is a much better place for it than slotAboutToFinish(). > > Reporters of CCed bugs, please re-test your bug with this commit > applied, it is possible it has been resolved by this patch. > > BUG: 299890 > CCBUG: 268892 > CCBUG: 277197 > CCBUG: 302652 > CCBUG: 303580 > CCBUG: 302240 > FIXED-IN: 2.6 > > MetaStream: match track by QUrl, not by QString > > Fixes a bug where MetaStram::Track didn't update tags here because > pretty url was used in signal, but it compared against encoded url > (the one with %20 instead of spaces). > > MetaStream: big clean-up, implement play statistics methods > > * remove many unused and useless methods such as setTitle() and > friends > * additionally take genre, comment, track number from Engine > Controller's signal > * implement rating, score, first & last play and play count. These are > rather debugging tool for EngineController, but they also give you > nice info on how many songs have played in the stream > * no need to reimplement observer pattern, use MetaBase implementation > * implement length, EngineController will emit length info soon > > EngineController: introduce trackFinishedPlaying() signal > > Used do call track->finishedPlaying() now, to be used in Last.fm > scrobbling when it is emitted for streams, too. > > EngineController: reduce code duplication among play() and stop() > > > EngineController: rework slotMetaDataChanged(), nearly no functional change > > * get data from Phonon to meta QVariantMap more intelligently > * m_lastTrack and trackChanged had effect only in debug message, ditch > them > * note that m_currentTrack may be inaccurate there > * rename isMetadataSpam() to isInRecentMetaDataHistory() to describe > the functionality better > * less debug() spam, specifically be rather quiet for duplicate > signals from phonon > * Phonon doesn't provide track length, but it does provide track > description that we save as "comments" now. > * trackData() method was unused and likely meant for > slotMetaDataChanged(); ditch it > > > This addresses bug 299890. > https://bugs.kde.org/show_bug.cgi?id=299890 > > > Diffs > ----- > > ChangeLog 1d4f1e92fb5a033500c0f18d3dca257a89f1a139 > src/EngineController.h ad2e0c4a5e7c80c79bf448bf74cd6b52cd1f0ed3 > src/EngineController.cpp 83f0a6ed0a92ae992e1809800cee65d9349dc680 > src/core-impl/meta/stream/Stream.h 2b0a5824eae49345807eef94a465e133996624a1 > src/core-impl/meta/stream/Stream.cpp > b0fcfad5808b9ae428cb6612bec1737b3af82d3b > src/core-impl/meta/stream/Stream_p.h > 58b715f27cb43d014d5837f1afec9d60cb71cc48 > src/playlist/PlaylistActions.h 4f27658f48fa7af65c1fbc26d5c184c3f6070f78 > src/playlist/PlaylistActions.cpp 1be1d5af6313eb0058c7c65ba7304fa2bd1c00a1 > > Diff: http://git.reviewboard.kde.org/r/105610/diff/ > > > Testing > ------- > > Works for me, fixes mentioned bug and more smaller glitches. > > > Thanks, > > Matěj Laitl > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
