New Sound Manager svn patch

[historic_bruno]

I wonder if these threading issues could also be responsible for #1763.

[Yves]

I wonder if these threading issues could also be responsible for #1763.

None of the soundmanagers' threading problems I discovered yet should cause problems with the UI.

I would not say that it's impossible but I don't see any signs for that, beside the fact that music also stopped playing.

EDIT: Hmm maybe if the mutex stays locked but I guess that would cause the whole main thread to stall.

[historic_bruno]

It could be triggering race conditions elsewhere, I suspect #1729 was one of them.

[stwf]

It could be triggering race conditions elsewhere, I suspect #1729 was one of them.

It seems that bug was fixed with no actual sound manager involvement, so I'm not sure why you say that. Also I didn't say there was a race condition just that the main thread might occasionally wait for the lock to delete items when it should just move on and do it some other time.

[Yves]

I have a new version.

It calculates the thread's wakeup-interval based on how much time could be loaded into the buffer and makes sure that there's always enough data for the music and environment sounds. Additionally, I'm now using more than one mutex to avoid the unnecessary locking that causes performance issues.

It's still not cleaned up, should probably be reorganized a bit and contains debugging code.

@stwf

How should we proceed now?

This implementation is obviously not ready to commit yet and there's some more work required to get it to that point. If you have a lot of changes too, it probably doesn't even make sense to make it ready to commit, because you'd have to change it again to match your changes.

How much do your changes overlap/conflict with my changes and when do you think you'll have the new version ready to commit? I have a lot of time this week and next week but then my vacation is over.

If our changes affect too much of the same code, one needs to take the lead and merge it. It's fine for me either way.

Another thing I noticed today. Did you know that there's only one piece of music looped over and over again even if more than one is specified in the [civ].json file? Also as far as I can tell there's just one ambient sound used (day_temperate_gen_03.ogg).

A callback function that allows scripts to set the next piece to be played would be a good solution IMO.

It would trigger when the end of the first peice is detected and will come soon (ogg->atFileEOF()), but not instantly (there's still some data queued in the buffers).

soundmanager_threading_v0.5.diff

[stwf]

@stwf

How should we proceed now?

This implementation is obviously not ready to commit yet and there's some more work required to get it to that point. If you have a lot of changes too, it probably doesn't even make sense to make it ready to commit, because you'd have to change it again to match your changes.

How much do your changes overlap/conflict with my changes and when do you think you'll have the new version ready to commit? I have a lot of time this week and next week but then my vacation is over.

If our changes affect too much of the same code, one needs to take the lead and merge it. It's fine for me either way.

Awesome work, thanks! I do have extensive changes, so I don't think it makes sense to put any more work into the cleanup. I can do that on top of my changes. When is a good question, but this weekend is a good bet. I want to make sure its behaving right even in low resource environments. But I do think these changes will go into the code very cleanly. The areas you hit aren't very much changed in the new code.

Did you mention you wanted to look into some other areas? That might be a good idea to do for the next few days if you wanted.

Another thing I noticed today. Did you know that there's only one piece of music looped over and over again even if more than one is specified in the [civ].json file? Also as far as I can tell there's just one ambient sound used (day_temperate_gen_03.ogg).

A callback function that allows scripts to set the next piece to be played would be a good solution IMO.

It would trigger when the end of the first peice is detected and will come soon (ogg->atFileEOF()), but not instantly (there's still some data queued in the buffers).

The music is handled from the simulation (javascript), so I just implemented the system as it was in the code previously. The javascript takes the first song and plays it looping. There has been a lot of talk about taking over that responsibility from the js code but its been hard to pin down a consensus. Hopefully its something the Design Committee can clear up.

[Yves]

Awesome work, thanks! I do have extensive changes, so I don't think it makes sense to put any more work into the cleanup. I can do that on top of my changes. When is a good question, but this weekend is a good bet. I want to make sure its behaving right even in low resource environments. But I do think these changes will go into the code very cleanly. The areas you hit aren't very much changed in the new code.

Did you mention you wanted to look into some other areas? That might be a good idea to do for the next few days if you wanted.

Sounds perfect.

I'll check the AI code.

The music is handled from the simulation (javascript), so I just implemented the system as it was in the code previously. The javascript takes the first song and plays it looping. There has been a lot of talk about taking over that responsibility from the js code but its been hard to pin down a consensus. Hopefully its something the Design Committee can clear up.

I think we all agree that it doesn't make sense to only loop one piece of music during a game if we have several others available.

We don't necessarily have to move everything to the engine and I think we shouldn't. We could just make some new functions available for javascript and change the scripts to use them. My understanding of the "Design Committee" is that it's more focused on changes that affect the gameplay signifficantly or result in a lot of additional work. I don't think either of them applies here.

[Mythos_Ruler]

Right, the Design Committee would decide if we want battle music or how complicated we want the music to be, rather than deciding whether to do something in Javascript or not.

[historic_bruno]

It seems that bug was fixed with no actual sound manager involvement, so I'm not sure why you say that.

It wasn't a bug in the sound manager, but the new threaded design calls the timer code much more frequently, which exposed the actual bug.

I'll try the proposed patch on my laptop and see if the game starts and hopefully sound will still play smoothly.

[historic_bruno]

The good news is with the patch, the game loads again on my laptop, the bad news is music stops playing even while loading the maps list and doesn't continue (back where we started on that). There has to be a sensible solution as I've never played another game with music issues like that.

[Yves]

the bad news is music stops playing even while loading the maps list and doesn't continue (back where we started on that).

Thank you for testing!

I've removed the workaround to start music again when it's stopped, which explains why it doesn't start again.

It doesn't explain why it even stopped (never happened on my computer with the latest patch).

Does it seem to be performance related? Please post mainlog.html.

At the moment it measures how much time it can load into the buffer at once and then triggers reloading at intervals of half of that time. In theory if it can load 30 seconds into the buffer it should have about 15 seconds to load additional 15 seconds and to avoid a buffer underrun.

I thougt that's enough even on systems with poor performance. Loading the buffers takes about 100msec on my system and there might be some msec left to complete the last frame (which shouldn't be more than 1 second even on very very slow systems).

Maybe it can load less time into the buffer at once on your system for some reason, but that's something we should see in the log if you didn't remove the LOGERROR calls.

EDIT: There's also some additional time we loose because a buffer hasn't completely finished playing.

However that shouldn't be more than a few hundred milliseconds (30sec divided by 50 buffers).

Maybe it would also help if you could check how long the soundmanager loop takes using the profiler. It should run at the interval you see in mainlog.html (~14 seconds on my system).

[historic_bruno]

Hmm, so as long as the main thread updates at least once during the period of the buffer there should be no problem?

So far the only case I can reproduce the sound stoppage is loading the map XML for the first time (uncached) which takes about 25-30 seconds on my system. Using a stopwatch it looks like about 28 seconds after music starts playing, it stops during the map loading, and the log would seem to explain that:

ERROR: Loaded data of audio/music/Honor_Bound.ogg for 27.863958 seconds
ERROR: Loaded data of audio/music/Honor_Bound.ogg for 0.557279 seconds

Both of these lines occurring immediately at startup, the next one occurs after the map list is loaded, by that time music is already broken.

This is probably a worst case scenario because releases have map data already cached and most users will have faster computers. But it does make me think we need to optimize the map XML loading because the number of maps will only grow over time and users might have hundreds of their own maps. We could use a thread or progressive loading for that instead of expecting it all to load instantly in the main thread.

I'll look for other cases of music stopping, I remember it happened sometimes in game as well.

[Yves]

It really looks like the main function wasn't able to call "g_SoundManager->Update(false);" for at least 15 seconds.

We could use a thread or progressive loading for that instead of expecting it all to load instantly in the main thread.

Yes I think we should do that.

[stwf]

Thanks for all of the good work on this. These are the reasons threading can be so tough, especially on low resource machines. By sleeping a thread for 15 seconds you aren't guaranteed of a call in 15 seconds, especially when the main thread is waiting on something time consuming like a large file read. In fact the system may mistake

such a large sleep value for licence to give this thread a very low priority.

Also possibly a solution is to use a smaller sleep value and load fewer buffers.

That being said I don't think that music stoppage during startup is the end of the world as long as the code to resume them is put back. I'm not even sure you would want the sound manager to get time in the case where resources were so tight that somebody has to be kept waiting...

My rewrite stuff is taking longer than I thought, I still can't get through the huge combat demo without errors, so if this gets settled down then maybe we should commit it. Especially if you want to make an interim release. I can always put my changes on top of it when the time comes.

[historic_bruno]

That being said I don't think that music stoppage during startup is the end of the world as long as the code to resume them is put back.

Not only music, but the UI stops responding as well since the main(rendering) thread is busy, so it's really bad behavior. I think we need both solutions, the sound manager should know if a buffer runs out of data unexpectedly and continue where it left off, and we should make sure the main thread is never blocked for such a long time.

[luziferius]

idk, if it works good, but it might be worth a try

you could use Virtualbox to create a low resource machine:

install Virtualbox and a VM with an SVN build of 0ad (+Guest Additions for OpenGL-passthrough; idk whether 0ad runs with that, haven’t tested it)

in the VM-Settings under System->Processor you can set the number of cpus the vm can use + a cpu limiter

the limiter is important. you can reduce the cpu-power the VM gets to eg. 20% of your physical cpu-power.

(eg you have a 3.6GHz cpu. you can test the game on a 1GHz cpu by setting the limiter to 28%)

[Yves]

Does anyone know how much overhead there is to wake up a thread? Unfortunately I have no Idea about that and can't tell if it's even worth waiting 10 seconds instead of 1 or 0.1.

My impression was that it should probably be a few seconds, but I don't really know it.

Restarting the music if we had a buffer underrun is no problem. We can simply check if the source is still playing after loading additional buffers (we did that before I removed it in the patch).

[quantumstate]

The overhead to wake up a thread shouldn't be very large. 100 microseconds on old hardware perhaps (vague conservative estimate). Recent CPUs should do it in about 5-10 microseconds. Waking every 1 second seems reasonable, why would you want it switching more often?

[Yves]

No I don't think more than once per second makes sense. I was just talking about intervals of 0.1 seconds to make clear what I mean.

If it's not significantly more than 100 µs it shouldn't be a problem. I think we can remove the code to calculate the play-time entirely and set the interval to 1 sec.

[stwf]

No I don't think more than once per second makes sense. I was just talking about intervals of 0.1 seconds to make clear what I mean.

If it's not significantly more than 100 µs it shouldn't be a problem. I think we can remove the code to calculate the play-time entirely and set the interval to 1 sec.

Yes, I agree for two reasons, thread switching SHOULD be extremely fast I mean what would be the point otherwise lol, and even 1 second is an eternity in processor time...

Also the thread will be doing more than that in my new update, it needs to check to see if non repeating sources are done playing, and if so return them to the pool of sources, so calling it infrequently could cause issues with action sound playback...

[stwf]

OK, I checked in a simplified version of the work done by Yves.

This uses a second mutex for items that are marked for deletion. It also uses SDL_Delay to sleep the thread. The delay is 1 second usually but falls to 0.1 if some music fading is going on. Also in this case if the music stops it should be restarted again.

Please let me know if this still causes problems on the single core machines. We could probably bump up the non-fade sleep time to 2 seconds with little issue if it is still getting in the way.

Thanks for all of the help on this!

[historic_bruno]

That seems to fix the problem with the game not starting. I did the same test of initial match setup loading on an old single core laptop. The music sounds very smooth to me with no stoppages. The tradeoff seems to be the setup screen takes longer to load and the game runs slower, but it does run and with smooth music

[stwf]

Cool, thanks for testing... If you want to experiment with values you can add more sleep time to the equation by changing line 157 of SoundManager.cpp. I have set this as high as 7000 without seeing any gaps in music, but I think trying it on a single core machine may be necessary to see if that is too long a timeout since we have to be prepared to wait longer than 7000 ms on a constrained machine.

[luziferius]

ok, I tested it on my machine using my limiter script:

I can provide tests with the following values:

CPU-number: 1 to 6

clock speeds(both maximum and minimum; in kHz): one of 2800000 2200000 1500000 800000

I just tested:

build: latest git master as of now

map: combat Demo(Huge)

singlecore 1,5GHz: no stalls at all(!), you can clearly hear the sim update: music plays smooth, every 3-4 seconds you hear a bunch of battle sounds from a sim update

dualcore 800MHz: no stalls, similar to singlecore 1.5GHz

this is below the mimimal system requirements:

singlecore 800MHz: some stalls(only directly after sim update;the stalls are infrequent and short), sim updates very infrequently(+10seconds/sim update in mid-battle)

Edited January 14, 2013 by luziferius

[stwf]

Thanks for the research! To me thats sounds pretty good. In the worst case the music stalls but comes back. Which is probably what you would want in a sense, since game play should get the nod over music if necessary and possible.