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

[Windows] fix playback of HDR material when Windows is already in HDR mode #23241

Merged
merged 1 commit into from
May 10, 2023

Conversation

CrystalP
Copy link
Contributor

@CrystalP CrystalP commented May 6, 2023

Description

The current code sets the color space matching the played media long after the creation of the swap chain, except when Kodi switches the Windows HDR mode, forcing the creation of a new swap chain (the only case that works properly).

After some experiments, it seems that setting a colorspace has no effect on a used swap chain, at least on recent AMD GPU. That is not mentioned in MS documentation.

Made changes to:

  • track the used status of a swap chain (was it presented)
  • SetHdrColorSpace:
    • recreate the swap chain if it's been used

(untested: this change may also help with the setting of the HDR10 metadata)

Motivation and context

Blown out picture for HDR video when the screen is in HDR mode when Kodi starts.

The issue sort of self-corrects when "Use display HDR capabilities" is enabled but it's not ideal. Play HDR 1st time: bad picture, Kodi switches screen to SDR when playback stops, play HDR 2nd time: Kodi switches HDR on, playback ok

The problem is possibly limited to AMD. I don't think it was reported before and I don't have other HDR-capable hardware to check.

How has this been tested?

Windows 10 x64, AMD GPU
Combinations of:

  • Windowed fullscreen and fullscreen exclusive
  • "Use display HDR capabilities" on and off
  • Kodi started with Windows HDR set to on and off
  • SDR and HDR videos

Other than pre-existing problems when Windows HDR is switched while Kodi is running, things worked as expected (good picture).

What is the effect on users?

Maybe recent AMD only.
Kodi plays HDR correctly when the screen is already in HDR mode.
=> it's possible to set and leave the screen in HDR mode and Kodi will play any media SDR/HDR correctly.

Screenshots (if appropriate):

Before:
unable to capture, taking a screenshots fixes the problem.

After: normal image
image

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

@CrystalP CrystalP added this to the Omega 21.0 Alpha 2 milestone May 6, 2023
@thexai
Copy link
Member

thexai commented May 6, 2023

The problem is possibly limited to AMD. I don't think it was reported before and I don't have other HDR-capable hardware to check.

I'm sure it's just AMD. This does not happen on Nvidia or Intel. With Windows HDR ON all the time the picture is correct even in the case of the first video played is HDR.

If the requirement for AMD is set initial swap chain color space probably is more easy do it here:

hr = swapChain.As(&m_swapChain); CHECK_ERR();

Insert SetHdrColorSpace(DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709); after swap chain creation.

Most of the others changes seems to me redundant or unnecessary: e.g. in debug info OSD is already tracked if color space is SDR or PQ. Note there are two different things: Windows HDR on/off and transfer PQ on/off

Windows HDR on - EOTF SDR ---> plays SDR video with Windows HDR on
Windows HDR on - EOTF PQ ---> plays HDR video with Windows HDR on
Windows HDR off - EOTF SDR ---> plays SDR video with Windows HDR off (or HDR video tone mapped to SDR)

@CrystalP
Copy link
Contributor Author

CrystalP commented May 7, 2023

Setting an initial value is not enough, I would have stopped there :-)

It could be AMD drivers yes, Windows is also manipulating the color spaces to get multiple apps to share the system.
There is no problem in the non-fullscreen modes for some reason.
Maybe re-creating the swap chain is just be a way to reset some internal state either in the driver or in Windows. There may be other ways, I don't know.

Setting SDR mode initially could be omitted, it's there only so that m_swapChainColorSpace is initialized when Kodi starts.
Can't hurt either, I didn't find documentation saying that swap chains are always created with DXGI_COLOR_SPACE_RGB_FULL_G22_NONE_P709.

Yes it looked like an overlap between m_IsTransferPQ and m_swapChainColorSpace because one commit to clean it up was missing. Now pushed.
Since SetHdrColorSpace() already receives the exact color space, keeping the more detailed information is free and makes the code more generic. m_IsTransferPQ / IsTransferPQ() can easily be deducted.

@thexai
Copy link
Member

thexai commented May 7, 2023

Now looks much worse :(

As this is a fix I not understand by is need change things in debug Info OSD and m_IsTransferPQ flag witch is used to tonemap GUI to HDR.

I'd like to see first the minimal changes that fix the problem on AMD without touching anything else. Then maybe I'll understand why it happens. There are other reasons: if it's really just a fix, backport to Nexus would be desirable but then you shouldn't include changes that aren't necessary (like adding color space in debug Info OSD or changing how m_IsTransferPQ works).

@thexai
Copy link
Member

thexai commented May 7, 2023

There is no problem in the non-fullscreen modes for some reason.

You men that full screen windowed works fine? (Use fullscreen window setting ON)

Maybe AMD driver not supports full screen exclusive. Anyway today there is no advantage to use full screen exclusive on Windows 11. Formerly improve performance a little but there are no differences anymore.

@CrystalP
Copy link
Contributor Author

CrystalP commented May 7, 2023

I was not thinking about a backport, but here you go, minimal version that still doesn't unnecessarily recreate the swap chain twice in a row when Windows HDR mode switching is used.

Both windowed fullscreen (most common) and fullscreen exclusive fail. Windowed non-fullscreen mode was OK.

Copy link
Member

@thexai thexai left a comment

Choose a reason for hiding this comment

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

Now looks much better and changes are clear. If this is enough to fix these AMD issues go ahead.

@CrystalP
Copy link
Contributor Author

CrystalP commented May 8, 2023

yes it's enough. I'll hold on a bit on the merge and will create a bug report with AMD just in case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants