Fixed [2.1.17] [Linux 64-bit] Crash when playing certain sounds in "doom format"

[ForgiveThisNewb]

Problem: When playing certain sounds in a certain format, SRB2 on 64-bit Linux will experience a partial crash. The same sound in a different format will not produce a crash.

Test Case: SUGOI's final boss.

Replication Instructions:
- Launch from terminal: ./lsdlsrb2.debug -file sugoi-v1.wad -warp 28
- In the in-game console: devmode 1; teleport -x -200 -y -100 -z 2400
- Wait a couple of seconds.

Results:
- The pre-fight taunt may or may not play.
- The system terminal will spit out an error such as *** Error in `lsdl2srb2.debug': corrupted double-linked list: 0x0000000004452e40 ***
- The game will refuse to process anything but sound and will not exit without a sigkill.
- Full Backtrace, Disassembly, and Register values: http://pastebin.com/qC8HydNv

Expected Results:
- The pre-fight taunt plays, and the fight ensues.

My OS: Ubuntu 14.04 64-bit
SRB2 Version: 2.1.16 [git revision e62c0794dce5df2c73374edcc73d8575e1d10a26]
Fails on 64-bit compilations, appears to work in 32-bit.

Additional Information:
- The two sounds triggering this crash in the test case are PHENT1 and PHENT2. SLADE says these are Doom Format sounds. Specifically, raw 8-bit mono 48kHz.
- Replacing the sounds with Ogg Vorbis encodings prevents the crash.
- Replacing the sounds with Wav files generated from the raw audio prevents the crash.

End-user workarounds:
- Use a 32-bit binary
- Reencode problematic sound files.
- Toggle SFX in the options menu as you approach the boss, and turn it back on only after the health meter appears.

 

[AlamGBC]

Can you recompile with valgrind support (VALGRIND=1), to see where it happens?

My best guess is that code that resample the sfx to the device's sample rate is not alloc the right amount of memory, that the size of the sfx lump does not match the sfx header's sample number (aka size)

Sent from my Nexus 7 using ********

 

[ForgiveThisNewb]

Resampling? Oh boy, it's like it's 2.1's release all over again.

==5283== More than 1000 different errors detected.  I'm not reporting any more.

Thanks Valgrind. Wouldn't even let the game finish starting before it threw in the towel.
With the error limit disabled, the log is so huge and filled with garbage I can't even pastebin it. So here, have a log made with the standard debug executable that still manages to capture the error. http://pastebin.com/yTRUgczc

==5321== Invalid write of size 2
==5321== at 0x519326: ds2chunk (mixer_sound.c:229)

Okay, let's take a look at ds2chunk around that line

	default: // convert arbitrary hz to 44100.
		step = 0;
		frac = ((UINT32)freq << FRACBITS) / 44100;
		while (i < samples)
		{
			o = (INT16)(*s+0x80)<<8; // changed signedness and shift up to 16 bits
			while (step < FRACUNIT) // this is as fast as I can make it.
			{
				*d++ = o; // left channel
				*d++ = o; // right channel
				step += frac;
			}
			do {
				i++; s++;
				step -= FRACUNIT;
			} while (step >= FRACUNIT);
		}
		break;

I think I can see the problem at hand. frac is an integer, and there's division in its assignment line. This results in truncation, which results in the game trying to play non-existent parts of the sound file. Frankly I'm surprised this doesn't crash more often. Easiest solution is to add 1 to the end of the frac assignment line, but it may not be perfectly ideal. At the very least, it prevents this crash.

 

[Monster Iestyn]

Your fix was merged to master just earlier, just so you know. I suppose we can call it fixed now? *shrugs*