<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/105813/">http://git.reviewboard.kde.org/r/105813/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 6th, 2012, 7:08 p.m., <b>George Kiagiadakis</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;">Much better now. It's a bit unclear how it behaves if there are multiple clients, but this is not a realistic scenario anyway. One other thing I just noticed (but don't hold this commit for this) is that if a client quits without stopping music first (i.e. the service gets unregistered), you may want to change presence back.</pre>
</blockquote>
<p>On August 7th, 2012, 10:02 a.m., <b>Martin Klapetek</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;">> It's a bit unclear how it behaves if there are multiple clients
It should be whichever player sends the Metadata last, will "win". Unfortunately Amarok seems retarted, because it sends with every manual track change the "PlaybackStatus=Stopped" signal first, which disables the plugin as it listens to that signal. This makes it effectively unusable with Amarok, but also JuK. Works perfectly fine with Clementine.
So the question is - should we deactivate this presence when you stop the playback (like it is now) or just leave it on the "Now playing..." and keep waiting for the playback status to change until the user changes the presence manually?
> if a client quits without stopping music first, you may want to change presence back
I tested with Clementine and that one behaves correctly - before it quits, it sends the "PlaybackStatus=Stopped" message, while Amarok just quits. So we'll have to add a connection for the service being unregistered and disable it then. I might try to fix amarok/report a bug.</pre>
</blockquote>
<p>On August 7th, 2012, 10:09 a.m., <b>George Kiagiadakis</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;">Hmm, I didn't notice this. I don't think the plugin should be disabled when you stop playing. You may well want to stop the player without quitting it and start it again later. I think it should only be disabled when the player quits. When it just stops, you could just reset the presence.</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;">Just in case it was misunderstood, this is how it works right now - user selects the "now playing" presence, the plugin is activated in kded where it waits for player signal, player starts playing, a temp presence is inserted into the contact list presence chooser and is selected, when the player stops/pauses, the plugin in kded is deactivated again and the previous user presence is restored, which causes the presence chooser to select different presence and thus deactivating this plugin (once again, but nothing happens as it is deactivated). At this point, this plugin won't be activated again until user selects the "now playing" presence in contact list again.
I did this with the assumption, that if you stop your music, you want your presence to go back to whatever it was. But this breaks badly with Amarok/JuK/possibly others, who send "PlaybackStatus=Stopped" first when changing songs manually (continuous play is fine).
I'm thinking we could change this whole thing to not be activated by an item in the presence dropdown (and deactivated when something else is selected), but rather by some checkbox "Set my presence to 'now playing' whenever there's a music playing". So it would be activated all the time and whenever you play music, it automatically sets the presence to the configured string. if you pause/stop it, it goes back to your previous presence. if you play it again, it sets the now playing presence again. Maybe a checkbox in the settings menu?
Anyway I'll commit this now as-is, to at least get rid of the sync dbus calls.</pre>
<br />
<p>- Martin</p>
<br />
<p>On August 6th, 2012, 9:13 a.m., Martin Klapetek 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 Telepathy.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated Aug. 6, 2012, 9:13 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;">I reworked the kded module to use async dbus calls. Now when it detects a player, it queries for all its properties (to save some roundtrips) and if it finds the player is playing, it sets the song info as the presence.</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 with clementine.</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>telepathy-mpris.h <span style="color: grey">(de45cec)</span></li>
<li>telepathy-mpris.cpp <span style="color: grey">(8386dd9)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105813/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>