Make Sound / No sound option live

[bluap]

it has been updated since... now here's the thing. it works fine for me, but I have reports of crashes on other computers...

it'd be appreciated it more people can test it all out and see where bugs may live

(although I'm almost ready to just blame the OpenAL libs for not working to spec, because at least one case looks strongly like that... but I'd prefer it not crashing no matter what)

I've been experimenting with your latest version of the patch (as of 2006-05-12 23:12). I still have the lock up problems and second time I free the dev, the mutex assert crash. However if comment out lines 909 910 and 911 from sound.c (the calls to alcGetCurrentContext() alcGetContextsDevice() and alcMakeContextCurrent() in the destroy_sound() function) the patch works almost perfectly. Excessive on/off toggling works with no hangs or crashes. This tallies with my original patch - having to replace the high level alut functions with alc versions and reusing the original context made in the ini_sound() calls. I'm not sure if you need the alutExit() either, it does not appear to make a difference.

 

Above I said "almost perfectly". This is because if I grab the device outside the client I have problems. The code correctly handles the case when the dev is not available - giving a warning. If I free the dev and try again, it grabs it and sounds and music appear to work as normal. If I then turn sound and music off, the client crashes in the call to alcDestroyContext() at line 912 in destroy_sound() with the same mutex asset problem.

 

I have fixed this in my own way by modifying init_sound(). I moved the alcMakeContextCurrent() and SDL_CreateMutex() calls after the status check. If the status check failed, I set inited = 0 and return from the function - in addition to what you currently do. Its appears to work OK.

 

So, I have a version of the client that works exactly as I wanted, I think we are nearly there....

Edited May 13, 2006 by bluap

[bluap]

Any news on the status of this patch?

[ttlanhil]

I have fixed this in my own way by modifying init_sound(). I moved the alcMakeContextCurrent() and SDL_CreateMutex() calls after the status check. If the status check failed, I set inited = 0 and return from the function - in addition to what you currently do. Its appears to work OK.

this works for me, and is logical enough, does this alone make it work? or did it require the other changes as well?

[bluap]

this works for me, and is logical enough, does this alone make it work? or did it require the other changes as well?

I had to make all the changes I mentioned (including the commenting out of lines 909 910 and 911 from sound.c as described). I've been using this version since and had no other problems. Thanks again for doing this change.

Edited May 19, 2006 by bluap

[ttlanhil]

I had to make all the changes I mentioned (including the commenting out of lines 909 910 and 911 from sound.c as described).

okay, done that as well, and updated the patch.

anyone interested should try it out, because if there are no problems reported soon it'll go in

couple of notes:

there are messages logged to the console when it's turned on/off. those will be removed before it goes into cvs

the label in config window will be wrong, you check the box to enable sound, not disable it (the label comes from XML files)

there's a checkbox for sound effects, and one for music. both need to be off for it to not grab the audio device

and possibly other important things I forgot

[bluap]

okay, done that as well, and updated the patch.

anyone interested should try it out, because if there are no problems reported soon it'll go in

Great, that works really nicely thank you.

 

I have noticed a couple of details that I didn't notice before.

 

1) If music is switched off when you start, you can switch it on and it works fine. However the on state is not saved in the config file. I think this is because there is a call to turn_music_off() in start_rendering() of main.c just before saving the configuration options. I moved the config save above the switch and the option is saved ok.

 

2) If you compile with -DNEW_WEATHER there is a compiler error in init_sound() of sound.c at the end when rain_sound is cleared. May be this should be #ifndef NEW_WEATHER protected?

 

couple of notes:

there are messages logged to the console when it's turned on/off. those will be removed before it goes into cvs

the label in config window will be wrong, you check the box to enable sound, not disable it (the label comes from XML files)

there's a checkbox for sound effects, and one for music. both need to be off for it to not grab the audio device

and possibly other important things I forgot

 

In my config window, the options look to be labelled correctly - both say enabled.

 

Once again, thanks.

[ttlanhil]

1) If music is switched off when you start, you can switch it on and it works fine. However the on state is not saved in the config file. I think this is because there is a call to turn_music_off() in start_rendering() of main.c just before saving the configuration options. I moved the config save above the switch and the option is saved ok.

d'oh. right.

2) If you compile with -DNEW_WEATHER there is a compiler error in init_sound() of sound.c at the end when rain_sound is cleared. May be this should be #ifndef NEW_WEATHER protected?

true... though other parts of NEW_WEATHER would also be problematic. i haven't worried about it too much since the NEW_WEATHER option has been getting worse and worse, so it's not used a lot. compiler errors shall be dealt with though

In my config window, the options look to be labelled correctly - both say enabled.

uhh. if so, it might not be finding options.xml

[ttlanhil]

okay, since there were no other issues reported, it's now committed.

I've tested it with NEW_WEATHER, and haven't had any problems (it seems to work fine), but that has been tested less than non NEW_WEATHER

note that shutting down/setting up OpenAL may take a second or two, so toggling may cause a slight delay

options.xml has been updated in both CVS and in languages rep (english only, I can't help with the rest ), so it should work properly for people using them

[bluap]

okay, since there were no other issues reported, it's now committed.

I have a nice clean cvs build and it all looks great. Thank you again for working on this.

[d000hg]

What's going on in turn_sound_off()?

 

Whether it loops through all playing sounds is dependant on whether music is enabled - if music is enabled then destroy_sound() gets called, if music is disabled then it simply loops through playing sounds manually.

 

What's the deal with this - destroy_sound does a lot more than just stop playing sounds? I ask 'cos I'm trying to merge your changes into my sound re-write. Has it been tested both with NO_MUSIC #defined and not #defined?

[ttlanhil]

the idea is to free the audio device if audio is no longer in use... which doesn't matter to windows users, but it does to some other people.

so if music is disabled (NO_MUSIC) or turned off (!music_on) when you turn sound off it'll pull the plug on the sound system. that means simply cleaning everything up in one hit

if music is enabled (maybe not playing, but turned on), then it needs to stop all sounds except the music thread

similar when music is turned off... if sound is also off, shut down the audio system. if not, only stop the music.

 

my debug builds have NO_MUSIC defined (lib issues) and my release build doesn't... both were tried with these changes, and as far as I know there are no issues (except that if you release sounds and don't restart them after the sound is reenabled they don't come back. except for rain, since that's done especially in the change. this isn't much of an issue now, but might be in the future (eg your changes))

[d000hg]

That does make sense. I did notice the special rain code, that old weather code is NOT GOOD from an OpenAL point of view, I wish we could permanently switch to NEW_WEATHER! Maybe I'll fix it somehow once the other things are done (like defining a NEW_SOUND option).

[Roja]

I was using the latest CVS, had the music on, turned it off in the client, then tried to turn it back on but it didn't go on.

 

I also started the client with the music off, turned the music on(it worked), turned it off, then back on and it did not come on.

[0ctane]

While sound seems to be working for me now (more testing needed) I am getting an error when turning sound off:

Error initializing sound: Invalid Enum Value

I find this a little odd since it happens when turning sound off but no errors posted when turning it on. I know OS X has some OpenAL issues, but is anyone else seeing this error?

 

BTW, I am using NO_MUSIC flag right now if it makes a difference.

 

[edit 2] I get "Too Many Sounds" at portland storage and portland mayor's house still. Can we get that fixed?

Edited May 28, 2006 by 0ctane

[ttlanhil]

I was using the latest CVS, had the music on, turned it off in the client, then tried to turn it back on but it didn't go on.

 

I also started the client with the music off, turned the music on(it worked), turned it off, then back on and it did not come on.

so turning music on works exactly once... is this with sound turned on the whole time, turned off the whole time, or being toggled as well?

where was it that this was happening? (doubt it's the problem, but I think I mostly only tested on IP)

anything sound or file related in error_log.txt?

I've played with the toggles a fair bit, so I suspect it might be the compiler/libraries or the location/playlists...

once I get my computer back from repairs I'll see if I can find what the problem may be

I am getting an error when turning sound off:

Error initializing sound: Invalid Enum Value

I find this a little odd since it happens when turning sound off but no errors posted when turning it on.

makes a certain amount of sence. I suspect it might be during the device closure... when the sound context is changed so that the sound device is no longer used.

in destroy_sound():

alcDestroyContext( mSoundContext );
if(mSoundDevice){
alcCloseDevice( mSoundDevice );
}
alutExit();

without more information, that's about all I can offer though

BTW, I am using NO_MUSIC flag right now if it makes a difference.

only in that it means music is always turned off, so toggling sound will always init or destroy the sound setup

[edit 2] I get "Too Many Sounds" at portland storage and portland mayor's house still. Can we get that fixed?

I'm not certain, but I strongly suspect that the sounds are added by the server... in which case only a server coder or mapmaker can fix it

to be on the safe side, though, I may add some logging to check where the last sound object was added from when that error comes up(currently that'd be server, weather, or particle)

Edited May 28, 2006 by ttlanhil

[Roja]

interesting

 

Ok, If I have the sound effects enabled, what I explained in my post above happens(reproduced it several times).

 

If I have sound effects turned off, I can turn the music on/off as many times as I want. It works.

 

If I have sound effects turned on, then turn the music off, then back on-it doesn't go back on BUT..if i then turn off the sound effects, and turn the music on then the music does go on! I can then turn on the sound effects again. (I can also the soundFX back on before I turn the music back on and it works too)

(note I tested this while it was raining so i heard soundFX & music at the same time).

[0ctane]

Well, for a VERY long time things were working great. I managed to have sound effects and music running (after a self compile of OpenAL via SubVersion). So, kudos to all of you.

 

Eventually music died when in Palon Vertas. Got "stream_music error: Invalid Operation Last message repeated 2 times" in my error log. Now I need to wait for some rain to determine if it killed sound effects too.

 

Looks like line 632 in sound.c posted the error. The error check then kills music, but it looks like sound effects should be fine. "I COMMAND RAIN!!!!" *0ctane twiddles thumbs and wonders if there is a weather forecast channel*

 

[edit] reproduced the error, but sound effects still work. =) I think the problem might be from trying to stop and start the same music score. The exit from Sedoculis to PV plays the same music as in PV. I noticed some gittering right before music stopped and the error appeared. I need to change the *.pll file to be sure.

 

[edit] problem is the same for both a different *.ogg and when I try the Kusamura exit from PV. Must be some oddity with PV map.

Edited May 29, 2006 by 0ctane

[ttlanhil]

Ok, If I have the sound effects enabled, what I explained in my post above happens(reproduced it several times).

 

If I have sound effects turned off, I can turn the music on/off as many times as I want. It works.

okay, so something about music isn't getting reset when only music is turned off, it takes de/re-initialisation of the sound system to fix it. that should narrow down the options a lot when I can get back to coding (grr mutter mutter... tech has a cold so my comp still isn't back...)

I managed to have sound effects and music running (after a self compile of OpenAL via SubVersion).

yay!

can you reproduce the problem in other maps, especially C1 (where the music setup would have been more thoroughly tested)?

apart from restarting the same track (which, theoretically, should either result in the music being flushed, closed, and reopened, or no change at all... but may not) causing issues, I dunno what else it could be.

I wouldn't expect the sound to die as well, the music system will shut itself down on errors (a typo in a single playlist can end all music... which is where the music toggle comes in handy)

 

oh, and with -DDEBUG you can trigger rain and thunder manually from memory, ctrl+shift+t for thunder, and ctrl+shift+home for start/stop rain (it starts a counter, though, so you'll have to wait 2-3 sec from the keystroke)

I may not remember the keys right, so if they don't work try without shift, with alt, etc

[0ctane]

I managed to have sound effects and music running (after a self compile of OpenAL via SubVersion).

yay!

can you reproduce the problem in other maps, especially C1 (where the music setup would have been more thoroughly tested)?

...

oh, and with -DDEBUG you can trigger rain and thunder manually from memory, ctrl+shift+t for thunder, and ctrl+shift+home for start/stop rain (it starts a counter, though, so you'll have to wait 2-3 sec from the keystroke)

yay indeed! Some OS X users have been wondering where the sound and music were. Well, thanks for the DEBUG suggestion. The key combinations are correct, and this will save a lot of time for me. However, the banners are throwing a bunch of errors which is making reading the error_log a pain.

 

I reproduced the music problem on C1 by taking the boat from WS to DP. It looks like the docks at both these places have different music. Also DP->PL by land, but not PL->DP by land. WS<->PL has no problems (I have made this trip many times). Does anyone else have their music cutout in these situations? If not, then I guess it is an OS X issue (but not endian since I use a i386 Mac).

[edit] Happens PL->DP by land sometimes too. Also noticed, in PL there is a music free zone around the sapphire cave, which might be why WS<->PL does not have an issue. Anyhow....

[edit2] Yup, I edited the map6nf.pll to put a gap at the North DP exit. Music did not fail. Need to learn OpenAL now....

 

This is getting a bit off topic though. If no one else has an issue, I will have to look into it on my own.

 

If I have sound effects turned on, then turn the music off, then back on-it doesn't go back on

I do not have this problem. Tested it while raining and dry. There is a delay and skip before music is turned off, but it does turn off fine. Music turns on fine too and did not effect sound effects. I did not test all your other combinations though.

Edited May 29, 2006 by 0ctane

[bluap]

Did I really offer to do this patch myself

Anyhow, I've nothing further to offer apart from my moral support. The changes appear to be working fine on Linux now. I certainly haven't seen problems switching on/off in any combination with the latest code. I'll keep watching the tread and help if I can...