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

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


- Martin


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

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to