GNOME Bugzilla – Bug 572953
Review remarks for alsa plugin, two serious
Last modified: 2009-04-19 09:01:26 UTC
Please describe the problem: Andrea has been trying to get current alsa plugin to work with pulse. When doing so, various timing problem has occurred. So I made a very small code review of the relevant read/write parts revealing some serious problems both in details and overall design. Steps to reproduce: See andreas' logs in http://mail.gnome.org/archives/ekiga-list/2009-February/msg00207.html. Long story short, the plugin does not behave as expected. Note also the extremely big hw buffer (100 ms) obviously required to hide the underlying problems Attaching more complete review remarks Actual results: Expected results: Does this happen every time? Other information:
Created attachment 129393 [details] Some review remarks
Created attachment 129792 [details] [review] Patch reducing the 5 periods alsa hw buffer on Linux to 2 periods, win32 to 5 periods Temporary, not tested, submitted for reference purposes
Reducing the default number of audio buffers reduces latency, but it also greatly increases the chances of broken audio on machines that have inferior hardware. Windows Vista also requires at least 10 audio buffers to avoid broken audio, whereas Windows XP on the same hardware does not.
I actually , in another life, did a small survey about the value used for other VOIP apps. At that point,my conclusions was that that value was 2-3 periods for Linux platforms. In my own case, I settled for 3 periods at 16 kHz. Question is if it's really OK if this value is hardcoded. 10 buffers of 160 frames (320 bytes) is at 8 kHz 1000 ms, quite a latency... OTOH, at 41 kHz it would be < 200 ms, a more acceptable value. Maybe buffer size and/or # buffers should depend on the codec (i. e., samplerate) used? Time permitting, I will make some stress tests on Linux with this patch.
Here is a comment from Lennart (Pulse Audio) (Any taker to write a patch?) : Yes. You shouldn't misuse the fragments logic of sound devices. It's like this: The latency is defined by the buffer size. The wakeup interval is defined by the fragment size. The buffer fill level will oscillate between 'full buffer' and 'full buffer minus 1x fragment size minus OS scheduling latency'. Setting smaller fragment sizes will increase the CPU load and decrease battery time since you force the CPU to wake up more often. OTOH it increases drop out safety, since you fill up playback buffer earlier. Choosing the fragment size is hence something which you should do balancing out your needs between power consumption and drop-out safety. With modern processors and a good OS scheduler like the Linux one setting the fragment size to anything other than half the buffer size does not make much sense. Your ALSA output is configured to set the the fragment size to the size of your codec audio frames. And that's a bad idea. Because the codec frame size has not been chosen based on power consumption or drop-out safety reasoning. It has been chosen by the codec designers based on different reasoning, such as latency. You probably configured your backend this ways because the ALSA library docs say that it is recommended to write to the sound card in multiples of the fragment size. However deducing from this that you hence should configure the fragment size to the codec frame size is wrong! The best way to implement playback these days for ALSA is to write as much as snd_pcm_avail() tells you to each time you wake up due to POLLOUT on the sound card. If that is not a multiple of your codec frame size then you need to buffer the the remainder of the decoded data yourself in system memory. The ALSA fragment size you should noirmally set as large as possible but that you have at least two fragments in your buffer size. I hope this explains a bit how frag_size/buffer_size should be chosen. If you have questions, just ask. (Oh, ALSA uses the term 'period' for what I call 'fragment' above. It's synonymous)
Created attachment 130324 [details] Updated version of problems (w linebreaks!)
Closing bug as notgnome and pushint it upstream: https://sourceforge.net/tracker/?func=detail&aid=2709434&group_id=204472&atid=989748
Hi, This bug has been reopened here: http://bugzilla.gnome.org/show_bug.cgi?id=577498 Best regards, Yannick