[MPris Plugin] Support Seek()
ClosedPublic

Authored by broulik on Sep 30 2018, 7:12 PM.

Details

Summary

Support Seek() command by issuing an appropriate SetPosition() call.
Fixes changing position through KDE Connect

Test Plan
  • Can now change position in track using KDE Connect app

Using SetPosition() (dbus exported) because that one actually sends command to browser, setPosition() (note the lowecase s) is the internal one that propagates a change via mpris

Diff Detail

Repository
R856 Plasma Browser Integration
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Sep 30 2018, 7:12 PM
Restricted Application added a project: Plasma. · View Herald TranscriptSep 30 2018, 7:12 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Sep 30 2018, 7:12 PM
fvogt requested changes to this revision.Oct 1 2018, 9:50 AM

Seek has a different behaviour for position() + offset < 0 and position() + offset == m_length:

A negative value seeks back. If this would mean seeking back further than the start of the track, the position is set to 0.

If the value passed in would mean seeking beyond the end of the track, acts like a call to Next.

SetPosition has:

If the Position argument is less than 0, do nothing.

If the Position argument is greater than the track length, do nothing.

which is different.

This revision now requires changes to proceed.Oct 1 2018, 9:50 AM
broulik updated this revision to Diff 42647.Oct 1 2018, 9:58 AM
  • Implement Seek according to spec
fvogt accepted this revision.Oct 1 2018, 11:16 AM

LGTM. I just tried how the currentTime property in JS works, it actually behaves like MPRIS' seek does and does not ignore invalid values. That might need a check in SetPosition.

This revision is now accepted and ready to land.Oct 1 2018, 11:16 AM
broulik updated this revision to Diff 42652.Oct 1 2018, 11:27 AM
  • Check bounds in SetPosition
  • Use > again since when I move the slider all the way to the end it sets the position to length and then would do nothing
broulik updated this revision to Diff 42653.Oct 1 2018, 11:29 AM
  • Use >= again :D bug is in media controller plasmoid
fvogt accepted this revision.Oct 1 2018, 11:31 AM
This revision was automatically updated to reflect the committed changes.