| Message ID | fe9744216d9d421a2dbb09bcf5fa0dbd18f77ac5.1580684275.git.DirtY.iCE.hu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | audio/dsound: fix invalid parameters error | expand |
On Mon, Feb 3, 2020 at 12:02 AM Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote: > Windows (unlike wine) bails out when IDirectSoundBuffer8::Lock is called > with zero length. Also, hw->pos_emul handling was incorrect when > calling this function for the first time. > > Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> > Reported-by: KJ Liew <liewkj@yahoo.com> > --- > > I've tested this patch on wine and a borrowed Windows 8.1 laptop, I > could only test audio playback, not recording. I've cross-compiled qemu > using the docker image, for 64-bit. > > --- > audio/dsound_template.h | 1 + > audio/audio.c | 6 ++---- > audio/dsoundaudio.c | 27 +++++++++++++++++++++++---- > 3 files changed, 26 insertions(+), 8 deletions(-) > > diff --git a/audio/dsound_template.h b/audio/dsound_template.h > index 7a15f91ce5..9c5ce625ab 100644 > --- a/audio/dsound_template.h > +++ b/audio/dsound_template.h > @@ -244,6 +244,7 @@ static int dsound_init_out(HWVoiceOut *hw, struct > audsettings *as, > goto fail0; > } > > + ds->first_time = true; > obt_as.endianness = 0; > audio_pcm_init_info (&hw->info, &obt_as); > > diff --git a/audio/audio.c b/audio/audio.c > index f63f39769a..cb1efc6dc5 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -1076,10 +1076,8 @@ static size_t audio_pcm_hw_run_out(HWVoiceOut *hw, > size_t live) > while (live) { > size_t size, decr, proc; > void *buf = hw->pcm_ops->get_buffer_out(hw, &size); > - if (!buf) { > - /* retrying will likely won't help, drop everything. */ > - hw->mix_buf->pos = (hw->mix_buf->pos + live) % > hw->mix_buf->size; > - return clipped + live; > + if (!buf || size == 0) { > + break; > } > > decr = MIN(size / hw->info.bytes_per_frame, live); > diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c > index c265c0094b..bd57082a8d 100644 > --- a/audio/dsoundaudio.c > +++ b/audio/dsoundaudio.c > @@ -53,12 +53,14 @@ typedef struct { > typedef struct { > HWVoiceOut hw; > LPDIRECTSOUNDBUFFER dsound_buffer; > + bool first_time; > dsound *s; > } DSoundVoiceOut; > > typedef struct { > HWVoiceIn hw; > LPDIRECTSOUNDCAPTUREBUFFER dsound_capture_buffer; > + bool first_time; > dsound *s; > } DSoundVoiceIn; > > @@ -414,21 +416,32 @@ 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, wpos, act_size; > size_t req_size; > int err; > void *ret; > > - hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL); > + hr = IDirectSoundBuffer_GetCurrentPosition( > + dsb, &ppos, ds->first_time ? &wpos : NULL); > if (FAILED(hr)) { > dsound_logerr(hr, "Could not get playback buffer position\n"); > *size = 0; > return NULL; > } > > + if (ds->first_time) { > + hw->pos_emul = wpos; > + ds->first_time = false; > + } > + > req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul); > req_size = MIN(req_size, hw->size_emul - hw->pos_emul); > > + if (req_size == 0) { > + *size = 0; > + return NULL; > + } > + > err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret, > NULL, > &act_size, NULL, false, ds->s); > if (err) { > @@ -508,18 +521,24 @@ static void *dsound_get_buffer_in(HWVoiceIn *hw, > size_t *size) > DSoundVoiceIn *ds = (DSoundVoiceIn *) hw; > LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer; > HRESULT hr; > - DWORD cpos, act_size; > + DWORD cpos, rpos, act_size; > size_t req_size; > int err; > void *ret; > > - hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos, NULL); > + hr = IDirectSoundCaptureBuffer_GetCurrentPosition( > + dscb, &cpos, ds->first_time ? &rpos : NULL); > if (FAILED(hr)) { > dsound_logerr(hr, "Could not get capture buffer position\n"); > *size = 0; > return NULL; > } > > + if (ds->first_time) { > + hw->pos_emul = rpos; > + ds->first_time = false; > + } > + > req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul); > req_size = MIN(req_size, hw->size_emul - hw->pos_emul); > > -- > 2.25.0 > Hi, I tested this patch running qemu-system-ppc with MacOS 9.2 and OSX 10.5. Qemu was cross-compiled on Fedora 31 from https://github.com/mcayland/qemu/tree/screamer. Host was Windows 10. The dsound locking errors are gone, so for this test case all seems OK. Best, Howard
Hi Howard, On 2/3/20 8:56 AM, Howard Spoelstra wrote: > On Mon, Feb 3, 2020 at 12:02 AM Kővágó, Zoltán <dirty.ice.hu@gmail.com > <mailto:dirty.ice.hu@gmail.com>> wrote: > > Windows (unlike wine) bails out when IDirectSoundBuffer8::Lock is called > with zero length. Also, hw->pos_emul handling was incorrect when > calling this function for the first time. > > Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com > <mailto:DirtY.iCE.hu@gmail.com>> > Reported-by: KJ Liew <liewkj@yahoo.com <mailto:liewkj@yahoo.com>> > --- > > I've tested this patch on wine and a borrowed Windows 8.1 laptop, I > could only test audio playback, not recording. I've cross-compiled qemu > using the docker image, for 64-bit. > > --- > audio/dsound_template.h | 1 + > audio/audio.c | 6 ++---- > audio/dsoundaudio.c | 27 +++++++++++++++++++++++---- > 3 files changed, 26 insertions(+), 8 deletions(-) > > diff --git a/audio/dsound_template.h b/audio/dsound_template.h > index 7a15f91ce5..9c5ce625ab 100644 > --- a/audio/dsound_template.h > +++ b/audio/dsound_template.h > @@ -244,6 +244,7 @@ static int dsound_init_out(HWVoiceOut *hw, > struct audsettings *as, > goto fail0; > } > > + ds->first_time = true; > obt_as.endianness = 0; > audio_pcm_init_info (&hw->info, &obt_as); > > diff --git a/audio/audio.c b/audio/audio.c > index f63f39769a..cb1efc6dc5 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -1076,10 +1076,8 @@ static size_t audio_pcm_hw_run_out(HWVoiceOut > *hw, size_t live) > while (live) { > size_t size, decr, proc; > void *buf = hw->pcm_ops->get_buffer_out(hw, &size); > - if (!buf) { > - /* retrying will likely won't help, drop everything. */ > - hw->mix_buf->pos = (hw->mix_buf->pos + live) % > hw->mix_buf->size; > - return clipped + live; > + if (!buf || size == 0) { > + break; > } > > decr = MIN(size / hw->info.bytes_per_frame, live); > diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c > index c265c0094b..bd57082a8d 100644 > --- a/audio/dsoundaudio.c > +++ b/audio/dsoundaudio.c > @@ -53,12 +53,14 @@ typedef struct { > typedef struct { > HWVoiceOut hw; > LPDIRECTSOUNDBUFFER dsound_buffer; > + bool first_time; > dsound *s; > } DSoundVoiceOut; > > typedef struct { > HWVoiceIn hw; > LPDIRECTSOUNDCAPTUREBUFFER dsound_capture_buffer; > + bool first_time; > dsound *s; > } DSoundVoiceIn; > > @@ -414,21 +416,32 @@ 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, wpos, act_size; > size_t req_size; > int err; > void *ret; > > - hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL); > + hr = IDirectSoundBuffer_GetCurrentPosition( > + dsb, &ppos, ds->first_time ? &wpos : NULL); > if (FAILED(hr)) { > dsound_logerr(hr, "Could not get playback buffer position\n"); > *size = 0; > return NULL; > } > > + if (ds->first_time) { > + hw->pos_emul = wpos; > + ds->first_time = false; > + } > + > req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul); > req_size = MIN(req_size, hw->size_emul - hw->pos_emul); > > + if (req_size == 0) { > + *size = 0; > + return NULL; > + } > + > err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, > &ret, NULL, > &act_size, NULL, false, ds->s); > if (err) { > @@ -508,18 +521,24 @@ static void *dsound_get_buffer_in(HWVoiceIn > *hw, size_t *size) > DSoundVoiceIn *ds = (DSoundVoiceIn *) hw; > LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer; > HRESULT hr; > - DWORD cpos, act_size; > + DWORD cpos, rpos, act_size; > size_t req_size; > int err; > void *ret; > > - hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos, > NULL); > + hr = IDirectSoundCaptureBuffer_GetCurrentPosition( > + dscb, &cpos, ds->first_time ? &rpos : NULL); > if (FAILED(hr)) { > dsound_logerr(hr, "Could not get capture buffer position\n"); > *size = 0; > return NULL; > } > > + if (ds->first_time) { > + hw->pos_emul = rpos; > + ds->first_time = false; > + } > + > req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul); > req_size = MIN(req_size, hw->size_emul - hw->pos_emul); > > -- > 2.25.0 > > > Hi, > > I tested this patch running qemu-system-ppc with MacOS 9.2 and OSX 10.5. > Qemu was cross-compiled on Fedora 31 from > https://github.com/mcayland/qemu/tree/screamer. Host was Windows 10. > > The dsound locking errors are gone, so for this test case all seems OK. Thanks for testing! Does that mean we can add your tag to this patch? Tested-by: Howard Spoelstra <hsp.cat7@gmail.com> Regards, Phil.
Cher Philippe, On Mon, Feb 3, 2020 at 10:18 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Howard, > > On 2/3/20 8:56 AM, Howard Spoelstra wrote: > > On Mon, Feb 3, 2020 at 12:02 AM Kővágó, Zoltán <dirty.ice.hu@gmail.com > > <mailto:dirty.ice.hu@gmail.com>> wrote: > > > > Windows (unlike wine) bails out when IDirectSoundBuffer8::Lock is > called > > with zero length. Also, hw->pos_emul handling was incorrect when > > calling this function for the first time. > > > > Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com > > <mailto:DirtY.iCE.hu@gmail.com>> > > Reported-by: KJ Liew <liewkj@yahoo.com <mailto:liewkj@yahoo.com>> > > --- > > > > I've tested this patch on wine and a borrowed Windows 8.1 laptop, I > > could only test audio playback, not recording. I've cross-compiled > qemu > > using the docker image, for 64-bit. > > > > --- > > audio/dsound_template.h | 1 + > > audio/audio.c | 6 ++---- > > audio/dsoundaudio.c | 27 +++++++++++++++++++++++---- > > 3 files changed, 26 insertions(+), 8 deletions(-) > > > > diff --git a/audio/dsound_template.h b/audio/dsound_template.h > > index 7a15f91ce5..9c5ce625ab 100644 > > --- a/audio/dsound_template.h > > +++ b/audio/dsound_template.h > > @@ -244,6 +244,7 @@ static int dsound_init_out(HWVoiceOut *hw, > > struct audsettings *as, > > goto fail0; > > } > > > > + ds->first_time = true; > > obt_as.endianness = 0; > > audio_pcm_init_info (&hw->info, &obt_as); > > > > diff --git a/audio/audio.c b/audio/audio.c > > index f63f39769a..cb1efc6dc5 100644 > > --- a/audio/audio.c > > +++ b/audio/audio.c > > @@ -1076,10 +1076,8 @@ static size_t audio_pcm_hw_run_out(HWVoiceOut > > *hw, size_t live) > > while (live) { > > size_t size, decr, proc; > > void *buf = hw->pcm_ops->get_buffer_out(hw, &size); > > - if (!buf) { > > - /* retrying will likely won't help, drop everything. */ > > - hw->mix_buf->pos = (hw->mix_buf->pos + live) % > > hw->mix_buf->size; > > - return clipped + live; > > + if (!buf || size == 0) { > > + break; > > } > > > > decr = MIN(size / hw->info.bytes_per_frame, live); > > diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c > > index c265c0094b..bd57082a8d 100644 > > --- a/audio/dsoundaudio.c > > +++ b/audio/dsoundaudio.c > > @@ -53,12 +53,14 @@ typedef struct { > > typedef struct { > > HWVoiceOut hw; > > LPDIRECTSOUNDBUFFER dsound_buffer; > > + bool first_time; > > dsound *s; > > } DSoundVoiceOut; > > > > typedef struct { > > HWVoiceIn hw; > > LPDIRECTSOUNDCAPTUREBUFFER dsound_capture_buffer; > > + bool first_time; > > dsound *s; > > } DSoundVoiceIn; > > > > @@ -414,21 +416,32 @@ 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, wpos, act_size; > > size_t req_size; > > int err; > > void *ret; > > > > - hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL); > > + hr = IDirectSoundBuffer_GetCurrentPosition( > > + dsb, &ppos, ds->first_time ? &wpos : NULL); > > if (FAILED(hr)) { > > dsound_logerr(hr, "Could not get playback buffer > position\n"); > > *size = 0; > > return NULL; > > } > > > > + if (ds->first_time) { > > + hw->pos_emul = wpos; > > + ds->first_time = false; > > + } > > + > > req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul); > > req_size = MIN(req_size, hw->size_emul - hw->pos_emul); > > > > + if (req_size == 0) { > > + *size = 0; > > + return NULL; > > + } > > + > > err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, > > &ret, NULL, > > &act_size, NULL, false, ds->s); > > if (err) { > > @@ -508,18 +521,24 @@ static void *dsound_get_buffer_in(HWVoiceIn > > *hw, size_t *size) > > DSoundVoiceIn *ds = (DSoundVoiceIn *) hw; > > LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer; > > HRESULT hr; > > - DWORD cpos, act_size; > > + DWORD cpos, rpos, act_size; > > size_t req_size; > > int err; > > void *ret; > > > > - hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos, > > NULL); > > + hr = IDirectSoundCaptureBuffer_GetCurrentPosition( > > + dscb, &cpos, ds->first_time ? &rpos : NULL); > > if (FAILED(hr)) { > > dsound_logerr(hr, "Could not get capture buffer > position\n"); > > *size = 0; > > return NULL; > > } > > > > + if (ds->first_time) { > > + hw->pos_emul = rpos; > > + ds->first_time = false; > > + } > > + > > req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul); > > req_size = MIN(req_size, hw->size_emul - hw->pos_emul); > > > > -- > > 2.25.0 > > > > > > Hi, > > > > I tested this patch running qemu-system-ppc with MacOS 9.2 and OSX 10.5. > > Qemu was cross-compiled on Fedora 31 from > > https://github.com/mcayland/qemu/tree/screamer. Host was Windows 10. > > > > The dsound locking errors are gone, so for this test case all seems OK. > > Thanks for testing! > > Does that mean we can add your tag to this patch? > > > Regards, > > Phil. > > Yes, so: Tested-by: Howard Spoelstra <hsp.cat7@gmail.com> Best, Howard
On Mon, Feb 03, 2020 at 12:02:23AM +0100, Kővágó, Zoltán wrote: > Windows (unlike wine) bails out when IDirectSoundBuffer8::Lock is called > with zero length. Also, hw->pos_emul handling was incorrect when > calling this function for the first time. > > Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> > Reported-by: KJ Liew <liewkj@yahoo.com> Patch queued. thanks, Gerd
diff --git a/audio/dsound_template.h b/audio/dsound_template.h index 7a15f91ce5..9c5ce625ab 100644 --- a/audio/dsound_template.h +++ b/audio/dsound_template.h @@ -244,6 +244,7 @@ static int dsound_init_out(HWVoiceOut *hw, struct audsettings *as, goto fail0; } + ds->first_time = true; obt_as.endianness = 0; audio_pcm_init_info (&hw->info, &obt_as); diff --git a/audio/audio.c b/audio/audio.c index f63f39769a..cb1efc6dc5 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1076,10 +1076,8 @@ static size_t audio_pcm_hw_run_out(HWVoiceOut *hw, size_t live) while (live) { size_t size, decr, proc; void *buf = hw->pcm_ops->get_buffer_out(hw, &size); - if (!buf) { - /* retrying will likely won't help, drop everything. */ - hw->mix_buf->pos = (hw->mix_buf->pos + live) % hw->mix_buf->size; - return clipped + live; + if (!buf || size == 0) { + break; } decr = MIN(size / hw->info.bytes_per_frame, live); diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c index c265c0094b..bd57082a8d 100644 --- a/audio/dsoundaudio.c +++ b/audio/dsoundaudio.c @@ -53,12 +53,14 @@ typedef struct { typedef struct { HWVoiceOut hw; LPDIRECTSOUNDBUFFER dsound_buffer; + bool first_time; dsound *s; } DSoundVoiceOut; typedef struct { HWVoiceIn hw; LPDIRECTSOUNDCAPTUREBUFFER dsound_capture_buffer; + bool first_time; dsound *s; } DSoundVoiceIn; @@ -414,21 +416,32 @@ 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, wpos, act_size; size_t req_size; int err; void *ret; - hr = IDirectSoundBuffer_GetCurrentPosition(dsb, &ppos, NULL); + hr = IDirectSoundBuffer_GetCurrentPosition( + dsb, &ppos, ds->first_time ? &wpos : NULL); if (FAILED(hr)) { dsound_logerr(hr, "Could not get playback buffer position\n"); *size = 0; return NULL; } + if (ds->first_time) { + hw->pos_emul = wpos; + ds->first_time = false; + } + req_size = audio_ring_dist(ppos, hw->pos_emul, hw->size_emul); req_size = MIN(req_size, hw->size_emul - hw->pos_emul); + if (req_size == 0) { + *size = 0; + return NULL; + } + err = dsound_lock_out(dsb, &hw->info, hw->pos_emul, req_size, &ret, NULL, &act_size, NULL, false, ds->s); if (err) { @@ -508,18 +521,24 @@ static void *dsound_get_buffer_in(HWVoiceIn *hw, size_t *size) DSoundVoiceIn *ds = (DSoundVoiceIn *) hw; LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer; HRESULT hr; - DWORD cpos, act_size; + DWORD cpos, rpos, act_size; size_t req_size; int err; void *ret; - hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, &cpos, NULL); + hr = IDirectSoundCaptureBuffer_GetCurrentPosition( + dscb, &cpos, ds->first_time ? &rpos : NULL); if (FAILED(hr)) { dsound_logerr(hr, "Could not get capture buffer position\n"); *size = 0; return NULL; } + if (ds->first_time) { + hw->pos_emul = rpos; + ds->first_time = false; + } + req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul); req_size = MIN(req_size, hw->size_emul - hw->pos_emul);
Windows (unlike wine) bails out when IDirectSoundBuffer8::Lock is called with zero length. Also, hw->pos_emul handling was incorrect when calling this function for the first time. Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> Reported-by: KJ Liew <liewkj@yahoo.com> --- I've tested this patch on wine and a borrowed Windows 8.1 laptop, I could only test audio playback, not recording. I've cross-compiled qemu using the docker image, for 64-bit. --- audio/dsound_template.h | 1 + audio/audio.c | 6 ++---- audio/dsoundaudio.c | 27 +++++++++++++++++++++++---- 3 files changed, 26 insertions(+), 8 deletions(-)