Re: [Qemu-devel] [4249] Improve audio api use in WM8750.

[andrzej zaborowski]

On 25/04/2008, Jan Kiszka <address@hidden> wrote:
> andrzej zaborowski wrote:
>  > On 24/04/2008, Jan Kiszka <address@hidden> wrote:
>  >> Hi Andrzej,
>  >>
>  >>  Andrzej Zaborowski wrote:
>  >>  > Revision: 4249
>  >>  ...
>  >>  >  static void wm8750_audio_out_cb(void *opaque, int free_b)
>  >>  >  {
>  >>  >      struct wm8750_s *s = (struct wm8750_s *) opaque;
>  >>  >
>  >>  > -    s->req_out = free_b;
>  >>  > -    s->data_req(s->opaque, free_b >> 2, s->req_in >> 2);
>  >>  > -    wm8750_out_flush(s);
>  >>  > +    if (s->idx_out >= free_b) {
>  >>  > +        s->idx_out = free_b;
>  >>  > +        s->req_out = 0;
>  >>  > +        wm8750_out_flush(s);
>  >>  > +    } else
>  >>  > +        s->req_out = free_b - s->idx_out;
>  >>  > +
>  >>  > +    s->data_req(s->opaque, s->req_out >> 2, s->req_in >> 2);
>  >>
>  >>  Please make sure that the callback is always issued _before_ the flush
>  >>  (keep in mind: it may increase the amount of data that has to be flushed
>  >>  ASAP!). And this change also leaves the MusicPal broken.
>  >
>  > The idea is to output free_b bytes immediately if we have that many in
>  > the buffer (it could happen assuming that free_b value can change
>
>
> Wrong ordering: If there has been a _relevant_ amount of data (not a few
>  10 bytes) hanging around in our internal buffer between the guest has
>  filled it during the last callbacks and the host reports demand via the
>  new callback, that data was missing in the host's audio buffer! That's
>  what I saw (heard) with the old version (the current one even causes
>  total silence).

Not sure I understand what you mean by missing data.  The host reports
that it can accept a write of free_b bytes.  If we have free_b bytes
in s->data_out then we write it.  Then we tell the guest how many more
samples we need (if any).  When the guest provides us with that many
samples then we write the whole buffer to host (no matter if that
happened inside the callback or whenever that data became ready).

>
>
>  > between callbacks).  I'm not sure how this can break something: if
>  > *inside* the data_req() callback we receive enough bytes to fill the
>  > the whole buffer then dac_dat() will call out_flush().
>  >
>  > Without that all buffering becomes useless because we flush every
>  > sample we receive and we start to crawl.
>
>
> Nothing is crawling here, just use a reasonable threshold for flushing
>  _after_ the callback.

We don't really care what happened in the callback.  We need to write
when we reach the threshold, and this is what is implemented.
-- 
Please do not print this email unless absolutely necessary. Spread
environmental awareness.