New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 788675 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

webrtc headers don't compile if _USE_MATH_DEFINES is defined

Project Member Reported by brat...@opera.com, Nov 27 2017

Issue description

_USE_MATH_DEFINES is a header that should be set before including Windows headers to include certain optional parts of those headers. Therefore a lot of code set it, including webrtc. Unfortunately that can result in compilation errors because of redefined macros (gn defines macros to 1, other code just define them without a value).

Suggested fix below:

diff --git a/modules/audio_processing/beamformer/nonlinear_beamformer.h b/modules/audio_processing/beamformer/nonlinear_beamformer.h
index 76556e7..5b79dc4 100644
--- a/modules/audio_processing/beamformer/nonlinear_beamformer.h
+++ b/modules/audio_processing/beamformer/nonlinear_beamformer.h
@@ -12,7 +12,9 @@
 #define MODULES_AUDIO_PROCESSING_BEAMFORMER_NONLINEAR_BEAMFORMER_H_
 
 // MSVC++ requires this to be set before any other includes to get M_PI.
+#ifndef _USE_MATH_DEFINES
 #define _USE_MATH_DEFINES
+#endif
 
 #include <math.h>
 
diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h
index a4245c6..fcda887 100644
--- a/modules/audio_processing/include/audio_processing.h
+++ b/modules/audio_processing/include/audio_processing.h
@@ -12,7 +12,9 @@
 #define MODULES_AUDIO_PROCESSING_INCLUDE_AUDIO_PROCESSING_H_
 
 // MSVC++ requires this to be set before any other includes to get M_PI.
+#ifndef _USE_MATH_DEFINES
 #define _USE_MATH_DEFINES
+#endif
 
 #include <math.h>
 #include <stddef.h>  // size_t

 

Comment 1 by foolip@chromium.org, Nov 28 2017

Owner: phoglund@chromium.org
Status: Assigned (was: Untriaged)
phoglund@, could you take a look at this, or reassign?
Owner: ehmaldonado@chromium.org
Edward, can you have a look?

Comment 3 by kwiberg@webrtc.org, Nov 28 2017

The suggested fix looks good to me; we want it to be defined, but we don't care if we have to do it ourselves or if someone else has already defined it.
Owner: phoglund@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 13 2017

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/3ff90f19d36e9d20c22a04a30f17c45003696ac3

commit 3ff90f19d36e9d20c22a04a30f17c45003696ac3
Author: Patrik Höglund <phoglund@webrtc.org>
Date: Wed Dec 13 09:39:20 2017

Fix macro clash with _USE_MATH_DEFINES.

Bug:  chromium:788675 
Change-Id: I4840fd013a81ffe157323b0bb876d64fd60d8a19
Reviewed-on: https://webrtc-review.googlesource.com/32304
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Patrik Höglund <phoglund@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21235}
[modify] https://crrev.com/3ff90f19d36e9d20c22a04a30f17c45003696ac3/modules/audio_processing/beamformer/nonlinear_beamformer.h
[modify] https://crrev.com/3ff90f19d36e9d20c22a04a30f17c45003696ac3/modules/audio_processing/include/audio_processing.h

Status: Fixed (was: Assigned)

Sign in to add a comment