-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
spu2-x:windows:Use XAudio2.8+ for Windows 8 and later #1253
Conversation
Just wanted to put in a 2 cents on this. It's a good idea but could be problematic. I'm running on Windows 10 with Visual Studio 2015 Pro but the windows 8.1 SDK is installed, which uses the old version of the GetVersion API, so windows 10 reports that I'm using windows 7 (or 8, i forget) so it just complains I need 8.1 or higher. So users would need to have the Windows 10 SDK installed, not sure if this will cause problems though, especially when it comes to releases. |
It should build fine against the Windows 8.1 SDK - it does on AppVeyor, and also on my Win10 machine for both VS2013/VS2015. PCSX2 should also be reporting the correct Windows version (it reports 10 for me on Windows 10). Or am I completely misunderstanding you? Currently I know there are build issues if the include paths are altered to look at the DirectX SDK directories before looking the Windows 8.1 SDK files (see 16ef7ca), which I need to look into properly before I can even merge this PR but I've been too lazy to do so (it's so not my idea of fun). |
The problem I have is the SDK which is the Windows 8.1 SDK uses the old GetVersion API call, which Windows 10 spoofs to be an older version of windows unless you use the new call, it's some compatibility bull they came up with, so it thinks I'm using an old version of windows. PCSX2 reports it fine, it's just Visual Studio, so it refuses to compile SPU2-X with the Win 8.1 SDK. Example explanation: http://www.freebasic.net/forum/viewtopic.php?t=23868 Anyway this isn't your bug, this is in the SDK, actually in xaudio2.h |
What specific errors come up? It compiles fine with the Win 8.1 SDK here (Windows 10, VS2013/VS2015 community) |
#if(_WIN32_WINNT < _WIN32_WINNT_WIN8) line 19 of xaudio2.h |
That error shouldn't come up unless Visual Studio can't find the DirectX SDK include files. Does SPU2-X compile on master for you? Do you have DXSDK_DIR set anywhere? It's usually set as a system/user environment variable but there's probably other ways (VS macro). |
the DX SDK has never installed properly for me under VS and i have never created a DXSDK_DIR, i have always installed it by manually adding the include/library paths to my microsoft.users.cpp property, which is how people on the web said to do it. Anyway, I removed those references to see if i could just use the SDK as it didn't matter about compatibility from my side, but that error happened of course. Just to add strangeness to the mix. If i open xaudio2.h on its own and check it, _WIN32_WINNT returns 0x603, which is right, if i check _WIN32_WINNT inside the PCSX2 solution under SPU2-X or GSDX (didn't try any other), it returns 0x600. |
omg, it's wgwidgets, it's defined in there as 0x600! wrapwin.h line 47 is where it's getting it from, it isn't finding the definition anywhere else. I guess we could define it in the preprocessor list as 0x602, that should sort it. |
Yeah I can do that for now, but aren't your proposed changes going to make it use the SDK and put me back in that position again? |
It shouldn't. I set _WIN32_WINNT to 0x602 when I include xaudio2.h from the 8.1 SDK (plugins/spu2-x/src/Windows/SndOut_XAudio2.cpp):
The DX SDK header files should be used for the other xaudio2.h include (plugins/spu2-x/src/Windows/SndOut_XAudio2_27.cpp), and that requires DXSDK_DIR to be set. |
That should be okay then :) |
Could the PR be merged? Or is there any remaining stuff to review/update? |
Well, it might break compilation on the Orphis buildbot. My last GSdx PR broke it and I had to fix it (7736c90 which isn't actually a Windows 7 compile fix). I suppose I should request access to the buildbot and try to fix the issue so that I can merge this at some point. |
Ok. I have the access. I could post you a log (if is KO and I don't forget) |
Well, if you check the log from one of the recent builds and the GSdx warnings are different from AppVeyor's (DXGI_* macro redefinition warnings), then this PR is likely to break the buildbot. |
I pushed your branch into a PCSX2 project branch so the buildbot will eventually built it. Edit: result in 30 minutes (if there is only a single build in queue) |
Here we go
Note: you can push a branch directly to PCSX2 (instead of your fork, this way it will trigger a build on the bot and I can copy/past the log) |
Yeah, that indicates that it's looking at the DX SDK files before it looks at the Windows SDK files. It's only supposed to do that for Sndout_XAudio2_27.cpp in this PR. The rest of the files shouldn't require the DX SDK at all. The fix is to set DXSDK_DIR (system/user variable, or VS user macro) to the DX SDK path (backslash at end is essential) and remove any other method of adding the DX SDK directory to the path. I assume that's what needs changed in the buildbot. |
The current config is
Only in system env. |
And there those parameters (maybe not env variable)
So what's wrong? |
Hmm. No clue. Is there any variable with include/library paths? |
Here the env setup. There are some stuff about cg.
|
Yes but I have 2.9 With WIndows 10, No 2.7, so it is normal i don't have very old SDK |
Windows 7 users have XAudio2.7 though. |
Yes, but, i waiting Spu works with Windows 10 only with Sdk 8.1, and i thinks this PR fixed this problerm but no :( |
Windows 7 EOL is 14/1/2020. You'll likely be waiting quite a while then. Your other options are to create and maintain a local branch that doesn't require the DirectX SDK (but which removes the ability for Windows 7 users to use XAudio 2.7), or just install/extract the necessary DirectX SDK files (you only really need the header files, so that's really not going to take up a lot of space). |
Can't you like just simply fallback to windows sdk stuff when directx sdk (files? env var?) isn't found? And I don't see why whatever microsoft says about support should be relevant to whether we still can support an OS or not. |
Newer isn't always better and when it comes to developing on Win10 you are doing dev builds on a dev OS and that isn't always best for stability. That said, for right now the target minimum OS should remain Win7 and up. I know I plan to use Win7 long after support 'officially' ends because I am not fond of doing gaming or other stuff on an unstable OS like Win10. In Linux Terms: Win7 = Long Term Support release = STABLE |
Windows 10 Very Best, Windows 7 euuu , very old, no support SSD, no support Dx12 etc, this OS is dead |
@jcdenton2k Windoes 8 just has a shitty UI, aside of that nothing changed since 95. The dev quality paradigm shift happened with W10 and its "special updates policies". @Zangetsu38 Even windows 95 can run on an ssd. If you are talking about TRIM that was indeed introduced in Windows 7. I'd stop the OT here. |
@mirh Yes, Trim support start by Windows 8, so yes, ssd install and use works, but Fail with no support trim |
I think __has_include directive is what we really would need here. I haven't tested myself, but a guy says VS15 update 2 supports it (part of the c++17 N4567 work I guess?) |
I'm not sure where you got your faulty information, but Win7 SP1 supports SSDs and TRIM without issues: http://www.ghacks.net/2010/09/14/verify-that-trim-is-enabled-in-windows-7/ Even though I'm using mechanical drives, TRIM is still enabled by default for me just in case I update to SSDs. Having a dual-architecture structure for the Win7/Win8.1 side and the Win10 side might be useful to do but we should be careful not to deprecate the most widely-used OS's on the market. Win10 is a tiny minority in the grand scheme of things even after M$ pushed out Win10 like a virus on everyone forcing them to update without notice or consent. |
No, Sorry, it is officiel By Microsoft, Windows 7 Not support Trim :) |
I juste Request, separate SDK Dx June 2010 for other OS, and actually, This emulator only Run Xaudio 2.7, Xaudio 2.8+ requierement Sdk Windows kit 8.1 minimum. |
Windows 7 does support TRIM from Service Pack 1. Microsoft officially said Windows 7 doesn't support TRIM, meaning WITHOUT the service pack, it won't support it. |
hmm I see |
Profit ? |
@mirh I have this error now
|
Lol what? |
Or.. now that I think.. Should it be put the other way? Like: use updated stuff if you aren't targeting older systems?
|
Just a quick thing. Depending on environmental variables in Windows may be unreliable depending on the OS version and other installed software. When building in a testing environment there may be multiple variations of those environmental variables depending on what order things were installed in. Certain variables are relatively certain like the main Windows system directory and the TEMP directory. Other variables are not as reliable. There should be a way to query the OS to determine the installed DirectX SDK version and then have branching decisions made based upon that result. |
I'd use dxsdk env var presence to detect dxsdk installation (easy sherlock), but I really dunno how could you check the presence of an external variables. And.. Cmon, if you have an environment with another "dxsdkver.h" file (or older dx sdks) you deserve problems. |
Up. |
I don't actually understand what is wanted. I'll just randomly pick stuff
Conflicting statements - adding to 3rdparty is just pretending that we're not using an old SDK.
Wrong. Windows 7 uses XAudio 2.7, Windows 8.1 uses XAudio 2.8, Windows 10 uses XAudio 2.9 (it loads the XAudio 2.8 dll, but that loads XAudio 2.9). Otherwise this PR would have been completely pointless.
You're probably looking to use I'm not interested in doing this by the way. |
Nice to know. |
This reverts commit e6bf77d, reversing changes made to c794085. # Conflicts: # plugins/spu2-x/src/PS2E-spu2.cpp # plugins/spu2-x/src/SndOut.cpp # plugins/spu2-x/src/SndOut.h # plugins/spu2-x/src/Spu2replay.cpp # plugins/spu2-x/src/Windows/Config.cpp # plugins/spu2-x/src/Windows/SndOut_XAudio2.cpp # plugins/spu2-x/src/Windows/SndOut_XAudio2_27.cpp # plugins/spu2-x/src/Windows/Spu2-X.vcxproj
This reverts commit e6bf77d, reversing changes made to c794085. # Conflicts: # plugins/spu2-x/src/PS2E-spu2.cpp # plugins/spu2-x/src/SndOut.cpp # plugins/spu2-x/src/SndOut.h # plugins/spu2-x/src/Spu2replay.cpp # plugins/spu2-x/src/Windows/Config.cpp # plugins/spu2-x/src/Windows/SndOut_XAudio2.cpp # plugins/spu2-x/src/Windows/SndOut_XAudio2_27.cpp # plugins/spu2-x/src/Windows/Spu2-X.vcxproj
Changes:
Combined with my GSdx (#1238) and XInput (#1237) PRs, Windows 8.1 and 10 users should be able to run PCSX2 without requiring the DirectX redists.