Page MenuHomeWildfire Games

Stop using WMI for detecting the sound card
ClosedPublic

Authored by Itms on Jun 13 2017, 10:23 AM.

Details

Summary

This revision makes sound card detection cross-platform by using OpenAL instead of Windows Management Instrumentation.
Admittedly, the information won't be useful on a number of non-Windows platforms, as the function might just return OpenAL Soft as the card name.
Additionally, the driver version detection is now fixed and made cross-platform.

With the patch, the WMI code is not used anymore, so remove it.

Test Plan

Test on Unix by taking a look at system_info.txt after starting pyrogenesis.

Test on Windows some more.

Check that #4561 does not happen anymore.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Itms created this revision.Jun 13 2017, 10:23 AM
Vulcan added a subscriber: Vulcan.Jun 13 2017, 11:11 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1552/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/193/ for more details.

echotangoecho edited edge metadata.

Seems to work well on linux, but can't test on windows atm, as my windows install is currently broken...

source/lib/snd.cpp
47 ↗(On Diff #2548)

NULL -> nullptr, and below here as well

50 ↗(On Diff #2548)

Do we actually want to get all devices in the device string (versus the one device that might be in use) if possible?

source/lib/sysdep/gfx.cpp
43 ↗(On Diff #2548)

Inaccurate comment? WMI can cause crashes with NVIDIA hardware, regardless of whether Optimus is used (or do the crashes only occur with Optimus?).

Imarok edited edge metadata.Jun 24 2017, 4:52 PM

Can't build.
I applied the patch, deleted vc2013, executed update-workspace.bat.
Error 1 error C1083: Cannot open include file: 'lib/snd.h': No such file or directory E:\0AD\source\ps\Util.cpp 24 1 engine

Itms added a comment.Jun 26 2017, 9:38 PM

Seems to work well on linux, but can't test on windows atm, as my windows install is currently broken...

Out of curiosity, can you post the resulting Linux system_info.txt?

In D636#26993, @Imarok wrote:

Can't build.
I applied the patch, deleted vc2013, executed update-workspace.bat.
Error 1 error C1083: Cannot open include file: 'lib/snd.h': No such file or directory E:\0AD\source\ps\Util.cpp 24 1 engine

I believe the patch didn't apply correctly, see if lib/snd.h is here, else patch it manually.

source/lib/snd.cpp
47 ↗(On Diff #2548)

Sure why not ?

50 ↗(On Diff #2548)

This is to be consistent with the previous behavior. And the more information the better, I'd say.

source/lib/sysdep/gfx.cpp
43 ↗(On Diff #2548)

This comment does not talk about the crash I'm fixing. I believe the WMI crashes when trying to detect the graphics cards did only happen with Optimus, whereas a recent driver update breaks our entire WMI initialization.

Imarok added a comment.EditedJul 1 2017, 10:37 AM

Compiles and runs on win.

Sound Card     : OpenAL Soft on Lautsprecher (Realtek High Definition Audio); 
Sound Drivers  : 1.1 ALSOFT 1.17.1

Should I test more?

Itms added a comment.Jul 1 2017, 11:42 AM
In D636#27911, @Imarok wrote:

Should I test more?

I'd be happy to see the output on Unix ?

Same machine with linux:

Sound Card     : Internes Audio Analog Stereo; HDA NVidia Digital Stereo (HDMI); 
Sound Drivers  : 1.1 ALSOFT 1.16.0
Stan added a subscriber: Stan.Jul 1 2017, 1:55 PM

Doesn't detect the same soundcard, and I don't think you are using the NVIDIA one on linux unless you output your sound through a monitor ?

elexis added a subscriber: elexis.Jul 1 2017, 6:47 PM

Rebased HWDetect.cpp:

Index: source/ps/GameSetup/HWDetect.cpp
===================================================================
--- source/ps/GameSetup/HWDetect.cpp	(revision 19863)
+++ source/ps/GameSetup/HWDetect.cpp	(working copy)
@@ -18,21 +18,21 @@
 #include "precompiled.h"
 
 #include "scriptinterface/ScriptInterface.h"
 
 #include "lib/ogl.h"
+#include "lib/snd.h"
 #include "lib/svn_revision.h"
 #include "lib/timer.h"
 #include "lib/utf8.h"
 #include "lib/external_libraries/libsdl.h"
 #include "lib/res/graphics/ogl_tex.h"
 #include "lib/posix/posix_utsname.h"
 #include "lib/sysdep/cpu.h"
 #include "lib/sysdep/gfx.h"
 #include "lib/sysdep/numa.h"
 #include "lib/sysdep/os_cpu.h"
-#include "lib/sysdep/snd.h"
 #if ARCH_X86_X64
 # include "lib/sysdep/arch/x86_x64/cache.h"
 # include "lib/sysdep/arch/x86_x64/topology.h"
 #endif
 #include "ps/CLogger.h"
@@ -264,12 +264,12 @@ void RunHardwareDetection()
 	scriptInterface.SetProperty(settings, "build_clang", (int)CLANG_VERSION);
 
 	scriptInterface.SetProperty(settings, "gfx_card", gfx::CardName());
 	scriptInterface.SetProperty(settings, "gfx_drv_ver", gfx::DriverInfo());
 
-	scriptInterface.SetProperty(settings, "snd_card", std::wstring(snd_card));
-	scriptInterface.SetProperty(settings, "snd_drv_ver", std::wstring(snd_drv_ver));
+	scriptInterface.SetProperty(settings, "snd_card", snd_card);
+	scriptInterface.SetProperty(settings, "snd_drv_ver", snd_drv_ver);
 
 	ReportGLLimits(scriptInterface, settings);
 
 	scriptInterface.SetProperty(settings, "video_xres", g_VideoMode.GetXRes());
 	scriptInterface.SetProperty(settings, "video_yres", g_VideoMode.GetYRes());

The test plan works for me on Ubuntu:

Sound Card     : SB Audigy (2 Platinum) Analog Stereo; Caicos HDMI Audio [Radeon HD 6400 Series] Digital Stereo (HDMI); 
Sound Drivers  : 1.1 ALSOFT 1.16.0
Itms added a comment.Jul 2 2017, 2:32 PM
In D636#27920, @Stan wrote:

Doesn't detect the same soundcard, and I don't think you are using the NVIDIA one on linux unless you output your sound through a monitor ?

This is normal, different OSes won't expose the same things to OpenAL. Specifically, sometimes they show the unplugged hardware, sometimes they don't. Not a big deal for us.

Stan added a comment.Jul 2 2017, 3:11 PM

Ah right, I didn't see there was two soundcards.

echotangoecho accepted this revision.Jul 5 2017, 1:34 PM

Looks fine to me, if those occurences of NULL are changed to nullptr.

This revision is now accepted and ready to land.Jul 5 2017, 1:34 PM
This revision was automatically updated to reflect the committed changes.