[alsa-devel] Long-standing SDL ALSA bug
Hey guys, we're in the process of getting ready to release SDL 1.2.14, and we're trying to fix a long standing issue with crackling and pops in ALSA with some configurations: http://bugzilla.libsdl.org/show_bug.cgi?id=650
What's really puzzling is that we're not able to reproduce it ourselves at all.
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. You can get the full pre-release here: http://www.libsdl.org/tmp/SDL-1.2.zip
Thanks!
12.10.2009 22:14, Sam Lantinga ?????:
Hey guys, we're in the process of getting ready to release SDL 1.2.14, and we're trying to fix a long standing issue with crackling and pops in ALSA with some configurations: http://bugzilla.libsdl.org/show_bug.cgi?id=650
What's really puzzling is that we're not able to reproduce it ourselves at all.
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. You can get the full pre-release here: http://www.libsdl.org/tmp/SDL-1.2.zip
Thanks!
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
I may be wrong but I have a feeling that the problem is in the pulseaudio actually not in alsa. There's a thing called alsa-plugins-pulseaudio which creates virtual alsa device. This device sends all sound output to pulseaudio. Many modern distributions like to use this device as default. And pulseaudio still has some crackling issues, yes.
On 13/10/09 14:36, The Source wrote:
12.10.2009 22:14, Sam Lantinga ?????:
I may be wrong but I have a feeling that the problem is in the pulseaudio actually not in alsa.
I'd take a look, but libsdl.org seems down atm.
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
Thanks for the feedback! I think that improved our ALSA driver significantly (attached!)
On Tue, Oct 13, 2009 at 12:03 AM, Clemens Ladisch clemens@ladisch.de wrote:
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
participants (4)
-
Clemens Ladisch
-
Peter Lawler
-
Sam Lantinga
-
The Source