-----------------------------------------------------------
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.
Changes
-------
Rebase on top of the current master, rearrange commits, better (real) fix for
bug 299890, which may fix some more Phonon shortcommings.
This is the shape of this patch-set I'd like to see in 2.6 RC, please review.
Ack if you agree with the direction, you don't need to review every single line
- I'm quite confident I've sorted out the details right. (oh noes, bragging on
reviewboard! :-) )
Description (updated)
-------
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 (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/playlist/PlaylistActions.h 4f27658f48fa7af65c1fbc26d5c184c3f6070f78
src/playlist/PlaylistActions.cpp 1be1d5af6313eb0058c7c65ba7304fa2bd1c00a1
Diff: http://git.reviewboard.kde.org/r/105610/diff/
Testing (updated)
-------
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
