Hi.
Takashi Iwai wrote:
I don't think it's a race. It appears to be just the matter of configuration mismatch (or misconception) to me.
OK then. The race will have to wait till I get to it hard and (in case there really is) produce a patch.
Possibly. Changing avail_min isn't a good idea, I believe, too.
Well, what I was trying to say is that there are a few serious bugs that were hidden by the hack... Never mind, I'll post the patches when the time will come. One step at a time.
size. Thus only for apps that fills arbitrary amount of data via snd_pcm_writei() triggers this hack.
I don't think so.
It is.
Firstly, it seems all or most apps write the arbitrary amounts.
mpg123: --- #7 0x00002aaaaad34afa in snd_pcm_mmap_writei (pcm=0x565dd0, buffer=0x5660b0, size=4096) at pcm_mmap.c:186 186 return snd_pcm_write_areas(pcm, areas, 0, size, (gdb) p pcm->period_size $1 = 5512 --- writes 4096, while period_size is 5512.
ogg123: --- #9 0x00002aaab2c48a69 in snd_pcm_writei (pcm=0x673430, buffer=0x2aaab4215108, size=11340) at pcm.c:1186 1186 return _snd_pcm_writei(pcm, buffer, size); (gdb) p pcm->period_size $2 = 2756 --- writes 11340 with period_size 2756.
But I don't agree with you even if some app does not. It will still be affected. Let me demonstrate. I added the following: --- +if (size > pcm->period_size && size % pcm->period_size) +size -= size % pcm->period_size; --- to snd_pcm_writei() and snd_pcm_mmap_writei() to simulate the condition when an app transfers by periods. Here we go: --- #5 0x00002aaab2c5dbc1 in snd_pcm_mmap_writei (pcm=0x672e30, buffer=0x2aaab4215108, size=11024) at pcm_mmap.c:188 188 return snd_pcm_write_areas(pcm, areas, 0, size, (gdb) p pcm->period_size $1 = 2756 (gdb) p pcm->period_size*4 $2 = 11024 (gdb) p pcm->period_size*4==size $3 = 1 --- OK, it is trying to write 4 periods.
--- Breakpoint 2, snd_pcm_rate_poll_descriptors (pcm=0x672e30, pfds=0x409ffdd0, space=1) at pcm_rate.c:720 720 snd_pcm_rate_t *rate = pcm->private_data; (gdb) n 724 ret = snd_pcm_generic_poll_descriptors(pcm, pfds, space); (gdb) n 725 if (ret < 0) (gdb) n 728 avail_min = rate->appl_ptr % pcm->period_size; (gdb) n 729 if (avail_min > 0) { (gdb) n 730 recalc(pcm, &avail_min); (gdb) p avail_min $4 = 4 --- See a small reminder? Why you ask? Here: (the ogg has the rate 22050, the driver has 48000) --- (gdb) p rate->gen.slave->period_size $10 = 6000 (gdb) p 6000/2756.0 $11 = 2.1770682148040637 (gdb) p 48000/22050.0 $12 = 2.1768707482993195 --- See a slight difference? I think this is exactly why _any_ app is affected, not only those that write arbitrary amounts (even though they are a majority, it seems to me). So let me challenge you on that. :)
The app must not think that the whole fragment can be written at a single call. That's the bug of the app!
Hmm, I don't think it does, but I don't understand what you mean here. So you say the app should write an arbitrary amounts? Well, then the above examples of mine makes no sense, but I don't see how it helps.
The problem is that two periods with the current rate plugin cannot work. Suppose the case of client period size 940, slave period size
Thanks for the example, I now understand what fundamental problem you have in mind.
So, without another wake-up source, the problem cannot be solved.
OK, it appears we are talking about two completely different problems. Yes, I now see the fundamental one and I admit my patch doesn't solve it. Neither it was intended to.
So what we have: 1. A fundamental problem with the different fragment sizes. This may (or may not, if the HW period is adjustable) give you underruns. 2. A bug in snd_pcm_rate_poll_descriptors(), which prevents an app from filling the second fragment before the first one is played. This _does_ give an underruns, constantly, unavoidably and badly. I have a patch, it needs to be discussed. 3. A bug in snd_pcm_write_areas() I mentioned earlier. I have a patch, will post it when the time will come. 4. A (possible) race in snd_pcm_playback_poll() which is harmless most of the times, but I can get underruns out of it by rapidly switching consoles. 5. Some other bug related to that problem, which makes me a headache, but is still to be located.
And you basically say: "if we have 1, then we are not interested in fixing 2,3 and investigating 4,5". That point of view may exist, but given that 2 gives some 90% of underruns with 1 being only theoretically harmfull, I am not satisfied. :)
Sorry for the lengthy posting, I tried to strip it as much as I could.