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: RenderManager: treat mono as non-3d mode when updating display resolution #13466

Closed
wants to merge 1 commit into from

Conversation

codesnake
Copy link
Contributor

@codesnake codesnake commented Feb 1, 2018

Treat "mono" as non-3D mode when updating display resolution.

Description

CVideoPlayer::OpenVideo() may pass "mono" as stereo mode to CRenderManager::TriggerUpdateResolution(), but CRenderManager::UpdateResolution() mistakenly treats this as a 3D mode, leading to selection of wrong display mode.

Motivation and Context

Kodi sometimes switches to display mode with refresh rate not matching FPS of the video being played.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

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

@FernetMenta
Copy link
Contributor

FernetMenta commented Feb 1, 2018

I want to get rid of this "mono" crap. It should not be set as a stereo mode. Either eliminate this label from the entire project or apply an intermediate fix at a higher level. i.e.:

--- a/xbmc/cores/VideoPlayer/VideoPlayerVideo.cpp
+++ b/xbmc/cores/VideoPlayer/VideoPlayerVideo.cpp
@@ -700,7 +700,7 @@ bool CVideoPlayerVideo::ProcessDecoderOutput(double &frametime, double &pts)
           stereoMode = m_hints.stereo_mode;
           break;
       }
-      if (!stereoMode.empty())
+      if (!stereoMode.empty() && stereoMode != "mono")
       {
         m_picture.stereoMode = stereoMode;
       }
--- a/xbmc/guilib/StereoscopicsManager.cpp
+++ b/xbmc/guilib/StereoscopicsManager.cpp
@@ -162,7 +162,7 @@ RENDER_STEREO_MODE CStereoscopicsManager::GetNextSupportedStereoMode(const RENDE
 
 std::string CStereoscopicsManager::DetectStereoModeByString(const std::string &needle)
 {
-  std::string stereoMode = "mono";
+  std::string stereoMode;
   std::string searchString(needle);
   CRegExp re(true);

@da-anda
Copy link
Member

da-anda commented Feb 1, 2018

mono is coming from MKV meta-data, see https://www.matroska.org/technical/specs/index.html#StereoMode . So we have to deal with it at some point. IIIRC we kept it, because this indicated that we at least know the current stream is monoscopic and not unknown (which an empty string could also mean).

edit: and it might be needed for some GUI labels/skin stuff. Don't remember all the details and need to check the code again

@FernetMenta
Copy link
Contributor

VP won‘t deal with mono at lower levels. If mkv uses this label, it has to be cleared at demuxer

@codesnake
Copy link
Contributor Author

Superseded by #13469.

@codesnake codesnake closed this Feb 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants