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