<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/105610/">http://git.reviewboard.kde.org/r/105610/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmm, I think I've found a solution for trackFinishedPlaying(), not a workaround, hold on with reviewing until I update this.</pre>
<br />
<p>- Matěj</p>
<br />
<p>On July 19th, 2012, 7:14 p.m., Matěj Laitl wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok and Bart Cerneels.</div>
<div>By Matěj Laitl.</div>
<p style="color: grey;"><i>Updated July 19, 2012, 7:14 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=299890">299890</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>ChangeLog <span style="color: grey">(1d4f1e92fb5a033500c0f18d3dca257a89f1a139)</span></li>
<li>src/EngineController.h <span style="color: grey">(ad2e0c4a5e7c80c79bf448bf74cd6b52cd1f0ed3)</span></li>
<li>src/EngineController.cpp <span style="color: grey">(83f0a6ed0a92ae992e1809800cee65d9349dc680)</span></li>
<li>src/core-impl/meta/stream/Stream.h <span style="color: grey">(2b0a5824eae49345807eef94a465e133996624a1)</span></li>
<li>src/core-impl/meta/stream/Stream.cpp <span style="color: grey">(b0fcfad5808b9ae428cb6612bec1737b3af82d3b)</span></li>
<li>src/core-impl/meta/stream/Stream_p.h <span style="color: grey">(58b715f27cb43d014d5837f1afec9d60cb71cc48)</span></li>
<li>src/playlistmanager/file/PlaylistFileProvider.cpp <span style="color: grey">(bafdf69f24e606e0a1e879fda78cedfef7325cbb)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105610/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>