Skip to content
Permalink
Browse files

opus-build: revert ff9086e and error out if built with CONFIG(sbcelt)…

… instead.

Stock Opus 1.1 doesn't build in C++ mode, so our
old hack doesn't work anymore.

For now, we instead error out in qmake if we detect
a scenario that could cause symbol clashes.
  • Loading branch information...
mkrautz committed Dec 17, 2013
1 parent ff9086e commit 338f0240f171fd658e9bb78b49849ee5c9633e05
Showing with 11 additions and 4 deletions.
  1. +11 −4 opus-build/opus-build.pro
@@ -44,10 +44,17 @@ win32 {

unix {
CONFIG += staticlib
# Build as C++ to ensure symbols are C++-mangled.
# This avoids symbol clashes with CELT 0.7 when
# building Mumble with CONFIG(sbcelt).
QMAKE_CFLAGS += -x c++
CONFIG(sbcelt) {
# Before Opus 1.1 we used to be able to build Opus
# as C++ code to get C++ name mangling for free,
# allowing us to statically build both libcelt
# and libopus into the same binary while avoiding
# symbol clashes between the two libraries.
#
# Stock Opus 1.1 doesn't build in C++ mode, so error
# out for now.
error(Mumble cannot be built in SBCELT mode with Opus 1.1 - aborting build.)
}
INCLUDEPATH += ../$$BUILDDIR
}

4 comments on commit 338f024

@hacst

This comment has been minimized.

Copy link
Member

replied Dec 18, 2013

It doesn't build in C++ mode? Really? If that's true you should report that upstream. Afaik they want it to be buildable in C++ mode. At least that's the impression I got when this came up the last time.

@mkrautz

This comment has been minimized.

Copy link
Member Author

replied Dec 18, 2013

Yes, I'm going to. There's an implicit conversion from const void * to const opus_int16 * that fails when built as C++ - and that's it. Minor single line patch.

@mkrautz

This comment has been minimized.

Copy link
Member Author

replied Dec 18, 2013

@dD0T

Had a glance at Opus's git repo. Seems like this build issue has already been fixed by the signature change of optimize_frame_size in http://git.xiph.org/?p=opus.git;a=commitdiff;h=32ada84b5b5a147c38c428fb0863d419dd802b37

Do we want to sync to that?

@mkrautz

This comment has been minimized.

Copy link
Member Author

replied Jan 11, 2014

@dD0T

Do you have an opinion about my comment above?

Please sign in to comment.
You can’t perform that action at this time.