Review Request 111512: MPRIS2: avoid updating Metadata when between tracks

[Mark Kretschmann]

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

Ship it!


Looking good, please push to master.

- Mark Kretschmann


On July 15, 2013, 1:18 p.m., Alex Merry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111512/
> -----------------------------------------------------------
> 
> (Updated July 15, 2013, 1:18 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> MPRIS2: avoid updating Metadata when between tracks
> 
> When changing tracks, we would emit PropertiesChanged for Metadata
> twice, once with mpris:trackid set to /org/kde/amarok/PendingTrack and
> once with the actual new trackid (because the first time the playlist
> code had not yet updated the active track).  If the track was changed
> manually (not just progressing to the next one) we would often also emit
> a PropertiesChanged with an empty Metadata before repopulating it.
> 
> This solves the first issue by making the signal connection for
> trackChanged from EngineController queued, meaning that by the time the
> MPRIS2 code gets the signal, the playlist has updated the activeTrack
> and we can easily figure out the correct trackid.
> 
> It solves the second issue by ignoring the trackLengthChanged signal
> when it has a meaningless value (<0), which seems to happen at some
> point during track changes that are not predictable.
> 
> BUG: 321602
> 
> 
> Diffs
> -----
> 
>   ChangeLog 5fe5d2e64c771d722f3f90bf6c98d5013e56553c 
>   src/dbus/mpris2/MediaPlayer2Player.cpp a633756bf558a89ba2a3db2307c0ebbc373a759b 
> 
> Diff: http://git.reviewboard.kde.org/r/111512/diff/
> 
> 
> Testing
> -------
> 
> Tested using a tool that listens to the PropertiesChanged signal of the MPRIS2 interface and lists when the mpris:trackid changes.
> 
> Without the patch, I get output like
> Track change: "/org/kde/amarok/Track/5739423209746661216" -> "/org/kde/amarok/PendingTrack"
> Track change: "/org/kde/amarok/PendingTrack" -> "/org/kde/amarok/Track/8264712350997591513"
> when the track progresses because the previous track finished, and
> Track change: "/org/kde/amarok/Track/5739423209746661216" -> ""
> Track change: "" -> "/org/kde/amarok/Track/8264712350997591513"
> when I manually change the track (eg: by clicking "next" or by double-clicking another track).
> 
> With the patch, I only get things like
> Track change: "/org/kde/amarok/Track/5739423209746661216" -> "/org/kde/amarok/Track/8264712350997591513"
> 
> 
> Thanks,
> 
> Alex Merry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130715/54afa4b7/attachment.html>