Sound Improvements

[Zini]

Yeah, that is obviously wrong.

[Zini]

Finally got around to have a closer look at your code. Sorry that it took so long.

I didn't examine all of it in detail, but the overall structure and the sound manager look reasonable.

There is one thing I noticed: SoundManager and probably a few other classes. Are not properly copyable or assignable. Which is okay. But in this case the automatically defined copy constructor and the assignment operator should be disabled by declaring them private and leaving them unimplemented.
Unfortunately this basic safety measure has been neglected in a significant fraction of the older parts of OpenMW.

[werdanith]

Since the sound refactoring, the game segfaults 20% of the time when you try to play music with libmpg123 version 1.12 ( the default in Ubuntu 11.10). The issues go away if you upgrade to 1.13 by manual installation. I don't know what the decoding situation was before and what it is now but this did not happen before. Here's the stack trace:

Code: Select all

(gdb) bt
#0  0x0088e5c1 in ?? () from /usr/lib/libmpg123.so.0
#1  0x0088f9b6 in ?? () from /usr/lib/libmpg123.so.0
#2  0x0087160f in ?? () from /usr/lib/libmpg123.so.0
#3  0x00871bdd in mpg123_decode () from /usr/lib/libmpg123.so.0
#4  0x00871d45 in mpg123_read () from /usr/lib/libmpg123.so.0
#5  0x0849efd0 in MWSound::MpgSnd_Decoder::read(char*, unsigned int) ()
#6  0x08492831 in MWSound::OpenAL_SoundStream::play() ()
#7  0x084946bb in MWSound::OpenAL_Output::streamSound(std::string const&, float, float) ()
#8  0x0848e5e5 in MWSound::SoundManager::streamMusicFull(std::string const&) ()
#9  0x0848e833 in MWSound::SoundManager::startRandomTitle() ()
#10 0x0848e8f4 in MWSound::SoundManager::playPlaylist(std::string const&) ()
#11 0x0852f245 in OMW::Engine::go() ()
#12 0x0852554e in main ()

[Chris]

Since the sound refactoring, the game segfaults 20% of the time when you try to play music with libmpg123 version 1.12 ( the default in Ubuntu 11.10). The issues go away if you upgrade to 1.13 by manual installation.

The issue is that the mpg123 decoder now uses the callbacks provided by the lib so that it can read from an Ogre Datastream. Apparently libmpg123 v1.12 has a bug with the callbacks that don't appear with standard I/O.

Upgrading to libmpg123 v1.13 fixes the bug, as does switching to the ffmpeg decoder. It may be worth switching to ffmpeg by default, particularly since we're going to need it for videos too, but I'd like to check with others (Zini) to make sure it's reasonable to do at this point.

[Greendogo]

-nevermind-

[Zini]

Upgrading to libmpg123 v1.13 fixes the bug, as does switching to the ffmpeg decoder. It may be worth switching to ffmpeg by default, particularly since we're going to need it for videos too, but I'd like to check with others (Zini) to make sure it's reasonable to do at this point.

Don't like the idea too much. I don't remember all the details, because that was shortly after I joined the OpenMW project (a long time ago). But from what I remember ffmpeg caused problems for some people. And I remember being one of them. Wasn't able to build OpenMW with ffmpeg, because some peculiarities of my system's setup stopped me from installing the ffmpeg dev packages. Never got around to investigate it, but I would rather not get into this right now.

[gus]

And it's impossible to use ffmpeg with visual studio.

[Ace (SWE)]

And it's impossible to use ffmpeg with visual studio.

Not completely, you have to use prebuilt binaries as ffmpeg doesn't compile under visual studio. You also need to fiddle a bit with the headers as they try to include cygwin headers and use functions from those.
My OpenMW 0.14.0 builds will come with ffmpeg though so it's very possible.