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

BACKENDS: SDL: Use null mixer if audio initialization fails #5275

Merged
merged 2 commits into from Nov 7, 2023

Conversation

elasota
Copy link
Contributor

@elasota elasota commented Aug 18, 2023

I've attempted to clean this up but unfortunately untangling all of the cases where mixer readiness is not being checked is complicated. EmulatedOPL doesn't do it, VideoDecoder doesn't do it, only 10 engines are actually checking isReady on the mixer because this is such a rare case.

However, it is a case that happens in the real world when the user has no audio output devices enabled.

Anything that fails to check immediately crashes in playStream. Since there is a lot of code that isn't handling this correctly, I think it is better to use a null mixer fallback and add a way of detecting that it is a null device to MixerManager instead.

@@ -822,6 +823,9 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
DO_LONG_OPTION_BOOL("disable-sdl-parachute")
END_OPTION

DO_LONG_OPTION_BOOL("disable-sdl-audio")
Copy link
Member

@sev- sev- Aug 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add it to the help text above, then. Also, since it is SDL-specific, shouldn't it be guarded around by #ifdef SDL_BACKEND?

@sev-
Copy link
Member

sev- commented Oct 16, 2023

@elasota any updates?

@sev-
Copy link
Member

sev- commented Nov 5, 2023

@elasota Still no updates?

…a flag to forcibly disable it for testing. Add an alternate call that returns true if the mixer manager is a null device.

SDL audio init will fail on Windows if all audio output devices are disabled.
Only about 10 engines are checking for this case and numerous pieces of common code (EmulatedOPL, VideoDecoder) fail as well, so this acts as a fallback to prevent instability.
@sev-
Copy link
Member

sev- commented Nov 7, 2023

Thank you!

@sev- sev- merged commit cc64a58 into scummvm:master Nov 7, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants