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

AudioCommon: Make the SoundStream global a unique_ptr #4074

Merged
merged 1 commit into from Jul 31, 2016

Conversation

lioncash
Copy link
Member

@lioncash lioncash commented Jul 31, 2016

Basic stuff.

Alternatively I could make the function act as a creational one and have it return a unique_ptr instance separate from the global which is then assigned to said global directly at the function usage site in Core.cpp, if preferred.


This change is Reviewable

@BhaaLseN
Copy link
Member

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


Source/Core/AudioCommon/AudioCommon.cpp, line 67 [r1] (raw file):

  UpdateSoundStream();

  if (!g_sound_stream->Start())

What will happen down here if g_sound_stream isn't initialized (when none of the backends was valid, and NullSound::isValid also returned false)?
It looks a little like NullSound::isValid is a tiny bit useless, in the sense of always being valid; or otherwise this piece of code down here (falling back to Null after the other backend failed to start) is rather...strange.


Comments from Reviewable

@lioncash
Copy link
Member Author

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


Source/Core/AudioCommon/AudioCommon.cpp, line 67 [r1] (raw file):

Previously, BhaaLseN (BhaaL) wrote…

What will happen down here if g_sound_stream isn't initialized (when none of the backends was valid, and NullSound::isValid also returned false)?
It looks a little like NullSound::isValid is a tiny bit useless, in the sense of always being valid; or otherwise this piece of code down here (falling back to Null after the other backend failed to start) is rather...strange.

Yeah, it's a bit silly. I can remove the isValid check(s) if it makes sense for this PR.

Comments from Reviewable

@delroth
Copy link
Member

delroth commented Jul 31, 2016

Let's fix that in a subsequent change.

@delroth delroth merged commit 007df77 into dolphin-emu:master Jul 31, 2016
@lioncash lioncash deleted the soundstream branch July 31, 2016 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants