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

spu2-x: Drop XAudio 2.7 #3244

Merged
merged 3 commits into from May 9, 2020
Merged

spu2-x: Drop XAudio 2.7 #3244

merged 3 commits into from May 9, 2020

Conversation

@MonJamp
Copy link
Contributor

@MonJamp MonJamp commented Feb 9, 2020

Simplifies code for spu2-x while removing support for Windows 7

@tadanokojin tadanokojin added this to the Release 1.8 milestone Feb 10, 2020
@seta-san
Copy link
Contributor

@seta-san seta-san commented Feb 20, 2020

don't forget in WinConfig.h you should change to this to specifically target xaudio2.9
"#ifndef _WIN32_WINNT
#define _WIN32_WINNT 0x0A00
#endif

@turtleli
Copy link
Member

@turtleli turtleli commented Feb 20, 2020

This PR is for dropping XAudio 2.7. Targeting XAudio 2.9 has no benefit if we're not using its features.

@seta-san
Copy link
Contributor

@seta-san seta-san commented Feb 20, 2020

This PR is for dropping XAudio 2.7. Targeting XAudio

either you target 2.8 or 2.9 after dropping 2.7.

https://docs.microsoft.com/en-us/windows/win32/xaudio2/xaudio2-redistributable

Microsoft provides a NuGet package for 2.9 that can be ran on windows 7 sp1 and windows 8. There's really no good reason for stopping off at 2.8 when we can simply target 2.9 and still have compatibility with all supported Operating Systems. The 2.9 dll is still supported and still receives updates and bug fixes from Microsoft.

@turtleli
Copy link
Member

@turtleli turtleli commented Feb 20, 2020

The plan is to drop Windows 7 support after 1.6 is released. Please see #3241 for previous discussion.

@seta-san
Copy link
Contributor

@seta-san seta-san commented Feb 20, 2020

The plan is to drop Windows 7 support after 1.6 is released. Please see #3241 for previous discussion.

cool to see you already had the discussion. i get not wanting to deal with a nuget but i still think targeting 2.9 is superior since Microsoft is still letting people with windows 8 upgrade to 10 for free although they aren't really advertising it. still, if we're targeting 2.8 the line in winconfig.h should be 0x602

@turtleli
Copy link
Member

@turtleli turtleli commented Feb 20, 2020

There's a line in SndOut_XAudio2.cpp that sets _WIN32_WINNT to 0x602 but yes, what you suggested also works and might be a better way of doing it.

@seta-san
Copy link
Contributor

@seta-san seta-san commented Feb 20, 2020

N32_WINNT to 0x602 but yes, what you suggested also works and might be a better way of doing it.

thanks for listening. This has been a pet issue of mine for a little bit.
https://forums.pcsx2.net/Thread-how-to-get-SPU2-to-compile-on-visual-studio-2019-and-to-modern-xaudio
:)

@MonJamp
Copy link
Contributor Author

@MonJamp MonJamp commented Feb 22, 2020

Using the ListDlls utility with this build I see that both xaudio 2.8 and xaudio 2.9 dlls are mapped on Windows 10. I assume that if W10 is detected there is a forward declaration to load xaudio 2.9 dll even if the exe is targeting 2.8. And just for fun I opened the xaudio2_8.dll found on W10 in a disassembler and confirm that all it's doing is forwarding function calls to xaudio2_9.dll

So there's no harm targeting xaudio 2.8 and it allows us to keep supporting W8. Although maybe I should add a comment to target xaudio 2.9 when W8 support is dropped.

Copy link
Contributor

@lightningterror lightningterror left a comment

Gave it a quick test to see if audio works, seems good.

@turtleli turtleli self-assigned this May 7, 2020
@turtleli
Copy link
Member

@turtleli turtleli commented May 7, 2020

SndOut_XAudio2_27.cpp and SndOut_XAudio2.inl also need to be removed from the SPU2-X CMakeLists.txt file.

@turtleli turtleli merged commit 9319ed1 into PCSX2:master May 9, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@turtleli
Copy link
Member

@turtleli turtleli commented May 9, 2020

Thanks.

@Anths2016
Copy link

@Anths2016 Anths2016 commented May 24, 2020

How can I use new plugin with Win7?

@refractionpcsx2
Copy link
Member

@refractionpcsx2 refractionpcsx2 commented May 24, 2020

Use the one that comes with 1.6.0

@kenshen112
Copy link
Contributor

@kenshen112 kenshen112 commented May 24, 2020

@Anths2016 You don't. We're not supporting Windows 7 anymore. You can either use 1.6's plugin or upgrade

@Anths2016
Copy link

@Anths2016 Anths2016 commented May 24, 2020

Are you affiliated with Microsoft? Otherwise I don't understand the reason ...

@refractionpcsx2
Copy link
Member

@refractionpcsx2 refractionpcsx2 commented May 24, 2020

We are not no. However we have been using the DirectX SDK since 2010 and that was the only thing left using it, everything has been integrated in to the Windows SDK since Windows 8, and now Windows 7 is end of life we figured it time to remove the legacy tools.

We're not stopping you using the current release (or just the audio plugin with the newer builds) but by the time 1.8.0 is released, Windows 7 will be end of life by about 4-5 years. We're not purposefully killing off old OS's, the availability of compatible tools is the problem and it hinders progress.

@KrossX
Copy link
Contributor

@KrossX KrossX commented May 24, 2020

You can still use the new plugin though, is just that old XAudio backend won't be available. You can use the other backends instead. However, I don't remember any fix going into SPU so using old plugin should be just fine and you wouldn't be missing anything.

@tadanokojin
Copy link
Member

@tadanokojin tadanokojin commented May 24, 2020

Any further discussion about this the hows and whys of this can happen at https://forums.pcsx2.net/
Don't need to fill the poor guys PR.

@PCSX2 PCSX2 locked and limited conversation to collaborators May 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
You can’t perform that action at this time.