From 3027cead01d8aacce03dc360cedaa4cc44a062fc Mon Sep 17 00:00:00 2001 From: Solomon Peachy Date: Wed, 18 Nov 2020 14:08:17 -0500 Subject: [PATCH] hosted: Improve buffer underrun handling in the ALSA driver * Bump internal mix buffer size by 4x, to 1K frames (matching ALSA period) * Handle an underrun that occurs when filling the audio buffer * Log underruns and make them available in the debug info Change-Id: I28d56dd35d88851fa167ad92368a5882937a758f --- firmware/export/pcm_mixer.h | 4 ++ firmware/target/hosted/agptek/debug-agptek.c | 1 + firmware/target/hosted/pcm-alsa.c | 65 ++++++++++++++++---- firmware/target/hosted/pcm-alsa.h | 15 ++--- 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/firmware/export/pcm_mixer.h b/firmware/export/pcm_mixer.h index 3d255a7345..39a814de6f 100644 --- a/firmware/export/pcm_mixer.h +++ b/firmware/export/pcm_mixer.h @@ -40,6 +40,10 @@ /* iBasso Devices: Match Rockbox PCM buffer size to ALSA PCM buffer size to minimize memory transfers. */ #define MIX_FRAME_SAMPLES 2048 +#elif (CONFIG_PLATFORM & PLATFORM_HOSTED) +/* Hosted targets need larger buffers for decent performance due to + OS locking/scheduling overhead */ +#define MIX_FRAME_SAMPLES 1024 #else /* Assume HW DMA engine is available or sufficient latency exists in the PCM pathway */ diff --git a/firmware/target/hosted/agptek/debug-agptek.c b/firmware/target/hosted/agptek/debug-agptek.c index 8ef0048945..a9b829f7ec 100644 --- a/firmware/target/hosted/agptek/debug-agptek.c +++ b/firmware/target/hosted/agptek/debug-agptek.c @@ -62,6 +62,7 @@ bool dbg_hw_info(void) } lcd_putsf(0, line++, "pcm srate: %d", pcm_alsa_get_rate()); + lcd_putsf(0, line++, "pcm xruns: %d", pcm_alsa_get_xruns()); #ifdef HAVE_HEADPHONE_DETECTION lcd_putsf(0, line++, "hp: %d", headphones_inserted()); #endif diff --git a/firmware/target/hosted/pcm-alsa.c b/firmware/target/hosted/pcm-alsa.c index 14e1c4cd3e..1854f67ba0 100644 --- a/firmware/target/hosted/pcm-alsa.c +++ b/firmware/target/hosted/pcm-alsa.c @@ -62,6 +62,12 @@ #define DEFAULT_PLAYBACK_DEVICE "plughw:0,0" #define DEFAULT_CAPTURE_DEVICE "default" +#if MIX_FRAME_SAMPLES < 512 +#error "MIX_FRAME_SAMPLES needs to be at least 512!" +#elif MIX_FRAME_SAMPLES < 1024 +#warning "MIX_FRAME_SAMPLES <1024 may cause dropouts!" +#endif + static const snd_pcm_access_t access_ = SND_PCM_ACCESS_RW_INTERLEAVED; /* access mode */ #if defined(SONY_NWZ_LINUX) || defined(HAVE_FIIO_LINUX_CODEC) /* Sony NWZ must use 32-bit per sample */ @@ -83,6 +89,8 @@ static sample_t *frames = NULL; static const void *pcm_data = 0; static size_t pcm_size = 0; +static unsigned int xruns = 0; + static snd_async_handler_t *ahandler = NULL; static pthread_mutex_t pcm_mtx; static char signal_stack[SIGSTKSZ]; @@ -117,17 +125,21 @@ static int set_hwparams(snd_pcm_t *handle) snd_pcm_hw_params_malloc(¶ms); /* Size playback buffers based on sample rate. - Note these are in FRAMES, and are sized to be about 10ms - for the buffer and 2.5ms for the period */ + + Buffer size must be at least 4x period size! + + Note these are in FRAMES, and are sized to be about 8.5ms + for the buffer and 2.1ms for the period + */ if (pcm_sampr > SAMPR_96) { - buffer_size = MIX_FRAME_SAMPLES * 16 * 4; /* 32k */ - period_size = MIX_FRAME_SAMPLES * 4 * 4; /* 4k */ + buffer_size = MIX_FRAME_SAMPLES * 4 * 4; + period_size = MIX_FRAME_SAMPLES * 4; } else if (pcm_sampr > SAMPR_48) { - buffer_size = MIX_FRAME_SAMPLES * 16 * 2; /* 16k */ - period_size = MIX_FRAME_SAMPLES * 4 * 2; /* 2k */ + buffer_size = MIX_FRAME_SAMPLES * 2 * 4; + period_size = MIX_FRAME_SAMPLES * 2; } else { - buffer_size = MIX_FRAME_SAMPLES * 16; /* 4k */ - period_size = MIX_FRAME_SAMPLES * 4; /* 1k */ + buffer_size = MIX_FRAME_SAMPLES * 4; + period_size = MIX_FRAME_SAMPLES; } /* choose all parameters */ @@ -407,7 +419,8 @@ static void async_callback(snd_async_handler_t *ahandler) if (state == SND_PCM_STATE_XRUN) { - logf("underrun!"); + xruns++; + logf("initial underrun!"); err = snd_pcm_recover(handle, -EPIPE, 0); if (err < 0) { logf("XRUN Recovery error: %s", snd_strerror(err)); @@ -432,8 +445,20 @@ static void async_callback(snd_async_handler_t *ahandler) { if (copy_frames(false)) { + retry: err = snd_pcm_writei(handle, frames, period_size); - if (err < 0 && err != period_size && err != -EAGAIN) + if (err == -EPIPE) + { + logf("mid underrun!"); + xruns++; + err = snd_pcm_recover(handle, -EPIPE, 0); + if (err < 0) { + logf("XRUN Recovery error: %s", snd_strerror(err)); + goto abort; + } + goto retry; + } + else if (err != period_size) { logf("Write error: written %i expected %li", err, period_size); break; @@ -452,7 +477,18 @@ static void async_callback(snd_async_handler_t *ahandler) while (snd_pcm_avail_update(handle) >= period_size) { int err = snd_pcm_readi(handle, frames, period_size); - if (err < 0 && err != period_size && err != -EAGAIN) + if (err == -EPIPE) + { + logf("rec mid underrun!"); + xruns++; + err = snd_pcm_recover(handle, -EPIPE, 0); + if (err < 0) { + logf("XRUN Recovery error: %s", snd_strerror(err)); + goto abort; + } + continue; /* buffer contents trashed, no sense in trying to copy */ + } + else if (err != period_size) { logf("Read error: read %i expected %li", err, period_size); break; @@ -751,11 +787,16 @@ void pcm_play_dma_postinit(void) #endif } -int pcm_alsa_get_rate(void) +unsigned int pcm_alsa_get_rate(void) { return real_sample_rate; } +unsigned int pcm_alsa_get_xruns(void) +{ + return xruns; +} + #ifdef HAVE_RECORDING void pcm_rec_lock(void) { diff --git a/firmware/target/hosted/pcm-alsa.h b/firmware/target/hosted/pcm-alsa.h index 1392593c0c..4c0b0d0b9d 100644 --- a/firmware/target/hosted/pcm-alsa.h +++ b/firmware/target/hosted/pcm-alsa.h @@ -1,10 +1,10 @@ /*************************************************************************** - * __________ __ ___. - * Open \______ \ ____ ____ | | _\_ |__ _______ ___ - * Source | _// _ \_/ ___\| |/ /| __ \ / _ \ \/ / - * Jukebox | | ( <_> ) \___| < | \_\ ( <_> > < < - * Firmware |____|_ /\____/ \___ >__|_ \|___ /\____/__/\_ \ - * \/ \/ \/ \/ \/ + * __________ __ ___. + * Open \______ \ ____ ____ | | _\_ |__ _______ ___ + * Source | _// _ \_/ ___\| |/ /| __ \ / _ \ \/ / + * Jukebox | | ( <_> ) \___| < | \_\ ( <_> > < < + * Firmware |____|_ /\____/ \___ >__|_ \|___ /\____/__/\_ \ + * \/ \/ \/ \/ \/ * * Copyright (C) 2016 Amaury Pouly * @@ -34,6 +34,7 @@ void pcm_alsa_set_playback_device(const char *device); void pcm_alsa_set_capture_device(const char *device); #endif -int pcm_alsa_get_rate(void); +unsigned int pcm_alsa_get_rate(void); +unsigned int pcm_alsa_get_xruns(void); #endif /* __PCM_ALSA_RB_H__ */