[alsa-devel] underruns and strange code in pcm_rate.c (and patch)

[Takashi Iwai]

At Thu, 08 Nov 2007 11:27:42 +0300,
Stas Sergeev wrote:
> 
> Hello.
> 
> Takashi Iwai wrote:
> >> OK then. The race will have to wait
> >> till I get to it hard and (in case
> >> there really is) produce a patch.
> > Yeah, a testcase is required to reproduce the bug anyway.
> No, the race doesn't have a test-case -
> it gives underruns only when I rapidly
> switch the consoles... Anyway, let's
> wait till I get to it.

The latency via console-switching is a long known problem in the kernel.
It's likely irrelevant with the sound system itself.

> > Well, I object it - as far as I know (through hundreds of SUSE
> > package I've been maintaining), most of them write the period_size
> > data at once.  Writing arbitrary size is rather minor.
> OK, thanks for info - I'll see about
> enlarging my test-suit.
> 
> >> mpg123:
> >> ogg123:
> > Both are for libao.
> Hmm, my mpg123 is not from libao, only
> ogg123 is. And mpg123 doesn't suffer an
> underruns therefore, but it is affected
> by a few other problems I mentioned.

Ah, OK, I thought it'g mpg321.  OK, I forgot about mpg123 supporting
ALSA.

> > The normal apps are more careful about the write timing and sync with
> > GUI.  So they tend to write the data at the time poll permits, instead
> > of dumb write sequence relying on blocking mode.  Maybe this is the
> > key to get or avoid the bug.
> OK, I'll try aplay later and post back.
> 
> > The difference seems to be whether you do simply writei() sequences
> > without checks which causes partial writes and eventually triggers the
> > hack.
> No, its not that simple. Or at least in
> my theory. :) The rounding error I was
> pointing into, causes the following:
> - the app writes a full period.
> - the rate plugin converts it to the
> slave period, which has a different
> size.
> - because of the rounding errors, that
> slave period is not filled properly -
> it is either slightly underfilled, or
> overfilled (and the subsequent period is
> started). That triggers the hack.
> Does this look realistic or not?

Not really.  It checks appl_ptr of the client, not hw_ptr.  appl_ptr
is what app writes.  So, as long as app writes only the period size,
the hack isn't triggered.

> > As mentioned, many apps do check the available spaces before
> > write, so the partial write doesn't happen practically.
> I don't think this will help, but I'll
> check with aplay later.
> 
> > No, what I meant is that the app should check always the return value
> > of the write properly.  Even in the blocking mode, write *may* return
> > without writing the whole data.  Well, it's off-topic right now.
> I know that, but I can't follow the
> logic. If write returns earlier, then
> the app will have to do the partial
> write next time, so you loose the
> alignment. So by saying that, you
> basically say that the arbitrary writes
> are unavoidable, and so I can't follow
> the logic.

The app can wait via poll until the period size becomes available.  In
that way, partial write can be avoided practically.

> > I'm not satisfied because I have no testcase to reproduce the
> > practical bug.  The patch looks fine, but I cannot test it because the
> > serious bug doesn't occur to me.  OK?  That's why I claim a testcase.
> I'll try to help with that, but I don't
> think this will be successfull (and
> you should try my asound.conf first).
> But we always can ask someone else
> with the problem to test the patch,
> right? I don't think there is a shortage
> of such people - google makes me to beleive
> so.

Sorry, no, testing the patch with specific apps alone isn't enough in
this case.

If the problem persists only with two-period cases, the real fix is to
change the hw_refine in rate plugin not to allow periods <= 2 for
unaligned sample rates, because two periods don't work anyway.  Then a
non-working configuration wouldn't be allowed instead of jerky sounds.


Takashi