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

Resolution: Choose most matching refreshrate #10243

Merged
merged 1 commit into from Oct 1, 2016

Conversation

fritsch
Copy link
Member

@fritsch fritsch commented Aug 6, 2016

Admiral Schneider on the forum had an issue with his new 120 hz TV. This TV also has a 119.8804 mode which is used for 23.97608 content. Because our metric always ends with a distance measure of 0.0 if (refreshrate / video_fps) / round(refreshrate / video_fps) == 0.0. So basically every division without rest gets this 0.0

As we render with refreshrate 120 hz and this puts immense load for 24.0 fps content onto GPUs, we shall punish modes so that 24.0 gets a lower measure than 120 fps.

Another example: 30 fps
TV has: 30 hz, 60 hz and 120 hz

Before this PR:

w(30) == w(60) == w(120) = 0.0

After this PR:

w(30) < w(60) < w(120)

but still:

w(60) < w(59.94) < w(118.88)

@popcornmix
Copy link
Member

I used to do this in Pi builds, but there was an issue with interlaced video.

They tend to vary between 25 and 50 (or 30 and 60) with different deinterlace modes.
You'll often start playback with 25fps and detect interlaced after the first frame is decoded and then the fps is doubled when deinterlace is activated.
This scheme is likely to be switching HDMI mode more often.

Special casing 25 and 30/29.97 to prefer 2x the framerate would be one way of avoiding this issue, but I guess @FernetMenta would be better placed to comment.

@fritsch
Copy link
Member Author

fritsch commented Aug 6, 2016

@popcornmix In fact I talked with @FernetMenta before that and he told me "we can get interlaced content detected properly", but yes - you are right - we need something ontop of that most likely.

@fritsch
Copy link
Member Author

fritsch commented Aug 6, 2016

Easy fix however: http://sprunge.us/MQcU but I would consider this a hack :-)

@FernetMenta
Copy link
Contributor

Special casing 25 and 30/29.97 to prefer 2x the framerate would be one way of avoiding this issue

It is not on renderer to decide but player.

@fritsch
Copy link
Member Author

fritsch commented Aug 6, 2016

Yeah, but we need a player change. If you play 1080i50 player does not know about the 50 hz at the beginning, so it will switch to 25 hz first and after some seconds it will switch to 50hz mode.

@FernetMenta
Copy link
Contributor

Yeah, but we need a player change

a tiny one. if it is interlaced, it can't be 25hz

@fritsch fritsch force-pushed the refreshratemax branch 2 times, most recently from ede269b to c00178d Compare September 30, 2016 18:46
@fritsch
Copy link
Member Author

fritsch commented Sep 30, 2016

@FernetMenta two hacks added - I am not sure how to solve it properly.

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.

drop this please. the strategy for videoplayer is making to kill all dependencies to pvr. this change does not cover all cases anyway

@fritsch
Copy link
Member Author

fritsch commented Oct 1, 2016

force pushed

@FernetMenta
Copy link
Contributor

the comment was for 2nd commit, don't know if github shows this correctly

// the content is interlaced at the start, only
// punish when refreshrate > 60 hz to not have to switch
// twice for 30i content
if (refresh > 60 && round > 1)

This comment was marked as spam.

@fritsch
Copy link
Member Author

fritsch commented Oct 1, 2016

Not possible. Some tvs only have 120 and 119.9x

Did I remove the right one?

Am 01.10.2016 7:47 vorm. schrieb "Rainer Hochecker" <
notifications@github.com>:

@FernetMenta commented on this pull request.

In xbmc/guilib/Resolution.cpp
#10243 (review):

else

  • return (float)fabs(div / round - 1.0);
  • weight = (float)fabs(div / round - 1.0);
  • // punish higher refreshrates and prefer better matching
  • // e.g. 30 fps content at 60 hz is better than
  • // 30 fps at 120 hz - as we sometimes don't know if
  • // the content is interlaced at the start, only
  • // punish when refreshrate > 60 hz to not have to switch
  • // twice for 30i content
  • if (refresh > 60 && round > 1)

lets just ignore all resolutions > 60


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#10243 (review), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABCfHcXivGdjs8qGXQZhaKE3e6vyiS8Xks5qvfPZgaJpZM4JeWPy
.

@FernetMenta
Copy link
Contributor

If tvs have only a single resolution, we can't switch anyway and stay at desktop. So it is possible to ignore all > 60

@FernetMenta
Copy link
Contributor

+1 after discussion with @fritsch on slack

@fritsch
Copy link
Member Author

fritsch commented Oct 1, 2016

jenkins build this please

@MartijnKaijser MartijnKaijser added this to the Krypton 17.0-beta3 milestone Oct 1, 2016
@fritsch fritsch merged commit cd4ee97 into xbmc:master Oct 1, 2016
@MartijnKaijser MartijnKaijser added Type: Fix non-breaking change which fixes an issue v17 Krypton labels Oct 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Type: Fix non-breaking change which fixes an issue v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants