[alsa-devel] Long-standing SDL ALSA bug

[Clemens Ladisch]

Sam Lantinga wrote:
> I'm attaching the latest versions of the SDL audio files, and I'd
> really appreciate it if you take a look and sanity check our code.

> 	device = SDL_getenv("AUDIODEV");	/* Is there a standard variable name? */

ALSA's devices look at certain environment variables; you are supposed
to use names like "default" or "surround51" to get that default
configuration.

> 	if (channels == 6) device = "surround51";
> 	else if (channels == 4) device = "surround40";

The devices do not have automatic sample format/rate conversion; you
might want to use "plug:surround.." instead.

> /* This function waits until it is possible to write a full sound buffer */
> static void ALSA_WaitAudio(_THIS)

This function doesn't actually wait ...

> 	if ( status == -EAGAIN ) {
> 		SDL_Delay(1);
> 		continue;
> 	}

Not necessary in blocking mode.

> 	rate = spec->freq;
> 	status = SDL_NAME(snd_pcm_hw_params_set_rate_near)(pcm_handle, hwparams, &rate, NULL);
> 	spec->freq = rate;

The returned rate could be wildly different, but I guess SDL correctly
handles the new value in spec->freq.

> 	/* Set the buffer size, in samples */
> 	frames = spec->samples;
> 	status = SDL_NAME(snd_pcm_hw_params_set_period_size_near)(pcm_handle, hwparams, &frames, NULL);

This does _not_ set the buffer size.

> 	periods = 2;
> 	SDL_NAME(snd_pcm_hw_params_set_periods_near)(pcm_handle, hwparams, &periods, NULL);

A bigger number of periods would make the writing of audio data less
bursty.

>	status = SDL_NAME(snd_pcm_sw_params_set_start_threshold)(pcm_handle, swparams, 0);

Zero doesn't really make sense; the default value 1 would be OK.

> 	status = SDL_NAME(snd_pcm_sw_params_set_avail_min)(pcm_handle, swparams, frames);

The default value is the period size anyway, so you can remove this.
(If you change the buffer size code above to use _set_buffer_size_near,
this call would be wrong.)


Best regards,
Clemens