RtAudio.cpp needs to include more headers for DirectSound

[weskeene]

In certain cases (an MFC app building on Windows 10), RtAudio.cpp pulling in dsound.h (platform copy, as it should be), will fail build.

The solution is to add:

#include "mmsystem.h"
#include "mmreg.h"

immediately before the line:

#include <dsound.h>

I am also aware there is another thread about distributing dsound.h. I don't care about the legal aspect, but it's probably a bad idea. When/if things go wrong, a user will not immediately know which dsound.h was pulled in, especially because <> are used instead of "".

[radarsat1]

any idea if adding those lines could break things on older systems?

[radarsat1]

By the way could you have a look at #75 and say whether you have had any related issues with Windows 10?

[weskeene]

With regard to breaking older systems, that seems most unlikely. A search of the copyright date of each header reveals that mmsystem.h has a copyright range between 1992-1998, where mmreg has a copyright between 1991-1998. So, it's probably safe to say that any reasonable system you'd want to support has those headers. Even if it does manage to break some very old system, installing the DirectX SDK would surely solve the problem.

With respect to #75 I have not experiences those issues. That's probably because that person was using mingw, and I am not. However, I also observed one of the commenters said he was trying to compile WASAPI, whereas my suggestion pertains only to DirectSound support.

It is possible that my suggestion, if implemented in the WASAPI section, may have fixed his issue. But since I don't use that interface, I can't confirm it.

[radarsat1]

Thanks! Yes, I think it's safe to add includes for mmsystem and mmreg, just weird that it wasn't necessary before and now apparently it is. I'll make a pull request for further discussion. Some more people who actually have Windows 10 should test, perhaps.

(I think most rtaudio developers do not have Windows, now that would be a useful case for using something like Travis... too bad it doesn't support Windows.)

[weskeene]

It was strange, too. For one of my projects, the existing headers in RtAudio worked great. But once I moved to an MFC project, it didn’t. It was probably a header order issue, but RtAudio should just explicitly include everything it needs.

From: Stephen Sinclair [mailto:notifications@github.com]
Sent: Wednesday, November 2, 2016 10:20 AM
To: thestk/rtaudio rtaudio@noreply.github.com
Cc: Wes Keene wes@keene.co; Author author@noreply.github.com
Subject: Re: [thestk/rtaudio] RtAudio.cpp needs to include more headers for DirectSound (#78)

Thanks! Yes, I think it's safe to add includes for mmsystem and mmreg, just weird that it wasn't necessary before and now apparently it is. I'll make a pull request for further discussion. Some more people who actually have Windows 10 should test, perhaps.

(I think most rtaudio developers do not have Windows, now that would be a useful case for using something like Travis... too bad it doesn't support Windows.)

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/78#issuecomment-257878081, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AWGrmS1tVZt-zAiAnp84eW5FhiGBN1bnks5q6JwBgaJpZM4KliR3.

[radarsat1]

Ah, but in that case I'd be interested to know more details of what the actual error is, because I'm looking through the DirectSound part of RtAudio.cpp and I don't see what could need anything in mmsystem or mmreg.

[radarsat1]

Maybe MUTEX stuff?

[weskeene]

If these headers are not included dsound.h will not see certain definitions it needs. First and foremost, WAVEFORMATEX will not be defined. So really MS should have included those in dsound.h, probably, but you know that will never change.

From: Stephen Sinclair [mailto:notifications@github.com]
Sent: Wednesday, November 2, 2016 10:27 AM
To: thestk/rtaudio rtaudio@noreply.github.com
Cc: Wes Keene wes@keene.co; Author author@noreply.github.com
Subject: Re: [thestk/rtaudio] RtAudio.cpp needs to include more headers for DirectSound (#78)

Ah, but in that case I'd be interested to know more details of what the actual error is, because I'm looking through the DirectSound part of RtAudio.cpp and I don't see what could need anything in mmsystem or mmreg.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/78#issuecomment-257880082, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AWGrmSa8QUnC29LA3mKj8jRs8NCnB5_Eks5q6J2cgaJpZM4KliR3.

[radarsat1]

Ah, yeah, I see.

[garyscavone]

This seems like an easy fix. Can someone verify the proposed PR doesn't break anything? Most of us don't have Windows machines.

[radarsat1]

I merged #79 because it just adds some header files that certainly exist. Whether this fixes the problem or not I can't be sure.

[garyscavone]

I'm closing this, as it should be fixed by the above merge. If a problem still persists, a new issue or PR should be created.