DirectSound: infinite loop after device change depending on top-level windows and initialization order about miniaudio

[mackron]

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 GetForegroundWindow(), and if that doesn't find anything, it'll fall back to GetDesktopWindow(). I guess maybe miniaudio could add a property in ma_context_config to let you pass in your own window handle, specifically for the DirectSound backend? Regardless, the DirectSound backend requires a window handle (why this is needed is anyone's guess...), and since miniaudio doesn't have any notion of a window, it needs to source one from somewhere, hence the logic above.

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.

[mackron]

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.

[LouisDuVerdier]

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.

[LouisDuVerdier]

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.

[mackron]

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 IDirectSound objects for each window? If that's the case, the only way to make that work would be to have a separate ma_device object for each window, with each of those device's being owned by their own ma_context object which have their own window handle set appropriately.

[LouisDuVerdier]

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!).

[LouisDuVerdier]

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)

Your application can use the IDirectSoundBuffer8::GetStatus method to determine if the buffer is playing or if it has stopped.

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):

MiniAudio: [DirectSound] State: 0, 0, 0.
MiniAudio: [DirectSound] State: 0, 11520, 11520.
MiniAudio: [DirectSound] State: 0, 23040, 23040.
MiniAudio: [DirectSound] State: 0, 26880, 26880.
MiniAudio: [DirectSound] State: 0, 23040, 23040.

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)

DSBSTATUS_PLAYING is set if the buffer is being heard. Because of latency, a call to Play or Stop might not immediately change the status.

I hope this helps, feel free to ask if you need more information,

Thank you,
Louis

[LouisDuVerdier]

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,
Louis

[mackron]

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 ma_device_start(). In your conditional I've added && isPlaybackDeviceStarted to only attempt this re-starting logic if miniaudio thinks DirectSound is playing.

Are you able to give the dev branch a try to confirm my adjustment is working as expected?

[LouisDuVerdier]

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.

[mackron]

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?

[LouisDuVerdier]

Yes, looks good to me, thanks a lot!