Skip to content
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

Audio playback issue after device disconnect using WASAPI backend #717

Open
day-garwood opened this issue Aug 21, 2023 · 18 comments
Open

Audio playback issue after device disconnect using WASAPI backend #717

day-garwood opened this issue Aug 21, 2023 · 18 comments
Labels

Comments

@day-garwood
Copy link

Hi there.
Thanks for your great work so far.
I made a Windows app recently and a friend of mine noticed a problem when coming out of sleep mode. When I tested I found this had something to do with MiniAudio.
A minimal test with debugging enabled showed that MA seems to make six attempts to change devices. This is likely due to the fact that, even after waking the computer from sleep, I have to press a few keys for the audio device to kick in again.
The result of this is that we now appear to be in an unknown state. Both loading and playing a sound return MA_SUCCESS but no audio actually plays, ma_sound_is_playing always returns true, and the cursor is stuck at 0.
I have attached the debug log for reference.
Thanks again.

@mackron
Copy link
Owner

mackron commented Aug 21, 2023

Thanks for the report. I'm actually not entirely sure what's going on there or what I might be able to do to solve this. Those "CHANGING DEVICE" messages are actually from WASAPI itself when it wants to reroute. I'll try to replicate this when I get a chance and report back.

@day-garwood day-garwood changed the title Audio playback issue due to unsuccessful device changes when exiting from low power states Audio playback issue after device disconnect using WASAPI backend Aug 22, 2023
@day-garwood
Copy link
Author

Hi there,
It seems the behaviour also occurs when a device has been disconnected. I haven't been able to test this myself since I require audio for TTS and don't have a backup device, but my friend informs me that my program exhibits the same behaviour of not playing and then hanging because it's waiting for the sound to finish which, according to MA, it never does.
That makes sense because of course most if not all peripherals are disconnected and/or powered down during sleep.
To me the bug is less about what MA does to try to recover from the situation, but more the inconsistent state it seems to be in afterwards - everything still shows as successful when in fact it isn't.
I will also run more tests my end. I can't say I'll come back with much - I am still learning and so at the moment understand very little about low-level audio processing, but I will certainly do my best.
I'm looking forward to collaborating with you to figure this out. Any further guidance you can provide on this issue would be greatly appreciated.
Cheers.

@mackron
Copy link
Owner

mackron commented Aug 23, 2023

OK, I saw your pull request and perhaps I misinterpreted your situation. The state of a ma_sound object is decoupled from the engine. It's absolutely possible and intentional for ma_sound_is_playing() to return true even when the engine (and the underlying device) is in a stopped state. ma_sound_is_playing() is just a simple getter of an internal flag for determining if the sound itself is in an active state from it's own isolated perspective. This makes sense - it's common that you will want to pre-prepare your sounds by putting them into a started state (which is also their default state) before you've actually started the engine. This way you can start the engine and sounds will start playback atomically and they will stay in sync. With your PR, you will never be able to put your sound into a started state before starting the engine.

Having said all that, having a ma_engine_is_started() might be a useful API. That would be a more appropriate way of determining whether or not audio as a whole is running.

In the situation you're talking about in this ticket, does sound playback start working again after calling ma_engine_start()?

@day-garwood
Copy link
Author

Now this, is interesting.
When I use ma_engine_start, the sound plays, but then crashes when the program exits.
On the other hand, when I built a debug version, the sound didn't play, there were no crashes, and I got the following error, presumably from MiniAudio's debug system:
ERROR: [WASAPI] Failed to start internal playback device. HRESULT = -2004287484.
I would say this is bizarre given that all it does is call ma_device_start under the hood just like my code does. This gets stranger by the minute.
Cheers.

@mackron
Copy link
Owner

mackron commented Sep 7, 2023

If it's possible, it would be handy to see the contents of your ma_engine object just before you start playing the sound, and if possible, immediately after playing it. It sounds to me like this is just a simple matter of a corrupt ma_engine or ma_device object. Are you definitely not destroying and recreating your ma_engine object or anything? And you're definitely passing in a valid ma_engine object?

@day-garwood
Copy link
Author

Interestingly, the WASAPI error seems to trigger when I attach a debugger and restart the device after waking up, regardless of build settings. The crash only happened the once, every other time it just hung because the device is trying to restart.
I am definitely passing engine objects to engine functions and device objects to device functions.
Oddly enough, destroying and recreating the engine with uninit/init actually works. So clearly the sleep/wake is putting it into some halfway state.
At this point I'm not ruling out a corrupt engine object - given I've no idea how the sleep power state actually works, yes it's meant to retain memory data but for all I know, other APIs and hardware devices might not have been programmed to wake up as gracefully as we might like. Given we've got both devices and thread sync failing, something's definitely happening somewhere.
Here are my engine states:
before sleep
after waking
This is from the code that doesn't attempt to restart the engine after waking up so this is just the original issue. If you feel you need the state after a restart attempt as well for the thread problem, let me know.
Thanks a lot for looking into this.

@mackron
Copy link
Owner

mackron commented Sep 8, 2023

So I tested the sleep/wakeup cycle with the deviceio test and it's working exactly as I would expect with the WASAPI backend. That test lets you pause and resume the device with the "P" key. It's successfully stopping the device when the computer is put to the sleep, and it's successfully resuming when I wakeup the machine and hit the "P" key in the program to resume it. You can test this out for yourself if you like (it's easy to compile).

I'm assuming the ma_device_uninit() calls are a result of you calling ma_engine_uninit() somewhere? Where are you calling that? It looks to me from your logs that it's being called when the device is in the process of being stopped, hence the state being set to "4" at the time of the uninit. Are you doing it inside any kind of callback or anything? If you are indeed calling that inside a callback, that's invalid usage.

@LouisDuVerdier
Copy link

LouisDuVerdier commented Sep 9, 2023

Hello,

If I may comment, and if that helps, I understand the source of this is indeed the ma_device being stopped by Windows going into sleep mode, and not being restarted by any mean. Experiencing the same issue, I somehow expected miniaudio to recover automatically from this (similarly to how it already handles device changes), maybe day-garwood did too, but there are good reasons for it not to do it as well.

Calling periodically:

if (!ma_device_is_started(engine.pDevice))
{
    std::cout << "Attempting to restarting audio device that was stopped" << std::endl;
    ma_device_start(engine.pDevice);
}

Or just before playing a new sound properly restarts the device and all sounds that were playing properly continue. I'm just not sure if that's correct to do such things with the high level API.

Thank you!

@day-garwood
Copy link
Author

The deviceio test is giving me an error:
"ERROR: [WASAPI] Failed to start internal playback device. HRESULT = -2004287484.Press Q to quit, P to resume."
Note oddly enough this is the same error message I got when I attached my test case to a debugger.

Yes, the device_uninit is a result of engine_uninit. The only times I'm calling engine_uninit are:

  1. If any functions fail,
  2. During the reinit test (if enabled)
  3. When all tests have been run.
    There are no callbacks and no other threads involved in my test case.
    Just so you have a bit of context to work with, it's literally just a single c file (about 70 lines) using ma_engine and ma_sound. It simply plays a sound, prompts you to put the machine to sleep and press enter when it's woken up, tries to play the sound again, and cleans up.
    The only functions I have are:
    int main(void);
    int run_sound_test(void);
    int run_restart_test(void);
    int restart_engine(void);
    int reinit_engine(void);
    The only MiniAudio functions I'm calling are:
    ma_engine_init
    ma_sound_init_from_file
    ma_sound_start
    ma_sound_uninit
    ma_engine_start (if restart test is enabled)
    ma_engine_uninit
    The only other external functions that are being called are printf and getchar from stdio.
    Cheers.

@day-garwood
Copy link
Author

Hi Louis,
Yes, that's the workaround I'm currently having to use in my application, is to check the device state whenever I want to play a sound. But it's when doing that in my test case, that I then found the stop event mt sync problem on uninit.
I can see why you think there are good reasons for not automatically recovering from that state, especially if the user is controlling their own low-level components like devices and decoders. But yes you're correct, I would have thought the higher level engine would automatically recover from that. Sleep state is different from a total disconnection, after all.
Even if not though, clearly there are errors somewhere. If we attempt to restart the device and we're getting silence one minute, errors the next, and causing thread sync issues on uninit in other tests, that says to me that there's a lot more than just a simple device stop when the system goes to sleep. Very mystifying.
Of course the other problem is I'm also guessing this type of scenario is likely dependent on machine, BIOS settings, OS settings and so on. Personally I've never liked these hybrid low-power states for that very reason.
Having said that, apps are definitely recovering a lot better from sleep now compared to when I last used it (back in the good ol' days when it was known as Standby).
As for calling ma_device_start, yes you can do that, or you can make things easier and call ma_engine_start instead. To check state though you would have to check the device with something like ma_device_state state=ma_device_get_state(ma_engine_get_device(pEngine)), as there doesn't seem to be a ma_engine_get_state or ma_engine_is_started.
Cheers.

@LouisDuVerdier
Copy link

LouisDuVerdier commented Sep 9, 2023

Hi day-garwood,

Indeed, now that you mention it, calling this ma_device_start() seems to cause issues, one of them being a deadlock on stop, when calling ma_engine_uninit() at the end of the test. Deadlock was on WaitForSingleObject (though there is a timeout so possible called repeatedly):

if (hr == MA_AUDCLNT_E_BUFFER_TOO_LARGE || hr == MA_AUDCLNT_E_BUFFER_ERROR) {
    /* Not enough data available. We need to wait for more. */
    if (WaitForSingleObject((HANDLE)pDevice->wasapi.hEventPlayback, MA_WASAPI_WAIT_TIMEOUT_MILLISECONDS) != WAIT_OBJECT_0) {
        result = MA_ERROR;
        break;   /* Wait failed. Probably timed out. */
    }
}

The main thread calling ma_engine_uninit() was a the time blocked in ma_device_uninit().

For your own tests, did you try to build in debug mode with asserts?

Once I started to call ma_device_start() from the main thread every 1sec (if state is not started), like in the code I mentioned previously, I started getting quite a lot of them related to device states, so maybe you'll get some too. Feels like one should not call ma_device_start() while miniaudio might be attempting to recover from device changes.

@day-garwood
Copy link
Author

I did build in debug mode. I got no assertion errors though.
The MT problem you found is very interesting. Wonder if that's related to the logs I posted on 730 where the device tried to restart itself on uninit because the call to wait on stop was returning immediately? As I say I'm very new to multithreaded programming (in fact I'm relatively new to C programming in general) so wouldn't have had a clue how to debug that other than extensive logging.
Cheers.

@LouisDuVerdier
Copy link

LouisDuVerdier commented Sep 9, 2023

Hello,

Just to try to isolate a bit the issue, I attempted to make a tiny console C++ example here:

issue_717_restart_device_test.txt

It requires "my_sound.wav" to be available (preferably a long one, so something continuously runs).

You can type 'S' then press Enter to stop and uninit the test, and 'R' to stop and restart a new engine.

Scenario 1):

  • Comment out block at line 104 => if (ma_device_get_state(engine.pDevice) == ma_device_state_stopped)
  • Start the test
  • Enter to sleep mode -- sound will obviously stop, this is expected
  • Go back from sleep mode
  • There will not be any sound anymore -- this is the issue of the ticket
  • Press 'R' => that will restart everything properly with sound, no crash/no issue -- but this is a new engine, so maybe your test is different here

Scenario 2)

  • Restore block at line 104
  • Start the test
  • Enter to sleep mode -- sound stops
  • Go back from sleep mode
  • Sound will resume after passing in the block of line 104, but plenty of asserts will be logged
  • Press 'S' then Enter => deadlock in ma_engine_uninit

@day-garwood Feel free to tell if your test looks different, I just attempted to make something easier for mackron to reproduce.

Thank you,
Louis

@day-garwood
Copy link
Author

Ah, didn't realise you had to define ma_assert yourself. My test doesn't do that.
Mine is written in C and somewhat shorter. Mine plays a sound, prompts for sleep, tries to restart based on the uncommented code in the restart function, and tries to play the sound again. It relies on a file called audio/test.flac.
sleep_wake_test.txt
My multiple scenarios lie in the uncommented code in the restart function, as follows:

  • Uncomment line 54, comment 55 and 56 (this is how it is right now): No restart, and no sound plays.
  • Uncomment line 55, comment 54 and 56: Engine restarts, thread problem.
  • Uncomment line 56, comment 54 and 55: Engine fully reinitialised, works as expected.
    Cheers.

mackron added a commit that referenced this issue Sep 9, 2023
If the state miniaudio side of the device does not match the actual
state of the backend side of it, such as when the device is stopped but
the backend doesn't post a notification, attempting to stop the device
might result in a deadlock.

This is a just a quick workaround hack for the moment while a more
robust solution is figured out.

#717
@mackron mackron added the bug label Sep 9, 2023
@mackron
Copy link
Owner

mackron commented Sep 9, 2023

Thanks team. It sounds like for some reason WASAPI isn't reporting your device as stopped. On my system, WASAPI is definitely reporting the device as invalid which miniaudio is detecting which then puts the ma_device object into a stopped state. This results in both miniaudio and the backend agreeing on the actual state of the device which means everything works as expected (except for the automatic restarting upon wakeup). It's surprising to me that it's seemingly inconsistent between our machines. If this state inconsistency theory is correct you won't be able to use ma_device_get_state() as the means for checking whether or not the device needs to be restarted or reinitialized.

Although I haven't yet been able to replicate any crash or deadlock, I've pushed an experimental hack to the dev branch to hopefully prevent the deadlock in uninit. It doesn't actually solve the underlying problem, and if my thinking is correct, would still result in a deadlock if ma_sound_stop() was to get called, so it's not really a hugely meaningful change.

I tested with DirectSound and WinMM backends, and both of these resume automatically upon wakeup. I guess the ideal solution would be for miniaudio to detect when the machine is put to sleep or woken up and cleanly handle it like DirectSound and WinMM are doing (they do this automatically - no direct involvement on miniaudio's part). I think the way to do this is to have a hidden window with a window proc running in a background thread which detects the power state changes. I'm not quite sure how to deal with that yet.

Marking this one as a bug.

@LouisDuVerdier
Copy link

LouisDuVerdier commented Sep 10, 2023

Hello,

On my side it unfortunately didn't help.

I took the liberty of improving a bit the test of day-garwood to add thread_id + date/time in logs, and to print MA logs. With latest dev branch, I uncommented line restart_engine() of the test (otherwise there is no sound after waking up) and got the same behavior with an infinite loop when stopping.

issue_717_updated_test.txt
issue_717_updated_log.txt
issue_717_updated_log_GetCurrentThreadId.txt

I documented the steps in the log. day-garwood made the test quite simple, and doesn't mix calls to device restarts while miniaudio is processing changes, so I'm thinking the test itself is not messing with miniaudio's state (at least not causing extra harm at the time it's called).

@mackron: I'm thinking miniaudio properly detects the change, but maybe the issue lies that there is not only a single change when computer wakes but multiple ones? Thread ids are interesting to watch as well in the log (I attached another version as well with GetCurrentThreadId() instead of pthread_self() because I'm not so sure how posix works on Windows for thread ids - scenario is exactly the same).

@day-garwood: just a quick note regarding asserts, you don't have to do it like I did to print them, the basic C assert() should work perfectly fine as long as NDEBUG is not defined (which is generally the case when building in debug mode). I just redefined it to bypass them (which is generally a wrong thing to do because they're here for a reason), but to see them at least.

I'm available for further testing if needed!

Thank you,
Louis

@LouisDuVerdier
Copy link

An additional note: I just noticed that with the latest change in dev, basic scenario (without sleep mode/restart of device) also ends up in infinite loop when closing.

@TempoLabGames
Copy link

The change in a47f065 has caused a hang for me in some circumstances. On Linux (I believe with the ALSA backend) setting a very small buffer (1ms) in exclusive mode will apparently succeed but no audio plays. Once the device is in this state, calling ma_device_uninit will hang indefinitely. I've been able to work around this by manually calling ma_device_stop first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants