Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[skin] show playspeed #10266

Merged
merged 2 commits into from Aug 15, 2016

Conversation

@Rechi
Copy link
Member

commented Aug 10, 2016

This shows the playspeed in the skins from #10216

@@ -75,6 +75,7 @@
<value condition="Player.Forwarding">$LOCALIZE[31044] $VAR[Speed]</value>
<value condition="Player.Rewinding">$LOCALIZE[31045] $VAR[Speed]</value>
<value condition="Player.Paused">[UPPERCASE]$LOCALIZE[31043][/UPPERCASE]</value>
<value condition="Player.IsTempo">$LOCALIZE[31046]: $INFO[Player.PlaySpeed]</value>

This comment has been minimized.

Copy link
@ronie

ronie Aug 10, 2016

Member

please use [UPPERCASE] here

@Rechi Rechi force-pushed the Rechi:skinPlayspeed branch from 1e65e85 to 3869766 Aug 10, 2016
@Rechi

This comment has been minimized.

Copy link
Member Author

commented Aug 10, 2016

@ronie updated

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Aug 10, 2016

great!

@Rechi

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2016

I thought it might be useful to replace the condition Player.IsTempo with Player.TempoEnabled + Player.Playing.
Then it will also display Play speed: 1.00, but only if the player can change speed. This would show the ability of changing speed for the actual stream.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Aug 12, 2016

Player.Playing is redundant in this context. TempoEnabled returns false if player is not active.

@Rechi

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2016

TempoEnabled also returns true if player is fastforwarding or rewinding, but I think Play speed: * only should be displayed if playing.
My question was if it also makes sense to display Play speed: 1.00 or should it display nothing in this case.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Aug 12, 2016

right, this differs from the internal implementation where IsPlaying is also true for ff/rw.

My question was if it also makes sense to display Play speed: 1.00 or should it display nothing in this case.

Good question. Now that we have a label for speed, does it still make sense to display "forwarding" or "rewinding"? Why not just show the speed label for all cases?

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Aug 15, 2016

@ronie

This comment has been minimized.

Copy link
Member

commented Aug 15, 2016

tested and works ok at my end.

thx!

@ronie ronie merged commit 1034f24 into xbmc:master Aug 15, 2016
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@Rechi Rechi deleted the Rechi:skinPlayspeed branch Aug 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.