[PATCH] xaudio2: Avoid redefining several F3DAudio related constants.
I noticed a bunch of compiler warnings like the following issued by GCC 10: In file included from ../xaudio2_7/x3daudio.c:29: .../include/F3DAudio.h:69: warning: "SPEAKER_2POINT1" redefined ../../include/x3daudio.h:127: note: this is the location of the previous definition Our own include/x3daudio.h tries to prevent that via #ifndef _SPEAKER_POSITIONS_ #define _SPEAKER_POSITIONS_ However, current it is included *before* the system provided F3DAudio.h, so this safety net falls short. Fixed thusly by changing the order of these two include files so that the system provided one comes first. Gerald Signed-off-by: Gerald Pfeifer <gerald(a)pfeifer.com> --- dlls/xaudio2_7/x3daudio.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dlls/xaudio2_7/x3daudio.c b/dlls/xaudio2_7/x3daudio.c index 240cd5c101..6f16ba3470 100644 --- a/dlls/xaudio2_7/x3daudio.c +++ b/dlls/xaudio2_7/x3daudio.c @@ -20,15 +20,13 @@ #include "config.h" #include <stdarg.h> - +#include <F3DAudio.h> #include "windef.h" #include "winbase.h" #include "x3daudio.h" #include "wine/debug.h" -#include <F3DAudio.h> - #if XAUDIO2_VER >= 8 || defined X3DAUDIO1_VER WINE_DEFAULT_DEBUG_CHANNEL(xaudio2); #endif -- 2.25.0
On Thu, Apr 30, 2020 at 12:28:10AM +0200, Gerald Pfeifer wrote:
I noticed a bunch of compiler warnings like the following issued by GCC 10:
In file included from ../xaudio2_7/x3daudio.c:29: .../include/F3DAudio.h:69: warning: "SPEAKER_2POINT1" redefined ../../include/x3daudio.h:127: note: this is the location of the previous definition
Our own include/x3daudio.h tries to prevent that via
#ifndef _SPEAKER_POSITIONS_ #define _SPEAKER_POSITIONS_
However, current it is included *before* the system provided F3DAudio.h, so this safety net falls short.
Fixed thusly by changing the order of these two include files so that the system provided one comes first.
I'm not opposed to this, but I think there's a better solution. The platform SDK and Wine's header both check for "#ifndef SPEAKER_MONO" before defining the speaker arrangements symbols. However, FAudio's header checks for "#ifndef _SPEAKER_COMBINATIONS_" instead. This symbol doesn't occur in the platform SDK. Perhaps we should change FAudio's header to check for SPEAKER_MONO instead, to match the platform SDK? Andrew
Signed-off-by: Gerald Pfeifer <gerald(a)pfeifer.com> --- dlls/xaudio2_7/x3daudio.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/dlls/xaudio2_7/x3daudio.c b/dlls/xaudio2_7/x3daudio.c index 240cd5c101..6f16ba3470 100644 --- a/dlls/xaudio2_7/x3daudio.c +++ b/dlls/xaudio2_7/x3daudio.c @@ -20,15 +20,13 @@ #include "config.h"
#include <stdarg.h> - +#include <F3DAudio.h> #include "windef.h" #include "winbase.h" #include "x3daudio.h"
#include "wine/debug.h"
-#include <F3DAudio.h> - #if XAUDIO2_VER >= 8 || defined X3DAUDIO1_VER WINE_DEFAULT_DEBUG_CHANNEL(xaudio2); #endif -- 2.25.0
On Thu, 30 Apr 2020, Andrew Eikum wrote:
In file included from ../xaudio2_7/x3daudio.c:29: .../include/F3DAudio.h:69: warning: "SPEAKER_2POINT1" redefined ../../include/x3daudio.h:127: note: this is the location of the previous definition
Our own include/x3daudio.h tries to prevent that via
#ifndef _SPEAKER_POSITIONS_ #define _SPEAKER_POSITIONS_
However, current it is included *before* the system provided F3DAudio.h, so this safety net falls short.
Fixed thusly by changing the order of these two include files so that the system provided one comes first. I'm not opposed to this, but I think there's a better solution. The platform SDK and Wine's header both check for "#ifndef SPEAKER_MONO" before defining the speaker arrangements symbols. However, FAudio's header checks for "#ifndef _SPEAKER_COMBINATIONS_" instead. This symbol doesn't occur in the platform SDK.
Perhaps we should change FAudio's header to check for SPEAKER_MONO instead, to match the platform SDK?
Yes, that looks like a valid idea. Even if you can get such a change accepted, how long until the next release and how long until that spreads widely? And it looks like an implementation detail (subject to change)? So I suggest to go with my patch for Wine now, and if you can push for such a change in FAudio upstream on top, all the better! Hope this makes sense? Gerald
On Fri, May 01, 2020 at 12:46:56AM +0200, Gerald Pfeifer wrote:
On Thu, 30 Apr 2020, Andrew Eikum wrote:
In file included from ../xaudio2_7/x3daudio.c:29: .../include/F3DAudio.h:69: warning: "SPEAKER_2POINT1" redefined ../../include/x3daudio.h:127: note: this is the location of the previous definition
Our own include/x3daudio.h tries to prevent that via
#ifndef _SPEAKER_POSITIONS_ #define _SPEAKER_POSITIONS_
However, current it is included *before* the system provided F3DAudio.h, so this safety net falls short.
Fixed thusly by changing the order of these two include files so that the system provided one comes first. I'm not opposed to this, but I think there's a better solution. The platform SDK and Wine's header both check for "#ifndef SPEAKER_MONO" before defining the speaker arrangements symbols. However, FAudio's header checks for "#ifndef _SPEAKER_COMBINATIONS_" instead. This symbol doesn't occur in the platform SDK.
Perhaps we should change FAudio's header to check for SPEAKER_MONO instead, to match the platform SDK?
Yes, that looks like a valid idea. Even if you can get such a change accepted, how long until the next release and how long until that spreads widely? And it looks like an implementation detail (subject to change)?
So I suggest to go with my patch for Wine now, and if you can push for such a change in FAudio upstream on top, all the better!
I've opened a pull request for this: https://github.com/FNA-XNA/FAudio/pull/187 FAudio makes a new release every month. If it's just fixing compiler warnings, I'm not sure it's worth the change in Wine. Andrew
On Fri, 1 May 2020, Andrew Eikum wrote:
I've opened a pull request for this: https://github.com/FNA-XNA/FAudio/pull/187
FAudio makes a new release every month. If it's just fixing compiler warnings, I'm not sure it's worth the change in Wine.
Thank you, Andrew! That worked quite well (and fast). :-) Still, don't we usually want to have system headers before ours? Or is this somewhat different? Gerald Signed-off-by: Gerald Pfeifer <gerald(a)pfeifer.com> --- dlls/xaudio2_7/x3daudio.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dlls/xaudio2_7/x3daudio.c b/dlls/xaudio2_7/x3daudio.c index 240cd5c101..6f16ba3470 100644 --- a/dlls/xaudio2_7/x3daudio.c +++ b/dlls/xaudio2_7/x3daudio.c @@ -20,15 +20,13 @@ #include "config.h" #include <stdarg.h> - +#include <F3DAudio.h> #include "windef.h" #include "winbase.h" #include "x3daudio.h" #include "wine/debug.h" -#include <F3DAudio.h> - #if XAUDIO2_VER >= 8 || defined X3DAUDIO1_VER WINE_DEFAULT_DEBUG_CHANNEL(xaudio2); #endif -- 2.25.0
participants (2)
-
Andrew Eikum -
Gerald Pfeifer