----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105610/ -----------------------------------------------------------
(Updated July 19, 2012, 7:14 p.m.) Review request for Amarok and Bart Cerneels. Changes ------- Rebase on top of the current master, so that this can be still tested. Description ------- 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). EngineController: reduce code duplication among play( TrackPtr ) and stop() EngineController: fix play count increased by 2 in some cases First, I tried to fix this using the "correct" approach by emitting trackFinishedPlaying() in slotFinished() [connected to Phonon finished()] and in slotNewTrackPlaying() [connected to Phonon currentSourceChanged()], but I failed, because even currentSourceChanged() is emitted twice by some back-ends (at least phonon-vlc 0.5) plus slotNewTrackPlaying() has hard time determining whether m_currentTrack was really played. So I've solved it by adding guard variable to detect slotAboutToFinish() being called twice per song. We would need this even for other things, I had following bug at the end of the playlist: first slotAboutToFinish() calls playlistActions()->requestNextTrack() which correctly says "nothing more to play", but the next call to it re-starts playback from the start of the playlist. BUG: 299890 FIXED-IN: 2.6 EngineController: fix isStream(), style fix for trackPosition() 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 ...to be used in Last.fm scrobbling when it is emitted for streams, too. 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 EngineController: remove unimplemented methods No one could have called them, so remove them altogether to reduce developer confusion. PlaylistFileProvider: fix switch { } compiler warning This addresses bug 299890. https://bugs.kde.org/show_bug.cgi?id=299890 Diffs (updated) ----- 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/playlistmanager/file/PlaylistFileProvider.cpp bafdf69f24e606e0a1e879fda78cedfef7325cbb Diff: http://git.reviewboard.kde.org/r/105610/diff/ Testing ------- Thanks, Matěj Laitl
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel