Review Request 115282: Fix mpris2 status flicker on active away plugin (priority override)

Fri Feb 7 23:49:50 UTC 2014

> On Jan. 24, 2014, 10:18 a.m., David Edmundson wrote:
> > telepathy-mpris.cpp, line 264
> > <https://git.reviewboard.kde.org/r/115282/diff/1/?file=235593#file235593line264>
> >
> >     I think this will cause problems.
> >     
> >     If you set yourself to Invisible and have a local-xmpp account which doesn't support invisible. When you listen to a track it will now move all your accounts "Away". 
> >     
> >     Could you explain the problem we're trying to solve?
> 
> James Smith wrote:
>     #114569 exposed an issue where the status will noticeably flicker in ktp-plasma-applet when the status is set by the AutoAway plugin (from Available state) and subsequent track changes in MPRIS2 change the status without updating the away message, which is set in AutoAway and takes precedence. The status will quickly switch back to Away from Available, returning the Away status message instead of the MPRIS2 status message (and associated Available state).
>     
>     So, don't even bother updating the MPRIS2 status and message.
> 
> David Edmundson wrote:
>     Why does it go back to available before it goes to whatever MPRIS is set to?
> 
> James Smith wrote:
>     No, I don't think so. Because we're requesting what the presence is RIGHT NOW, removing the current status message and pasting a new one in and relying on the plugin priority to show a change in persistent status or not, the MC plugins shouldn't receive a status change and instead should remain at what the Ktp plugin allowed or explicitly (not persistently) set. The away KTp plugin, being higher priority, won't allow the mpris2 KTp plugin to maintain a changed status or changed status message.
>     
>     requestedPresence() should (will) return only what the global presence was requested to be set at, not what currentPresence() reported the global presence to actually be (plugins are taken into account by requestedPresence()). 
>     
>     The jump to global Available was caused by requestedPresence() reporting the user-set presence, not the plugin-priority forced Away presence. The mpris2 status message was ignored (properly) by the plugin ordering; however the status flickered while mpris2 KTp plugin set the requested user presence, changed the status message, and then returned (only to have the Away KTp plugin re-take control over global status based on its timer, which caused a reset to away hopefully before anybody noticed what happened by KTp mpris2.) 
>     
>     I don't know if anyone would need mpris2 status messages while the machine is autoway or on screensaver away? Probably bad netiquette.
> 
> David Edmundson wrote:
>     >The jump to global Available was caused by requestedPresence() reporting the user-set presence, not the plugin-priority forced Away presence. The mpris2 status message was ignored (properly) by the plugin ordering; 
>     
>     Why would it be ignored?
>     
>      - user is playing music
>      - mpris plugin activates
>      - presence is [online - "Some Song"]
>      - user goes away
>      - autoaway plugin activates
>      - presence is [away - "I am not here right now"]
>      - user comes 
>      - auto away plugin deactivates
>      - presence gets set to that of the mpris plugin [online - "Some song"]
>     
>     I can imagine a bug in which a track change whilst away sets you to [away - "Some song"] when you come back; but this patch won't fix that, and that's not this flickering you describe.
>
> 
> James Smith wrote:
>     Since KTp MPRIS2 doesn't actually change the global status, but does set the status message, it probably belongs in its own class of plugin.
>     
>     This approach seems proper, since taking the current status and adding a status message is correct when relying on the plugin ordering mechanism to properly set and unset the global presence. Any change in global status unsets the MPRIS2 status message, which is probably great for people who leave both music and IM open while physically away from the machine or with a screensaver, providing a bit more privacy.
>     
>     Status messages are bound by plugin ordering, just like global presences.
>     
>     Reordering the plugins with mpris2 before the state-change plugins doesn't work either, causing the away plugin to wait on the track change. I think I might be right in thinking the plugins are ordered the way they are out of necessity? MPRIS2 won't interoperate with Away at all unless its pulling up the rear and only then with #114569 and this patch applied.
>     
>     Unrelated, but there are currently no checks for hidden status and for preventing a plugin from picking a visible state while manually the global presence is hidden.
> 
> David Edmundson wrote:
>     I still don't understand why the current system breaks. Until I understand that I can't sign off on anything.
>     
>     Can you explain in steps like above how and why this flicker occurs?
> 
> James Smith wrote:
>     1 run juk on a long playlist
>     2 open dragon and run a long movie without setting off autoaway
>     3 watch as between tracks in juk ktp tray jumps to available with new track information and then back to away
>     4 verify you arent seeing things by watching a tp plugin for presence change in a dbus
>     
>     I'm working on a more robust fix for these two issues.

I finally got round to trying to reproduce this. (sorry for the delay btw)
Using Tomahawk (not Juk) I didn't see any flicker my setup was:

1) start listening to music
2) set myself to "Now listening to..." (worked correctly)
3) ..waiting
4) went to autoaway
5) on track changes everything stayed as away.

Moving out of autoaway brought me back to NowPlaying. Pausing that brought me back to Online.
Everything seemed ok \o/

Maybe JUK sends super weird MPRIS signals and sends a pause and play. Perhaps we can capture this in bustle?

- David

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

On Jan. 24, 2014, 3:43 a.m., James Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115282/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2014, 3:43 a.m.)
> 
> 
> Review request for Telepathy and Xuetian Weng.
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> -------
> 
> Fixes state flicker when away plugin is active and track changes in a media player.
> 
> 
> Diffs
> -----
> 
>   telepathy-mpris.cpp 1c7b98c 
> 
> Diff: https://git.reviewboard.kde.org/r/115282/diff/
> 
> 
> Testing
> -------
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140207/22ae8c3a/attachment-0001.html>