Differential D11359

MPRIS: for images report the slideshow interval as well as timer resets
ClosedPublic
Actions

Authored by kossebau on Mar 15 2018, 6:01 PM.

Details

Reviewers
rkflx
Group Reviewers
Gwenview
Commits
R260:4fbffe22bab8: MPRIS: for images report the slideshow interval as well as timer resets
Summary

Setting "mpris:length" "Metadata" property of images matching the interval of the
slideshow, signalling resets of the "Position" and delivering the
current timeout progress in the autoadvance timer, all that allows
MPRIS controllers to show the timely progress in the slideshow.

Supporting the same for video files is left-out for now, that needs
some bigger refactoring to have this information properly modelled
by the slideshow object.

((The Plasma media controller applet of <= 5.12 sadly only shows time
progress information if CanSeek=true. That got fixed for Plasma 5.13,
which will show a read-only progressbar instead of a slider))

Diff Detail

Repository
R260 Gwenview
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau requested review of this revision.Mar 15 2018, 6:01 PM
kossebau created this revision.
kossebau added a comment.Mar 15 2018, 6:03 PM

Plasma media controller applet got fixed only today (motivated by this patch :) ), so if running some Plasma from master branches, make sure to update to something from after the commit of D11356

rkflx accepted this revision.Mar 17 2018, 12:52 AM
rkflx added a subscriber: rkflx.

Thanks, works great. A bit unusual at first, but totally makes sense in the end.

lib/slideshow.cpp
251

The first condition should be equivalent to isRunning().

Is there any situation we might need the second condition?

252

Spaces: *

lib/slideshow.h
57–62

Insert linebreak.

This revision is now accepted and ready to land.Mar 17 2018, 12:52 AM
Closed by commit R260:4fbffe22bab8: MPRIS: for images report the slideshow interval as well as timer resets (authored by kossebau). · Explain WhyMar 17 2018, 9:12 AM
This revision was automatically updated to reflect the committed changes.
kossebau marked 3 inline comments as done.
kossebau added inline comments.Mar 17 2018, 9:17 AM
lib/slideshow.cpp
251

Second condition in needed because QTimer::remainingTime(): "If the timer is inactive, the returned value will be -1." So this is needed to protect against this.
And there is some state with the existing code when mState == Started, but timer already done. Added an inline comment to make this more clear.

251

Actually the whole logic was not complete as I found when reasoning, changed it now that for Stopped (&WaitForUrl) there is always 0, and only calculate a value if Started and give extra case for timer already done == end reached.

Makes sense as is now?

rkflx added inline comments.Mar 17 2018, 11:05 PM
lib/slideshow.cpp
251

Makes sense as is now?

Much more straightforward now ;)

Revision Contents
Changeset List

PathPackages
Mlib/mpris2/mprismediaplayer2player.cpp (14 lines)
Mlib/slideshow.h (12 lines)
Mlib/slideshow.cpp (19 lines)

Diff 29740

View Options

lib/mpris2/mprismediaplayer2player.cpp

Loading...
View Options

lib/slideshow.h

Loading...
View Options

lib/slideshow.cpp

Loading...