[Windows] Refactor DXVA renderer to enumerate and use the supported conversions #23513

CrystalP · 2023-07-14T01:36:23Z

Description

This is the refactor that the previous ones enabled and mostly finishes a transition to have the dxva render method rely on the Windows 10 ID3D11VideoProcessorEnumerator1 interface (with provisions to keep Windows 8 working)

There are barely any functionality changes, with very minor improvements.
One change by commit, clearly labelled (I hope!). A few of the commits were kept separate for review and will be combined for the merge.

Reorganized the DXVA renderer so that it contains the rules to decide how to handle the rendering. The processor is now "dumb", the enumerator still contains some rules to pre-select which conversions make sense for a source/output combination.
The process goes like this:

GetWeight:

Create enumerator
List supported conversions for the given source and destination
- the list has separate entries for the different output formats 8 bits, 10 bits
- special case for Windows 8: return all SDR conversions as supported, in the absence of the Enumerator1 interface
At least one result => mark dxva as a candidate render method

Configure:

Create enumerator
List supported conversions (same function used by GetWeight)
Chose best candidate based on source, destination, desired quality, VSR use, ...
Create processor
Configure processor for the chosen conversion
Turn on VSR when requested

CheckVideoParameters (detect changes during playback):

compare render buffer attributes that impact conversion choice with values in previous frame
in case of change, list supported conversions and choose new best match
- no support available: keep using the same conversion. This avoids a black screen. A better way (fallback to Pixel Shaders) requires changes to WinRenderer to switch render methods mid-stream.
if the new best conversion is different from current conversion, reconfigure the renderer and processor

notes:

HLG to SDR conversion is supported by AMD processor and used to be activated, but gave strange results. With this PR it will not be used at all, with better visual results. HLG could be revisited in future PR.
the previous code did not react to changes in VSR setting, high quality setting or source size / pixel format and that was not added here in order to limit the scope. Could be implemented in future PR.
an enumerator still gets created twice in succession for GetWeight and then Configure, due to the way WinRenderer probes the render methods. Could be improved in future PR.

Motivation and context

Reduces logic duplication in the code, enable reacting to changes during playback in a saner way by centralizing the rules in one place.

How has this been tested?

Windows 10, AMD, Intel gen4 and gen13
no behaviour changes for combinations of SDR/HDR/HLG playback to limited/full/SDR/HDR output.

No nVidia hardware at hand to test impact on VSR. It seems to activate when needed on Intel gen13 but not much visual effect.

What is the effect on users?

none, will make further improvements possible

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

thexai

I agree with all the changes and I think it's a great improvement not only because of the re-usability of the code, but also by using the same logic for configure and rendering. This keeps things simple and prevents weird behavior / black screens. It will also make it easier to maintain or extend in the future.

HLG thing: I agree to remove this particular case. Very few systems support it and also (although the code comment seems to indicate otherwise) I currently think that this color space can only be used for HLG to HLG (not HLG to SDR). So it can never be used on Windows (desktop) because no video driver supports HLG output.

HLG is backwards compatible with SDR but then the BT.2020 SDR legacy color space should be used.

Runtime tested and no issues found. All seems work as before. No performance regressions found playing 4K HDR 59.94 fps

Only minor code cosmetic things:

xbmc/cores/VideoPlayer/VideoRenderers/HwDecRender/DXVAEnumeratorHD.cpp

xbmc/cores/VideoPlayer/VideoRenderers/HwDecRender/DXVAHD.h

xbmc/cores/VideoPlayer/VideoRenderers/windows/RendererDXVA.cpp

xbmc/cores/VideoPlayer/VideoRenderers/windows/RendererDXVA.h

xbmc/cores/VideoPlayer/VideoRenderers/windows/RendererDXVA.cpp

CrystalP · 2023-07-15T17:33:10Z

HLG thing: I agree to remove this particular case. Very few systems support it and also (although the code comment seems to indicate otherwise) I currently think that this color space can only be used for HLG to HLG (not HLG to SDR). So it can never be used on Windows (desktop) because no video driver supports HLG output.

Yes Windows doesn't support HLG output.

However AMD claims support and does the HLG to HDR conversion properly (same results as Kodi's own method) so I plan to add the support later.

AMD also claims support for HLG to SDR but by default it looks weird. Adjusting brightness and contrast can make it look good. I suppose it's the same type of issue noticed with tonemapping HDR to SDR in general, so I would still keep that one out. At least for v21.

About HLG back compatibility with SDR: it's compatible yes so converting just the primaries works, but I don't think it's the optimal result. It should benefit from tonemapping the picture from PQ to SDR and I plan to play a little with that.

Thanks for the comments, I'll take care of them.

…ssary struct init

CrystalP · 2023-07-15T18:15:06Z

Comments integrated, except for the const & return, maybe someone will weight in

thexai · 2023-07-16T07:38:07Z

xbmc/cores/VideoPlayer/VideoRenderers/windows/RendererDXVA.cpp

+      {
+        m_enumerator->LogSupportedConversions(
+            dxgi_format, CProcessorHD::AvToDxgiColorSpace(DXVA::DXGIColorSpaceArgs(picture)),
+            m_intermediateTargetFormat);


When code reach this point m_intermediateTargetFormat is not yet initialized, then in LogSupportedConversions the list of conversions not indicates which is picked

Ah yes, I took a note to remove that parameter, which doesn't make sense anymore, and forgot to apply it. It may come back in another refactor.

done, removed.

thexai · 2023-07-17T07:48:29Z

xbmc/cores/VideoPlayer/VideoRenderers/HwDecRender/DXVAEnumeratorHD.cpp

+
+  const DXGI_COLOR_SPACE_TYPE inputCS = isSourceFullRange
+                                            ? DXGI_COLOR_SPACE_YCBCR_FULL_G22_LEFT_P709
+                                            : DXGI_COLOR_SPACE_YCBCR_STUDIO_G22_LEFT_P709;


There at least one use case with old SD videos (DVD) not handled properly: these MPEG2 videos should use primaries P601 and color space DXGI_COLOR_SPACE_YCBCR_STUDIO_G22_LEFT_P601. As this is working properly in current master could be considered a regression, then please add here the code to handle these cases:

Mainly use colors space:

DXGI_COLOR_SPACE_YCBCR_STUDIO_G22_LEFT_P601
DXGI_COLOR_SPACE_YCBCR_FULL_G22_LEFT_P601

when primaries reported by FFmpeg are AVCOL_PRI_BT470BG or AVCOL_PRI_SMPTE170M (see old code)

This is a sample video:
tsbuffer.zip

Note that some times these old videos not has this information's properly signaled (AVCOL_PRI_UNSPECIFIED ) then can be used video resolution to infer that is SD video. As this is not implemented in current code is not a blocking thing and can be implemented later. But RenderShaders has it implemented:

xbmc/xbmc/cores/VideoPlayer/VideoRenderers/windows/RendererShaders.cpp

Lines 195 to 206 in 022ef38

AVColorPrimaries CRendererShaders::GetSrcPrimaries(AVColorPrimaries srcPrimaries, unsigned width, unsigned height)

{

AVColorPrimaries ret = srcPrimaries;

if (ret == AVCOL_PRI_UNSPECIFIED)

{

if (width > 1024 || height >= 600)

ret = AVCOL_PRI_BT709;

else

ret = AVCOL_PRI_BT470BG;

}

return ret;

}

There also some special case with DXGI_COLOR_SPACE_YCBCR_FULL_G22_NONE_P709_X601 (primaries BT.709 and tranfer BT.601) and RGB color spaces but again this can be done on a follow-up PR.

As a requirement I think it is enough to support regular SD videos here.

General info about this:
https://guide.encode.moe/archived-websites/bt601-vs-bt709.html

Agreed. I never saw any video reporting proper flags and it slipped my mind, so I'll integrate the flag-based detection.
I'll try playing a raw DVD in case ffmpeg reports flagging for those.

Something similar to PS to guess BT601 is OK, I was thinking of another PR but can do a separate commit here.
If the information is available, maybe extend with conditions on codec or container type, like mpeg-2, xvid, avi... all that predate HD basically)

edit: and might as well do it at RendererBase level so that all render methods benefit.

If the information is available, maybe extend with conditions on codec or container type, like mpeg-2, xvid, avi... all that

I vote for keep it simple same as is in PS for now. Codecs and containers not helps much... i.e. MPEG-2 is in both DVD and Blu-Ray and guess only when primaries are AVCOL_PRI_UNSPECIFIED

Sure.

For P601/P709_X601, rather than duplicate the logic, I'm going to change the Query*Conversions to use AvToDxgiColorSpace(). I kept it around for that purpose for a future refactor, might as well do it now. It only needs minor changes. Will be a separate commit.

Refactor of Query*Conversions to use AvToDxgiColorSpace() will come in another PR for clarity. Added the few missing lines to QuerySDRConversions() for now. Tested with your sample, with DVD (flags properly reported) > OK.

MS indicates that DXGI_COLOR_SPACE_YCBCR_FULL_G22_NONE_P709_X601 is for jpg, which Kodi won't decode with dxva, so the code will likely never be used.

Improvement for AVCOL_PRI_UNSPECIFIED requires touching other render methods to share the logic and code, so beyond the scope of this PR.

…ed GetWeight Refactor of the existing logic to present GetWeight with a list of the supported conversions, using the ID3D11VideoProcessorEnumerator1 interface of Windows 10 and above. Support for SDR is assumed in the absence of the D3D11 interface (Windows 8) At least one conversion: the dxva processor supports the rendering. Keep the previous API around for now as it is used by the Configure() path. Removed in later commits, including the temporary adaptor functions.

…ist of supported conversions Configure retrieves the same list of conversions as GetWeight, applies some rules to pick the best (refactor of CalcIntermediateTargetFormat) and tells CProcessorHD to use it. No need to check the input format in Configure() anymore, it is checked by extracting the supported conversion, and when setting the conversion in the processor. The Configure and GetWeight paths now execute the same logic to gather information, even though it's still executed twice.

…e it throughout

…e it with the processor It will make it easier to get the renderer react to video attributes changes during playback. Removed all methods that CProcessorHD was simply forwarding to CEnumeratorHD. Removed PreInit method of CProcessorHD since the enumerator is created externally and has already been created successfully by the time PreInit was called.

…fter recovery from lost device

First do everything enumerator related, then processor related. to be merged with previous commit once reviewed

…g playback. Compare attributes of new frame with old frame. Differences could require a different conversion, re-enumerate conversions and choose the best one again. Similar to the replaced code, does not react to: - VSR setting change - High precision processing setting change - Source dimensions or input dxgi format changes - in practice doesn't seem to have an effect on D1D11 enumerator interfaces behavior , but enumerator should be reopened. To be added in future changes.

CrystalP added Type: Improvement non-breaking change which improves existing functionality Component: Video rendering Platform: Windows Platform: WindowsStore Component: DirectX rendering v21 Omega labels Jul 14, 2023

CrystalP added this to the Omega 21.0 Alpha 3 milestone Jul 14, 2023

CrystalP requested review from afedchin and thexai July 14, 2023 01:36

CrystalP force-pushed the dxva-refactor3 branch from 98c7f39 to d7e79b5 Compare July 14, 2023 02:34

thexai reviewed Jul 15, 2023

View reviewed changes

xbmc/cores/VideoPlayer/VideoRenderers/windows/RendererDXVA.cpp Show resolved Hide resolved

[Windows][dxva] Fix input format in conversions list / removed unnece…

9346eaa

…ssary struct init

CrystalP force-pushed the dxva-refactor3 branch from d7e79b5 to 1d78da7 Compare July 15, 2023 18:14

afedchin approved these changes Jul 15, 2023

View reviewed changes

CrystalP force-pushed the dxva-refactor3 branch from 1d78da7 to 70a4c99 Compare July 15, 2023 18:55

thexai reviewed Jul 16, 2023

View reviewed changes

CrystalP force-pushed the dxva-refactor3 branch from 70a4c99 to f70256a Compare July 16, 2023 18:24

thexai requested changes Jul 17, 2023

View reviewed changes

CrystalP force-pushed the dxva-refactor3 branch from f70256a to b343355 Compare July 17, 2023 19:56

CrystalP added 8 commits July 17, 2023 16:32

[Windows][dxva] Cleanup now-unused functions of the old logic

eecdbd3

[Windows][dxva] Keep a copy of the conversion in the processor and us…

28c3c8a

…e it throughout

[Windows][dxva] Add CEnumeratorHD handling of D3D11 device creation a…

df8485b

…fter recovery from lost device

[Windows][dxva] reorder steps in Configure() more logically

80fa78a

First do everything enumerator related, then processor related. to be merged with previous commit once reviewed

CrystalP force-pushed the dxva-refactor3 branch from b343355 to 852cd7f Compare July 17, 2023 20:33

thexai approved these changes Jul 18, 2023

View reviewed changes

CrystalP merged commit 60ae8f5 into xbmc:master Jul 18, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Windows] Refactor DXVA renderer to enumerate and use the supported conversions #23513

[Windows] Refactor DXVA renderer to enumerate and use the supported conversions #23513

CrystalP commented Jul 14, 2023 •

edited

thexai left a comment

CrystalP commented Jul 15, 2023

CrystalP commented Jul 15, 2023

thexai Jul 16, 2023

CrystalP Jul 16, 2023

CrystalP Jul 16, 2023

thexai Jul 17, 2023 •

edited

CrystalP Jul 17, 2023 •

edited

thexai Jul 17, 2023

CrystalP Jul 17, 2023

CrystalP Jul 17, 2023

	AVColorPrimaries CRendererShaders::GetSrcPrimaries(AVColorPrimaries srcPrimaries, unsigned width, unsigned height)
	{
	AVColorPrimaries ret = srcPrimaries;
	if (ret == AVCOL_PRI_UNSPECIFIED)
	{
	if (width > 1024 \|\| height >= 600)
	ret = AVCOL_PRI_BT709;
	else
	ret = AVCOL_PRI_BT470BG;
	}
	return ret;
	}

[Windows] Refactor DXVA renderer to enumerate and use the supported conversions #23513

[Windows] Refactor DXVA renderer to enumerate and use the supported conversions #23513

Conversation

CrystalP commented Jul 14, 2023 • edited

Description

Motivation and context

How has this been tested?

What is the effect on users?

Types of change

Checklist:

thexai left a comment

Choose a reason for hiding this comment

CrystalP commented Jul 15, 2023

CrystalP commented Jul 15, 2023

thexai Jul 16, 2023

Choose a reason for hiding this comment

CrystalP Jul 16, 2023

Choose a reason for hiding this comment

CrystalP Jul 16, 2023

Choose a reason for hiding this comment

thexai Jul 17, 2023 • edited

Choose a reason for hiding this comment

CrystalP Jul 17, 2023 • edited

Choose a reason for hiding this comment

thexai Jul 17, 2023

Choose a reason for hiding this comment

CrystalP Jul 17, 2023

Choose a reason for hiding this comment

CrystalP Jul 17, 2023

Choose a reason for hiding this comment

CrystalP commented Jul 14, 2023 •

edited

thexai Jul 17, 2023 •

edited

CrystalP Jul 17, 2023 •

edited