Review Request: Make the kded module use async dbus calls

[George Kiagiadakis]

> On Aug. 1, 2012, 12:51 p.m., George Kiagiadakis wrote:
> > telepathy-mpris.cpp, lines 80-99
> > <http://git.reviewboard.kde.org/r/105813/diff/1/?file=75589#file75589line80>
> >
> >     This code could be improved a lot. First by just checking the "PlaybackStatus" property instead of all of them and second by caching properties for each player so that you don't have to fetch them all again for each player... I suspect it works, though, but it is a bit fragile imho.
> 
> Martin Klapetek wrote:
>     > checking the "PlaybackStatus" property instead of all of them
>     
>     True, but if the status is "Playing", we need to fetch the song metadata - do another dbus call and handle it. So I fetch it all at once, check for the playback status (again..) and if "Playing", the track data are available right away. That's why I did it like this. But I can change it to fetch only those properties really needed.
>     
>     > caching properties for each player so that you don't have to fetch them all again for each player
>     
>     Yeah that's a bit crap. We actually only need the PlaybackStatus and Metadata properties and caching these is no use.
>     
>     --
>     
>     I can rework this to check for PlaybackStatus in detectPlayers() and then fetch the track data only if the status is Playing. That way we will query all players for only one property and one more if it is playing. Then I can try changing the onPlayerSignalReceived(..) method to actually be aware which player emitted that signal and only query that one player for the song data.
>     
>     Would that work?

I was more referring to checking the "PlaybackStatus" property instead of doing a Q_FOREACH() going through all the properties and seeing which one has an acceptable value... I am not talking about GetAll here.

Also, about caching properties... the spec says that the PropertiesChanged signal is emitted for all properties, including the Metadata one. So, instead of asking the player for the property you could wait for the signal and cache the value - only for the Metadata property. This is how tp-qt gets property values 99% of the time.


- George


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105813/#review16762
-----------------------------------------------------------


On Aug. 1, 2012, 12:41 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105813/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2012, 12:41 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   telepathy-mpris.h de45cec 
>   telepathy-mpris.cpp 8386dd9 
> 
> Diff: http://git.reviewboard.kde.org/r/105813/diff/
> 
> 
> Testing
> -------
> 
> Tested with clementine.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120802/15312d03/attachment-0001.html>