[PATCH v6] dsound: Commit next audio chunk between play cursor and write cursor to playing.
Andrew Eikum
aeikum at codeweavers.com
Mon Sep 20 15:14:14 CDT 2021
Signed-off-by: Andrew Eikum <aeikum at codeweavers.com>
On Tue, Sep 14, 2021 at 01:05:18PM +0300, Eduard Permyakov wrote:
> This region of the audio buffer is forbidden to be written to by the
> DirectSound specification. The documentation states: "The write cursor
> is the point after which it is safe to write data into the buffer. The
> block between the play cursor and the write cursor is already committed
> to be played, and cannot be changed safely." However, some applications
> still do this, which has lead to audio glitches only when using the Wine
> DirectSound implementation. Experiments showed that the native DirctSound
> implementation will still play the old audio the first time around when the
> buffer region gets overwritten. Use an approach of copying the next forbidden
> region into a "committed buffer" to add the same behavior to the Wine
> implementation.
>
> Out of performance considerations, only copy data to the committed buffer
> when we detect that an overwrite is possible (i.e. the current mixing
> region of the buffer gets locked).
>
> Signed-off-by: Eduard Permyakov <epermyakov at codeweavers.com>
> ---
> dlls/dsound/buffer.c | 66 ++++++++++++++++++++++++++++++++++--
> dlls/dsound/dsound_convert.c | 33 +++++++++---------
> dlls/dsound/dsound_private.h | 11 ++++--
> dlls/dsound/mixer.c | 48 ++++++++++++++++++++------
> 4 files changed, 125 insertions(+), 33 deletions(-)
>
> diff --git a/dlls/dsound/buffer.c b/dlls/dsound/buffer.c
> index dc3b54906ce..c595b19dd63 100644
> --- a/dlls/dsound/buffer.c
> +++ b/dlls/dsound/buffer.c
> @@ -101,6 +101,27 @@ static int __cdecl notify_compar(const void *l, const void *r)
> return 1;
> }
>
> +static void commit_next_chunk(IDirectSoundBufferImpl *dsb)
> +{
> + void *dstbuff = dsb->committedbuff, *srcbuff = dsb->buffer->memory;
> + DWORD srcoff = dsb->sec_mixpos, srcsize = dsb->buflen, cpysize = dsb->writelead;
> +
> + if(dsb->state != STATE_PLAYING)
> + return;
> +
> + if(cpysize > srcsize - srcoff) {
> + DWORD overflow = cpysize - (srcsize - srcoff);
> + memcpy(dstbuff, (BYTE*)srcbuff + srcoff, srcsize - srcoff);
> + memcpy((BYTE*)dstbuff + (srcsize - srcoff), srcbuff, overflow);
> + }else{
> + memcpy(dstbuff, (BYTE*)srcbuff + srcoff, cpysize);
> + }
> +
> + dsb->use_committed = TRUE;
> + dsb->committed_mixpos = 0;
> + TRACE("committing %u bytes from offset %u\n", dsb->writelead, dsb->sec_mixpos);
> +}
> +
> static HRESULT WINAPI IDirectSoundNotifyImpl_SetNotificationPositions(IDirectSoundNotify *iface,
> DWORD howmuch, const DSBPOSITIONNOTIFY *notify)
> {
> @@ -244,6 +265,7 @@ static HRESULT WINAPI IDirectSoundBufferImpl_SetFrequency(IDirectSoundBuffer8 *i
> {
> IDirectSoundBufferImpl *This = impl_from_IDirectSoundBuffer8(iface);
> DWORD oldFreq;
> + void *newcommitted;
>
> TRACE("(%p,%d)\n",This,freq);
>
> @@ -274,6 +296,13 @@ static HRESULT WINAPI IDirectSoundBufferImpl_SetFrequency(IDirectSoundBuffer8 *i
> This->freqAdjustDen = This->device->pwfx->nSamplesPerSec;
> This->nAvgBytesPerSec = freq * This->pwfx->nBlockAlign;
> DSOUND_RecalcFormat(This);
> +
> + newcommitted = HeapReAlloc(GetProcessHeap(), 0, This->committedbuff, This->writelead);
> + if(!newcommitted) {
> + ReleaseSRWLockExclusive(&This->lock);
> + return DSERR_OUTOFMEMORY;
> + }
> + This->committedbuff = newcommitted;
> }
>
> ReleaseSRWLockExclusive(&This->lock);
> @@ -319,6 +348,8 @@ static HRESULT WINAPI IDirectSoundBufferImpl_Stop(IDirectSoundBuffer8 *iface)
> if (This->state == STATE_PLAYING || This->state == STATE_STARTING)
> {
> This->state = STATE_STOPPED;
> + This->use_committed = FALSE;
> + This->committed_mixpos = 0;
> DSOUND_CheckEvent(This, 0, 0);
> }
>
> @@ -503,8 +534,10 @@ static HRESULT WINAPI IDirectSoundBufferImpl_Lock(IDirectSoundBuffer8 *iface, DW
>
> if (writecursor+writebytes <= This->buflen) {
> *(LPBYTE*)lplpaudioptr1 = This->buffer->memory+writecursor;
> - if (This->sec_mixpos >= writecursor && This->sec_mixpos < writecursor + writebytes && This->state == STATE_PLAYING)
> + if (This->sec_mixpos >= writecursor && This->sec_mixpos < writecursor + writebytes && This->state == STATE_PLAYING) {
> WARN("Overwriting mixing position, case 1\n");
> + commit_next_chunk(This);
> + }
> *audiobytes1 = writebytes;
> if (lplpaudioptr2)
> *(LPBYTE*)lplpaudioptr2 = NULL;
> @@ -519,16 +552,20 @@ static HRESULT WINAPI IDirectSoundBufferImpl_Lock(IDirectSoundBuffer8 *iface, DW
> *(LPBYTE*)lplpaudioptr1 = This->buffer->memory+writecursor;
> *audiobytes1 = This->buflen-writecursor;
> This->buffer->lockedbytes += *audiobytes1;
> - if (This->sec_mixpos >= writecursor && This->sec_mixpos < writecursor + writebytes && This->state == STATE_PLAYING)
> + if (This->sec_mixpos >= writecursor && This->sec_mixpos < writecursor + writebytes && This->state == STATE_PLAYING) {
> WARN("Overwriting mixing position, case 2\n");
> + commit_next_chunk(This);
> + }
> if (lplpaudioptr2)
> *(LPBYTE*)lplpaudioptr2 = This->buffer->memory;
> if (audiobytes2) {
> *audiobytes2 = writebytes-(This->buflen-writecursor);
> This->buffer->lockedbytes += *audiobytes2;
> }
> - if (audiobytes2 && This->sec_mixpos < remainder && This->state == STATE_PLAYING)
> + if (audiobytes2 && This->sec_mixpos < remainder && This->state == STATE_PLAYING) {
> WARN("Overwriting mixing position, case 3\n");
> + commit_next_chunk(This);
> + }
> TRACE("Locked %p(%i bytes) and %p(%i bytes) writecursor=%d\n", *(LPBYTE*)lplpaudioptr1, *audiobytes1, lplpaudioptr2 ? *(LPBYTE*)lplpaudioptr2 : NULL, audiobytes2 ? *audiobytes2: 0, writecursor);
> }
>
> @@ -552,6 +589,9 @@ static HRESULT WINAPI IDirectSoundBufferImpl_SetCurrentPosition(IDirectSoundBuff
> newpos -= newpos%This->pwfx->nBlockAlign;
> This->sec_mixpos = newpos;
>
> + This->use_committed = FALSE;
> + This->committed_mixpos = 0;
> +
> /* at this point, do not attempt to reset buffers, mess with primary mix position,
> or anything like that to reduce latency. The data already prebuffered cannot be changed */
>
> @@ -1081,6 +1121,12 @@ HRESULT secondarybuffer_create(DirectSoundDevice *device, const DSBUFFERDESC *ds
> /* calculate fragment size and write lead */
> DSOUND_RecalcFormat(dsb);
>
> + dsb->committedbuff = HeapAlloc(GetProcessHeap(), 0, dsb->writelead);
> + if(!dsb->committedbuff) {
> + IDirectSoundBuffer8_Release(&dsb->IDirectSoundBuffer8_iface);
> + return DSERR_OUTOFMEMORY;
> + }
> +
> if (dsb->dsbd.dwFlags & DSBCAPS_CTRL3D) {
> dsb->ds3db_ds3db.dwSize = sizeof(DS3DBUFFER);
> dsb->ds3db_ds3db.vPosition.x = 0.0;
> @@ -1135,6 +1181,7 @@ void secondarybuffer_destroy(IDirectSoundBufferImpl *This)
>
> HeapFree(GetProcessHeap(), 0, This->notifies);
> HeapFree(GetProcessHeap(), 0, This->pwfx);
> + HeapFree(GetProcessHeap(), 0, This->committedbuff);
>
> if (This->filters) {
> int i;
> @@ -1157,6 +1204,7 @@ HRESULT IDirectSoundBufferImpl_Duplicate(
> {
> IDirectSoundBufferImpl *dsb;
> HRESULT hres = DS_OK;
> + VOID *committedbuff;
> TRACE("(%p,%p,%p)\n", device, ppdsb, pdsb);
>
> dsb = HeapAlloc(GetProcessHeap(),0,sizeof(*dsb));
> @@ -1166,6 +1214,13 @@ HRESULT IDirectSoundBufferImpl_Duplicate(
> return DSERR_OUTOFMEMORY;
> }
>
> + committedbuff = HeapAlloc(GetProcessHeap(),0,pdsb->writelead);
> + if (committedbuff == NULL) {
> + HeapFree(GetProcessHeap(),0,dsb);
> + *ppdsb = NULL;
> + return DSERR_OUTOFMEMORY;
> + }
> +
> AcquireSRWLockShared(&pdsb->lock);
>
> CopyMemory(dsb, pdsb, sizeof(*dsb));
> @@ -1175,6 +1230,7 @@ HRESULT IDirectSoundBufferImpl_Duplicate(
> ReleaseSRWLockShared(&pdsb->lock);
>
> if (dsb->pwfx == NULL) {
> + HeapFree(GetProcessHeap(),0,committedbuff);
> HeapFree(GetProcessHeap(),0,dsb);
> *ppdsb = NULL;
> return DSERR_OUTOFMEMORY;
> @@ -1192,6 +1248,9 @@ HRESULT IDirectSoundBufferImpl_Duplicate(
> dsb->notifies = NULL;
> dsb->nrofnotifies = 0;
> dsb->device = device;
> + dsb->committedbuff = committedbuff;
> + dsb->use_committed = FALSE;
> + dsb->committed_mixpos = 0;
> DSOUND_RecalcFormat(dsb);
>
> InitializeSRWLock(&dsb->lock);
> @@ -1202,6 +1261,7 @@ HRESULT IDirectSoundBufferImpl_Duplicate(
> list_remove(&dsb->entry);
> dsb->buffer->ref--;
> HeapFree(GetProcessHeap(),0,dsb->pwfx);
> + HeapFree(GetProcessHeap(),0,dsb->committedbuff);
> HeapFree(GetProcessHeap(),0,dsb);
> dsb = NULL;
> }else
> diff --git a/dlls/dsound/dsound_convert.c b/dlls/dsound/dsound_convert.c
> index 4735178a9a8..67344565860 100644
> --- a/dlls/dsound/dsound_convert.c
> +++ b/dlls/dsound/dsound_convert.c
> @@ -55,26 +55,25 @@ WINE_DEFAULT_DEBUG_CHANNEL(dsound);
> #define le32(x) (x)
> #endif
>
> -static float get8(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel)
> +static float get8(const IDirectSoundBufferImpl *dsb, BYTE *base, DWORD channel)
> {
> - const BYTE* buf = dsb->buffer->memory;
> - buf += pos + channel;
> + const BYTE *buf = base + channel;
> return (buf[0] - 0x80) / (float)0x80;
> }
>
> -static float get16(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel)
> +static float get16(const IDirectSoundBufferImpl *dsb, BYTE *base, DWORD channel)
> {
> - const BYTE* buf = dsb->buffer->memory;
> - const SHORT *sbuf = (const SHORT*)(buf + pos + 2 * channel);
> + const BYTE *buf = base + 2 * channel;
> + const SHORT *sbuf = (const SHORT*)(buf);
> SHORT sample = (SHORT)le16(*sbuf);
> return sample / (float)0x8000;
> }
>
> -static float get24(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel)
> +static float get24(const IDirectSoundBufferImpl *dsb, BYTE *base, DWORD channel)
> {
> LONG sample;
> - const BYTE* buf = dsb->buffer->memory;
> - buf += pos + 3 * channel;
> + const BYTE *buf = base + 3 * channel;
> +
> /* The next expression deliberately has an overflow for buf[2] >= 0x80,
> this is how negative values are made.
> */
> @@ -82,32 +81,32 @@ static float get24(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel)
> return sample / (float)0x80000000U;
> }
>
> -static float get32(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel)
> +static float get32(const IDirectSoundBufferImpl *dsb, BYTE *base, DWORD channel)
> {
> - const BYTE* buf = dsb->buffer->memory;
> - const LONG *sbuf = (const LONG*)(buf + pos + 4 * channel);
> + const BYTE *buf = base + 4 * channel;
> + const LONG *sbuf = (const LONG*)(buf);
> LONG sample = le32(*sbuf);
> return sample / (float)0x80000000U;
> }
>
> -static float getieee32(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel)
> +static float getieee32(const IDirectSoundBufferImpl *dsb, BYTE *base, DWORD channel)
> {
> - const BYTE* buf = dsb->buffer->memory;
> - const float *sbuf = (const float*)(buf + pos + 4 * channel);
> + const BYTE *buf = base + 4 * channel;
> + const float *sbuf = (const float*)(buf);
> /* The value will be clipped later, when put into some non-float buffer */
> return *sbuf;
> }
>
> const bitsgetfunc getbpp[5] = {get8, get16, get24, get32, getieee32};
>
> -float get_mono(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel)
> +float get_mono(const IDirectSoundBufferImpl *dsb, BYTE *base, DWORD channel)
> {
> DWORD channels = dsb->pwfx->nChannels;
> DWORD c;
> float val = 0;
> /* XXX: does Windows include LFE into the mix? */
> for (c = 0; c < channels; c++)
> - val += dsb->get_aux(dsb, pos, c);
> + val += dsb->get_aux(dsb, base, c);
> val /= channels;
> return val;
> }
> diff --git a/dlls/dsound/dsound_private.h b/dlls/dsound/dsound_private.h
> index bdb9ebee544..9ec96504a50 100644
> --- a/dlls/dsound/dsound_private.h
> +++ b/dlls/dsound/dsound_private.h
> @@ -43,7 +43,7 @@ typedef struct IDirectSoundBufferImpl IDirectSoundBufferImpl;
> typedef struct DirectSoundDevice DirectSoundDevice;
>
> /* dsound_convert.h */
> -typedef float (*bitsgetfunc)(const IDirectSoundBufferImpl *, DWORD, DWORD);
> +typedef float (*bitsgetfunc)(const IDirectSoundBufferImpl *, BYTE *, DWORD);
> typedef void (*bitsputfunc)(const IDirectSoundBufferImpl *, DWORD, DWORD, float);
> extern const bitsgetfunc getbpp[5] DECLSPEC_HIDDEN;
> void putieee32(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel, float value) DECLSPEC_HIDDEN;
> @@ -153,7 +153,12 @@ struct IDirectSoundBufferImpl
> LONG64 freqAccNum;
> /* used for mixing */
> DWORD sec_mixpos;
> -
> + /* Holds a copy of the next 'writelead' bytes, to be used for mixing. This makes it
> + * so that these bytes get played once even if this region of the buffer gets overwritten,
> + * which is more in-line with native DirectSound behavior. */
> + BOOL use_committed;
> + LPVOID committedbuff;
> + DWORD committed_mixpos;
> /* IDirectSoundNotify fields */
> LPDSBPOSITIONNOTIFY notifies;
> int nrofnotifies;
> @@ -171,7 +176,7 @@ struct IDirectSoundBufferImpl
> struct list entry;
> };
>
> -float get_mono(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel) DECLSPEC_HIDDEN;
> +float get_mono(const IDirectSoundBufferImpl *dsb, BYTE *base, DWORD channel) DECLSPEC_HIDDEN;
> void put_mono2stereo(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel, float value) DECLSPEC_HIDDEN;
> void put_mono2quad(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel, float value) DECLSPEC_HIDDEN;
> void put_stereo2quad(const IDirectSoundBufferImpl *dsb, DWORD pos, DWORD channel, float value) DECLSPEC_HIDDEN;
> diff --git a/dlls/dsound/mixer.c b/dlls/dsound/mixer.c
> index 124d1e4fba1..e9d8a6e3cdd 100644
> --- a/dlls/dsound/mixer.c
> +++ b/dlls/dsound/mixer.c
> @@ -276,22 +276,35 @@ void DSOUND_CheckEvent(const IDirectSoundBufferImpl *dsb, DWORD playpos, int len
> }
>
> static inline float get_current_sample(const IDirectSoundBufferImpl *dsb,
> - DWORD mixpos, DWORD channel)
> + BYTE *buffer, DWORD buflen, DWORD mixpos, DWORD channel)
> {
> - if (mixpos >= dsb->buflen && !(dsb->playflags & DSBPLAY_LOOPING))
> + if (mixpos >= buflen && !(dsb->playflags & DSBPLAY_LOOPING))
> return 0.0f;
> - return dsb->get(dsb, mixpos % dsb->buflen, channel);
> + return dsb->get(dsb, buffer + (mixpos % buflen), channel);
> }
>
> static UINT cp_fields_noresample(IDirectSoundBufferImpl *dsb, UINT count)
> {
> UINT istride = dsb->pwfx->nBlockAlign;
> UINT ostride = dsb->device->pwfx->nChannels * sizeof(float);
> + UINT committed_samples = 0;
> DWORD channel, i;
> - for (i = 0; i < count; i++)
> +
> + if(dsb->use_committed) {
> + committed_samples = (dsb->writelead - dsb->committed_mixpos) / istride;
> + committed_samples = committed_samples <= count ? committed_samples : count;
> + }
> +
> + for (i = 0; i < committed_samples; i++)
> + for (channel = 0; channel < dsb->mix_channels; channel++)
> + dsb->put(dsb, i * ostride, channel, get_current_sample(dsb, dsb->committedbuff,
> + dsb->writelead, dsb->committed_mixpos + i * istride, channel));
> +
> + for (; i < count; i++)
> for (channel = 0; channel < dsb->mix_channels; channel++)
> - dsb->put(dsb, i * ostride, channel, get_current_sample(dsb,
> - dsb->sec_mixpos + i * istride, channel));
> + dsb->put(dsb, i * ostride, channel, get_current_sample(dsb, dsb->buffer->memory,
> + dsb->buflen, dsb->sec_mixpos + i * istride, channel));
> +
> return count;
> }
>
> @@ -300,6 +313,7 @@ static UINT cp_fields_resample(IDirectSoundBufferImpl *dsb, UINT count, LONG64 *
> UINT i, channel;
> UINT istride = dsb->pwfx->nBlockAlign;
> UINT ostride = dsb->device->pwfx->nChannels * sizeof(float);
> + UINT committed_samples = 0;
>
> LONG64 freqAcc_start = *freqAccNum;
> LONG64 freqAcc_end = freqAcc_start + count * dsb->freqAdjustNum;
> @@ -326,16 +340,24 @@ static UINT cp_fields_resample(IDirectSoundBufferImpl *dsb, UINT count, LONG64 *
> fir_copy = dsb->device->cp_buffer;
> intermediate = fir_copy + fir_cachesize;
>
> + if(dsb->use_committed) {
> + committed_samples = (dsb->writelead - dsb->committed_mixpos) / istride;
> + committed_samples = committed_samples <= required_input ? committed_samples : required_input;
> + }
>
> /* Important: this buffer MUST be non-interleaved
> * if you want -msse3 to have any effect.
> * This is good for CPU cache effects, too.
> */
> itmp = intermediate;
> - for (channel = 0; channel < channels; channel++)
> - for (i = 0; i < required_input; i++)
> - *(itmp++) = get_current_sample(dsb,
> - dsb->sec_mixpos + i * istride, channel);
> + for (channel = 0; channel < channels; channel++) {
> + for (i = 0; i < committed_samples; i++)
> + *(itmp++) = get_current_sample(dsb, dsb->committedbuff,
> + dsb->writelead, dsb->committed_mixpos + i * istride, channel);
> + for (; i < required_input; i++)
> + *(itmp++) = get_current_sample(dsb, dsb->buffer->memory,
> + dsb->buflen, dsb->sec_mixpos + i * istride, channel);
> + }
>
> for(i = 0; i < count; ++i) {
> UINT int_fir_steps = (freqAcc_start + i * dsb->freqAdjustNum) * dsbfirstep / dsb->freqAdjustDen;
> @@ -389,6 +411,12 @@ static void cp_fields(IDirectSoundBufferImpl *dsb, UINT count, LONG64 *freqAccNu
> }
>
> dsb->sec_mixpos = ipos;
> +
> + if(dsb->use_committed) {
> + dsb->committed_mixpos += adv * dsb->pwfx->nBlockAlign;
> + if(dsb->committed_mixpos >= dsb->writelead)
> + dsb->use_committed = FALSE;
> + }
> }
>
> /**
> --
> 2.25.1
>
>
More information about the wine-devel
mailing list