Revert XBMC commit

Development Feature Requests
1

There is a fairly serious issue in CoreELEC where 25fps content is not played at 50fps by default. Recently, CPM tracked this issue down and I tracked down the original Kodi commit. From the PR notes, it’s fairly clear that this is an ancient workaround applied without the greatest depth of thought. Therefore, can this commit be reverted?

Edit: I believe that a 25fps file with 48 kHz audio is needed to test this. Whitelist 25fps and see if audio plays. If yes, then the workaround is no longer needed.

2

You shouldn’t post this here. It’s a Kodi thing. Better create a new issue here:

And include the links that you found from your other post.

1 Like
3

I created an issue on XBMC because I couldn’t figure out how to create a PR.

4

We have some code for “better” 1080i25hz detection what is not in XBMC.
So this will maybe cause that the commit is maybe not need.

I will try tonight a 1080i25hz sample without whitelist what happen on resolution switch.

5

Untouched like nightly:

2024-09-28 19:57:50.851 T:1043     info <general>: [WHITELIST] Searching the whitelist for: width: 1920, height: 1080, fps: 25.000, 3D: false:(0x4), stereo mode: 0
2024-09-28 19:57:50.851 T:1043    debug <general>: [WHITELIST] Using the default whitelist because the user whitelist is empty
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] Searching for an exact resolution with an exact refresh rate
2024-09-28 19:57:50.852 T:1191    debug <general>: CVideoPlayer::ProcessVideoData size:38135 dts:11.540 pts:11.540 dur:20.000ms, clock:-0.028 level:100
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] No match for an exact resolution with an exact refresh rate
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] Searching for an exact resolution with double the refresh rate
2024-09-28 19:57:50.852 T:1191    debug <general>: CVideoPlayer::ProcessVideoData size:28548 dts:11.560 pts:11.560 dur:20.000ms, clock:-0.028 level:100
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] No match for an exact resolution with double the refresh rate
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] Searching for an exact resolution with a 3:2 pulldown refresh rate
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] No match for a resolution with a 3:2 pulldown refresh rate
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] Searching for a closest resolution with an exact refresh rate
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] No match for a closest resolution with an exact refresh rate
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] Searching for a closest resolution with double refresh rate
2024-09-28 19:57:50.852 T:1043    debug <general>: [WHITELIST] Matched a closest resolution with double refresh rate 3840x2160 @ 50.00 - Full Screen (16), penalty 3000

With the commit revert:

2024-09-28 20:04:31.470 T:1044     info <general>: [WHITELIST] Searching the whitelist for: width: 1920, height: 1080, fps: 25.000, 3D: false:(0x4), stereo mode: 0
2024-09-28 20:04:31.470 T:1044    debug <general>: [WHITELIST] Using the default whitelist because the user whitelist is empty
2024-09-28 20:04:31.470 T:1044    debug <general>: [WHITELIST] Searching for an exact resolution with an exact refresh rate
2024-09-28 20:04:31.470 T:1185    debug <general>: CVideoPlayer::ProcessVideoData size:27682 dts:9.580 pts:9.580 dur:20.000ms, clock:-0.153 level:100
2024-09-28 20:04:31.470 T:1044    debug <general>: [WHITELIST] No match for an exact resolution with an exact refresh rate
2024-09-28 20:04:31.470 T:1044    debug <general>: [WHITELIST] Searching for an exact resolution with double the refresh rate
2024-09-28 20:04:31.471 T:1044    debug <general>: [WHITELIST] No match for an exact resolution with double the refresh rate
2024-09-28 20:04:31.471 T:1044    debug <general>: [WHITELIST] Searching for an exact resolution with a 3:2 pulldown refresh rate
2024-09-28 20:04:31.471 T:1044    debug <general>: [WHITELIST] No match for a resolution with a 3:2 pulldown refresh rate
2024-09-28 20:04:31.471 T:1044    debug <general>: [WHITELIST] Searching for a closest resolution with an exact refresh rate
2024-09-28 20:04:31.472 T:1044    debug <general>: [WHITELIST] Matched a closest resolution with an exact refresh rate 4096x2160 @ 25.00 - Full Screen (22), penalty 3256
2024-09-28 20:04:31.472 T:1044    debug <general>: [WHITELIST] Matched a closest resolution with an exact refresh rate 3840x2160 @ 25.00 - Full Screen (30), penalty 3000
2024-09-28 20:04:31.472 T:1044    debug <general>: [WHITELIST] Searching for a closest resolution with double refresh rate
2024-09-28 20:04:31.472 T:1044    debug <general>: [WHITELIST] Matched a closest resolution with double refresh rate 3840x2160 @ 50.00 - Full Screen (16), penalty 3000
2024-09-28 20:04:31.472 T:1185    debug <general>: CVideoPlayer::ProcessVideoData size:134083 dts:9.600 pts:9.600 dur:20.000ms, clock:-0.151 level:100
2024-09-28 20:04:31.472 T:1044    debug <general>: [WHITELIST] Matched a closest resolution with double refresh rate 3840x2160 @ 50.00 - Full Screen (16), penalty 3000

So it does choose on end same resolution because the “disable double resolution” option is deisabled when not using whitelist.

Because of:

  if (noWhiteList || CServiceBroker::GetSettingsComponent()->GetSettings()->GetBool(
          SETTING_VIDEOSCREEN_WHITELIST_DOUBLEREFRESHRATE))

noWhiteList is true, SETTING_VIDEOSCREEN_WHITELIST_DOUBLEREFRESHRATE is false but doesn’t matter.

What is bad on using 50/60 Hz for 25/30fps?

6

As reported by @R3S3T_9999 and others, it breaks Dolby Vision output by causing the HDMI handshake to fail and the TV to flicker repeatedly switch Dolby Vision on and off.

7

There was a good discussion on this including that snippet here Kodi doesn't choose the correct resolution from the whitelist earlier this year.

8

Is there 25fps DV content available?

9

Duration…: 00:46:01
Video Bit Rate…: 8618 kb/s
Bit Depth…: 10 bits
Frame Rate…: 25.00 FPS
Video Codec…: HEVC
HDR Format…: Dolby Vision, dvhe.05, BL+RPU,
Resolution…: 3840x1608

10

Yes, it’s the last major issue with Dolby Vision on CoreELEC.

Also, this is a workaround for an issue that does not seem to exist on CoreELEC. So this workaround breaks Dolby Vision playback, causes videos to be played at double refresh rate, and fixes nothing.

As @chuckster tested, 25fps content with 48khz audio does output audio. However, a double refresh rate does occur. So reverting this PR will uncover another issue.

11

A lot if VS10 HDR->DV is considered

12

I have no DV 25hz sample.
And revert the commit will not change anything when whitelist is empty. It automatic perform a double refresh rate search, not controllable by user.

13

@R3S3T_9999 Do you think you can provide a DV 25hz sample?

14

Maybe it makes no difference for me as I have GUI at 4k. But video is 1080p.

15

I know that for me I have seen 25hz and 30hz video being outputted at double refresh rate and I have experienced flashing Dolby Vision. This is clearly a workaround and I don’t think it’s a good one. Why force users to watch at a double refresh rate because of a bug in Kodi 18?

16

Just tested with 1080p GUI, still 50hz as it uses double refresh rate because whitelist is empty.

So the revert does change nothing for resolution choose.

17

Thanks for checking this out. It seems that this bug requires more investigation to correct the default behavior for no whitelist.

Edit: What do you think of CPM’s fix?

18

He just skip double refresh search when the option is not enabled.
But this does not look correct for me for the case nowhitelist.

More correct would be to revert the exclude of < 30hz if no whitelist AND to make the option “use double refresh time” available, also when no whitelist is set. But what should be the default? On or off?

19

Double Refresh should be off by default because it is more correct to output at the original refresh rate.

20

This looks more correct for me:

diff --git a/system/settings/settings.xml b/system/settings/settings.xml
index 15de604ff8..c59b5f2859 100755
--- a/system/settings/settings.xml
+++ b/system/settings/settings.xml
@@ -3174,10 +3174,7 @@
         </setting>
         <setting id="videoscreen.whitelistdoublerefreshrate" type="boolean" parent="videoscreen.whitelist" label="14128" help="36445">
           <level>3</level>
-          <default>false</default>
-          <dependencies>
-            <dependency type="enable" setting="videoscreen.whitelist" operator="!is"></dependency>
-          </dependencies>
+          <default>true</default>
           <control type="toggle" />
         </setting>
       </group>
diff --git a/xbmc/windowing/Resolution.cpp b/xbmc/windowing/Resolution.cpp
index 9e591c01e2..50c0fde590 100644
--- a/xbmc/windowing/Resolution.cpp
+++ b/xbmc/windowing/Resolution.cpp
@@ -138,15 +138,8 @@ void CResolutionUtils::FindResolutionFromWhitelist(float fps, int width, int hei
            (info.iScreenHeight >= curr.iScreenHeight && info.iScreenWidth >= curr.iScreenWidth)) &&
            (info.dwFlags & dwFlags) == dwFlags)
       {
-        // do not add half refreshrates (25, 29.97 by default) as kodi cannot cope with
-        // them on playback start. Especially interlaced content is not properly detected
-        // and this causes ugly double switching.
-        // This won't allow 25p / 30p playback on native refreshrate by default
-        if ((info.fRefreshRate > 30) || (MathUtils::FloatEquals(info.fRefreshRate, 24.0f, 0.1f)))
-        {
-          resString = CDisplaySettings::GetInstance().GetStringFromRes(c);
-          indexList.emplace_back(resString);
-        }
+        resString = CDisplaySettings::GetInstance().GetStringFromRes(c);
+        indexList.emplace_back(resString);
       }
     }
   }
@@ -184,7 +177,7 @@ void CResolutionUtils::FindResolutionFromWhitelist(float fps, int width, int hei
   if (!found)
     CLog::Log(LOGDEBUG, "[WHITELIST] No match for an exact resolution with an exact refresh rate");
 
-  if (noWhiteList || CServiceBroker::GetSettingsComponent()->GetSettings()->GetBool(
+  if (CServiceBroker::GetSettingsComponent()->GetSettings()->GetBool(
           SETTING_VIDEOSCREEN_WHITELIST_DOUBLEREFRESHRATE))
   {
     CLog::Log(LOGDEBUG,
@@ -286,7 +279,7 @@ void CResolutionUtils::FindResolutionFromWhitelist(float fps, int width, int hei
   if (!found)
     CLog::Log(LOGDEBUG, "[WHITELIST] No match for a closest resolution with an exact refresh rate");
 
-  if (noWhiteList || CServiceBroker::GetSettingsComponent()->GetSettings()->GetBool(
+  if (CServiceBroker::GetSettingsComponent()->GetSettings()->GetBool(
           SETTING_VIDEOSCREEN_WHITELIST_DOUBLEREFRESHRATE))
   {
     CLog::Log(LOGDEBUG, "[WHITELIST] Searching for a closest resolution with double refresh rate");
@@ -342,7 +335,7 @@ void CResolutionUtils::FindResolutionFromWhitelist(float fps, int width, int hei
 
   CLog::Log(LOGDEBUG, "[WHITELIST] No match for a desktop resolution with an exact refresh rate");
 
-  if (noWhiteList || CServiceBroker::GetSettingsComponent()->GetSettings()->GetBool(
+  if (CServiceBroker::GetSettingsComponent()->GetSettings()->GetBool(
           SETTING_VIDEOSCREEN_WHITELIST_DOUBLEREFRESHRATE))
   {
     CLog::Log(LOGDEBUG,

So the option “double refresh allow” is available also if no white list is set.
Default is enabled, so behavior is same like before with the difference the user can disable this option and 25hz should be chosen.