Re: [PATCH] audio/dsound: fix invalid parameters error

[Howard Spoelstra <hsp.cat7@gmail.com>]

[-- 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 --]