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

[Jan Kiszka]

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).

> 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. Need to check, but maybe we can even wait the a
full buffer.

Jan

signature.asc
Description: OpenPGP digital signature