[PATCH] xaudio2: Avoid redefining several F3DAudio related constants.

[Gerald Pfeifer]

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 
---
 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

[Andrew Eikum]

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
>
> 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

[Gerald Pfeifer]

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

[Andrew Eikum]

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

[Gerald Pfeifer]

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 
---
 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