At Fri, 20 Feb 2009 02:22:20 +0100, Lennart Poettering wrote:
On Thu, 19.02.09 07:52, Takashi Iwai (tiwai@suse.de) wrote:
At Thu, 19 Feb 2009 03:46:11 +0100, Lennart Poettering wrote:
Heya!
As you might now PulseAudio schedules audio by system timers, not by sound card interrupts. Unfortunately there are are couple of issues with the reliability of the timing/buffering information we get from ALSA.
For example, on an ens1371 I have tracked down the following issue: I use the full buffer that the sound card provides (64K, i.e. 371ms). I sleep for about 350 ms, then fill up again, and sleep 350ms, and so and so on. Everytime I wake up I check snd_pcm_avail() and write exactly as much as this call tells me to. I also listen to POLLOUT events on the fd, however I call snd_pcm_sw_params_set_period_event(, 0) for it to make sure I actually don't get any real events on that.
After a couple of minutes of running this loop I can reliably reproduce that _avail() tells me I should fill the buffer up, I do so, query it again and it shows the buffer is filled up. I then go to sleep but immediately get POLLIN (although I shouldnt, given that I called snd_pcm_sw_params_set_period_event()), wake up again, call snd_pcm_avail() and according to it the buffer ran completely empty. The poll() took less than 1 ms time, and the last thing I did before going to sleep was checking that the buffer was full. So certainly ALSA is lying to me here... The buffer couldn't have run empty in less than 1 ms!
The same with real numbers as one example:
- ... we are woken up
- _avail() returns 15496 samples (i.e. 350ms to fill up)
- we gather and write 15496 samples
- _avail() returns 48 samples, which we consider "filled up enough" to go to sleep
- we call poll()
- after < 1ms we are woken up by a POLLIN (which we actually thought we had explicitly disabled)
- _avail() returns 16448 samples (i.e. 372ms to fill up -- larger than the actual buffer of 16384 samples!)
This smells a lot like some kind of overflow to me, given that in every case where I could reproduce this it was possible to 'fix' the issue by substracting the buffer size from the _avail() after the poll(). After that substraction the value started to make sense again. (i.e. in the example above it then becomes 64 samples, which is reasonable after sleep less than 1ms I guess).
The stop threshold is set to the boundary here btw.
If necessary I could extract a test case for this, but my hope that you guys might have an idea what goes on without a test case, given that this smells so "overflowy" ;-)
And of course, how come ALSA triggers POLLIN if I asked it not to?
Are you using *_period_event() stuff?
Yes, I am. But ignore that part for now. I have now commented the use of that call. Now I certainly get more POLLOUTs as expected, but the real problem stays: after a few minutes _avail() will suddenly jump from next to zero to more then the hwbuf size in less than 1ms without any further inteference and with a buffer size of 350ms! There is something really wrong with the behaviour of _avail().
I can reproduce this only on ens1371 for now.
The ens1371 driver itself is damn simple. The pointer callback just returns the read value. So, it implies that it's basically a hardware issue.
Does the patch below have any influence?
Unless you use dmix, this is the only place where strange POLLIN may come in. There are many new hacks there...
There is no plug in used but "hw".
Yes but *_period_event() is implemented in "hw".
thanks,
Takashi
--- diff --git a/sound/pci/ens1370.c b/sound/pci/ens1370.c index 18f4d1e..3a7739b 100644 --- a/sound/pci/ens1370.c +++ b/sound/pci/ens1370.c @@ -991,15 +991,36 @@ static int snd_ensoniq_capture_prepare(struct snd_pcm_substream *substream) return 0; }
+#define MAX_PTR_TIMEOUT 50 + +static unsigned int get_cur_ptr(struct ensoniq *ensoniq, unsigned int map, + unsigned int reg) +{ + unsigned int optr, ptr; + int t; + + outl(map, ES_REG(ensoniq, MEM_PAGE)); + inl(ES_REG(ensoniq, MEM_PAGE)); + optr = inl(reg); + for (t = 0; t < MAX_PTR_TIMEOUT; t++) { + ptr = inl(reg); + if (ptr == optr) + return ptr; + optr = ptr; + } + return ptr; +} + static snd_pcm_uframes_t snd_ensoniq_playback1_pointer(struct snd_pcm_substream *substream) { struct ensoniq *ensoniq = snd_pcm_substream_chip(substream); size_t ptr;
spin_lock(&ensoniq->reg_lock); - if (inl(ES_REG(ensoniq, CONTROL)) & ES_DAC1_EN) { - outl(ES_MEM_PAGEO(ES_PAGE_DAC), ES_REG(ensoniq, MEM_PAGE)); - ptr = ES_REG_FCURR_COUNTI(inl(ES_REG(ensoniq, DAC1_SIZE))); + if (ensoniq->ctrl & ES_DAC1_EN) { + ptr = get_cur_ptr(ensoniq, ES_MEM_PAGEO(ES_PAGE_DAC), + ES_REG(ensoniq, DAC1_SIZE)); + ptr = ES_REG_FCURR_COUNTI(ptr); ptr = bytes_to_frames(substream->runtime, ptr); } else { ptr = 0; @@ -1014,9 +1035,10 @@ static snd_pcm_uframes_t snd_ensoniq_playback2_pointer(struct snd_pcm_substream size_t ptr;
spin_lock(&ensoniq->reg_lock); - if (inl(ES_REG(ensoniq, CONTROL)) & ES_DAC2_EN) { - outl(ES_MEM_PAGEO(ES_PAGE_DAC), ES_REG(ensoniq, MEM_PAGE)); - ptr = ES_REG_FCURR_COUNTI(inl(ES_REG(ensoniq, DAC2_SIZE))); + if (ensoniq->ctrl & ES_DAC2_EN) { + ptr = get_cur_ptr(ensoniq, ES_MEM_PAGEO(ES_PAGE_DAC), + ES_REG(ensoniq, DAC2_SIZE)); + ptr = ES_REG_FCURR_COUNTI(ptr); ptr = bytes_to_frames(substream->runtime, ptr); } else { ptr = 0; @@ -1031,9 +1053,10 @@ static snd_pcm_uframes_t snd_ensoniq_capture_pointer(struct snd_pcm_substream *s size_t ptr;
spin_lock(&ensoniq->reg_lock); - if (inl(ES_REG(ensoniq, CONTROL)) & ES_ADC_EN) { - outl(ES_MEM_PAGEO(ES_PAGE_ADC), ES_REG(ensoniq, MEM_PAGE)); - ptr = ES_REG_FCURR_COUNTI(inl(ES_REG(ensoniq, ADC_SIZE))); + if (ensoniq->ctrl & ES_ADC_EN) { + ptr = get_cur_ptr(ensoniq, ES_MEM_PAGEO(ES_PAGE_ADC), + ES_REG(ensoniq, ADC_SIZE)); + ptr = ES_REG_FCURR_COUNTI(ptr); ptr = bytes_to_frames(substream->runtime, ptr); } else { ptr = 0;