Android/OpenSLES: prevent lots or error log in background #12632
Conversation
while loop that handles audio playback sequence has changed from PlayDevice(), WaitDevice() to WaitDevice(), PlayDevice() So the semaphore need to be incremented by 1
|
What's the reasoning behind this change? @icculus, thoughts? |
|
maybe this affects some other backend ? and so SDL_audio.c wait/play should be inversed ? |
|
Yes, I think this is a bug. The thinking was probably that we should start feeding buffers to the device when the backend says it's ready, but I would bet almost all backends end up waiting an extra buffer because of this. The fix is almost certainly to change this from a With that change, though, we probably don't need to change the semaphore count in this PR. |
|
(fwiw, I get a single underrun error on ALSA, too, and it's probably exactly because of this.) |
|
Okay, a quick look through all of the backends suggests this change would work. PulseAudio looks like the most fragile, but its WriteCallback fires before PlayDevice here, asking for a large initial block of audio, so it all worked out. I think if it is a race and it doesn't work out, GetDeviceBuf will return a zero-byte buffer size and the higher level already treats this as "go back and wait again, the system isn't ready." The ALSA underrun was not fixed by this (but this plus moving the snd_pcm_start call to a ThreadInit() implementation, so it starts the device processing after the thread is going and we're literally about to feed the device does seem to fix it). I'm going to push both of these changes. @1bsyl, would you mind checking android again after I push? It's possible we need to adjust the semaphore count still. |
This prevents the waste of an initial buffer of audio on many backends, and is hopefully harmless on all of them. Reference PR #12632.
… work. Otherwise, in the time it takes the thread to start and other init tasks to complete, we tend to get an underrun on some systems, which ALSA logs to stderr. So this is moved to an InitThread implementation, which runs from the device thread, right before it begins its main loop. Reference PR #12632.
|
Okay, pushed! |
|
@icculus ok i ll check today. It seems that recording loop has also the same thing: wait/iterate |
|
@icculus so as expected, not more error log with opensles. so the PR isn't need. |
|
Cool, thanks for tracking this down! I'm going to flip the loop on recording tonight and then we'll close this PR. |
|
Actually, several of these recording things (including aaudio, directsound, maybe others) look like they'll fail if there wasn't an initial wait. This loop probably makes sense to stay with a wait to start with, as we can't consume data until data is available, as opposed to output, which we can provide right away. @1bysl, if we still need this semaphore change for recording on Android, let me know. |
|
@icculus, ok, maybe this makes sense as recording is the inverse process of playback |
… work. Otherwise, in the time it takes the thread to start and other init tasks to complete, we tend to get an underrun on some systems, which ALSA logs to stderr. So this is moved to an InitThread implementation, which runs from the device thread, right before it begins its main loop. Reference PR #12632. (cherry picked from commit ae17b04)
This prevents the waste of an initial buffer of audio on many backends, and is hopefully harmless on all of them. Reference PR libsdl-org#12632. (cherry picked from commit 4163695)
… work. Otherwise, in the time it takes the thread to start and other init tasks to complete, we tend to get an underrun on some systems, which ALSA logs to stderr. So this is moved to an InitThread implementation, which runs from the device thread, right before it begins its main loop. Reference PR libsdl-org#12632. (cherry picked from commit ae17b04)
3 participants
Android/OpenSLES, The while loop that handles audio playback sequence has changed from
PlayDevice(), WaitDevice()
to
WaitDevice(), PlayDevice()
( 905c4ff )
SDL_audio.c:
to
(see also libsdl-org/SDL_mixer#684)