[PATCH 5/6] dsound: rework ugly mixer logic

[Andrew Eikum]

Mostly good cleanup in this one. Some thoughts below...

On Tue, Oct 16, 2012 at 02:06:29PM +0200, Maarten Lankhorst wrote:
> diff --git a/dlls/dsound/dsound_private.h b/dlls/dsound/dsound_private.h
> index feef787..7817b88 100644
> --- a/dlls/dsound/dsound_private.h
> +++ b/dlls/dsound/dsound_private.h
> @@ -73,10 +73,8 @@ struct DirectSoundDevice
> -    DWORD                       writelead, buflen, state, playpos, mixpos;
> +    DWORD                       writelead, buflen, aclen, fraglen, state, playpos, pad;

If you're introducing new variables, surely you can come up with
something more descriptive than "aclen." Something like
"ac_buffer_frames," maybe? I'm a big fan of units on my variable
names so mistakes like "pos_bytes = ac_frames;" are obvious.

Similar for "pad," maybe something like "in_mmdev_bytes" (Is that
actually used any more after your patches? You could re-use it if
not.)

> @@ -209,7 +207,6 @@ HRESULT DSOUND_PrimaryCreate(DirectSoundDevice *device) DECLSPEC_HIDDEN;
>  HRESULT DSOUND_PrimaryDestroy(DirectSoundDevice *device) DECLSPEC_HIDDEN;
>  HRESULT DSOUND_PrimaryPlay(DirectSoundDevice *device) DECLSPEC_HIDDEN;
>  HRESULT DSOUND_PrimaryStop(DirectSoundDevice *device) DECLSPEC_HIDDEN;
> -HRESULT DSOUND_PrimaryGetPosition(DirectSoundDevice *device, LPDWORD playpos, LPDWORD writepos) DECLSPEC_HIDDEN;

Good riddance.

> @@ -682,147 +623,100 @@ static void DSOUND_PerformMix(DirectSoundDevice *device)
>  		LeaveCriticalSection(&device->mixlock);
>  		return;
>  	}
> -
> -	to_mix_frags = device->prebuf - (pad * device->pwfx->nBlockAlign + device->fraglen - 1) / device->fraglen;
> -
> -	to_mix_bytes = to_mix_frags * device->fraglen;
> -
> -	if(device->in_mmdev_bytes > 0){
> -		DWORD delta_bytes = min(to_mix_bytes, device->in_mmdev_bytes);
> -		device->in_mmdev_bytes -= delta_bytes;
> -		device->playing_offs_bytes += delta_bytes;
> -		device->playing_offs_bytes %= device->buflen;
> +	block = device->pwfx->nBlockAlign;

Bleh. Do we really need to alias that?

> +	if (maxq > device->fraglen * 3)
> +		maxq = device->fraglen * 3;
> +

This seems to be replacing ds_snd_queue_max, right? Why remove the
configurability? Why 3 instead of the old default of 10? This should
be a separate patch at least.

> +	writepos = (device->playpos + pad) % device->buflen;
>  
>  	if (device->priolevel != DSSCL_WRITEPRIMARY) {
> -		BOOL recover = FALSE, all_stopped = FALSE;
> -		DWORD playpos, writepos, writelead, maxq, prebuff_max, prebuff_left, size1, size2;
> -		LPVOID buf1, buf2;
> +		BOOL all_stopped = FALSE;
>  		int nfiller;
> +		BOOL native = device->normfunction == normfunctions[4];
> +		DWORD bpp = device->pwfx->wBitsPerSample>>3;

Again, do we need to alias that?

> -static DWORD DSOUND_fraglen(DirectSoundDevice *device)
> -{
> -    REFERENCE_TIME period;
> -    HRESULT hr;
> -    DWORD ret;
> -
> -    hr = IAudioClient_GetDevicePeriod(device->client, &period, NULL);
> -    if(FAILED(hr)){
> -        /* just guess at 10ms */
> -        WARN("GetDevicePeriod failed: %08x\n", hr);
> -        ret = MulDiv(device->pwfx->nBlockAlign, device->pwfx->nSamplesPerSec, 100);
> -    }else
> -        ret = MulDiv(device->pwfx->nSamplesPerSec * device->pwfx->nBlockAlign, period, 10000000);
> -
> -    ret -= ret % device->pwfx->nBlockAlign;
> -    return ret;
> -}
> -
...
> +    device->fraglen = MulDiv(device->pwfx->nSamplesPerSec, period, 10000000) * device->pwfx->nBlockAlign;

This should be a separate patch. I don't have an argument /against/
it, but why do you prefer 10ms over whatever the driver prefers?

Andrew