[quake2] Choppy sound with default (non-SDL) client

[Ozkan Sezer]

On 4/28/13, Alexey Dokuchaev <danfe at nsu.ru> wrote:
> On Sat, Apr 27, 2013 at 01:24:18PM +0300, Ozkan Sezer wrote:
>> On 4/27/13, Alexey Dokuchaev <danfe at nsu.ru> wrote:

>> BTW, a maintained and updated version of icculus-quake2 is at:
>> http://www.yamagi.org/quake2/
>
> Cool, I'll add it to FreeBSD ports collection.  I can instate you as
> maintainer of it if you wish. :)

Nope, I can't do it right now (I am more of a Linux guy. However Yamagi
himself is a FreeBSD guy, he might be interested in.)

>> > I've heard several reports over quite some time that default (non-SDL)
>> > client executable gives choppy sound on FreeBSD (that is, OSS).
>>
>> It is because the SNDCTL_DSP_GETOSPACE ioctl is issued before setting
>> other important parameters: if it is move to the place just below the
>> mmaping it is fixed. See the patch I just cooked up (which contains a
>> few additional bits)
>
> I confirm that moving SNDCTL_DSP_GETOSPACE ioctl() call lower does fix the
> problem for me here on FreeBSD 8.3, thanks!
>

My pleasure,

> May I ask a few questions on the remaining bits of the patch?
>
>> @@ -76,7 +74,7 @@
>>  	{
>>  		seteuid(saved_euid);
>>
>> -		audio_fd = open(snddevice->string, O_RDWR);
>> +		audio_fd = open(snddevice->string, O_RDWR|O_NONBLOCK);
>
> What happens when opening file in blocking mode?  I did not observe any
> problems...  Although I've seen this change in some other quake engine
> source code.
>

It has been a long time since I added that flag: it has been in my
uHexen2 for more than 7-8 years, and quakeforge does the same too.
I vaguely remember having some issues without it back at the time and
adding it helped, however it didn't change anything on my modern Linux
setup when testing the patch I sent you.

>> -		if (fmt & AFMT_S16_LE) dma.samplebits = 16;
>> +		if (fmt & AFMT_S16_NE) dma.samplebits = 16;
>>  		else if (fmt & AFMT_U8) dma.samplebits = 8;
>>  	}
>>
>>  	if (dma.samplebits == 16)
>>  	{
>> -        	rc = AFMT_S16_LE;
>> +        	rc = AFMT_S16_NE;
>
> Do I understand correctly that using AFMT_S16_LE would break on big-endian
> machines?  I.e., AFMT_S16_NE makes code CPU (endian) agnostic, ergo
> correct?
>

Exactly.  The sound file loaders in the engine already converts the
data to host-endian, therefore the device must be open in host-endian
mode, too.

>> +	mmaplen = info.fragstotal * info.fragsize;
>> +	sz = sysconf (_SC_PAGESIZE);
>> +	mmaplen += sz - 1;
>> +	mmaplen &= ~(sz - 1);
>
> What benefits forcing page size aligned mmaplen brings us here?
>

Depends on the arch you are running on. I remember inheriting that one
from quakeforge to fix some weirdness (or was it a crash?) on a foreign
arch, but it was a very long time ago..

>> +		munmap (dma.buffer, mmaplen);
>> +		dma.buffer=NULL;
>
> Fixing resource leaks are clear.

Yup,

>
> ./danfe

--
O.S.