-
-
Notifications
You must be signed in to change notification settings - Fork 367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DirectSound: infinite loop after device change depending on top-level windows and initialization order #779
Comments
I don't know what would be causing this and I'm not sure what miniaudio can do to work around this, but DirectSound does indeed need a window handle for initialization. Here's what miniaudio is doing: hWnd = ((MA_PFN_GetForegroundWindow)pContext->win32.GetForegroundWindow)();
if (hWnd == 0) {
hWnd = ((MA_PFN_GetDesktopWindow)pContext->win32.GetDesktopWindow)();
}
hr = ma_IDirectSound_SetCooperativeLevel(pDirectSound, hWnd, (shareMode == ma_share_mode_exclusive) ? MA_DSSCL_EXCLUSIVE : MA_DSSCL_PRIORITY); So miniaudio uses Having said all that, I'm not even sure if the above code is even responsible for the issue you're getting, and I don't know what else miniaudio could do. |
Also, further to my comment above, any automatic device change logic is handled by DirectSound itself. miniaudio does not have any direct control over that. |
Indeed, letting the developer pass the handle could help, as using EnumThreadWindows to find out what is the first window of GetCurrentThreadId might be a bit too hazardous, not sure. Regarding the sound not working properly, I'm thinking it might have to do with this thing about windows, based on the observations that I described, likely related to this SetCooperativeLevel() being reset/other, though it's hard to tell. Somehow, the infinite loop seems to be related to the device not flushing, but I'm not familiar enough with the code of this backend to be able to point out anything right now unfortunately. |
Additional point that might be interesting, using GetDesktopWindow() when there is a single top-level window seems to work as well, maybe it's a valid approach as well? This multi-windows stuff is a real mystery, though. |
When you're in the infinite loop, do you have any ability to pause execution via the debugger or anything to maybe narrow it down a bit? I've added support for specifying a custom window handle. You need to do it via the context config, not the device config. It's like this: contextConfig.dsound.hWnd = (ma_handle)YourHWND; This is in the dev branch if you were wanting to try that. I don't know what's going on with the multi-window issue to be honest. I almost wonder if DirectSound just doesn't like multiple windows... Or maybe it's expecting you to have a separate |
Thank you! I can indeed attach a debugger, feel free to ask me to test things if you have specific ideas in mind! So far, I could notice that "GetStatus" on the direct sound buffer, when positioned after the sleep, returned no error but state 0 in case the device was in a wrong state (it was returning 5 in "normal" cases). Also, interestingly, increasing the sleep time by a factor of 10 made it work with two windows, so I think maybe issue is caused by miniaudio locking the buffer while resource is unavailable due to the device change, or something around those lines. I am planning to experiment a bit more on this, I'll revert to you around Wednesday with my results (sorry, very busy start of week!). |
Hello, I could make dsound.hWnd work correctly, it was just a bit tricky to access the object, I ended-up doing this in the code: [...]
ma_engine_config engineConfig = ma_engine_config_init();
engineConfig.pLog = &d->log;
#ifdef MA_SUPPORT_DSOUND
ma_context_config contextConfig = ma_context_config_init();
contextConfig.allocationCallbacks = engineConfig.allocationCallbacks;
contextConfig.pLog = engineConfig.pLog;
if (parent) // Attach DirectSound to the main window if any
contextConfig.dsound.hWnd = (ma_handle)parent->winId();
else // Otherwise, use the desktop window (mostly for tests)
contextConfig.dsound.hWnd = GetDesktopWindow();
ma_backend backends[] = {
ma_backend_dsound
};
result = ma_context_init(backends, sizeof(backends)/sizeof(backends[0]), &contextConfig, &d->context);
if (result != MA_SUCCESS)
{
qCritical() << "Failed to initialize audio engine due to error:" << static_cast<int>(result);
return false;
}
engineConfig.pContext = &d->context;
#else
(void)parent;
#endif
result = ma_engine_init(&engineConfig, &d->engine); It seemed to attach properly, so I suppose this part of the issue is solved by this. I'm wondering if DesktopWindow shouldn't be used by default instead of the ForegroundWindow, but I don't use sound capture, only playback, so maybe it's better to leave it as it is to avoid to break anything. Regarding the issue itself of sound not resuming after change of device, leading to infinite loop, I suppose we're just lucky that it currently works most of the time, because we do not seem to handle properly cases were buffer might stop (it's not really clear on docs though). For example: https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ee416241(v=vs.85)
I attempted something using GetStatus just for testing purpose, that worked pretty well: #define MA_DSBSTATUS_PLAYING 0x00000001
#define MA_DSBSTATUS_BUFFERLOST 0x00000002
#define MA_DSBSTATUS_LOOPING 0x00000004
#define MA_DSBSTATUS_LOCHARDWARE 0x00000008
#define MA_DSBSTATUS_LOCSOFTWARE 0x00000010
#define MA_DSBSTATUS_TERMINATED 0x00000020
[...]
DWORD playbackBufferStatus = 0;
[...]
case ma_device_type_playback:
{
DWORD availableBytesPlayback;
DWORD physicalPlayCursorInBytes;
DWORD physicalWriteCursorInBytes;
hr = ma_IDirectSoundBuffer_GetCurrentPosition((ma_IDirectSoundBuffer*)pDevice->dsound.pPlaybackBuffer, &physicalPlayCursorInBytes, &physicalWriteCursorInBytes);
if (FAILED(hr)) {
break;
}
/* Test here */
if (SUCCEEDED(ma_IDirectSoundBuffer_GetStatus((ma_IDirectSoundBuffer*)pDevice->dsound.pPlaybackBuffer, &playbackBufferStatus)) && (playbackBufferStatus & MA_DSBSTATUS_PLAYING) == 0)
{
ma_log_postf(ma_device_get_log(pDevice), MA_LOG_LEVEL_INFO, "[DirectSound] State: %d, %d, %d.", (int)playbackBufferStatus, (int)physicalPlayCursorInBytes, (int)physicalWriteCursorInBytes);
hr = ma_IDirectSoundBuffer_Play((ma_IDirectSoundBuffer*)pDevice->dsound.pPlaybackBuffer, 0, 0, MA_DSBPLAY_LOOPING);
if (FAILED(hr)) {
ma_log_post(ma_device_get_log(pDevice), MA_LOG_LEVEL_ERROR, "[DirectSound] IDirectSoundBuffer_Play() failed.");
return ma_result_from_HRESULT(hr);
}
isPlaybackDeviceStarted = MA_TRUE;
} Logs when changing device that trigger the Play (once at startup which makes sense, then once per device change - somehow only when I unplug my USB headphones to go back to my Minifuse 4 sound interface, the other way around works properly without need for Pay - might be driver-dependent):
Location of the code of the test is pretty bad, because calling GetStatus at every loop looks to be an overhead that we likely want to avoid - it could probably be called elsewhere (e.g., around the "ma_sleep" mentioned previously), but the essence of it is that "isPlaybackDeviceStarted" is not good enough to know if buffer was already started or not. Also, it probably requires to be done in multiple places. However, I'm not so sure that checking GetStatus like this and calling Play right away is the best approach, because doc mentions Play() might have a delay to refresh GetStatus (I had a case that I couldn't reproduce where I suppose that I called Play multiple times in a row due to this, ending up with ma_IDirectSoundBuffer_Lock failing and printing its log): https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ee418070(v=vs.85)
I hope this helps, feel free to ask if you need more information, Thank you, |
Hi @mackron, Just wondered if I could do anything more to help you on this? So far my tests with GetStatus are working great, I did the following changes (same as above but with an extra sleep in case something goes wrong for any reason to prevent too much spam): diff --git a/miniaudio.h b/miniaudio.h
index 1f4822c..378bd55 100644
--- a/miniaudio.h
+++ b/miniaudio.h
@@ -23667,10 +23667,17 @@ DirectSound Backend
#define MA_DSBPLAY_LOCSOFTWARE 0x00000004
#define MA_DSBPLAY_TERMINATEBY_TIME 0x00000008
#define MA_DSBPLAY_TERMINATEBY_DISTANCE 0x00000010
#define MA_DSBPLAY_TERMINATEBY_PRIORITY 0x00000020
+#define MA_DSBSTATUS_PLAYING 0x00000001
+#define MA_DSBSTATUS_BUFFERLOST 0x00000002
+#define MA_DSBSTATUS_LOOPING 0x00000004
+#define MA_DSBSTATUS_LOCHARDWARE 0x00000008
+#define MA_DSBSTATUS_LOCSOFTWARE 0x00000010
+#define MA_DSBSTATUS_TERMINATED 0x00000020
+
#define MA_DSCBSTART_LOOPING 0x00000001
typedef struct
{
DWORD dwSize;
@@ -24829,10 +24836,11 @@ static ma_result ma_device_data_loop__dsound(ma_device* pDevice)
DWORD virtualWriteCursorInBytesPlayback = 0;
ma_bool32 virtualWriteCursorLoopFlagPlayback = 0;
ma_bool32 isPlaybackDeviceStarted = MA_FALSE;
ma_uint32 framesWrittenToPlaybackDevice = 0; /* For knowing whether or not the playback device needs to be started. */
ma_uint32 waitTimeInMilliseconds = 1;
+ DWORD playbackBufferStatus = 0;
MA_ASSERT(pDevice != NULL);
/* The first thing to do is start the capture device. The playback device is only started after the first period is written. */
if (pDevice->type == ma_device_type_capture || pDevice->type == ma_device_type_duplex) {
@@ -25157,10 +25165,24 @@ static ma_result ma_device_data_loop__dsound(ma_device* pDevice)
hr = ma_IDirectSoundBuffer_GetCurrentPosition((ma_IDirectSoundBuffer*)pDevice->dsound.pPlaybackBuffer, &physicalPlayCursorInBytes, &physicalWriteCursorInBytes);
if (FAILED(hr)) {
break;
}
+ hr = ma_IDirectSoundBuffer_GetStatus((ma_IDirectSoundBuffer*)pDevice->dsound.pPlaybackBuffer, &playbackBufferStatus);
+ if (SUCCEEDED(hr) && (playbackBufferStatus & MA_DSBSTATUS_PLAYING) == 0)
+ {
+ ma_log_postf(ma_device_get_log(pDevice), MA_LOG_LEVEL_INFO, "[DirectSound] Attempting to resume audio due to state: %d.", (int)playbackBufferStatus);
+ hr = ma_IDirectSoundBuffer_Play((ma_IDirectSoundBuffer*)pDevice->dsound.pPlaybackBuffer, 0, 0, MA_DSBPLAY_LOOPING);
+ if (FAILED(hr)) {
+ ma_log_post(ma_device_get_log(pDevice), MA_LOG_LEVEL_ERROR, "[DirectSound] IDirectSoundBuffer_Play() failed.");
+ return ma_result_from_HRESULT(hr);
+ }
+ isPlaybackDeviceStarted = MA_TRUE;
+ ma_sleep(waitTimeInMilliseconds);
+ continue;
+ }
+
if (physicalPlayCursorInBytes < prevPlayCursorInBytesPlayback) {
physicalPlayCursorLoopFlagPlayback = !physicalPlayCursorLoopFlagPlayback;
}
prevPlayCursorInBytesPlayback = physicalPlayCursorInBytes; There are other places that could be affected but I'm not sure if your strategy is to fix things case by case or if you would want to apply such change more globally to other places as well. Thank you, |
I've gone ahead and integrated your suggested fix but with a slight change. There's a subtle detail where the buffer needs to be filled with valid data before playback begins. This is relevant when the device is first started with Are you able to give the dev branch a try to confirm my adjustment is working as expected? |
Hi, thank you for this! I could test your change and it looks to be working great! The adjustment also works properly, I consequently no longer see "MiniAudio: [DirectSound] Attempting to resume audio due to state: 0" at startup, only when changing the device. |
Thanks for checking that. So with that change, and the ability to pass in a custom window handle, is this issue ready to be closed? |
Yes, looks good to me, thanks a lot! |
* DirectSound: Add support for specifying an explicit HWND. Public issue mackron#779 * Update version. * ALSA: Fix some warnings relating to unhandled return value of `read()`. * Web Audio: Fix ScriptProcessNode path when compiling with --closure=1. Audio Worklets do not work with --closure=1 because the callback used with emscripten_create_wasm_audio_worklet_processor_async never gets fired which means miniaudio will never be able to escape from it's busy wait loop. Public issue mackron#778 * Update split version for testing. * Update change history. * Web: Try fixing a runtime error. Public issue mackron#781 * Update dr_wav. * Update change history. * Add support for customizing the min SDK version for AAudio. Define MA_AAUDIO_MIN_ANDROID_SDK_VERSION to specify the minimum required SDK version for enabling the AAudio backend. * Fix bug in ma_node_detach_full(...) * AAudio: Increase default min SDK version from 26 to 27. Use `#define MA_AAUDIO_MIN_ANDROID_SDK_VERSION 26` if you need to support SDK version 26. * Update change history. * Fix an error where the pthread priority is not being set correctly. * Stop using MA_ASSERT in examples. This is useful because MA_ASSERT is only defined in the implementation section of miniaudio.h which can cause issues when people copy/paste the code and use it in a file that does not have visibility of the implementation. Note that there are still more references to implementation-defined macros, but these have been moved to the public section in the dev-0.12 branch so I'm not bothering to change those just yet. Public issue mackron#787 * Add support for customizing pthread priorities at compile time. Use the MA_PTHREAD_REALTTIME_THREAD_PRIORITY define. * Fix a typo. * Remove an accidental change to the deviceio test. * DirectSound: Attempted fix for an error when switching devices. Public issue mackron#779 * Formatting. * Fix amplification with `ma_device_set_master_volume()`. Public issue mackron#800 * Fix compilation error on Android. Public issue mackron#805 * Update dr_wav. * Adding ma_decoder_get_encoding_format * No resource manager also disables VFS. * More VFS disable * fix * more fix * mr * one * :| * more * remove resource manager vfs --------- Co-authored-by: David Reid <mackron@gmail.com> Co-authored-by: xielock <1812822378@qq.com>
* DirectSound: Add support for specifying an explicit HWND. Public issue mackron#779 * Update version. * ALSA: Fix some warnings relating to unhandled return value of `read()`. * Web Audio: Fix ScriptProcessNode path when compiling with --closure=1. Audio Worklets do not work with --closure=1 because the callback used with emscripten_create_wasm_audio_worklet_processor_async never gets fired which means miniaudio will never be able to escape from it's busy wait loop. Public issue mackron#778 * Update split version for testing. * Update change history. * Web: Try fixing a runtime error. Public issue mackron#781 * Update dr_wav. * Update change history. * Add support for customizing the min SDK version for AAudio. Define MA_AAUDIO_MIN_ANDROID_SDK_VERSION to specify the minimum required SDK version for enabling the AAudio backend. * Fix bug in ma_node_detach_full(...) * AAudio: Increase default min SDK version from 26 to 27. Use `#define MA_AAUDIO_MIN_ANDROID_SDK_VERSION 26` if you need to support SDK version 26. * Update change history. * Fix an error where the pthread priority is not being set correctly. * Stop using MA_ASSERT in examples. This is useful because MA_ASSERT is only defined in the implementation section of miniaudio.h which can cause issues when people copy/paste the code and use it in a file that does not have visibility of the implementation. Note that there are still more references to implementation-defined macros, but these have been moved to the public section in the dev-0.12 branch so I'm not bothering to change those just yet. Public issue mackron#787 * Add support for customizing pthread priorities at compile time. Use the MA_PTHREAD_REALTTIME_THREAD_PRIORITY define. * Fix a typo. * Remove an accidental change to the deviceio test. * DirectSound: Attempted fix for an error when switching devices. Public issue mackron#779 * Formatting. * Fix amplification with `ma_device_set_master_volume()`. Public issue mackron#800 * Fix compilation error on Android. Public issue mackron#805 * Update dr_wav. * Adding ma_decoder_get_encoding_format * No resource manager also disables VFS. * More VFS disable * fix * more fix * mr * one * :| * more * remove resource manager vfs * hiding path api --------- Co-authored-by: David Reid <mackron@gmail.com> Co-authored-by: xielock <1812822378@qq.com>
* DirectSound: Add support for specifying an explicit HWND. Public issue mackron#779 * Update version. * ALSA: Fix some warnings relating to unhandled return value of `read()`. * Web Audio: Fix ScriptProcessNode path when compiling with --closure=1. Audio Worklets do not work with --closure=1 because the callback used with emscripten_create_wasm_audio_worklet_processor_async never gets fired which means miniaudio will never be able to escape from it's busy wait loop. Public issue mackron#778 * Update split version for testing. * Update change history. * Web: Try fixing a runtime error. Public issue mackron#781 * Update dr_wav. * Update change history. * Add support for customizing the min SDK version for AAudio. Define MA_AAUDIO_MIN_ANDROID_SDK_VERSION to specify the minimum required SDK version for enabling the AAudio backend. * Fix bug in ma_node_detach_full(...) * AAudio: Increase default min SDK version from 26 to 27. Use `#define MA_AAUDIO_MIN_ANDROID_SDK_VERSION 26` if you need to support SDK version 26. * Update change history. * Fix an error where the pthread priority is not being set correctly. * Stop using MA_ASSERT in examples. This is useful because MA_ASSERT is only defined in the implementation section of miniaudio.h which can cause issues when people copy/paste the code and use it in a file that does not have visibility of the implementation. Note that there are still more references to implementation-defined macros, but these have been moved to the public section in the dev-0.12 branch so I'm not bothering to change those just yet. Public issue mackron#787 * Add support for customizing pthread priorities at compile time. Use the MA_PTHREAD_REALTTIME_THREAD_PRIORITY define. * Fix a typo. * Remove an accidental change to the deviceio test. * DirectSound: Attempted fix for an error when switching devices. Public issue mackron#779 * Formatting. * Fix amplification with `ma_device_set_master_volume()`. Public issue mackron#800 * Fix compilation error on Android. Public issue mackron#805 * Update dr_wav. * Adding ma_decoder_get_encoding_format * No resource manager also disables VFS. * More VFS disable * fix * more fix * mr * one * :| * more * remove resource manager vfs * hiding path api * more warnings removed --------- Co-authored-by: David Reid <mackron@gmail.com> Co-authored-by: xielock <1812822378@qq.com>
Hello,
First, thank you for your work!
To work around issues #717 and #777, I tried to use DirectSound backend, as you advised here: raysan5/raylib#3535. I was trying it to have both sleep mode and device change work seamlessly on Windows.
It seemed to work pretty well in my unit tests, but actual project didn't work. After some troubleshooting, I could isolate multiple behaviors:
Consequence of the failure described in 1) and 3) is that there is no sound after device change because the thread is stuck in ma_device_data_loop__dsound() in ma_sleep(waitTimeInMilliseconds):
Consequently, the app will go to infinite loop at stop time, when calling ma_engine_uninit().
Stack of the backend at stop time - not the same sleep as above, but similar reason (i.e. failure to drain pending bytes from the device):
In case it helps, I checked what windows my process had through this code:
I also logged in ma_context_create_IDirectSound__dsound() what window is used to sets the cooperative level - seems like it'll always find a ForegroundWindow(), but not necessarily the right one, depending on user actions, as mentioned above:
ma_log_postf(ma_context_get_log(pContext), MA_LOG_LEVEL_INFO, "Main window is: %p", hWnd);
Also, miniaudio doesn't seem to require exclusive mode (I don't mind, just a note in case that rings any bell to you).
Miniaudio logs:
Feel free to ask if you need anything!
Thank you,
Louis
The text was updated successfully, but these errors were encountered: