xiph / vorbis

Reference implementation of the Ogg Vorbis audio format.
BSD 3-Clause "New" or "Revised" License
450 stars 183 forks source link

Cannot build tests on MSVC #75

Closed SegaraRai closed 3 years ago

SegaraRai commented 3 years ago

MSVC displayed following error when I tried to build vorbis with CMake.

vorbis\test\util.c(37,41): error C2065: "M_PI": undeclared identifier

MSVC seems to require #define _USE_MATH_DEFINES before #include <math.h> to use M_PI. Build succeeded once I add it to L16.

rillian commented 3 years ago

I see. M_PI is also referenced by several files in lib but the library source generally includes os.h which has a fallback definition of M_PI for MSVC. The tone utility should have the same problem, but it looks like the CMake build doesn't try to compile it.

gcc enables M_PI by default as a gnu extension (part of X/Open) In my editor, clang-complete also complains about the missing definition.

So defining _USE_MATH_DEFINES here, possibly protected by an #ifdef MSVC is probably the correct fix, but it might be better to change the line in os.h similarly. tone.c can just include os.h since it's internal to the library source.

rillian commented 3 years ago

And obviously we should add the building and running the tests to the appveyor config!

rillian commented 3 years ago

This` should be addressed by 894d1b1f22f2fb8ab6d3aeba863e143416c71ce2. Thanks for reporting!