Jump to content

New Sound Manager svn patch


Recommended Posts

There's a new, re-mastered main menu track, so thats why fabio finds it different :) But it seems the new sound manager (maybe) cuts of the highs and lows of the track, giving it a sorta bland feel.

I hav been, and will continue to look at he old code to see if they we modifying the sound at all. But I can't find it. I will also look into the positioning, etc. perhaps that affects the sound. Personally I worry more about the stability, etc. Especially since the old sound system had so many issues. We can fix the audio quality later, after some more objective observations. To figure out what is going on... But I will keep at it

Link to comment
Share on other sites

Erik and I both received the following error at the end of a multiplayer match using autobuild and latest svn:

ERROR: OpenAL error: Invalid Value; called from CSoundItem::~CSoundItem (line 48)

Deiz who was also playing, did not see the error. Erik and I are on PCs

Edit: This was fixed in the latest autobuild :)

Link to comment
Share on other sites

I hav been, and will continue to look at he old code to see if they we modifying the sound at all. But I can't find it. I will also look into the positioning, etc. perhaps that affects the sound. Personally I worry more about the stability, etc. Especially since the old sound system had so many issues. We can fix the audio quality later, after some more objective observations. To figure out what is going on... But I will keep at it

Mainly I feel like maybe the sound engine isn't picking up the stereo tracks correctly or something. The music sounds almost mono and the highs and lows seem cut off. But I agree, stability is most important for now and we can focus on sound quality after the release.

EDIT: This is a pretty close approximation of how the game's music sounds to me and others:

Link to comment
Share on other sites

The game segfaults when starting up, Atlas segfaults when clicking the "Play" button. There was no issue a few revisions back, and it still works fine with the -nosound flag.

gdb backtrace:


(gdb) bt
#0 0x0000000000000000 in ?? ()
#1 0x00000000005cae8e in COggData::FetchDataIntoBuffer (this=0x4c013f0, count=<optimized out>, buffers=0x4c0142c)
at ../../../source/soundmanager/data/OggData.cpp:106
#2 0x00000000005cafb7 in COggData::InitOggFile (this=0x4c013f0, itemPath=<optimized out>) at ../../../source/soundmanager/data/OggData.cpp:63
#3 0x00000000005ca335 in SoundDataFromOgg (itemPath=...) at ../../../source/soundmanager/data/SoundData.cpp:99
#4 CSoundData::SoundDataFromFile (itemPath=...) at ../../../source/soundmanager/data/SoundData.cpp:79
#5 0x00000000005c8f5b in CSoundManager::LoadItem (this=0xbaaaf0, itemPath=...) at ../../../source/soundmanager/SoundManager.cpp:182
#6 0x00000000005d923e in Loop (this=<optimized out>) at ../../../source/soundmanager/js/MusicSound.cpp:50
#7 CNativeFunction<JMusicSound, false, bool, &JMusicSound::Loop>::JSFunction (cx=0xc335e0, argc=<optimized out>, vp=0x7fffbd2e0218)
at ../../../source/scripting/ScriptableObject.h:188
#8 0x00007ffff710100a in js::Interpret(JSContext*, JSStackFrame*, unsigned int, JSInterpMode) ()
from trunk/binaries/system/libmozjs185-ps-release.so.1.0
#9 0x00007ffff710a895 in js::RunScript(JSContext*, JSScript*, JSStackFrame*) ()
from trunk/binaries/system/libmozjs185-ps-release.so.1.0
#10 0x00007ffff710ad32 in js::Invoke(JSContext*, js::CallArgs const&, unsigned int) ()
from trunk/binaries/system/libmozjs185-ps-release.so.1.0
#11 0x00007ffff710bc1c in js::ExternalInvoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) ()
from trunk/binaries/system/libmozjs185-ps-release.so.1.0
#12 0x00007ffff7088922 in JS_CallFunctionName ()
from trunk/binaries/system/libmozjs185-ps-release.so.1.0
#13 0x000000000052e7ac in ScriptInterface::CallFunction_ (this=0xbbcb20, val=<optimized out>, name=0x87d9d7 "init", argc=2, argv=0x7fffea90eaf0,
ret=@0x7fffea90ea90) at ../../../source/scriptinterface/ScriptInterface.cpp:740
#14 0x00000000007b75f1 in CallFunctionVoid<CScriptValRooted, CScriptValRooted> (a1=..., a0=..., name=0x87d9d7 "init", val=18445618172679858216,
this=0xbbcb20) at ../../../source/scriptinterface/ScriptInterface.h:392
#15 CGUIManager::LoadPage (this=0xcc74a0, page=...) at ../../../source/gui/GUIManager.cpp:164
#16 0x00000000007b8bd6 in CGUIManager::PushPage (this=0xcc74a0, pageName=..., initData=...) at ../../../source/gui/GUIManager.cpp:80
#17 0x00000000007b9452 in CGUIManager::SwitchPage (this=0xcc74a0, pageName=..., initData=...) at ../../../source/gui/GUIManager.cpp:72
#18 0x000000000071fd5d in fGuiSwitchPage (msg=<optimized out>) at ../../../source/tools/atlas/GameInterface/Handlers/MiscHandlers.cpp:155
#19 AtlasMessage::fGuiSwitchPage_wrapper (msg=<optimized out>) at ../../../source/tools/atlas/GameInterface/Handlers/MiscHandlers.cpp:153
#20 0x0000000000701bc8 in RunEngine (data=<optimized out>) at ../../../source/tools/atlas/GameInterface/GameLoop.cpp:172
#21 0x00007ffff49f2efc in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#22 0x00007ffff472d59d in clone () from /lib/x86_64-linux-gnu/libc.so.6
#23 0x0000000000000000 in ?? ()

Can you try reproducing this now and see if there are more helpful errors being logged?

Link to comment
Share on other sites

Can you try reproducing this now and see if there are more helpful errors being logged?

No new errors, though the problem seems to be intermittent now. Sometimes everything works fine, though other times it crashes on startup (I guess a segfault can be random, depending on what data gets overwritten). I'll clean workspaces and and recompile everything, just to make sure everything is current, and I'll let you know if it happens again.

Link to comment
Share on other sites

My brother got this. It started somewhere in the middle of a game and then continued spamming. Later it crashed, but I don't know if it's related.

ERROR: OpenAL error: Invalid Name; called from CSoundBase::SetLocation (line 168)

ERROR: OpenAL error: Invalid Name; called from CSoundBase::SetRollOff (line 89)

ERROR: OpenAL error: Invalid Name; called from CSoundBase::SetPitch (line 112)

ERROR: OpenAL error: Invalid Name; called from CSoundBase::SetCone (line 102)

ERROR: OpenAL error: Invalid Name; called from CSoundBase::SetCone (line 104)

ERROR: OpenAL error: Invalid Name; called from CSoundBase::SetCone (line 106)

ERROR: OpenAL error: Invalid Name; called from CSoundBase::SetGain (line 83)

crashlog.txt

crashlog.dmp

Link to comment
Share on other sites

My brother got this. It started somewhere in the middle of a game and then continued spamming. Later it crashed, but I don't know if it's related.

At a glance the crash doesn't appear to be related to sound, I would create a Trac ticket for it and attach the files, so it's not forgotten in this topic. Also if you could find and attach interestinglog.html, that could potentially be helpful.


4b817083()
mozjs185-ps-release-1.0.dll!722137d0()
[Frames below may be incorrect and/or missing, no symbols loaded for mozjs185-ps-release-1.0.dll]
mozjs185-ps-release-1.0.dll!72130601()
mozjs185-ps-release-1.0.dll!72131353()
mozjs185-ps-release-1.0.dll!721318d5()
mozjs185-ps-release-1.0.dll!7221945c()
mozjs185-ps-release-1.0.dll!72215d1c()
mozjs185-ps-release-1.0.dll!7216690a()
mozjs185-ps-release-1.0.dll!721673b3()
mozjs185-ps-release-1.0.dll!72167af1()
mozjs185-ps-release-1.0.dll!7212846a()
> pyrogenesis.exe!ScriptInterface::CallFunction_(unsigned __int64 val=0xffff00071655a360, const char * name=0x00c986ac, unsigned int argc=0x00000001, unsigned __int64 * argv=0x0044f098, unsigned __int64 & ret=0x292269dc2e8d2a78) Line 740 + 0x21 bytes C++
pyrogenesis.exe!CComponentTypeScript::HandleMessage(const CMessage & msg={...}, bool global=false) Line 59 + 0x49 bytes C++
pyrogenesis.exe!CComponentManager::BroadcastMessage(const CMessage & msg={...}) Line 829 + 0xd bytes C++
pyrogenesis.exe!CSimulation2Impl::UpdateComponents(CSimContext & simContext={...}, CFixed<int,2147483647,32,15,16,65536> turnLengthFixed={...}, const std::vector<SimulationCommand,std::allocator<SimulationCommand> > & commands=[0x00000000]()) Line 489 C++
pyrogenesis.exe!CSimulation2Impl::Update(int turnLength=0x000001f4, const std::vector<SimulationCommand,std::allocator<SimulationCommand> > & commands=[0x00000000]()) Line 358 + 0x8 bytes C++
pyrogenesis.exe!CNetTurnManager::Update(float simFrameLength=0.060138527, unsigned int maxTurns=0x00000001) Line 173 C++
pyrogenesis.exe!CGame::Update(const double deltaRealTime=0.060138527303934097, bool doInterpolate=true) Line 281 + 0x15 bytes C++
pyrogenesis.exe!Frame() Line 379 C++
pyrogenesis.exe!RunGameOrAtlas(int argc=0x00000001, const char * * argv=0x0060b400) Line 507 + 0x5 bytes C++
pyrogenesis.exe!main(int argc=0x00000001, char * * argv=0x0060b400) Line 550 + 0xf bytes C++
pyrogenesis.exe!wmain(int argc=0x00000001, wchar_t * * argv=0x0060b8b8) Line 380 + 0xb bytes C++
pyrogenesis.exe!__tmainCRTStartup() Line 583 + 0x17 bytes C
pyrogenesis.exe!CallStartupWithinTryBlock() Line 397 C++
kernel32.dll!@BaseThreadInitThunk@12() + 0x12 bytes
ntdll.dll!___RtlUserThreadStart@8() + 0x27 bytes
ntdll.dll!__RtlUserThreadStart@8() + 0x1b bytes

Link to comment
Share on other sites

Got a bunch of the same OpenAL errors Yves got (and I later got a crash, but that was probably related to me Alt+F4ing to shut it down or something. I've attached the relevant files in case someone wants to take a look at it). They might have been caused by there being a lot of sounds which should be played or something as it always seemed to happen during fights, they didn't seem to cause any actual problems though as I couldn't really tell of any trouble with the sounds, they all seemed to play (hard to tell for sure, but I mean I didn't notice any difference in what sounds did or did not play depending on whether or not the error was displayed).

interestinglog.html

crashlog.dmp

crashlog.txt

system_info.txt

Link to comment
Share on other sites

I am having sound issues in Alpha 11 with Windows XP. I have sound but there is a lot of static for all sounds, including music, to the point its really annoying to play with sound. I haven't had sound issues, beyond the known problem of music stopping with the old sound manager, with previous releases.

Link to comment
Share on other sites

I am having sound issues in Alpha 11 with Windows XP. I have sound but there is a lot of static for all sounds, including music, to the point its really annoying to play with sound. I haven't had sound issues, beyond the known problem of music stopping with the old sound manager, with previous releases.

Could you please provide your systeminfo.txt and interestinglog.html (see http://trac.wildfire...ReportingErrors for info on where to find those files). Preferably in a new topic in the bug reports forum.
Link to comment
Share on other sites

  • 3 months later...

I've found out that the profiler gets an assertion failuere because it can't find any sync markers in the soundmanager's buffer.

For testing I've put "g_Profiler2.RecordSyncMarker();" on line 149 of soundmanager.cpp which fixes the problem... but I'm not quite sure yet how often/when this should be called.

That's the description in Profiler2.h


* The RecordSyncMarker calls are necessary to correct for time drift and to
* let the buffer parser accurately detect the start of an item in the byte stream.
...
* Other threads should call g_Profiler2.RecordSyncMarker occasionally,
* especially if it's been a long time since their last call to the profiler,
* or if they've made thousands of calls since the last sync marker.

It looks like you currently don't really use the profiler for the soundmanager.

Do you think it makes sense to add some profiler macros to the code?

Link to comment
Share on other sites

I have checked the profiling a bit more closely and discovered some issues with the soundmanager's threading implementation.

The worker thread always runs at full speed and newer sleeps. This is not an issue if your CPU has enough idle cores but can theoretically have a significant impact if you only have one core or if your system decides to put the main thread and the soundmanager worker-thread on the same core.

I've done a quick and dirty implementation with semaphores, similar to the user reporter. It wakes the thread up every 10 seconds.

This implementation currently breaks fading for obvious reasons, the music on the main menu starts delayed and most likely there are a bunch of other issues I haven't discovered yet. However it's good enough for a first comparision:

current svn implementation:

post-7202-0-14396500-1356027913_thumb.jp

my modified version:

post-7202-0-43271900-1356027912_thumb.jp

As you can see it makes quite a difference.

Obviously my implementation will become a bit slower too as the bugs get fixed.

@stwf

Is it ok for you if I continue my work and ask you for review as soon as it's more stable?

If you like to fix it yourself it's fine for me, in this case I'd start checking wraitii's new AI for performance issues. I have some ideas there too. :)

Link to comment
Share on other sites

@stwf

Is it ok for you if I continue my work and ask you for review as soon as it's more stable?

If you like to fix it yourself it's fine for me, in this case I'd start checking wraitii's new AI for performance issues. I have some ideas there too. :)

Absolutely, its ok with me! I appreciate the help! I am in the middle of a large rewrite of the SoundManager code (going to be a huge improvement!) but the threading code isn't part of that. So I'm sure your changes will still be an improvement. Whenever you wat to move on send me your changes and I'll incorporate them as soon as the rewrite is done.

Thanks!

Link to comment
Share on other sites

I have checked the profiling a bit more closely and discovered some issues with the soundmanager's threading implementation.

The worker thread always runs at full speed and newer sleeps. This is not an issue if your CPU has enough idle cores but can theoretically have a significant impact if you only have one core or if your system decides to put the main thread and the soundmanager worker-thread on the same core.

Indeed, thanks for looking into that. In some cases it actually prevents the game from loading on single core systems and can lead to an error in the WMI code on Windows (presumably it times out or something, since the sound worker thread begins very early in init). If you get a fix, please commit it ASAP.

Link to comment
Share on other sites

I've created a first version that basically works. I haven't spent much time testing and cleaning it up but I'd like to know what you guys think about the approach.

The main thread pings the worker thread every 10 seconds. If something needs more updates like when fading is started, the ISoundItem object sets a flag and triggers an immediate update. The worker thread then loops until the flag is false again(fading done) and goes to sleep only if no ISoundItem is left that has the flag set. This means that the CPU-Core is up to 100% during the time of fading(if not limited due to other load), but I guess there's no way around that if we want to be independent of the main thread's performance.

I've moved the call to CSoundManager::IdleTask() from the main thread to the worker thread because apparently it can block the main thread for quite a while under some circumstances. I guess the blocking got worse because m_WorkerMutex is blocked much longer inside Run() when it isn't called that often anymore.

I've also added some profiling macros, especially at places where time could be lost while waiting for the mutex. I did not add information about how long the thread is sleeping because IMO the profiler should only show load and not idling that has no impact on performance.

The 10 seconds are currently hardcoded. In theory this time depends on how much play-time is left in the buffers that are queued in OpenAL.

I tend to say that it's not worth the effort to calculate this sleep-time on the fly and a hardcoded value should be fine.

However these 10 seconds are quite random and I didn't do any analysis to get to that value. What do you think?

I'm not sure about this code:


if (m_CurrentTune)
m_CurrentTune->EnsurePlay();
if (m_CurrentEnvirons)
m_CurrentEnvirons->EnsurePlay();

It seems to be a workaround because the playing stops sometimes.

It that's true, it could take up to 10 seconds now until the playing resumes again. However I didn't notice such problems during testing.

Here's the patch.

EDIT: Hmm the music stops playing after a while.

EDIT2: Uff I seem to have found the cause for the problem with music stopping and also why the workaround mentioned above is even needed... after searching for hours.

CStreamItem::IdleTask() should call "theData->ResetFile();" also if proc_state is AL_STOPPED (plus some other conditions).

While debugging this, I've found various other curiosities I'll have to examine more closely. I'll probably post a new version of the patch tomorrow. Now I must do something different. :fool:

soundmanager_threading_v0.4.diff

Link to comment
Share on other sites

Thanks so much Yves, I'm actually in the middle of restructuring the SoundManager stuff to fix a few problems we've run into, and also due to a deeper understanding of OpenAL than I had when this started! But I'll incorporate the ideas in this fix at least as well as any further input you can give.

I've moved the call to CSoundManager::IdleTask() from the main thread to the worker thread because apparently it can block the main thread for quite a while under some circumstances. I guess the blocking got worse because m_WorkerMutex is blocked much longer inside Run() when it isn't called that often anymore.

I've also added some profiling macros, especially at places where time could be lost while waiting for the mutex. I did not add information about how long the thread is sleeping because IMO the profiler should only show load and not idling that has no impact on performance.

The 10 seconds are currently hardcoded. In theory this time depends on how much play-time is left in the buffers that are queued in OpenAL.

I tend to say that it's not worth the effort to calculate this sleep-time on the fly and a hardcoded value should be fine.

However these 10 seconds are quite random and I didn't do any analysis to get to that value. What do you think?

The thread is also not only working during fades it is required to keep the music and ambient music queues fed. The timeout value you'll need will probably also vary widely with sound cards and/or various driver implementations. It also depends on how much memory you choose to use for your buffers. To me 10 seconds sounds like too long but I don't have any data on this. I would mind an algorithm that could start shortening if it detected issues where it wasn't getting called quickly enough.

I'm trying to be careful about what get moved in to thread. I definitely think the call to CleanupItems (where finished sounds actually get deleted) needs to happen in the main thread for safetys sake.


if (m_CurrentTune)
m_CurrentTune->EnsurePlay();
if (m_CurrentEnvirons)
m_CurrentEnvirons->EnsurePlay();

As you noted if the music or ambient sound buffers run out the sound will stop and not restart whn you re-supply the buffers. I put that code in a while ago, I don't think it is needed often but it doesn't seem to cause too much trouble, so I left it in.

Link to comment
Share on other sites

The thread is also not only working during fades it is required to keep the music and ambient music queues fed. The timeout value you'll need will probably also vary widely with sound cards and/or various driver implementations. It also depends on how much memory you choose to use for your buffers. To me 10 seconds sounds like too long but I don't have any data on this. I would mind an algorithm that could start shortening if it detected issues where it wasn't getting called quickly enough.

Yes it certainly varies how much play-time you get each time you load data into a CStreamItem, but since the speed of playing sound effects and musfic is fixed, it should be possible to know how much additional play-time you got after loading a new chunk of data.

Probably we should make the thread's sleep-time variable and calculate it each time after reading additional data. However certain events like fading and playing sounds instantly should ignore this sleep time and wake the thread up immediately.

That seems to be the cleanest solution for me.

I'll try to make a proof of concept implementation to see if it works in practice.

I'm trying to be careful about what get moved in to thread. I definitely think the call to CleanupItems (where finished sounds actually get deleted) needs to happen in the main thread for safetys sake.

What's more safe if this is in the main thread? That's the first time I'm working with threads, so I might miss something.

I've already mentioned my main argument for moving it to the worker thread above but I'll elaborate a bit. We use m_WorkerMutex to protect any data that is shared between the main thread and the worker thread. This mutex can block the main thread for quite a while under some circumstances. I think it's mainly when loading of sound data happens which involves relatively slow reads from the harddisk.

The mutex is locked for the whole duration and if the main thread triggers any code that is at least partially protected by the mutex, it has to wait until loading (of all items!) is over. The main thread is the performance bottleneck and I would move as much as possible to other threads and avoid such locking problems.

post-7202-0-30771100-1356469469_thumb.jp

I've measured delays of about 10-20ms which is already too much IMO but could probably be worse depending on processor power, harddisc-load operating system etc...

I wonder if this or that could be related to this locking problem, but unfortunately no one has been able to provide the required information yet.

It seems to be more than 10-20 ms for these people but it could as well be something completely different.

Maybe we should also use more than one mutex to lock different data and limit the locking to less lines of code. For example there's no need to protect access to m_DeadItems while we are only changing m_Items.

EDIT:

It doesn't read it from the harddisk everytime it queues new data... that was a wrong assumption.

I've now written a test-function that checks the amount of playtime that was added. It's possible to get the data but I still have to check if it is useful for what I hope.


float fSec = 0;
for(int i=0; i<num_processed; i++)
{
ALint iFreq, iBits, iBytes, iChannels;

AL_CHECK;
alGetBufferi(al_buf[i], AL_FREQUENCY, &iFreq);
AL_CHECK;
alGetBufferi(al_buf[i], AL_BITS, &iBits);
AL_CHECK;
alGetBufferi(al_buf[i], AL_CHANNELS, &iChannels);
AL_CHECK;
alGetBufferi(al_buf[i], AL_SIZE, &iBytes);
AL_CHECK;
fSec += (float)iBytes * 8 / ((float)iFreq * (float)iBits * (float)iChannels);
AL_CHECK;
}

Link to comment
Share on other sites

Hi and Merry Christmas if you celebrate! Merry times anyway if you don't!

Yes it certainly varies how much play-time you get each time you load data into a CStreamItem, but since the speed of playing sound effects and musfic is fixed, it should be possible to know how much additional play-time you got after loading a new chunk of data.

Probably we should make the thread's sleep-time variable and calculate it each time after reading additional data. However certain events like fading and playing sounds instantly should ignore this sleep time and wake the thread up immediately.

That seems to be the cleanest solution for me.

I'll try to make a proof of concept implementation to see if it works in practice.

Yes also you never really know the quality of the audio you'll be loading so memory size is not going to be easily translated into an amount of time, I think. it's better to have something that could react to conditions.

What's more safe if this is in the main thread? That's the first time I'm working with threads, so I might miss something.

lol I've worked with threads in the past and gotten only pain! But the truth is they are fine as long as you are careful.

My usual fear is things getting deleted out from under you, so while OpenAL is fine with deleting sources at any time, we're deleting the associated ISoundItem too. This could happen before the main thread even returns from the alSourcePlay command, so if you use the SoundItem to play the sound it could return into an object that was already gone. That sort of thing.

Plus as you noted I doubt its the deleting thats slowing anything down, its the file loading. THAT can probably be moved into the thread with little problem, its a good idea. To some degree the streaming should already be going on in the background. But that first seek and read maybe not.

Anyway now I'm thinking if there may be some unnecessary contention with the deleting and the threads run action, I will look into that. You're right that one shouldn't have to wait for the other.

But my experience in this project tells me there isn't a whole lot of deleting going on, the sounds played are a select few over and over and they are usually cached and played by OpenAL directly. In practice I believe the thread is mostly just handling the music and ambient sounds.

I've now written a test-function that checks the amount of playtime that was added. It's possible to get the data but I still have to check if it is useful for what I hope.

ok sounds good, although We probably could come up with a decent enough number on our own and then ut something in so that if it detected a buffer outage it would be smart enough to shorten the sleep pattern.

Link to comment
Share on other sites

Hi and Merry Christmas if you celebrate! Merry times anyway if you don't!

Thank you, I hope you had a merry Christmas too! :)

Yes also you never really know the quality of the audio you'll be loading so memory size is not going to be easily translated into an amount of time, I think. it's better to have something that could react to conditions.

I think the data is alway uncompressed in the buffer and the code I posted above should always work.

It returned about 27 seconds after calling CStreamItem::Attach for playing "Honor bound" on the main menu. I've disabled the reloading and could confirm that it stops playing exactly after 27 seconds.

My usual fear is things getting deleted out from under you, so while OpenAL is fine with deleting sources at any time, we're deleting the associated ISoundItem too. This could happen before the main thread even returns from the alSourcePlay command, so if you use the SoundItem to play the sound it could return into an object that was already gone. That sort of thing.

Indeed, we'll have to be very careful about that.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

×
×
  • Create New...