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

[VideoPlayer] SetResolution synchronously instead RM::TriggerResolution #14755

Merged
merged 1 commit into from Nov 1, 2018

Conversation

peak3d
Copy link
Contributor

@peak3d peak3d commented Oct 31, 2018

Description

Process initial resolution change (if requested in settings / from stream) synchronously on VideoPlayer startup instead using the asynchronous RenderManager::TriggerUpdateResolution.

Motivation and Context

If user has enabled "Adjust display refresh rate" and whitelist includes a resolution matching to the stream to play, we switch resolution / refresh rate on supported devices.

The current async workflow can lead to rough video startup.

This PR processes the ResolutionChange (for fullscreen video playback) before VP initializes the underlying processes. Video playback with resolution change starts with getting screen black and smooth playback afterwards.

Please note, that handling of resolution changes during playback is not changed, its still async.

How Has This Been Tested?

  • Android NVIDIA Shield
  • Start kodi in 59.97 Hz
  • Enable "Adjust display refresh rate"
  • Activate 1080p@50 in whitelist
  • Play a 1080p@50 movie

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)

@peak3d peak3d added Type: Improvement non-breaking change which improves existing functionality Component: Video v18 Leia labels Oct 31, 2018
@peak3d peak3d added this to the Leia 18.0-beta5 milestone Oct 31, 2018
m_renderManager.TriggerUpdateResolution(static_cast<float>(framerate), hint.width, hint.stereo_mode);
RESOLUTION res = CResolutionUtils::ChooseBestResolution(static_cast<float>(framerate), hint.width, hint.height, !hint.stereo_mode.empty());
CServiceBroker::GetWinSystem()->GetGfxContext().SetVideoResolution(res, false);
//RenderManager::UpdateLatencyTweak() should be called automatically as soon GUIFullScreenWindow is ready

This comment was marked as spam.

@@ -128,7 +129,6 @@ class CRenderManager
void DeleteRenderer();
void ManageCaptures();

void UpdateLatencyTweak();

This comment was marked as spam.

@peak3d
Copy link
Contributor Author

peak3d commented Oct 31, 2018

@FernetMenta PR updated.

@@ -141,6 +141,7 @@ bool CRenderManager::Configure(const VideoPicture& picture, float fps, unsigned
m_dvdClock.SetVsyncAdjust(0);
m_pConfigPicture.reset(new VideoPicture());
m_pConfigPicture->CopyRef(picture);
UpdateLatencyTweak();

This comment was marked as spam.

This comment was marked as spam.

@peak3d
Copy link
Contributor Author

peak3d commented Oct 31, 2018

jenkins build this please

Copy link
Contributor

@FernetMenta FernetMenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

For the record:
Some may think that asynchronous startup of players has zero advantage. Actually it makes behaviour like this possible because main/render thread is free for tasks like this.

@peak3d peak3d merged commit a34b770 into xbmc:master Nov 1, 2018
@peak3d peak3d deleted the refresh branch November 1, 2018 17:40
@smp79
Copy link
Contributor

smp79 commented Nov 3, 2018

This PR introduced playback issues on Raspberry Pi. If during a live TV playback I try to play a file - I get a spinning circle and playback does not start. This does not happen with all videos, mostly with .ts and .m2ts.

Debug log - https://paste.kodi.tv/epixewujah.kodi
Sample m2ts video - https://www.dropbox.com/s/aypt4ejky3sfv0y/bd_sample.m2ts?dl=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Type: Improvement non-breaking change which improves existing functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants