Review Request 116940: State-affinity and engaging now Playing from a custom status message fixes.

[James Smith]

> On May 8, 2014, 9:45 p.m., David Edmundson wrote:
> > status-handler.cpp, line 128
> > <https://git.reviewboard.kde.org/r/116940/diff/11/?file=271992#file271992line128>
> >
> >     Martin, is this true?
> >     This seems really bad
> 
> Martin Klapetek wrote:
>     This was not added by me --> 
>     
>     commit fffcc2e011b6380a5478a9833d4dcd331b7906e6
>     Author: James Smith <smithjd15 at gmail.com>
>     Date:   Thu Feb 13 11:22:40 2014 +0100
>     
>         Fix delayed status update for KTp
>     
>         Fixes bug where state changes are slow to be returned to user-set values
>         after autoaway / screensaveraway interruption.
>     
>         REVIEW: 114569
>     
>     
>     ...however it was reviewed by me.
>     
>     Patch: https://git.reviewboard.kde.org/r/114569/diff/
> 
> James Smith wrote:
>     I don't know if this is necessarily bad, but a consequence of mpris2 polling. I think the mpris2 plugin needs to be re-worked to better take into account multiple players and then it might also be possible to filter the extra ticks in the plugin. It's not a bad addition here.
> 
> David Edmundson wrote:
>     Seems you're right. Good spot.
>     Fixed properly hopefully.
> 
> Martin Klapetek wrote:
>     What mpris polling are you talking about? It's fully signal based, there is no polling...
> 
> James Smith wrote:
>     The Juk player combined with the VLC Phonon backend emits > 20 presence changes per track change. This is a fairly bad state of affairs. There should be a lower-level filter to prevent flooding MC account plugins with presence message updates.
> 
> Martin Klapetek wrote:
>     Right; that's that player's bug and should be fixed there in the first place. Have you filed a bug? Does any other player have similar problems?
> 
> James Smith wrote:
>     Most of the players I tested with GStreamer + Phonon emitted a track change between twice and six times. It varies between players with the general trend being no less than two emits per track change. Dragon, Juk, Clementine, Tomahawk all had this problem. Juk with VLC Phonon backend was the worst performer with ~26 track change emits, the VLC backend was often similarly performing with different players, emitting close to the same number of re-signals. The results were sometimes different between first play and later plays. Overall, I think wide variations in implementing the mpris2 specification cause this issue, and the best possible way to ignore it is by making sure that the presence change is only emitted once. A number of players even cause a presence change re-signal upon opening with no file playing. I don't know if it's worth going to the trouble of filing bugs, simply because of the massive number of players involved that implement mpris2 in any number of different languages to varying degrees of compliance.
> 
> Martin Klapetek wrote:
>     I just tested it - you're right. Juk is broken (or some part underneath), it sends "PropertiesChanged" signal which change different properties, but metadata actually stay the same (and for some reason are part of the dbus message). Couldn't reproduce at all with Clementine.
>     
>     Anyways, I've pushed a fix for this in both 0.8 branch and master, so should be fixed now.

Excessive PropertiesChanged signals might have been implicated in bug #334492.


- James


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116940/#review57616
-----------------------------------------------------------


On May 11, 2014, 9:25 a.m., James Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116940/
> -----------------------------------------------------------
> 
> (Updated May 11, 2014, 9:25 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Bugs: 332082, 334492 and 334542
>     http://bugs.kde.org/show_bug.cgi?id=332082
>     http://bugs.kde.org/show_bug.cgi?id=334492
>     http://bugs.kde.org/show_bug.cgi?id=334542
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> -------
> 
> This patch returns the ability to engage status message plug-ins from custom status messages. Also working is the disabling of non-visible status message plug-ins. State-affinity in the 95% of previously noted cases has been vastly improved also, the few remaining issues should be due to "lite" protocols that don't have a full complement of on-line presences.
> 
> 
> Diffs
> -----
> 
>   status-handler.h 06240ff 
>   status-handler.cpp 4b9c25a 
>   telepathy-kded-module-plugin.h 4c16169 
>   telepathy-kded-module-plugin.cpp daf73c6 
>   telepathy-mpris.cpp 69e8562 
> 
> Diff: https://git.reviewboard.kde.org/r/116940/diff/
> 
> 
> Testing
> -------
> 
> Disconnect / reconnect, autoconnect / no autoconnect, suspend / resume. Enable / disable via kcm module. Added a new custom presence and engaged the now playing plugin in the contact list from the new presence. Disabled the plugin by activating another presence.
> 
> 
> Thanks,
> 
> James Smith
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140513/6f5076b6/attachment.html>