From 8de64347e83202b4539b99b80c34539b7abca391 Mon Sep 17 00:00:00 2001 From: Alberto Mardegan Date: Mon, 15 Jan 2024 19:13:29 +0300 Subject: [PATCH] ogc: rework audio driver to avoid clicks and gaps When playing an audio stream the previous implementation was producing frequent clicks (also audible in the emulator). The reason is that SDL runs a thread that does these operations in a loop: 1. Ask the driver for a buffer (GetDeviceBuf()) 2. Fill the buffer with audio data from the app 3. Call PlayAudio() on the driver 4. Call WaitDevice() on the driver The clicks occurred because after a buffer had finished playing, our completion callback would set the condition variable, allowing the audio thread (which was blocking in point 4) to continue. However, since libaesnd does not offer an API to queue audio buffer, we were sending the next audio buffer only in PlayAudio(), that is after the points 1 and 2 were performed. And since point 2 involves copying buffers and possibly even converting them, this would often generate an audible delay. The solution is to play the next buffer directly from the completion callback. The waiting logic has been rewritten to use a semaphore blocking on the availability of a buffer for writing (not for playback!). The number of buffers has also been increase to protect against sudden spikes in CPU usage, although in the test applications 2 buffers are good enough too. --- src/audio/ogc/SDL_ogcaudio.c | 58 ++++++++++++++++++++++++++---------- src/audio/ogc/SDL_ogcaudio.h | 9 +++--- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/audio/ogc/SDL_ogcaudio.c b/src/audio/ogc/SDL_ogcaudio.c index 9d70e430df9db..7a770f1abcb19 100644 --- a/src/audio/ogc/SDL_ogcaudio.c +++ b/src/audio/ogc/SDL_ogcaudio.c @@ -109,7 +109,28 @@ static void audio_frame_finished(AESNDPB *pb, u32 state, void *arg) SDL_AudioDevice *this = (SDL_AudioDevice *)arg; if (state == VOICE_STATE_STREAM) { - LWP_CondBroadcast(this->hidden->cv); + const size_t buffer_size = DMA_BUFFER_SIZE; + s8 playing_buffer; + void *buffer; + + /* Immediately send the next buffer to the DSP. It's important that + * AESND_SetVoiceBuffer() gets called before this callback returns, or + * some audio gaps might be audible. */ + contextLock(this); + playing_buffer = (this->hidden->playing_buffer + 1) % NUM_BUFFERS; + buffer = this->hidden->dma_buffers[playing_buffer]; + AESND_SetVoiceBuffer(pb, buffer, buffer_size); + this->hidden->playing_buffer = playing_buffer; + contextUnlock(this); + + /* If a frame has finished playing, it means that the corresponding + * buffer is no longer in use and can be filled up again. We signal + * this event to the audio thread via a semaphore. */ + LWP_SemPost(this->hidden->available_buffers); + } if (state == VOICE_STATE_STOPPED) { + contextLock(this); + this->hidden->playing_buffer = -1; + contextUnlock(this); } } @@ -126,6 +147,7 @@ static int OGCAUDIO_OpenDevice(_THIS, const char *devname) devname, this->spec.freq, this->spec.channels); memset(hidden, 0, sizeof(*hidden)); + hidden->playing_buffer = -1; this->hidden = hidden; AESND_Init(); @@ -133,7 +155,11 @@ static int OGCAUDIO_OpenDevice(_THIS, const char *devname) /* Initialise internal state */ LWP_MutexInit(&hidden->lock, false); - LWP_CondInit(&hidden->cv); + /* We set the initial number of available buffers to NUM_BUFFERS - 1, since + * SDL first calls GetDeviceBuf() and starts filling it without first + * calling WaitDevice(). So we consider the first buffer to be busy already + * at start. */ + LWP_SemInit(&hidden->available_buffers, NUM_BUFFERS - 1, NUM_BUFFERS); if (this->spec.freq <= 0 || this->spec.freq > 144000) this->spec.freq = DSP_DEFAULT_FREQ; @@ -170,26 +196,27 @@ static int OGCAUDIO_OpenDevice(_THIS, const char *devname) static void OGCAUDIO_PlayDevice(_THIS) { void *buffer; - size_t nextbuf; - size_t buffer_size = DMA_BUFFER_SIZE; + /* This only sends the first audio buffer. The following ones will always + * be send from the audio_frame_finished() callback, without having to + * switch between threads. */ contextLock(this); - - nextbuf = this->hidden->nextbuf; - this->hidden->nextbuf = (nextbuf + 1) % NUM_BUFFERS; - + if (this->hidden->playing_buffer < 0) { + buffer = this->hidden->dma_buffers[++this->hidden->playing_buffer]; + AESND_SetVoiceBuffer(this->hidden->voice, buffer, DMA_BUFFER_SIZE); + } contextUnlock(this); - - buffer = this->hidden->dma_buffers[nextbuf]; - DCStoreRange(buffer, buffer_size); - AESND_SetVoiceBuffer(this->hidden->voice, buffer, buffer_size); } static void OGCAUDIO_WaitDevice(_THIS) { - contextLock(this); - LWP_CondWait(this->hidden->cv, this->hidden->lock); - contextUnlock(this); + s8 nextbuf; + + /* This will block until at least one buffer is available for writing. */ + LWP_SemWait(this->hidden->available_buffers); + + nextbuf = this->hidden->nextbuf; + this->hidden->nextbuf = (nextbuf + 1) % NUM_BUFFERS; } static Uint8 *OGCAUDIO_GetDeviceBuf(_THIS) @@ -201,6 +228,7 @@ static void OGCAUDIO_CloseDevice(_THIS) { struct SDL_PrivateAudioData *hidden = this->hidden; + LWP_SemDestroy(hidden->available_buffers); if (hidden->voice) { AESND_SetVoiceStop(hidden->voice, true); AESND_FreeVoice(hidden->voice); diff --git a/src/audio/ogc/SDL_ogcaudio.h b/src/audio/ogc/SDL_ogcaudio.h index 151f4101601e8..a575ac5aaac55 100644 --- a/src/audio/ogc/SDL_ogcaudio.h +++ b/src/audio/ogc/SDL_ogcaudio.h @@ -25,13 +25,13 @@ #include #include -#include #include +#include /* Hidden "this" pointer for the audio functions */ #define _THIS SDL_AudioDevice *this -#define NUM_BUFFERS 2 /* -- Minimum 2! */ +#define NUM_BUFFERS 4 /* -- Minimum 2! */ #define SAMPLES_PER_DMA_BUFFER (DSP_STREAMBUFFER_SIZE) #define DMA_BUFFER_SIZE (SAMPLES_PER_DMA_BUFFER * 2 * sizeof(short)) @@ -44,9 +44,10 @@ struct SDL_PrivateAudioData /* Speaker data */ Uint32 format; Uint8 bytes_per_sample; - Uint32 nextbuf; + s8 nextbuf; + s8 playing_buffer; mutex_t lock; - cond_t cv; + sem_t available_buffers; }; #endif /* _SDL_ogcaudio_h_ */