<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/104681/">http://git.reviewboard.kde.org/r/104681/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 20th, 2012, 12:31 a.m., <b>Eike Hein</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You currently can't commit it, since the kdemultimedia repositories are still locked.
And to be honest I'd be slightly miffed if this went in over my version, since, well, I did get there first. I also don't like some aspects of your implementation, e.g. the superfluos extra abstraction of generating adaptors from XML that subsequently need to be backed by nearly-identical classes anyway. I also don't think there's -that- much use to caching the values in the property system since the spec largely avoids the need for polling (due to PropertiesChanged) and so dynamic return value computation shouldn't make for a big difference, save for nicer code.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ah, I also want to add that I disagree with mpyne's decision to use _HH encoding for the track ids. Michael really wants the file URL in there because it means he can reconstruct it from a request, but because D-Bus imposes a 255 character limit on path components I find using the URL too brittle and went for a SHA1 hash instead. And also, since the TrackList interface spec demands uniqueness even for duplicate entries of the same file in a playlist, using the file URL alone will not be good enough anyway.</pre>
<br />
<p>- Eike</p>
<br />
<p>On April 20th, 2012, 12:19 a.m., Alex Merry 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 KDE Multimedia and Michael Pyne.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated April 20, 2012, 12:19 a.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;">This adds basic support for MPRIS2 to JuK (http://specifications.freedesktop.org/mpris-spec/latest/)
Some (optional) things are still unsupported as I have not figured out how to make them work. Notably, it is not possible to seek, as the naive implementation (calling PlayerManager->seek()) behaves strangely (the existing D-Bus interface doesn't support seeking, either).
The existing collection interface is not replicated. Most things from the existing player interface are implemented in this MPRIS2 interface (I think the only exception is that MPRIS2 does not have an "album random" option).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Tested using the MPRIS2 tester application (http://github.com/randomguy3/mpristester).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(9bd57f6c08abd9b144b0abc1775702284d6af753)</span></li>
<li>mprisproxy.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>mprisproxy.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>org.mpris.MediaPlayer2.Player.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>org.mpris.MediaPlayer2.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>playermanager.h <span style="color: grey">(0eec655b487c5e0e37856b39e128087038987f05)</span></li>
<li>playermanager.cpp <span style="color: grey">(54f601d19f801327f45b3157ed090aa785370583)</span></li>
<li>volumepopupbutton.h <span style="color: grey">(6fb621b10c9a29f11d66869aac6451ed52935d79)</span></li>
<li>volumepopupbutton.cpp <span style="color: grey">(76bd95d8e8713ace3afcba0ee510fdb19d6777b3)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104681/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>