* [PATCH] audio/dsound: fix invalid parameters error
@ 2020-02-02 23:02 Kővágó, Zoltán
2020-02-03 7:56 ` Howard Spoelstra
2020-02-06 13:38 ` Gerd Hoffmann
0 siblings, 2 replies; 10+ messages in thread
From: Kővágó, Zoltán @ 2020-02-02 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, KJ Liew
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] audio/dsound: fix invalid parameters error
2020-02-02 23:02 [PATCH] audio/dsound: fix invalid parameters error Kővágó, Zoltán
@ 2020-02-03 7:56 ` Howard Spoelstra
2020-02-03 9:18 ` Philippe Mathieu-Daudé
2020-02-06 13:38 ` Gerd Hoffmann
1 sibling, 1 reply; 10+ messages in thread
From: Howard Spoelstra @ 2020-02-03 7:56 UTC (permalink / raw)
To: Kővágó, Zoltán
Cc: KJ Liew, qemu-devel qemu-devel, Gerd Hoffmann
[-- Attachment #1: Type: text/plain, Size: 4900 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 6212 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] audio/dsound: fix invalid parameters error
2020-02-03 7:56 ` Howard Spoelstra
@ 2020-02-03 9:18 ` Philippe Mathieu-Daudé
2020-02-03 9:25 ` Howard Spoelstra
0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-03 9:18 UTC (permalink / raw)
To: Howard Spoelstra, Kővágó, Zoltán
Cc: Gerd Hoffmann, qemu-devel qemu-devel, KJ Liew
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] audio/dsound: fix invalid parameters error
2020-02-03 9:18 ` Philippe Mathieu-Daudé
@ 2020-02-03 9:25 ` Howard Spoelstra
0 siblings, 0 replies; 10+ messages in thread
From: Howard Spoelstra @ 2020-02-03 9:25 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: KJ Liew, Gerd Hoffmann, qemu-devel qemu-devel,
Kővágó,
Zoltán
[-- Attachment #1: Type: text/plain, Size: 6218 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 8768 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] audio/dsound: fix invalid parameters error
2020-02-02 23:02 [PATCH] audio/dsound: fix invalid parameters error Kővágó, Zoltán
2020-02-03 7:56 ` Howard Spoelstra
@ 2020-02-06 13:38 ` Gerd Hoffmann
1 sibling, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2020-02-06 13:38 UTC (permalink / raw)
To: Kővágó, Zoltán; +Cc: qemu-devel, KJ Liew
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] audio/dsound: fix invalid parameters error
2020-01-27 1:46 ` Zoltán Kővágó
2020-01-31 7:55 ` Gerd Hoffmann
@ 2020-02-01 18:28 ` KJ Liew
1 sibling, 0 replies; 10+ messages in thread
From: KJ Liew @ 2020-02-01 18:28 UTC (permalink / raw)
To: Zoltán Kővágó
Cc: Philippe Mathieu-Daudé, QEMU Developers, Gerd Hoffmann
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");
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] audio/dsound: fix invalid parameters error
2020-01-27 1:46 ` Zoltán Kővágó
@ 2020-01-31 7:55 ` Gerd Hoffmann
2020-02-01 18:28 ` KJ Liew
1 sibling, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2020-01-31 7:55 UTC (permalink / raw)
To: Zoltán Kővágó
Cc: Philippe Mathieu-Daudé, QEMU Developers, KJ Liew
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] audio/dsound: fix invalid parameters error
2020-01-18 6:30 ` Philippe Mathieu-Daudé
@ 2020-01-27 1:46 ` Zoltán Kővágó
2020-01-31 7:55 ` Gerd Hoffmann
2020-02-01 18:28 ` KJ Liew
0 siblings, 2 replies; 10+ messages in thread
From: Zoltán Kővágó @ 2020-01-27 1:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, KJ Liew, QEMU Developers, Gerd Hoffmann
[-- Attachment #1: Type: text/plain, Size: 4917 bytes --]
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
[-- Attachment #2: dsound.patch --]
[-- Type: text/x-patch, Size: 2389 bytes --]
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");
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] audio/dsound: fix invalid parameters error
2020-01-17 18:26 ` KJ Liew
@ 2020-01-18 6:30 ` Philippe Mathieu-Daudé
2020-01-27 1:46 ` Zoltán Kővágó
0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-18 6:30 UTC (permalink / raw)
To: KJ Liew, qemu-devel, Kővágó, Zoltán, Gerd Hoffmann
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;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] audio/dsound: fix invalid parameters error
[not found] <20200117182621.GB13724.ref@chuwi-lt0>
@ 2020-01-17 18:26 ` KJ Liew
2020-01-18 6:30 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 10+ messages in thread
From: KJ Liew @ 2020-01-17 18:26 UTC (permalink / raw)
To: qemu-devel
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.
--- ../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;
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-06 13:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-02 23:02 [PATCH] audio/dsound: fix invalid parameters error Kővágó, Zoltán
2020-02-03 7:56 ` Howard Spoelstra
2020-02-03 9:18 ` Philippe Mathieu-Daudé
2020-02-03 9:25 ` Howard Spoelstra
2020-02-06 13:38 ` Gerd Hoffmann
[not found] <20200117182621.GB13724.ref@chuwi-lt0>
2020-01-17 18:26 ` KJ Liew
2020-01-18 6:30 ` Philippe Mathieu-Daudé
2020-01-27 1:46 ` Zoltán Kővágó
2020-01-31 7:55 ` Gerd Hoffmann
2020-02-01 18:28 ` KJ Liew
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.