winecoreaudio.drv: Improve underrun handling

Andrew,

The good news is that your underrun patch prevents mmdevapi from writing
less and less frames in mmdevapi/tests/render:test_worst_case
http://test.winehq.org/data/f0cfa7cf46d7c832fb84555529d1a1b7c49c46c0/mac_jch...
> render.c:1631: Released 86436=98x882 -0 frames at 44100 worth 1960ms in 2035ms
> render.c:1631: Released 71442=81x882 -0 frames at 44100 worth 1620ms in 2037ms
> render.c:1631: Released 56448=64x882 -0 frames at 44100 worth 1280ms in 2039ms

That is abnormal behavior.

+        sc = AudioQueueEnqueueBufferWithParameters(This->aqueue,
+                This->public_buffer, 0, NULL, 0, 0, 0, NULL, &req_time, &start_time);
I advise against setting req_time.  This can only fragilize the
system.  What if req_time is miscomputed or there's a delay?
NULL=ASAP is exactly what we want.  We don't want ALSA/dmix behavior,
where it'll silently skip over late frames in order to catch up
(dunno if MacOSX does this but in my eyes that doesn't matter).
If you need to memorize a time, I believe it's best stored in our
AQBuffer struct.
> if(list_count(...) == 0)

I once wrote a patch to turn such a pattern into if(list_empty(...))

Regards,
    Jörg

[Andrew Eikum]

On Mon, Jan 30, 2012 at 11:03:52AM +0100, [e-mail] wrote:
> -
>    sc = AudioQueueEnqueueBufferWithParameters(This->aqueue,
> -
>            This->public_buffer, 0, NULL, 0, 0, 0, NULL, &req_time, &start_time);
>
> I advise against setting req_time.  This can only fragilize the
> system.  What if req_time is miscomputed or there's a delay?
> NULL=ASAP is exactly what we want.  We don't want ALSA/dmix behavior,
> where it'll silently skip over late frames in order to catch up
> (dunno if MacOSX does this but in my eyes that doesn't matter).

The AudioQueue API does do this, and that's the behavior I was trying
to work around. See bug 29602. Other options like AudioQueueReset() or
AudioQueueFlush() don't seem to do what we want, so this is the best
solution I found. What do you mean by it doesn't matter?
> > if(list_count(...) == 0)
>
> I once wrote a patch to turn such a pattern into if(list_empty(...))

Oops, I forgot that existed. I'll fix that in the next version.

Andrew

Hi,

Andre Eikum wrote:
> > > -
> > >    sc = AudioQueueEnqueueBufferWithParameters(This->aqueue,
> > > -
> > >            This->public_buffer, 0, NULL, 0, 0, 0, NULL, 
> > > - &req_time, &start_time);
> >
> > NULL=ASAP is exactly what we want.  We don't want ALSA/dmix behavior, 
> > where it'll silently skip over late frames in order to catch up (dunno 
> > if MacOSX does this but in my eyes that doesn't matter).
>
> The AudioQueue API does do this

What "this" do you mean exactly?
a) like dmix, silently skip over megabytes of submitted frames
   should they happen to be late w.r.t. wall time?
b) Play any submitted frames ASAP to the speaker?
   You said in #29585 that OSS4 does this.

What mmdevapi needs is b). It does not appear to care about underruns
and has no dmix-like notion of trying to catch up.
> What do you mean by it doesn't matter?

If req_time NULL means ASAP, which is what we need, then the
behaviour of req_time NOT NULL as either a or b does not matter,
because we should not call it that way.

Regards,
    Jörg Höhle

[Andrew Eikum]

On Mon, Jan 30, 2012 at 06:52:36PM +0100, [e-mail] wrote:
> Andre Eikum wrote:
> > > > -
> > > >    sc = AudioQueueEnqueueBufferWithParameters(This->aqueue,
> > > > -
> > > >            This->public_buffer, 0, NULL, 0, 0, 0, NULL, 
> > > > - &req_time, &start_time);
> > >
> > > NULL=ASAP is exactly what we want.  We don't want ALSA/dmix behavior, 
> > > where it'll silently skip over late frames in order to catch up (dunno 
> > > if MacOSX does this but in my eyes that doesn't matter).
> >
> > The AudioQueue API does do this
>
> What "this" do you mean exactly?
> a) like dmix, silently skip over megabytes of submitted frames
>    should they happen to be late w.r.t. wall time?
> b) Play any submitted frames ASAP to the speaker?
>    You said in #29585 that OSS4 does this.
>
> What mmdevapi needs is b). It does not appear to care about underruns
> and has no dmix-like notion of trying to catch up.

Yes. AudioQueue does (a), which I'm trying to work around by telling
the queue to play the buffer at any time after the queue's current
time when we detect an underrun (although detecting when the queue is
empty needs some work, as Ken pointed out).

In ALSA, we use snd_pcm_recover() after underruns, but AudioQueue
doesn't seem to have an analogous function that works well.

Andrew