[dxva] added dxva-hd renderer as alternative native dxva video processing.

[afedchin]

In order to resolve issue with deinterlacing with dxva on Intel platforms discussed here http://forum.xbmc.org/showthread.php?tid=147171.

Previous PR #2916 solves image freeze with best dxva deinterlacing but provides the quality is even worse than in soft mode, so I've added support of dxva-hd renderer. dxva-hd is similar to native dxva video processing, but offers enhanced features and a simpler processing model.

I've tested it during a few months on my HTPC with Intel HD4000 and it provides the good deinterlacing quality. Also I've recieved a few positive comments from users who tried it:
http://forum.xbmc.org/showthread.php?tid=147171&pid=1523020#pid1523020
http://forum.xbmc.org/showthread.php?tid=147171&pid=1523631#pid1523631
http://forum.xbmc.org/showthread.php?tid=147171&pid=1525835#pid1525835

Unfortunately, dxva-hd in only available on windows 7 and later, so it can be only enabled into as.xml.

Code based on native dxva with a few improvements.

[Voyager1]

nice work! instead of the AS, you could query CSysInfo::IsWindowsVersionAtLeast(WindowsVersionWin7) to see whether it's available. Then, you could make it an additional choice in the list of renderers (Software, PS, DXVA, DXVA-HD) so you don't force it either-or.

[afedchin]

Sure, but in situation that after dxva decoding the rendering method forced to dxva, so which dxva method be prefer (if both are exist)?

[Voyager1]

good point. When rendering setting is explicitly chosen (e.g. DXVA) you respect that, otherwise, when the setting is not explicitly chosen in the GUI (thus rendering is set to AUTO), in that case you could go for the best one available, based on windows version.

[wsoltys]

Nice work. I really wish I would know more about the whole video processing stuff. Please go as Voyager1 suggested and try to prevent the advanced setting. Do I got it right that we need DXVA.* then only for vista and that DXVAHD is the prefert solution under win7 and up? If yes we might drop DXVA one day because it doubles a lot of code.
It would really good when @elupus would put an eye on it.
In the meanwhile anything else you could help us with in the DXVA area? :)

[afedchin]

@wsoltys, You are right. When this will be merged then DXVA.* only needed for Vista.

Sure, currently I'm working on fix issue with 3D+DXVA+Anaglyph. I've got some good results, but need more time for coding.

Edit: DXVA.* only needed for Vista - not exactly. DXVA.* contains the code for dxva decoding.

[afedchin]

Updated according to @Voyager1 suggestion.

[Memphiz]

Video Orientation Support for Dxva would be Great too ;)

[MartijnKaijser]

Not really know the internals but will this make dxva-hd the default if you run win7 and up (which would make sense to me)?

[afedchin]

Yes, if video render method set to Auto and dxva decode enable and you run on win7 and up.

[Voyager1]

with this you can still choose "DXVA" for rendering, and you get the old functionality.

[afedchin]

Yes, you can still choose "DXVA" for rendering.

[wsoltys]

Is there any reason you don't delete m_pInputFormats, m_pOutputFormats, m_pVPCaps and the other allocations?

[da-anda]

@afedchin, elupus said that "to fix the dxva anaglyph issue the dxva processor has to render to a buffer object instead of directly to the backbuffer". Whatever that means. hth.

[afedchin]

@wsoltys, there is no any reason, I've missed it.

@da-anda, yes, it is main idea.

[wsoltys]

You can also lower the scope for m_pInputFormats and m_pOutputFormats (no need for a class variable). Would be nice if you could also make the error messages a little more descriptive to know where they fail (two times out of memory, add HRESULT on error and make it a LOGERROR if it is an error :).

[a11599]

Thanks for picking up this topic. Without going into details a few observations first:

What is the reason of introducing m_processorHD in WinRenderer.cpp? To me it seems to serve the same purpose as m_processor. If so, just use m_processor and point it to a DXVA-HD processor instead of a DXVA2 proc when doing DXVA-HD processing. This should make the diff smaller and the code more readable.

There seems to be a lot of code duplications between DXVA.cpp and DXVAHD.cpp. Would it be possible to merge the two? (This would also take care of the m_processorHD stuff above.) If this would blow up the size of DXVA.cpp too much I would rather prefer splitting DXVA.cpp into a decoder and processor part where the latter has support for both DXVA2 and DXVA-HD processors.

I fear we will have to keep supporting DXVA2 (DXVA-VP) with DXVA-HD in parallel for quite a while. At least none of the ATI hardware around here supports DXVA-HD according to DXVAChecker.

[MartijnKaijser]

Just did a test-build http://mirrors.xbmc.org/test-builds/win32/XBMCSetup-20131107-27590a8-dxva-hd.exe but i don't see the dxva-hd in video settings during playback (on a win7). Only dxva

[afedchin]

@wsoltys, @a11599 thanks for your suggestions. I'll update PR to according they.
@MartijnKaijser, System -> Settings -> Video -> Playback -> Render method would possible to select DXVA-HD.

[MartijnKaijser]

ok it's there in the settings.

[afedchin]

Updated.

@a11599, split dxva decoder and processor is good idea. As for merge DXVA and DXVAHD I think it is no good because the difference is big. There is only one common method, but all other is very different. Take a look at updated commit.

[Voyager1]

@afedchin I think @a11599 makes a great point that DXVA-HD is not necessarily supported on all brands (such as AMD). So next to the check for Win7 there should be a test to see if GPU supports this render method.

[afedchin]

@Voyager1, good point.
Updated. Added if the method CProcessorHD::PreInit() returns false then drop the processor through to native dxva.

[Voyager1]

just gave it a try on my AMD 7650M, I selected DXVA-HD in the rendering setting, and it properly falls back to normal DXVA. as expected. Seems to be working fine. I'll test DXVA-HD on another machine.

[afedchin]

@FernetMenta, @Voyager1, thanks.

[Voyager1]

could you rebase it, GUIDToString doesn't compile anymore since the CStdString changes...

[afedchin]

@Voyager1, done. rebased and removed CStdString usage.

[MartijnKaijser]

jenkins build this please

[afedchin]

rebased

[wsoltys]

@Voyager1, @a11599 ready to merge or still some comments?

[a11599]

I do not know much about DXVA-HD so if it works and does not break existing functionality then I am fine with it. Maybe a generic code review from someone who has more C++-fu than me would be beneficial.

[Voyager1]

I have been running with this code for a while, although not really using the DXVA-HD piece. I can confirm it's not breaking anything in the regular DXVA2 world. The only risk introduced is the processor is now a pointer (so null pointers and memleaks) but AFAICT, new/delete are properly handled e.g. WinRenderer::Uninit(). I haven't checked thoroughly, but in my opinion it's good.

[wsoltys]

@afedchin I get a lot of ambiguous warnings in cppcheck because a lot of member variables in dxvahd.h are defined in the parent class as well:

[DXVAHD.h:77] -> [DXVA.h:146]: (warning) The class 'CProcessorHD' defines member variable with name 'm_caps' also defined in its parent class 'CProcessor'.
[DXVAHD.h:79] -> [DXVA.h:144]: (warning) The class 'CProcessorHD' defines member variable with name 'm_device' also defined in its parent class 'CProcessor'.
[DXVAHD.h:104] -> [DXVA.h:174]: (warning) The class 'CProcessorHD' defines member variable with name 'm_context' also defined in its parent class 'CProcessor'.
[DXVAHD.h:105] -> [DXVA.h:154]: (warning) The class 'CProcessorHD' defines member variable with name 'm_size' also defined in its parent class 'CProcessor'.
[DXVAHD.h:106] -> [DXVA.h:155]: (warning) The class 'CProcessorHD' defines member variable with name 'm_max_back_refs' also defined in its parent class 'CProcessor'.
[DXVAHD.h:107] -> [DXVA.h:156]: (warning) The class 'CProcessorHD' defines member variable with name 'm_max_fwd_refs' also defined in its parent class 'CProcessor'.
[DXVAHD.h:108] -> [DXVA.h:160]: (warning) The class 'CProcessorHD' defines member variable with name 'm_index' also defined in its parent class 'CProcessor'.
[DXVAHD.h:110] -> [DXVA.h:173]: (warning) The class 'CProcessorHD' defines member variable with name 'm_surfaces' also defined in its parent class 'CProcessor'.
[DXVAHD.h:111] -> [DXVA.h:171]: (warning) The class 'CProcessorHD' defines member variable with name 'm_section' also defined in its parent class 'CProcessor'.

[Memphiz]

@afedchin can you have a look at the reported black level issue here please?

http://forum.xbmc.org/showthread.php?tid=188083&pid=1645799#pid1645799