Skip to content

Commit

Permalink
Replace static size to dynamic array for DSound
Browse files Browse the repository at this point in the history 
Both DS Buffer and Stream are now using C++ std vector instead of static
size array which is in C.

It should improve performance for lower-end computers. Before, high-end
computers will only notice very light performance impact. Now appear to
be no performance impact unless certain titles are preserving maximum
size.
  • Loading branch information
@RadWolfie
RadWolfie committed May 1, 2018
1 parent 43c77a1 commit 270a821
Showing 1 changed file with 56 additions and 76 deletions.
132 changes: 56 additions & 76 deletions src/CxbxKrnl/EmuDSound.cpp
View file
Expand Up @@ -177,14 +177,10 @@ XTL::X_XFileMediaObject::_vtbl XTL::X_XFileMediaObject::vtbl =
there is chance of failure which contain value greater than 0.
*/

// size of DirectSound cache max size
#define X_DIRECTSOUND_CACHE_MAX 0x800

// TODO: Both buffer and stream cache size need to merge as one, there is no such thing as 4094 SGE

// size of sound buffer cache (used for periodic sound buffer updates)
#define SOUNDBUFFER_CACHE_SIZE 0x800 //Maximum is 2047 SGE overall

// size of sound stream cache (used for periodic sound stream updates)
#define SOUNDSTREAM_CACHE_SIZE 0x800
#define X_DIRECTSOUND_CACHE_COUNT (g_pDSoundBufferCache.size() + g_pDSoundStreamCache.size())

//Currently disabled since below may not be needed since under -6,400 is just silence yet accepting up to -10,000
// Xbox to PC volume ratio format (-10,000 / -6,400 )
Expand All @@ -193,15 +189,18 @@ XTL::X_XFileMediaObject::_vtbl XTL::X_XFileMediaObject::vtbl =
// Xbox maximum synch playback audio
#define DSOUND_MAX_SYNCHPLAYBACK_AUDIO 29

#define vector_ds_buffer std::vector<XTL::X_CDirectSoundBuffer*>
#define vector_ds_stream std::vector<XTL::X_CDirectSoundStream*>

// Static Variable(s)
XBAudio g_XBAudio = XBAudio();
extern LPDIRECTSOUND8 g_pDSound8 = nullptr; //This is necessary in order to allow share with EmuDSoundInline.hpp
static LPDIRECTSOUNDBUFFER g_pDSoundPrimaryBuffer = nullptr;
//TODO: RadWolfie - How to implement support if primary does not permit it for DSP usage?
static LPDIRECTSOUNDBUFFER8 g_pDSoundPrimaryBuffer8 = nullptr;
static LPDIRECTSOUND3DLISTENER8 g_pDSoundPrimary3DListener8 = nullptr;
static XTL::X_CDirectSoundBuffer* g_pDSoundBufferCache[SOUNDBUFFER_CACHE_SIZE] = { 0 }; //Default initialize to all zero'd
static XTL::X_CDirectSoundStream* g_pDSoundStreamCache[SOUNDSTREAM_CACHE_SIZE] = { 0 }; //Default initialize to all zero'd
vector_ds_buffer g_pDSoundBufferCache;
vector_ds_stream g_pDSoundStreamCache;
static int g_bDSoundCreateCalled = FALSE;
unsigned int g_iDSoundSynchPlaybackCounter = 0;
// Managed memory xbox audio variables
Expand Down Expand Up @@ -281,24 +280,21 @@ HRESULT WINAPI XTL::EMUPATCH(DirectSoundCreate)
CxbxKrnlCleanup("g_pDSound8->SetCooperativeLevel Failed!");
}

int v = 0;
// clear sound buffer cache
XTL::X_CDirectSoundBuffer* *pDSBuffer = g_pDSoundBufferCache;
for (v = 0; v < SOUNDBUFFER_CACHE_SIZE; v++) {
if ((*pDSBuffer) == nullptr)
continue;
while (XTL::EMUPATCH(IDirectSoundBuffer_Release)((*pDSBuffer))) {};
g_pDSoundBufferCache[v] = 0;
vector_ds_buffer::iterator ppDSBuffer = g_pDSoundBufferCache.begin();
for (; ppDSBuffer != g_pDSoundBufferCache.end();) {
while (XTL::EMUPATCH(IDirectSoundBuffer_Release)((*ppDSBuffer))) {};

This comment has been minimized.

Sign in to view

Copy link
@PatrickvL

PatrickvL May 1, 2018

Member

Could it be possible that the interface in ppDSBuffer was already released somewhere else? If it's memory isn't overwritten yet, that would cause it's reference count to be zero, which would lead this loop to cycle through all 32 bit values before returning... That would be a noticeable pause indeed

This comment has been minimized.

Sign in to view

Copy link
@RadWolfie

RadWolfie May 1, 2018

Author Member

If buffer/stream class is released, it is erased from IDirectSoundBuffer_Release and CDirectSoundStream_Release (see the code further down). It does not re-release here unless there are existing buffer/stream class in used. Release is just a ref count decrement, the same way with AddRef increment the ref count.

When Release is returned 0, the class is in final release plus cannot be re-used forever. To re-use, we have to create a brand new buffer class from DirectSound function directly.

Beside that, DirectSound is only create once at the moment. It is not emulating to reset to default with brand new DirectSound.

This comment has been minimized.

Sign in to view

Copy link
@RadWolfie

RadWolfie May 1, 2018 •
edited

Author Member

Also if Release was already at 0, calling it again will cause exception anyway.

EDIT: I believe I see a bug, erase calls in here shouldn't be here. It need to be replace with begin function later on. Like I said, DirectSoundCreate emulation is only created once even afterward it will pass down the existing creation. Not the highest priority since it's inactive.

This comment has been minimized.

Sign in to view

Copy link
@PatrickvL

PatrickvL May 1, 2018

Member

I know all that, but thanks for describing it.
What you could try, is adding a safeguard on the return value of Release : if that's negative (or an unlikely high unsigned value), then the code hit an unexpected situation, which would be very helpful in finding a cause for the strange pause people report

This comment has been minimized.

Sign in to view

Copy link
@RadWolfie

RadWolfie May 1, 2018

Author Member

ULONG can have negative value? How so? 😉 Since you're so concern about the release. I had checked both release functions. Both are getting the value from host's release. IF it has 0, from host side, AND called release ONE more time. It will CAUSE exception, period. That's the way DirectSound design is. No safeguard requirement. The only thing I can do about this is just use != operator against 0UL.

P.S. When the release emulation has 0 from uRet, it is erased from the cache internally. It won't get called again since it is no longer exist.

P.S.S. This will not cause a pause since both buffer and stream are now in C++ using std vector if they're active.

This comment has been minimized.

Sign in to view

Copy link
@RadWolfie

RadWolfie May 1, 2018

Author Member

Could it be possible that the interface in ppDSBuffer was already released somewhere else?

The ONLY place it can be released is from the patch emulated release functions for both buffer and stream. There's nowhere else it can be released.

This comment has been minimized.

Sign in to view

Copy link
@PatrickvL

PatrickvL May 1, 2018 •
edited

Member

Oh I believe you, no question about that. The suggestion to test for strange return values from Release was merely to safeguard against any unexpected side-effect (for example when an unpatched function or rogue pointer causes the ref count to be changed in any way). Since you seem to say it won't help at all in finding the cause for pauses, I'll stop whining about it ;)

This comment has been minimized.

Sign in to view

Copy link
@RadWolfie

RadWolfie May 1, 2018

Author Member

Well, there is no controller unpatched function using these 3 classes will cause it to go rogue. Such as SetEG, SetVelocity, SetLoopRegion, etc. All LLE audio are handled by 3 classes, DirectSound, DS Buffer, and DS Stream. XACT, DirectMusic, etc are using DirectSound classes straightforward. Only difference is the timing from parent class function, such as XACT, are directly accessing to LLE audio section. It is patched and forwarded thanks to @LukeUsher implement support.

One thing I do know, both creation and release patches for stream and buffer are 100% correct from oldest 3911 to unofficial 5933 XDK. Nothing need to be worry about, okay? 😉

One thing I do know, the pause itself isn't coming from here. It's one of the existing functions such as SetBufferData, Flush, and so on from buffer/stream class. And it is not highest priority atm, we can manage few seconds delay for time being. Seriously, I need a break from working on HLE DSound. 😛

P.S. We don't need extra safeguarding. More, extras, safeguard == more CPU cycle == slower gameplay experience == terrible design. Only time we need to put safeguards are conversion between xbox and host. It must be well experienced how the design the APIs perform and how it works. What I'm telling you is I know how it works. 😛

Since you seem to say it won't help at all in finding the cause for pauses, I'll stop whining about it ;)

👍
ppDSBuffer = g_pDSoundBufferCache.erase(ppDSBuffer);
}
g_pDSoundBufferCache.reserve(X_DIRECTSOUND_CACHE_MAX);

// clear sound stream cache
XTL::X_CDirectSoundStream* *pDSStream = g_pDSoundStreamCache;
for (v = 0; v < SOUNDSTREAM_CACHE_SIZE; v++) {
if ((*pDSStream) == nullptr)
continue;
while (XTL::EMUPATCH(CDirectSoundStream_Release)((*pDSStream))) {};
g_pDSoundStreamCache[v] = 0;
vector_ds_stream::iterator ppDSStream = g_pDSoundStreamCache.begin();
for (; ppDSStream != g_pDSoundStreamCache.end();) {
while (XTL::EMUPATCH(CDirectSoundStream_Release)((*ppDSStream)));

This comment has been minimized.

Sign in to view

Copy link
@PatrickvL

PatrickvL May 1, 2018

Member

Same here
ppDSStream = g_pDSoundStreamCache.erase(ppDSStream);
}
g_pDSoundStreamCache.reserve(X_DIRECTSOUND_CACHE_MAX);

//Create Primary Buffer in order for Xbox's DirectSound to manage complete control of it.
DSBUFFERDESC bufferDesc = { 0 };
Expand Down Expand Up @@ -475,9 +471,9 @@ VOID WINAPI XTL::EMUPATCH(DirectSoundDoWork)()
xboxkrnl::LARGE_INTEGER getTime;
xboxkrnl::KeQuerySystemTime(&getTime);

XTL::X_CDirectSoundBuffer* *ppDSBuffer = g_pDSoundBufferCache;
for (int v = 0; v < SOUNDBUFFER_CACHE_SIZE; v++, ppDSBuffer++) {
if ((*ppDSBuffer) == nullptr || (*ppDSBuffer)->Host_lock.pLockPtr1 == nullptr || (*ppDSBuffer)->EmuBufferToggle != X_DSB_TOGGLE_DEFAULT) {
vector_ds_buffer::iterator ppDSBuffer = g_pDSoundBufferCache.begin();
for (; ppDSBuffer != g_pDSoundBufferCache.end(); ppDSBuffer++) {
if ((*ppDSBuffer)->Host_lock.pLockPtr1 == nullptr || (*ppDSBuffer)->EmuBufferToggle != X_DSB_TOGGLE_DEFAULT) {
continue;
}
XTL::X_CDirectSoundBuffer* pThis = *ppDSBuffer;
Expand All @@ -500,13 +496,12 @@ VOID WINAPI XTL::EMUPATCH(DirectSoundDoWork)()
}

// Actually, DirectSoundStream need to process buffer packets here.
XTL::X_CDirectSoundStream* *ppDSStream = g_pDSoundStreamCache;
for (int v = 0; v < SOUNDSTREAM_CACHE_SIZE; v++, ppDSStream++) {
if ((*ppDSStream) == nullptr || (*ppDSStream)->Host_BufferPacketArray.size() == 0) {
vector_ds_stream::iterator ppDSStream = g_pDSoundStreamCache.begin();
for (; ppDSStream != g_pDSoundStreamCache.end(); ppDSStream++) {
if ((*ppDSStream)->Host_BufferPacketArray.size() == 0) {
continue;
}
XTL::X_CDirectSoundStream* pThis = *ppDSStream;

X_CDirectSoundStream* pThis = (*ppDSStream);
// TODO: Do we need this in async thread loop?
if (pThis->Xb_rtPauseEx != 0 && pThis->Xb_rtPauseEx <= getTime.QuadPart) {
pThis->Xb_rtPauseEx = 0LL;
Expand Down Expand Up @@ -536,14 +531,13 @@ static void dsound_thread_worker(LPVOID nullPtr)

enterCriticalSection;

XTL::X_CDirectSoundStream* *pDSStream = g_pDSoundStreamCache;
for (int v = 0; v < SOUNDSTREAM_CACHE_SIZE; v++, pDSStream++) {
if ((*pDSStream) == nullptr || (*pDSStream)->Host_BufferPacketArray.size() == 0) {
vector_ds_stream::iterator ppDSStream = g_pDSoundStreamCache.begin();
for (; ppDSStream != g_pDSoundStreamCache.end(); ppDSStream++) {
if ((*ppDSStream)->Host_BufferPacketArray.size() == 0) {
continue;
}
XTL::X_CDirectSoundStream* pThis = *pDSStream;
if ((pThis->EmuFlags & DSE_FLAG_FLUSH_ASYNC) > 0 && pThis->Xb_rtFlushEx == 0) {
DSoundStreamProcess(pThis);
if (((*ppDSStream)->EmuFlags & DSE_FLAG_FLUSH_ASYNC) > 0 && (*ppDSStream)->Xb_rtFlushEx == 0) {
DSoundStreamProcess((*ppDSStream));
}
}

Expand Down Expand Up @@ -922,16 +916,9 @@ HRESULT WINAPI XTL::EMUPATCH(DirectSoundCreateBuffer)
LOG_FUNC_END;

HRESULT hRet = DS_OK;
int v = 0;
X_CDirectSoundBuffer** ppDSoundBufferCache = nullptr;
for (v = 0; v < SOUNDBUFFER_CACHE_SIZE; v++) {
if (g_pDSoundBufferCache[v] == nullptr) {
ppDSoundBufferCache = &g_pDSoundBufferCache[v];
break;
}
}

//If out of space, return out of memory.
if (ppDSoundBufferCache == nullptr || !DSoundSGEMenAllocCheck(pdsbd->dwBufferBytes)) {
if (X_DIRECTSOUND_CACHE_COUNT == X_DIRECTSOUND_CACHE_MAX || !DSoundSGEMenAllocCheck(pdsbd->dwBufferBytes)) {

hRet = DSERR_OUTOFMEMORY;
*ppBuffer = xbnullptr;
Expand Down Expand Up @@ -980,7 +967,7 @@ HRESULT WINAPI XTL::EMUPATCH(DirectSoundCreateBuffer)
HybridDirectSoundBuffer_SetVolume((*ppBuffer)->EmuDirectSoundBuffer8, 0L, (*ppBuffer)->EmuFlags, nullptr,
(*ppBuffer)->Xb_VolumeMixbin, (*ppBuffer)->Xb_dwHeadroom);

*ppDSoundBufferCache = *ppBuffer;
g_pDSoundBufferCache.push_back(*ppBuffer);
}

leaveCriticalSection;
Expand Down Expand Up @@ -1359,11 +1346,11 @@ ULONG WINAPI XTL::EMUPATCH(IDirectSoundBuffer_Release)
if (pThis->EmuDirectSound3DBuffer8 != nullptr) {
pThis->EmuDirectSound3DBuffer8->Release();
}

// remove cache entry
for (int v = 0; v < SOUNDBUFFER_CACHE_SIZE; v++) {
if (g_pDSoundBufferCache[v] == pThis) {
g_pDSoundBufferCache[v] = 0;
}
vector_ds_buffer::iterator ppDSBuffer = std::find(g_pDSoundBufferCache.begin(), g_pDSoundBufferCache.end(), pThis);
if (ppDSBuffer != g_pDSoundBufferCache.end()) {
g_pDSoundBufferCache.erase(ppDSBuffer);
}

if (pThis->EmuBufferDesc.lpwfxFormat != nullptr) {
Expand Down Expand Up @@ -1731,16 +1718,9 @@ HRESULT WINAPI XTL::EMUPATCH(DirectSoundCreateStream)
LOG_FUNC_END;

HRESULT hRet = DS_OK;
int v = 0;
X_CDirectSoundStream** ppDSoundStreamCache = nullptr;
for (v = 0; v < SOUNDBUFFER_CACHE_SIZE; v++) {
if (g_pDSoundStreamCache[v] == nullptr) {
ppDSoundStreamCache = &g_pDSoundStreamCache[v];
break;
}
}

//If out of space, return out of memory.
if (ppDSoundStreamCache == nullptr) {
if (X_DIRECTSOUND_CACHE_COUNT == X_DIRECTSOUND_CACHE_MAX) {

hRet = DSERR_OUTOFMEMORY;
*ppStream = xbnullptr;
Expand Down Expand Up @@ -1801,7 +1781,7 @@ HRESULT WINAPI XTL::EMUPATCH(DirectSoundCreateStream)
HybridDirectSoundBuffer_SetVolume((*ppStream)->EmuDirectSoundBuffer8, 0L, (*ppStream)->EmuFlags, nullptr,
(*ppStream)->Xb_VolumeMixbin, (*ppStream)->Xb_dwHeadroom);

*ppDSoundStreamCache = *ppStream;
g_pDSoundStreamCache.push_back(*ppStream);
}

leaveCriticalSection;
Expand Down Expand Up @@ -1934,11 +1914,11 @@ ULONG WINAPI XTL::EMUPATCH(CDirectSoundStream_Release)
if (pThis->EmuDirectSound3DBuffer8 != NULL) {
pThis->EmuDirectSound3DBuffer8->Release();
}

// remove cache entry
for (int v = 0; v < SOUNDSTREAM_CACHE_SIZE; v++) {
if (g_pDSoundStreamCache[v] == pThis) {
g_pDSoundStreamCache[v] = 0;
}
vector_ds_stream::iterator ppDSStream = std::find(g_pDSoundStreamCache.begin(), g_pDSoundStreamCache.end(), pThis);
if (ppDSStream != g_pDSoundStreamCache.end()) {
g_pDSoundStreamCache.erase(ppDSStream);
}

for (auto buffer = pThis->Host_BufferPacketArray.begin(); buffer != pThis->Host_BufferPacketArray.end();) {
Expand Down Expand Up @@ -2207,26 +2187,26 @@ HRESULT WINAPI XTL::EMUPATCH(CDirectSound_SynchPlayback)

//TODO: Test case Rayman 3 - Hoodlum Havoc, Battlestar Galactica, Miami Vice, and ...?

XTL::X_CDirectSoundBuffer* *pDSBuffer = g_pDSoundBufferCache;
for (int v = 0; v < SOUNDBUFFER_CACHE_SIZE; v++, pDSBuffer++) {
if ((*pDSBuffer) == nullptr || (*pDSBuffer)->X_BufferCache == nullptr) {
vector_ds_buffer::iterator ppDSBuffer = g_pDSoundBufferCache.begin();
for (; ppDSBuffer != g_pDSoundBufferCache.end(); ppDSBuffer++) {
if ((*ppDSBuffer)->X_BufferCache == nullptr) {
continue;
}

if (((*pDSBuffer)->EmuFlags & DSE_FLAG_SYNCHPLAYBACK_CONTROL) > 0) {
DSoundBufferSynchPlaybackFlagRemove((*pDSBuffer)->EmuFlags);
(*pDSBuffer)->EmuDirectSoundBuffer8->Play(0, 0, (*pDSBuffer)->EmuPlayFlags);
if (((*ppDSBuffer)->EmuFlags & DSE_FLAG_SYNCHPLAYBACK_CONTROL) > 0) {
DSoundBufferSynchPlaybackFlagRemove((*ppDSBuffer)->EmuFlags);
(*ppDSBuffer)->EmuDirectSoundBuffer8->Play(0, 0, (*ppDSBuffer)->EmuPlayFlags);
}
}

XTL::X_CDirectSoundStream* *pDSStream = g_pDSoundStreamCache;
for (int v = 0; v < SOUNDSTREAM_CACHE_SIZE; v++, pDSStream++) {
if ((*pDSStream) == nullptr || (*pDSStream)->Host_BufferPacketArray.size() == 0) {
vector_ds_stream::iterator ppDSStream = g_pDSoundStreamCache.begin();
for (; ppDSStream != g_pDSoundStreamCache.end(); ppDSStream++) {
if ((*ppDSStream)->Host_BufferPacketArray.size() == 0) {
continue;
}
if (((*pDSStream)->EmuFlags & DSE_FLAG_SYNCHPLAYBACK_CONTROL) > 0) {
DSoundBufferSynchPlaybackFlagRemove((*pDSStream)->EmuFlags);
DSoundStreamProcess((*pDSStream));
if (((*ppDSStream)->EmuFlags & DSE_FLAG_SYNCHPLAYBACK_CONTROL) > 0) {
DSoundBufferSynchPlaybackFlagRemove((*ppDSStream)->EmuFlags);
DSoundStreamProcess((*ppDSStream));
}
}

Expand Down

0 comments on commit 270a821

Please sign in to comment.