--===============0074836738225699852== Content-Type: multipart/alternative; boundary="===============0523481759318868312==" --===============0523481759318868312== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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 > > --===============0523481759318868312== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111512/ |
Ship it!
Looking good, please push to master.
- Mark
On July 15th, 2013, 1:18 p.m. UTC, Alex Merry wrote:
Review request for Amarok.
By Alex Merry.
Updated July 15, 2013, 1:18 p.m. Description
Testing
Diffs
|