<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 />


<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 24, 2012, 4:07 p.m.</i></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">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! :-) )</pre>
  </td>
 </tr>
</table>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description  (updated)</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;">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</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing (updated)</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;">Works for me, fixes mentioned bug and more smaller glitches.</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> (updated)</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/playlist/PlaylistActions.h <span style="color: grey">(4f27658f48fa7af65c1fbc26d5c184c3f6070f78)</span></li>

 <li>src/playlist/PlaylistActions.cpp <span style="color: grey">(1be1d5af6313eb0058c7c65ba7304fa2bd1c00a1)</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>