[PATCH v4 2/2] dsound: Make GlobalHandle() for the sound buffer failed.

[Akihiro Sagawa]

In v4, retire VirtualAlloc/VirtualFree and use HeapAlloc + offset
method, again.

On Mon, 02 Mar 2020 20:37:26 +0100, Alexandre Julliard wrote:
> Note that VirtualAlloc allocates in 64K chunks. Is the typical buffer
> size large enough to justify this?
> It turns out that VirtualAlloc's 64K chunk isn't suitable for dsound
> buffer. I examined some applications and they use 88,200, 176,400,
> 192,000 or 352,800 as a buffer size. In most cases, it's multiple of a
> sample size, i.e. 44,100 or 48,000. Though, it's often greater than 64K,
> it isn't large enough. I give up this way.

In this patch, I employ a pointer size alignment. Andrew, do we need to
allocate in larger chunk, e.g. 4K?

Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com>
---
 dlls/dsound/buffer.c       | 15 +++++----------
 dlls/dsound/tests/dsound.c |  2 +-
 2 files changed, 6 insertions(+), 11 deletions(-)

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=66517

Your paranoid android.


=== w8adm (32 bit report) ===

dsound:
dsound.c:1081: Test failed: got 00000000

[Andrew Eikum]

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>

On Fri, Mar 06, 2020 at 09:52:46PM +0900, Akihiro Sagawa wrote:
> In v4, retire VirtualAlloc/VirtualFree and use HeapAlloc + offset
> method, again.
>
> On Mon, 02 Mar 2020 20:37:26 +0100, Alexandre Julliard wrote:
> > Note that VirtualAlloc allocates in 64K chunks. Is the typical buffer
> > size large enough to justify this?
> > It turns out that VirtualAlloc's 64K chunk isn't suitable for dsound
> > buffer. I examined some applications and they use 88,200, 176,400,
> > 192,000 or 352,800 as a buffer size. In most cases, it's multiple of a
> > sample size, i.e. 44,100 or 48,000. Though, it's often greater than 64K,
> > it isn't large enough. I give up this way.
>
> In this patch, I employ a pointer size alignment. Andrew, do we need to
> allocate in larger chunk, e.g. 4K?
>
> Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com>
> ---
>  dlls/dsound/buffer.c       | 15 +++++----------
>  dlls/dsound/tests/dsound.c |  2 +-
>  2 files changed, 6 insertions(+), 11 deletions(-)
> diff --git a/dlls/dsound/buffer.c b/dlls/dsound/buffer.c
> index 8d4a3b9..ea428c0 100644
> --- a/dlls/dsound/buffer.c
> +++ b/dlls/dsound/buffer.c
> @@ -999,6 +999,7 @@ HRESULT secondarybuffer_create(DirectSoundDevice *device, const DSBUFFERDESC *ds
>   LPWAVEFORMATEX wfex = dsbd->lpwfxFormat;
>   HRESULT err = DS_OK;
>   DWORD capf = 0;
> +	size_t bufsize;
>
> TRACE("(%p,%p,%p)\n", device, dsbd, buffer);
>
> @@ -1054,19 +1055,15 @@ HRESULT secondarybuffer_create(DirectSoundDevice *device, const DSBUFFERDESC *ds
>   TRACE("capf = 0x%08x, device->drvcaps.dwFlags = 0x%08x\n", capf, device->drvcaps.dwFlags);
>
> /* Allocate an empty buffer */
> -	dsb->buffer = HeapAlloc(GetProcessHeap(),0,sizeof(*(dsb->buffer)));
> +	bufsize = (sizeof(*(dsb->buffer)) + sizeof(void *) - 1) & ~(sizeof(void *) - 1);
> +	dsb->buffer = HeapAlloc(GetProcessHeap(),0,bufsize + dsb->buflen);
>          if (!dsb->buffer) {
>                  IDirectSoundBuffer8_Release(&dsb->IDirectSoundBuffer8_iface);
>       return DSERR_OUTOFMEMORY;
>   }
>
> /* Allocate system memory for buffer */
> -	dsb->buffer->memory = HeapAlloc(GetProcessHeap(),0,dsb->buflen);
> -        if (!dsb->buffer->memory) {
> -		WARN("out of memory\n");
> -                IDirectSoundBuffer8_Release(&dsb->IDirectSoundBuffer8_iface);
> -		return DSERR_OUTOFMEMORY;
> -	}
> +	dsb->buffer->memory = (BYTE *)dsb->buffer + bufsize;
>
> dsb->buffer->ref = 1;
>   dsb->buffer->lockedbytes = 0;
> @@ -1136,10 +1133,8 @@ void secondarybuffer_destroy(IDirectSoundBufferImpl *This)
>
> This->buffer->ref--;
>      list_remove(&This->entry);
> -    if (This->buffer->ref == 0) {
> -        HeapFree(GetProcessHeap(), 0, This->buffer->memory);
> +    if (This->buffer->ref == 0)
>          HeapFree(GetProcessHeap(), 0, This->buffer);
> -    }
>
> HeapFree(GetProcessHeap(), 0, This->notifies);
>      HeapFree(GetProcessHeap(), 0, This->pwfx);
> diff --git a/dlls/dsound/tests/dsound.c b/dlls/dsound/tests/dsound.c
> index 0e77961..2bf87eb 100644
> --- a/dlls/dsound/tests/dsound.c
> +++ b/dlls/dsound/tests/dsound.c
> @@ -1161,7 +1161,7 @@ static HRESULT test_duplicate(LPGUID lpGuid)
>                      /* broken apps like Asuka 120% Return BURNING Fest,
>                         pass the pointer to GlobalHandle. */
>                      HGLOBAL hmem = GlobalHandle(buf);
> -                    todo_wine ok(!hmem,"GlobalHandle should return NULL "
> +                    ok(!hmem,"GlobalHandle should return NULL "
>                         "for buffer %p, got %p\n",buf,hmem);
>                  }
>                  ZeroMemory(buf,bufsize);