Don't busy wait in the audio thread (ALSA) #2719
Conversation
| @@ -6,6 +6,8 @@ | |||
|
|
|||
| #include <atomic> | |||
| #include <thread> | |||
| #include <mutex> | |||
| #include <condition_variable> | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| m_mixer->Mix(reinterpret_cast<short *>(mix_buffer), frames_to_deliver); | ||
| int rc = m_muted ? 1337 : snd_pcm_writei(handle, mix_buffer, frames_to_deliver); | ||
| int rc = snd_pcm_writei(handle, mix_buffer, frames_to_deliver); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@dolphin-emu-bot rebuild |
|
The fifoci errors are likely due to bug 6001, which is now also triggered by ALSA because in this PR we moved the ALSA backend initialization out of the audio thread. I submitted PR #2824 to fix this bug too. |
|
Please rebase on master, else LGTM |
moncefmechri
force-pushed
the
bugfix-7729
branch
2 times, most recently
from
6be3fb1
to
b020391
Compare
| @@ -26,6 +28,11 @@ AlsaSound::~AlsaSound() | |||
| bool AlsaSound::Start() | |||
| { | |||
| m_thread_status.store(ALSAThreadStatus::RUNNING); | |||
| if (!AlsaInit()) { | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
moncefmechri
force-pushed
the
bugfix-7729
branch
4 times, most recently
from
7d6a509
to
c5bad46
Compare
This fixes a race condition: Before this commit, there was a race condition when starting a game: Core::EmuThread(), after having started (but not necessarily completed) the initialization of the audio thread, calls Core::SetState() which calls CCPU::EnableStepping(), which in turns calls AudioCommon::ClearAudioBuffer(). This means that SoundStream::Clear() can be called before AlsaSound::AlsaInit() has completed.
When the emulation is paused and the ALSA backend is used, make the audio thread wait on a condition variable instead of busy-waiting. This commit fixes bug dolphin-emu#7729 Since the ALSA API is not thread-safe, calls to snd_pcm_drop() and snd_pcm_prepare() in AlsaSound::Clear() are protected by the same mutex as the condition variable in AlsaSound::SoundLoop() to make sure that we do not call these functions while a call to snd_pcm_writei() is ongoing.
moncefmechri
force-pushed
the
bugfix-7729
branch
from
652fe22
to
333f998
Compare
|
FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:
automated-fifoci-reporter |
Don't busy wait in the audio thread (ALSA)
|
This PR appears to be the cause of most of our FifoCI random false positives recently. It appears that Alsa is somehow broken on FifoCI, it opens fine the first time, but blocks forever on the snd_pcm_writei call. The next time fifoci launches dolphin, alsa fails to open and falls back to the null audio backend. After about 20-30 seconds, alsa will reset and the next instance of dolphin will manage to open alsa again. I have no idea why this behaviour is happening. The problem is that this PR causes SoundLoop() to hold a mutex open while it's blocking on snd_pcm_writei, and if Clear() is called after SoundLoop() has grabbed that mutex, it will block forever. Clear() is called right after alsa is initialized, so it is up in the air if Clear() will be called before or after the SoundLoop(). If Clear() is too slow and dolphin didn't fall back to the null audio backend, then it will block forever, until the timeout kills dolphin. Log file, search for "killed" to find the two failed fifos. alsa was initialized in 6 out of 50 runs. I suspect we should move the two snd_pcm_* calls back into the audio thread and make Clear() signal the thread when the status of m_muted changes. |
|
#3105 should fix this issue. |
When the emulation is paused and the ALSA backend is used, make the audio
thread wait on a condition variable instead of busy-waiting. This commit
fixes bug #7729