audio/dsound: fix invalid parameters error
diff mbox series

Message ID 20200117182621.GB13724@chuwi-lt0
State New
Headers show
Series
  • audio/dsound: fix invalid parameters error
Related show

Commit Message

KJ Liew Jan. 17, 2020, 6:26 p.m. UTC
QEMU Windows has broken dsound backend since the rewrite of audio API in
version 4.2.0. Both playback and capture buffers failed to lock with
invalid parameters error.

Comments

Philippe Mathieu-Daudé Jan. 18, 2020, 6:30 a.m. UTC | #1
On 1/17/20 7:26 PM, KJ Liew wrote:
> QEMU Windows has broken dsound backend since the rewrite of audio API in
> version 4.2.0. Both playback and capture buffers failed to lock with
> invalid parameters error.

Fixes: 7fa9754ac88 (dsoundaudio: port to the new audio backend api)

Cc'ing Zoltán who wrote 7fa9754ac88, and Gerd (the maintainer of this file):

   $ ./scripts/get_maintainer.pl -f audio/dsoundaudio.c
   Gerd Hoffmann <kraxel@redhat.com> (maintainer:Audio)

> --- ../orig/qemu-4.2.0/audio/dsoundaudio.c	2019-12-12 10:20:47.000000000 -0800
> +++ ../qemu-4.2.0/audio/dsoundaudio.c	2020-01-17 08:05:46.783966900 -0800
> @@ -53,6 +53,7 @@
>   typedef struct {
>       HWVoiceOut hw;
>       LPDIRECTSOUNDBUFFER dsound_buffer;
> +    void *last_buf;
>       dsound *s;
>   } DSoundVoiceOut;
>   
> @@ -414,10 +415,10 @@
>       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
>       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
>       HRESULT hr;
> -    DWORD ppos, act_size;
> +    DWORD ppos, act_size, last_size;
>       size_t req_size;
>       int err;
> -    void *ret;
> +    void *ret, *last_ret;
>   
>       hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL);
>       if (FAILED(hr)) {
> @@ -426,17 +427,24 @@
>           return NULL;
>       }
>   
> +    if (ppos == hw->pos_emul) {
> +        *size = 0;
> +        return ds->last_buf;
> +    }
> +
>       req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul);
>       req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
>   
> -    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret, NULL,
> -                          &act_size, NULL, false, ds->s);
> +    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret, &last_ret,
> +                          &act_size, &last_size, false, ds->s);
>       if (err) {
>           dolog("Failed to lock buffer\n");
>           *size = 0;
>           return NULL;
>       }
>   
> +    ds->last_buf = g_realloc(ds->last_buf, act_size);
> +    memcpy(ds->last_buf, ret, act_size);
>       *size = act_size;
>       return ret;
>   }
> @@ -445,6 +453,8 @@
>   {
>       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
>       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
> +    if (len == 0)
> +        return 0;
>       int err = dsound_unlock_out(dsb, buf, NULL, len, 0);
>   
>       if (err) {
> @@ -508,10 +518,10 @@
>       DSoundVoiceIn *ds = (DSoundVoiceIn *) hw;
>       LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer;
>       HRESULT hr;
> -    DWORD cpos, act_size;
> +    DWORD cpos, act_size, last_size;
>       size_t req_size;
>       int err;
> -    void *ret;
> +    void *ret, *last_ret;
>   
>       hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos, NULL);
>       if (FAILED(hr)) {
> @@ -520,11 +530,16 @@
>           return NULL;
>       }
>   
> +    if (cpos == hw->pos_emul) {
> +        *size = 0;
> +        return NULL;
> +    }
> +
>       req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul);
>       req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
>   
> -    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size, &ret, NULL,
> -                         &act_size, NULL, false, ds->s);
> +    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size, &ret, &last_ret,
> +                         &act_size, &last_size, false, ds->s);
>       if (err) {
>           dolog("Failed to lock buffer\n");
>           *size = 0;
>
=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= Jan. 27, 2020, 1:46 a.m. UTC | #2
On 2020-01-18 07:30, Philippe Mathieu-Daudé wrote:
> On 1/17/20 7:26 PM, KJ Liew wrote:
>> QEMU Windows has broken dsound backend since the rewrite of audio API in
>> version 4.2.0. Both playback and capture buffers failed to lock with
>> invalid parameters error.
> 
> Fixes: 7fa9754ac88 (dsoundaudio: port to the new audio backend api)

Hmm, I see the old code specified those parameters, but MSDN reads:

If the application passes NULL for the ppvAudioPtr2 and pdwAudioBytes2 
parameters, the lock extends no further than the end of the buffer and 
does not wrap.

Looks like this means that if the lock doesn't fit in the buffer it 
fails instead of truncating it.  I'm sure I tested the code under wine, 
and probably in a win8.1 vm too, and it worked there, maybe it's 
dependent on the windows version or sound driver?

> 
> Cc'ing Zoltán who wrote 7fa9754ac88, and Gerd (the maintainer of this 
> file):
> 
>    $ ./scripts/get_maintainer.pl -f audio/dsoundaudio.c
>    Gerd Hoffmann <kraxel@redhat.com> (maintainer:Audio)
> 
>> --- ../orig/qemu-4.2.0/audio/dsoundaudio.c    2019-12-12 
>> 10:20:47.000000000 -0800
>> +++ ../qemu-4.2.0/audio/dsoundaudio.c    2020-01-17 08:05:46.783966900 
>> -0800
>> @@ -53,6 +53,7 @@
>>   typedef struct {
>>       HWVoiceOut hw;
>>       LPDIRECTSOUNDBUFFER dsound_buffer;
>> +    void *last_buf;
>>       dsound *s;
>>   } DSoundVoiceOut;
>> @@ -414,10 +415,10 @@
>>       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
>>       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
>>       HRESULT hr;
>> -    DWORD ppos, act_size;
>> +    DWORD ppos, act_size, last_size;
>>       size_t req_size;
>>       int err;
>> -    void *ret;
>> +    void *ret, *last_ret;
>>       hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL);
>>       if (FAILED(hr)) {
>> @@ -426,17 +427,24 @@
>>           return NULL;
>>       }
>> +    if (ppos == hw->pos_emul) {
>> +        *size = 0;
>> +        return ds->last_buf;
>> +    }
>> +
>>       req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul);
>>       req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
>> -    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, 
>> &ret, NULL,
>> -                          &act_size, NULL, false, ds->s);
>> +    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, 
>> &ret, &last_ret,
>> +                          &act_size, &last_size, false, ds->s);
>>       if (err) {
>>           dolog("Failed to lock buffer\n");
>>           *size = 0;
>>           return NULL;
>>       }
>> +    ds->last_buf = g_realloc(ds->last_buf, act_size);
>> +    memcpy(ds->last_buf, ret, act_size);
>>       *size = act_size;
>>       return ret;
>>   }

I don't really understand what's happening here, why do you need that 
memory allocation and memcpy?  This function should return a buffer 
where the caller will write data, that *size = 0; when returning 
ds->last_buf also looks incorrect to me (the calling function won't 
write anything into it).

I'm attaching a patch with a probably better (and totally untested) way 
to do this (if someone can tell me how to copy-paste a patch into 
thunderbird without it messing up long lines, please tell me).


>> @@ -445,6 +453,8 @@
>>   {
>>       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
>>       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
>> +    if (len == 0)
>> +        return 0;
>>       int err = dsound_unlock_out(dsb, buf, NULL, len, 0);
>>       if (err) {

Msdn says "The second pointer is needed even if nothing was written to 
the second pointer." so that NULL doesn't look okay.

>> @@ -508,10 +518,10 @@
>>       DSoundVoiceIn *ds = (DSoundVoiceIn *) hw;
>>       LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer;
>>       HRESULT hr;
>> -    DWORD cpos, act_size;
>> +    DWORD cpos, act_size, last_size;
>>       size_t req_size;
>>       int err;
>> -    void *ret;
>> +    void *ret, *last_ret;
>>       hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos, 
>> NULL);
>>       if (FAILED(hr)) {
>> @@ -520,11 +530,16 @@
>>           return NULL;
>>       }
>> +    if (cpos == hw->pos_emul) {
>> +        *size = 0;
>> +        return NULL;
>> +    }
>> +
>>       req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul);
>>       req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
>> -    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size, 
>> &ret, NULL,
>> -                         &act_size, NULL, false, ds->s);
>> +    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size, 
>> &ret, &last_ret,
>> +                         &act_size, &last_size, false, ds->s);
>>       if (err) {
>>           dolog("Failed to lock buffer\n");
>>           *size = 0;
>>

You're completely ignoring last_ret and last_size here.  Don't you lose 
samples here?  I think it's possible to do something like I posted above 
with output here.

Regards,
Zoltan
Gerd Hoffmann Jan. 31, 2020, 7:55 a.m. UTC | #3
On Mon, Jan 27, 2020 at 02:46:58AM +0100, Zoltán Kővágó wrote:
> On 2020-01-18 07:30, Philippe Mathieu-Daudé wrote:
> > On 1/17/20 7:26 PM, KJ Liew wrote:
> > > QEMU Windows has broken dsound backend since the rewrite of audio API in
> > > version 4.2.0. Both playback and capture buffers failed to lock with
> > > invalid parameters error.
> > 
> > Fixes: 7fa9754ac88 (dsoundaudio: port to the new audio backend api)
> 
> Hmm, I see the old code specified those parameters, but MSDN reads:
> 
> If the application passes NULL for the ppvAudioPtr2 and pdwAudioBytes2
> parameters, the lock extends no further than the end of the buffer and does
> not wrap.
> 
> Looks like this means that if the lock doesn't fit in the buffer it fails
> instead of truncating it.  I'm sure I tested the code under wine, and
> probably in a win8.1 vm too, and it worked there, maybe it's dependent on
> the windows version or sound driver?

Ping.  Any news here?  I'm busy collecting all pending audio fixes for
the next pull request ...

> 
> > 
> > Cc'ing Zoltán who wrote 7fa9754ac88, and Gerd (the maintainer of this
> > file):
> > 
> >    $ ./scripts/get_maintainer.pl -f audio/dsoundaudio.c
> >    Gerd Hoffmann <kraxel@redhat.com> (maintainer:Audio)
> > 
> > > --- ../orig/qemu-4.2.0/audio/dsoundaudio.c    2019-12-12
> > > 10:20:47.000000000 -0800
> > > +++ ../qemu-4.2.0/audio/dsoundaudio.c    2020-01-17
> > > 08:05:46.783966900 -0800
> > > @@ -53,6 +53,7 @@
> > >   typedef struct {
> > >       HWVoiceOut hw;
> > >       LPDIRECTSOUNDBUFFER dsound_buffer;
> > > +    void *last_buf;
> > >       dsound *s;
> > >   } DSoundVoiceOut;
> > > @@ -414,10 +415,10 @@
> > >       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
> > >       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
> > >       HRESULT hr;
> > > -    DWORD ppos, act_size;
> > > +    DWORD ppos, act_size, last_size;
> > >       size_t req_size;
> > >       int err;
> > > -    void *ret;
> > > +    void *ret, *last_ret;
> > >       hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL);
> > >       if (FAILED(hr)) {
> > > @@ -426,17 +427,24 @@
> > >           return NULL;
> > >       }
> > > +    if (ppos == hw->pos_emul) {
> > > +        *size = 0;
> > > +        return ds->last_buf;
> > > +    }
> > > +
> > >       req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul);
> > >       req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
> > > -    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size,
> > > &ret, NULL,
> > > -                          &act_size, NULL, false, ds->s);
> > > +    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size,
> > > &ret, &last_ret,
> > > +                          &act_size, &last_size, false, ds->s);
> > >       if (err) {
> > >           dolog("Failed to lock buffer\n");
> > >           *size = 0;
> > >           return NULL;
> > >       }
> > > +    ds->last_buf = g_realloc(ds->last_buf, act_size);
> > > +    memcpy(ds->last_buf, ret, act_size);
> > >       *size = act_size;
> > >       return ret;
> > >   }
> 
> I don't really understand what's happening here, why do you need that memory
> allocation and memcpy?  This function should return a buffer where the
> caller will write data, that *size = 0; when returning ds->last_buf also
> looks incorrect to me (the calling function won't write anything into it).
> 
> I'm attaching a patch with a probably better (and totally untested) way to
> do this (if someone can tell me how to copy-paste a patch into thunderbird
> without it messing up long lines, please tell me).
> 
> 
> > > @@ -445,6 +453,8 @@
> > >   {
> > >       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
> > >       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
> > > +    if (len == 0)
> > > +        return 0;
> > >       int err = dsound_unlock_out(dsb, buf, NULL, len, 0);
> > >       if (err) {
> 
> Msdn says "The second pointer is needed even if nothing was written to the
> second pointer." so that NULL doesn't look okay.
> 
> > > @@ -508,10 +518,10 @@
> > >       DSoundVoiceIn *ds = (DSoundVoiceIn *) hw;
> > >       LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer;
> > >       HRESULT hr;
> > > -    DWORD cpos, act_size;
> > > +    DWORD cpos, act_size, last_size;
> > >       size_t req_size;
> > >       int err;
> > > -    void *ret;
> > > +    void *ret, *last_ret;
> > >       hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos,
> > > NULL);
> > >       if (FAILED(hr)) {
> > > @@ -520,11 +530,16 @@
> > >           return NULL;
> > >       }
> > > +    if (cpos == hw->pos_emul) {
> > > +        *size = 0;
> > > +        return NULL;
> > > +    }
> > > +
> > >       req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul);
> > >       req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
> > > -    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size,
> > > &ret, NULL,
> > > -                         &act_size, NULL, false, ds->s);
> > > +    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size,
> > > &ret, &last_ret,
> > > +                         &act_size, &last_size, false, ds->s);
> > >       if (err) {
> > >           dolog("Failed to lock buffer\n");
> > >           *size = 0;
> > > 
> 
> You're completely ignoring last_ret and last_size here.  Don't you lose
> samples here?  I think it's possible to do something like I posted above
> with output here.
> 
> Regards,
> Zoltan
KJ Liew Feb. 1, 2020, 6:28 p.m. UTC | #4
On Mon, Jan 27, 2020 at 02:46:58AM +0100, Zoltán Kővágó wrote:
> On 2020-01-18 07:30, Philippe Mathieu-Daudé wrote:
> > On 1/17/20 7:26 PM, KJ Liew wrote:
> > > QEMU Windows has broken dsound backend since the rewrite of audio API in
> > > version 4.2.0. Both playback and capture buffers failed to lock with
> > > invalid parameters error.
> > 
> > Fixes: 7fa9754ac88 (dsoundaudio: port to the new audio backend api)
> 
> Hmm, I see the old code specified those parameters, but MSDN reads:
> 
> If the application passes NULL for the ppvAudioPtr2 and pdwAudioBytes2
> parameters, the lock extends no further than the end of the buffer and does
> not wrap.
> 
> Looks like this means that if the lock doesn't fit in the buffer it fails
> instead of truncating it.  I'm sure I tested the code under wine, and
> probably in a win8.1 vm too, and it worked there, maybe it's dependent on
> the windows version or sound driver?
>

I was testing on several real Win10 machines. QEMU built with
MSYS2/mingw-w64-x86_64 GNU toolchain.

> > 
> > Cc'ing Zoltán who wrote 7fa9754ac88, and Gerd (the maintainer of this
> > file):
> > 
> >    $ ./scripts/get_maintainer.pl -f audio/dsoundaudio.c
> >    Gerd Hoffmann <kraxel@redhat.com> (maintainer:Audio)
> > 
> > > --- ../orig/qemu-4.2.0/audio/dsoundaudio.c    2019-12-12
> > > 10:20:47.000000000 -0800
> > > +++ ../qemu-4.2.0/audio/dsoundaudio.c    2020-01-17
> > > 08:05:46.783966900 -0800
> > > @@ -53,6 +53,7 @@
> > >   typedef struct {
> > >       HWVoiceOut hw;
> > >       LPDIRECTSOUNDBUFFER dsound_buffer;
> > > +    void *last_buf;
> > >       dsound *s;
> > >   } DSoundVoiceOut;
> > > @@ -414,10 +415,10 @@
> > >       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
> > >       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
> > >       HRESULT hr;
> > > -    DWORD ppos, act_size;
> > > +    DWORD ppos, act_size, last_size;
> > >       size_t req_size;
> > >       int err;
> > > -    void *ret;
> > > +    void *ret, *last_ret;
> > >       hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL);
> > >       if (FAILED(hr)) {
> > > @@ -426,17 +427,24 @@
> > >           return NULL;
> > >       }
> > > +    if (ppos == hw->pos_emul) {
> > > +        *size = 0;
> > > +        return ds->last_buf;
> > > +    }
> > > +
> > >       req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul);
> > >       req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
> > > -    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size,
> > > &ret, NULL,
> > > -                          &act_size, NULL, false, ds->s);
> > > +    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size,
> > > &ret, &last_ret,
> > > +                          &act_size, &last_size, false, ds->s);
> > >       if (err) {
> > >           dolog("Failed to lock buffer\n");
> > >           *size = 0;
> > >           return NULL;
> > >       }
> > > +    ds->last_buf = g_realloc(ds->last_buf, act_size);
> > > +    memcpy(ds->last_buf, ret, act_size);
> > >       *size = act_size;
> > >       return ret;
> > >   }
> 
> I don't really understand what's happening here, why do you need that memory
> allocation and memcpy?  This function should return a buffer where the
> caller will write data, that *size = 0; when returning ds->last_buf also
> looks incorrect to me (the calling function won't write anything into it).

I was trying to fix the invalid parameters errors from
dsound_lock_out()/dsound_lock_in(). I have to say that I wasn't quite sure if the
content of buffer matters to the caller, but an assumption that safe buffer for
read/write got to be there. So I just memcpy to keep the last good
buffer.

The lock seemed to fail for dsound_lock_out()/dsound_lock_in() when
(ppos == hw->pos_emul) for _out and (cpos == hw->pos_emul) for _in.
Hence, the last_buf was returned to the caller. Since the lock did not
take place, the *size = 0 provides the hint to skip the unlock,
otherwise, dsound_unlock_out() failed for buffer that has never been
locked.

> I'm attaching a patch with a probably better (and totally untested) way to
> do this (if someone can tell me how to copy-paste a patch into thunderbird
> without it messing up long lines, please tell me). 
 
I checked out your patch. Unfortunately, it did not
address the invalid paramters errors. The console still flooded with error
messages:
dsound: Could not lock playback buffer
dsound: Reason: An invalid parameter was passed to the returning function
dsound: Failed to lock buffer

> > > @@ -445,6 +453,8 @@
> > >   {
> > >       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
> > >       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
> > > +    if (len == 0)
> > > +        return 0;
> > >       int err = dsound_unlock_out(dsb, buf, NULL, len, 0);
> > >       if (err) {
> 
> Msdn says "The second pointer is needed even if nothing was written to the
> second pointer." so that NULL doesn't look okay.
> 
> > > @@ -508,10 +518,10 @@
> > >       DSoundVoiceIn *ds = (DSoundVoiceIn *) hw;
> > >       LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer;
> > >       HRESULT hr;
> > > -    DWORD cpos, act_size;
> > > +    DWORD cpos, act_size, last_size;
> > >       size_t req_size;
> > >       int err;
> > > -    void *ret;
> > > +    void *ret, *last_ret;
> > >       hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos,
> > > NULL);
> > >       if (FAILED(hr)) {
> > > @@ -520,11 +530,16 @@
> > >           return NULL;
> > >       }
> > > +    if (cpos == hw->pos_emul) {
> > > +        *size = 0;
> > > +        return NULL;
> > > +    }
> > > +
> > >       req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul);
> > >       req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
> > > -    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size,
> > > &ret, NULL,
> > > -                         &act_size, NULL, false, ds->s);
> > > +    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size,
> > > &ret, &last_ret,
> > > +                         &act_size, &last_size, false, ds->s);
> > >       if (err) {
> > >           dolog("Failed to lock buffer\n");
> > >           *size = 0;
> > > 
> 
> You're completely ignoring last_ret and last_size here.  Don't you lose
> samples here?  I think it's possible to do something like I posted above
> with output here.
> 
> Regards,
> Zoltan

> diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
> index c265c0094b..b6bc241faa 100644
> --- a/audio/dsoundaudio.c
> +++ b/audio/dsoundaudio.c
> @@ -53,6 +53,9 @@ typedef struct {
>  typedef struct {
>      HWVoiceOut hw;
>      LPDIRECTSOUNDBUFFER dsound_buffer;
> +    void *buffer1, buffer2;
> +    DWORD size1, size2;
> +    bool has_remaining;
>      dsound *s;
>  } DSoundVoiceOut;
>  
> @@ -414,10 +417,16 @@ static void *dsound_get_buffer_out(HWVoiceOut *hw, size_t *size)
>      DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
>      LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
>      HRESULT hr;
> -    DWORD ppos, act_size;
> +    DWORD ppos, act_size1, act_size2;
>      size_t req_size;
>      int err;
> -    void *ret;
> +    void *ret1, *ret2;
> +
> +    if (ds->has_remaining) {
> +        ds->has_remaining = false;
> +        *size = ds->size2;
> +        return ds->buffer2;
> +    }
>  
>      hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL);
>      if (FAILED(hr)) {
> @@ -429,15 +438,20 @@ static void *dsound_get_buffer_out(HWVoiceOut *hw, size_t *size)
>      req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul);
>      req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
>  
> -    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret, NULL,
> -                          &act_size, NULL, false, ds->s);
> +    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret1, &ret2,
> +                          &act_size1, &act_size2, false, ds->s);
>      if (err) {
>          dolog("Failed to lock buffer\n");
>          *size = 0;
>          return NULL;
>      }
>  
> -    *size = act_size;
> +    *size = act_size1;
> +    ds->buffer1 = ret1;
> +    ds->buffer2 = ret2;
> +    ds->size1 = act_size1;
> +    ds->size2 = act_size2;
> +    ds->has_remaining = ret2 != NULL;
>      return ret;
>  }
>  
> @@ -445,7 +459,16 @@ static size_t dsound_put_buffer_out(HWVoiceOut *hw, void *buf, size_t len)
>  {
>      DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
>      LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
> -    int err = dsound_unlock_out(dsb, buf, NULL, len, 0);
> +    int err;
> +
> +    if (ds->has_remaining) {
> +        ds->size1 = len;
> +        hw->pos_emul = (hw->pos_emul + len) % hw->size_emul;
> +        return len;
> +    }
> +
> +    *(ds->buffer2 ? &ds->size2 : &ds->size1) = len;
> +    err = dsound_unlock_out(dsb, ds->buffer1, ds->buffer2, ds->size1, ds->size2);
>  
>      if (err) {
>          dolog("Failed to unlock buffer!!\n");

Patch
diff mbox series

--- ../orig/qemu-4.2.0/audio/dsoundaudio.c	2019-12-12 10:20:47.000000000 -0800
+++ ../qemu-4.2.0/audio/dsoundaudio.c	2020-01-17 08:05:46.783966900 -0800
@@ -53,6 +53,7 @@ 
 typedef struct {
     HWVoiceOut hw;
     LPDIRECTSOUNDBUFFER dsound_buffer;
+    void *last_buf;
     dsound *s;
 } DSoundVoiceOut;
 
@@ -414,10 +415,10 @@ 
     DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
     LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
     HRESULT hr;
-    DWORD ppos, act_size;
+    DWORD ppos, act_size, last_size;
     size_t req_size;
     int err;
-    void *ret;
+    void *ret, *last_ret;
 
     hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL);
     if (FAILED(hr)) {
@@ -426,17 +427,24 @@ 
         return NULL;
     }
 
+    if (ppos == hw->pos_emul) {
+        *size = 0;
+        return ds->last_buf;
+    }
+
     req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul);
     req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
 
-    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret, NULL,
-                          &act_size, NULL, false, ds->s);
+    err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret, &last_ret,
+                          &act_size, &last_size, false, ds->s);
     if (err) {
         dolog("Failed to lock buffer\n");
         *size = 0;
         return NULL;
     }
 
+    ds->last_buf = g_realloc(ds->last_buf, act_size);
+    memcpy(ds->last_buf, ret, act_size);
     *size = act_size;
     return ret;
 }
@@ -445,6 +453,8 @@ 
 {
     DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
     LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
+    if (len == 0)
+        return 0;
     int err = dsound_unlock_out(dsb, buf, NULL, len, 0);
 
     if (err) {
@@ -508,10 +518,10 @@ 
     DSoundVoiceIn *ds = (DSoundVoiceIn *) hw;
     LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer;
     HRESULT hr;
-    DWORD cpos, act_size;
+    DWORD cpos, act_size, last_size;
     size_t req_size;
     int err;
-    void *ret;
+    void *ret, *last_ret;
 
     hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos, NULL);
     if (FAILED(hr)) {
@@ -520,11 +530,16 @@ 
         return NULL;
     }
 
+    if (cpos == hw->pos_emul) {
+        *size = 0;
+        return NULL;
+    }
+
     req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul);
     req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
 
-    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size, &ret, NULL,
-                         &act_size, NULL, false, ds->s);
+    err = dsound_lock_in(dscb, &hw->info, hw->pos_emul, req_size, &ret, &last_ret,
+                         &act_size, &last_size, false, ds->s);
     if (err) {
         dolog("Failed to lock buffer\n");
         *size = 0;