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

[GraphicsContext] - add setting for only adjusting refresh rate on start #15506

Merged
merged 1 commit into from Mar 1, 2019

Conversation

Projects
None yet
4 participants
@Memphiz
Copy link
Member

commented Feb 12, 2019

Description

Long standing issue on my TODO list and some requests for it in the forum over the last years aswell.
Currently we have 3 settings for refresh rate adjustment:
0 - off (refresh rate is not adjusted by Kodi)
1 - always (refresh rate is adjusted to match the movie and goes back to desktop whenever the UI is in the foreground)
2 - on start/stop (refresh rate is adjusted on video playback start and goes back to desktop on video playback stop

This PR adds option:
3 - on start (refresh rate is adjusted on video playback start only and stays there until a movie with a different fps/refresh rate is started).

This new option has the benefit to reduce the refresh rate changes which improves experience on slow changing devices (like some TVs and projectors which need a long time to adjust refresh rate).
It comes at the cost that the UI might be sluggish after movie playback when refresh rate stays at 24Hz for example).

Motivation and Context

I don't care about the sluggish UI after movie playback - I rather have one refresh rate switch less with option 3.

How Has This Been Tested?

Set option to "On start". Std refresh rate is 50Hz.
Play movie with 24Hz and stop it
Screen stays at 24Hz.
Play another movie with 24Hz and stop it.
No refresh rate change at all.
Play another movie with 25fps
Refresh rate goes to 50Hz and stays there and so on.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed
@fritsch

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

Code is fine.

@fritsch
Copy link
Member

left a comment

Code is fine - if this is needed, go ahead.

@smp79

This comment has been minimized.

Copy link

commented Feb 15, 2019

"On start/stop" setting does not work after applying this. Tested on Raspberry Pi.

@fritsch

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

Post a debuglog please

@smp79

This comment has been minimized.

Copy link

commented Feb 15, 2019

@fritsch

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2019

@smp79 could you try again with current PR state?

@smp79

This comment has been minimized.

Copy link

commented Feb 15, 2019

@Memphiz It works fine now, thanks

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2019

Thx for the test and feedback.

@fritsch

This comment has been minimized.

Copy link
Member

commented Feb 16, 2019

Thanks. This RES_DESKTOP vs. CurrentResolution stuff is still confusing at times. Please squash and then it's ready to go.

Not sure if it "should" go in directly or did we branch Leia in the meantime?

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

@Memphiz ok to merge after squash

@Memphiz Memphiz force-pushed the Memphiz:refreshrate_onstart branch from a86dca6 to fce7ce7 Mar 1, 2019

@Memphiz Memphiz merged commit 2d8a36b into xbmc:master Mar 1, 2019

1 check was pending

default Yeah yeah I'll start on it when I get some time.
Details

@Memphiz Memphiz deleted the Memphiz:refreshrate_onstart branch Mar 1, 2019

@MartijnKaijser MartijnKaijser added this to the Leia 18.2-rc1 milestone Mar 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.