1 files changed, 119 insertions, 135 deletions

diff --git a/src/audioOutputs/audioOutput_alsa.c b/src/audioOutputs/audioOutput_alsa.c
index f0aa32713..b7b0a1151 100644
--- a/src/audioOutputs/audioOutput_alsa.c
+++ b/src/audioOutputs/audioOutput_alsa.c
@@ -23,6 +23,8 @@
 #define ALSA_PCM_NEW_HW_PARAMS_API
 #define ALSA_PCM_NEW_SW_PARAMS_API
 
+static const char default_device[] = "default";
+
 #define MPD_ALSA_BUFFER_TIME_US 500000
 /* the default period time of xmms is 50 ms, so let's use that as well.
  * a user can tweak this parameter via the "period_time" config parameter.
@@ -36,26 +38,42 @@
 
 #include <alsa/asoundlib.h>
 
+/* #define MPD_SND_PCM_NONBLOCK SND_PCM_NONBLOCK */
+#define MPD_SND_PCM_NONBLOCK 0
+
+/*
+ * This macro will evaluate both statements, but only returns the result
+ * of the second statement to the reader.  Thus it'll stringify the
+ * command name and assign it to the scoped cmd variable.
+ * Note that ALSA is strictly for Linux , and anybody compiling
+ * on Linux will have gcc or a gcc-compatible compiler anyways.
+ */
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
+#  define E(command , arg1 , ...) \
+	(err_cmd = #command, command( arg1 , __VA_ARGS__ ))
+#else /* ! C99, this works for gcc 2.95 at least: */
+#  define E(command , arg1 , args...) \
+	(err_cmd = #command, command( arg1 , ##args ))
+#endif /* ! C99 */
+
 typedef snd_pcm_sframes_t alsa_writei_t(snd_pcm_t * pcm, const void *buffer,
 					snd_pcm_uframes_t size);
 
 typedef struct _AlsaData {
-	char *device;
+	const char *device;
 	snd_pcm_t *pcmHandle;
 	alsa_writei_t *writei;
 	unsigned int buffer_time;
 	unsigned int period_time;
 	int sampleSize;
 	int useMmap;
-	int canPause;
-	int canResume;
 } AlsaData;
 
 static AlsaData *newAlsaData(void)
 {
 	AlsaData *ret = xmalloc(sizeof(AlsaData));
 
-	ret->device = NULL;
+	ret->device = default_device;
 	ret->pcmHandle = NULL;
 	ret->writei = snd_pcm_writei;
 	ret->useMmap = 0;
@@ -67,20 +85,27 @@ static AlsaData *newAlsaData(void)
 
 static void freeAlsaData(AlsaData * ad)
 {
-	if (ad->device)
-		free(ad->device);
-
+	if (ad->device && ad->device != default_device)
+		free(deconst_ptr(ad->device));
 	free(ad);
 }
 
 static int alsa_initDriver(AudioOutput * audioOutput, ConfigParam * param)
 {
+	/* no need for pthread_once thread-safety when reading config */
+	static int free_global_registered;
 	AlsaData *ad = newAlsaData();
 
+	if (!free_global_registered) {
+		atexit((void(*)(void))snd_config_update_free_global);
+		free_global_registered = 1;
+	}
+
 	if (param) {
-		BlockParam *bp = getBlockParam(param, "device");
-		ad->device = bp ? xstrdup(bp->value) : xstrdup("default");
+		BlockParam *bp;
 
+		if ((bp = getBlockParam(param, "device")))
+			ad->device = xstrdup(bp->value);
 		ad->useMmap = getBoolBlockParam(param, "use_mmap", 1);
 		if (ad->useMmap == CONF_BOOL_UNSET)
 			ad->useMmap = 0;
@@ -88,8 +113,7 @@ static int alsa_initDriver(AudioOutput * audioOutput, ConfigParam * param)
 			ad->buffer_time = atoi(bp->value);
 		if ((bp = getBlockParam(param, "period_time")))
 			ad->period_time = atoi(bp->value);
-	} else
-		ad->device = xstrdup("default");
+	}
 	audioOutput->data = ad;
 
 	return 0;
@@ -106,12 +130,10 @@ static int alsa_testDefault(void)
 {
 	snd_pcm_t *handle;
 
-	int ret = snd_pcm_open(&handle, "default", SND_PCM_STREAM_PLAYBACK,
-			       SND_PCM_NONBLOCK);
-	snd_config_update_free_global();
-
+	int ret = snd_pcm_open(&handle, default_device,
+	                       SND_PCM_STREAM_PLAYBACK, SND_PCM_NONBLOCK);
 	if (ret) {
-		WARNING("Error opening default alsa device: %s\n",
+		WARNING("Error opening default ALSA device: %s\n",
 			snd_strerror(-ret));
 		return -1;
 	} else
@@ -120,6 +142,17 @@ static int alsa_testDefault(void)
 	return 0;
 }
 
+static snd_pcm_format_t get_bitformat(const AudioFormat * af)
+{
+	switch (af->bits) {
+	case 8: return SND_PCM_FORMAT_S8;
+	case 16: return SND_PCM_FORMAT_S16;
+	case 24: return SND_PCM_FORMAT_S24;
+	case 32: return SND_PCM_FORMAT_S32;
+	}
+	return SND_PCM_FORMAT_UNKNOWN;
+}
+
 static int alsa_openDevice(AudioOutput * audioOutput)
 {
 	AlsaData *ad = audioOutput->data;
@@ -129,84 +162,57 @@ static int alsa_openDevice(AudioOutput * audioOutput)
 	snd_pcm_sw_params_t *swparams;
 	unsigned int sampleRate = audioFormat->sampleRate;
 	unsigned int channels = audioFormat->channels;
-	snd_pcm_uframes_t alsa_buffer_size;
-	snd_pcm_uframes_t alsa_period_size;
+	snd_pcm_uframes_t buffer_size;
+	snd_pcm_uframes_t period_size;
 	int err;
-	const char *cmd = NULL;
+	const char *err_cmd = NULL;
 	int retry = MPD_ALSA_RETRY_NR;
 	unsigned int period_time, period_time_ro;
 	unsigned int buffer_time;
 
-	switch (audioFormat->bits) {
-	case 8:
-		bitformat = SND_PCM_FORMAT_S8;
-		break;
-	case 16:
-		bitformat = SND_PCM_FORMAT_S16;
-		break;
-	case 24:
-		bitformat = SND_PCM_FORMAT_S24;
-		break;
-	case 32:
-		bitformat = SND_PCM_FORMAT_S32;
-		break;
-	default:
+	if ((bitformat = get_bitformat(audioFormat)) == SND_PCM_FORMAT_UNKNOWN)
 		ERROR("ALSA device \"%s\" doesn't support %i bit audio\n",
 		      ad->device, audioFormat->bits);
-		return -1;
-	}
 
-	err = snd_pcm_open(&ad->pcmHandle, ad->device,
-			   SND_PCM_STREAM_PLAYBACK, SND_PCM_NONBLOCK);
-	snd_config_update_free_global();
+	err = E(snd_pcm_open, &ad->pcmHandle, ad->device,
+	        SND_PCM_STREAM_PLAYBACK, MPD_SND_PCM_NONBLOCK);
 	if (err < 0) {
 		ad->pcmHandle = NULL;
 		goto error;
 	}
 
-	cmd = "snd_pcm_nonblock";
-	err = snd_pcm_nonblock(ad->pcmHandle, 0);
-	if (err < 0)
+#if MPD_SND_PCM_NONBLOCK == SND_PCM_NONBLOCK
+	if ((err = E(snd_pcm_nonblock, ad->pcmHandle, 0)) < 0)
 		goto error;
+#endif /* MPD_SND_PCM_NONBLOCK == SND_PCM_NONBLOCK */
 
 	period_time_ro = period_time = ad->period_time;
 configure_hw:
 	/* configure HW params */
 	snd_pcm_hw_params_alloca(&hwparams);
-
-	cmd = "snd_pcm_hw_params_any";
-	err = snd_pcm_hw_params_any(ad->pcmHandle, hwparams);
-	if (err < 0)
+	if ((err = E(snd_pcm_hw_params_any, ad->pcmHandle, hwparams)) < 0)
 		goto error;
 
 	if (ad->useMmap) {
-		err = snd_pcm_hw_params_set_access(ad->pcmHandle, hwparams,
-						   SND_PCM_ACCESS_MMAP_INTERLEAVED);
-		if (err < 0) {
-			ERROR("Cannot set mmap'ed mode on alsa device \"%s\": "
-			      " %s\n", ad->device, snd_strerror(-err));
-			ERROR("Falling back to direct write mode\n");
-			ad->useMmap = 0;
-		} else
+		if (!(err = snd_pcm_hw_params_set_access(ad->pcmHandle,
+		                hwparams, SND_PCM_ACCESS_MMAP_INTERLEAVED))) {
 			ad->writei = snd_pcm_mmap_writei;
-	}
-
-	if (!ad->useMmap) {
-		cmd = "snd_pcm_hw_params_set_access";
-		err = snd_pcm_hw_params_set_access(ad->pcmHandle, hwparams,
-						   SND_PCM_ACCESS_RW_INTERLEAVED);
-		if (err < 0)
-			goto error;
-		ad->writei = snd_pcm_writei;
-	}
+		} else {
+			ERROR("ALSA cannot enable mmap on device \"%s\": %s. "
+			      "Falling back to direct write mode\n",
+			      ad->device, snd_strerror(-err));
+			ad->useMmap = 0;
+		}
+	} else if ((err = E(snd_pcm_hw_params_set_access, ad->pcmHandle,
+		            hwparams, SND_PCM_ACCESS_RW_INTERLEAVED)) < 0)
+		goto error;
 
 	err = snd_pcm_hw_params_set_format(ad->pcmHandle, hwparams, bitformat);
 	if (err < 0) {
-		ERROR("ALSA device \"%s\" does not support %i bit audio: "
-		      "%s\n", ad->device, audioFormat->bits, snd_strerror(-err));
+		ERROR("ALSA device \"%s\" does not support %i bit audio:%s\n",
+		      ad->device, audioFormat->bits, snd_strerror(-err));
 		goto fail;
 	}
-
 	err = snd_pcm_hw_params_set_channels_near(ad->pcmHandle, hwparams,
 						  &channels);
 	if (err < 0) {
@@ -227,92 +233,62 @@ configure_hw:
 	audioFormat->sampleRate = sampleRate;
 
 	buffer_time = ad->buffer_time;
-	cmd = "snd_pcm_hw_params_set_buffer_time_near";
-	err = snd_pcm_hw_params_set_buffer_time_near(ad->pcmHandle, hwparams,
-						     &buffer_time, NULL);
-	if (err < 0)
+	if ((err = E(snd_pcm_hw_params_set_buffer_time_near, ad->pcmHandle,
+	             hwparams, &buffer_time, NULL)) < 0)
 		goto error;
 
 	period_time = period_time_ro;
-	cmd = "snd_pcm_hw_params_set_period_time_near";
-	err = snd_pcm_hw_params_set_period_time_near(ad->pcmHandle, hwparams,
-						     &period_time, NULL);
-	if (err < 0)
+
+	if ((err = E(snd_pcm_hw_params_set_period_time_near,
+	             ad->pcmHandle, hwparams, &period_time, NULL)) < 0)
 		goto error;
 
-	cmd = "snd_pcm_hw_params";
-	err = snd_pcm_hw_params(ad->pcmHandle, hwparams);
+	err = E(snd_pcm_hw_params, ad->pcmHandle, hwparams);
 	if (err == -EPIPE && --retry > 0) {
 		period_time_ro = period_time_ro >> 1;
 		goto configure_hw;
 	} else if (err < 0)
 		goto error;
-	if (retry != MPD_ALSA_RETRY_NR)
-		DEBUG("ALSA period_time set to %d\n", period_time);
 
-	cmd = "snd_pcm_hw_params_get_buffer_size";
-	err = snd_pcm_hw_params_get_buffer_size(hwparams, &alsa_buffer_size);
-	if (err < 0)
+	DEBUG("ALSA(%s) period_time: %u, buffer_time: %u\n",
+	      ad->device, period_time, buffer_time);
+	if ((err = E(snd_pcm_hw_params_get_buffer_size, hwparams,
+	             &buffer_size)) < 0)
 		goto error;
 
-	cmd = "snd_pcm_hw_params_get_period_size";
-	err = snd_pcm_hw_params_get_period_size(hwparams, &alsa_period_size,
-						NULL);
-	if (err < 0)
+	if ((err = E(snd_pcm_hw_params_get_period_size, hwparams,
+	             &period_size, NULL)) < 0)
 		goto error;
-
-	ad->canPause = snd_pcm_hw_params_can_pause(hwparams);
-	ad->canResume = snd_pcm_hw_params_can_resume(hwparams);
+	DEBUG("ALSA(%s) period_size: %lu buffer_size: %lu\n",
+	      ad->device, period_size, buffer_size);
 
 	/* configure SW params */
 	snd_pcm_sw_params_alloca(&swparams);
 
-	cmd = "snd_pcm_sw_params_current";
-	err = snd_pcm_sw_params_current(ad->pcmHandle, swparams);
-	if (err < 0)
-		goto error;
-
-	cmd = "snd_pcm_sw_params_set_start_threshold";
-	err = snd_pcm_sw_params_set_start_threshold(ad->pcmHandle, swparams,
-						    alsa_buffer_size -
-						    alsa_period_size);
-	if (err < 0)
+	if ((err = E(snd_pcm_sw_params_current, ad->pcmHandle, swparams)) < 0)
 		goto error;
-
-	cmd = "snd_pcm_sw_params_set_avail_min";
-	err = snd_pcm_sw_params_set_avail_min(ad->pcmHandle, swparams,
-					      alsa_period_size);
-	if (err < 0)
+	if ((err = E(snd_pcm_sw_params_set_start_threshold, ad->pcmHandle,
+	              swparams, buffer_size - period_size)) < 0)
 		goto error;
-
-	cmd = "snd_pcm_sw_params_set_xfer_align";
-	err = snd_pcm_sw_params_set_xfer_align(ad->pcmHandle, swparams, 1);
-	if (err < 0)
+	if ((err = E(snd_pcm_sw_params_set_avail_min, ad->pcmHandle,
+	             swparams, period_size)) < 0)
 		goto error;
-
-	cmd = "snd_pcm_sw_params";
-	err = snd_pcm_sw_params(ad->pcmHandle, swparams);
-	if (err < 0)
+	if ((err = E(snd_pcm_sw_params, ad->pcmHandle, swparams)) < 0)
 		goto error;
 
 	ad->sampleSize = (audioFormat->bits / 8) * audioFormat->channels;
 
 	audioOutput->open = 1;
 
-	DEBUG("alsa device \"%s\" will be playing %i bit, %i channel audio at "
+	DEBUG("ALSA device \"%s\" will be playing %i bit, %i channel audio at "
 	      "%i Hz\n", ad->device, (int)audioFormat->bits,
 	      channels, sampleRate);
 
 	return 0;
 
 error:
-	if (cmd) {
-		ERROR("Error opening alsa device \"%s\" (%s): %s\n",
-		      ad->device, cmd, snd_strerror(-err));
-	} else {
-		ERROR("Error opening alsa device \"%s\": %s\n", ad->device,
-		      snd_strerror(-err));
-	}
+	ERROR("Error opening ALSA device \"%s\" (%s): %s\n",
+	      ad->device, (err_cmd ? err_cmd : ""), snd_strerror(-err));
 fail:
 	if (ad->pcmHandle)
 		snd_pcm_close(ad->pcmHandle);
@@ -323,24 +299,25 @@ fail:
 
 static int alsa_errorRecovery(AlsaData * ad, int err)
 {
-	if (err == -EPIPE) {
-		DEBUG("Underrun on alsa device \"%s\"\n", ad->device);
-	} else if (err == -ESTRPIPE) {
-		DEBUG("alsa device \"%s\" was suspended\n", ad->device);
-	}
+	snd_pcm_state_t state = snd_pcm_state(ad->pcmHandle);
+	const char *err_cmd = NULL;
+
+	if (err == -EPIPE)
+		DEBUG("Underrun on ALSA device \"%s\"\n", ad->device);
+	else if (err == -ESTRPIPE)
+		DEBUG("ALSA device \"%s\" was suspended\n", ad->device);
 
-	switch (snd_pcm_state(ad->pcmHandle)) {
+	switch (state) {
 	case SND_PCM_STATE_PAUSED:
-		err = snd_pcm_pause(ad->pcmHandle, /* disable */ 0);
+		err = E(snd_pcm_pause, ad->pcmHandle, /* disable */ 0);
 		break;
 	case SND_PCM_STATE_SUSPENDED:
-		err = ad->canResume ?
-		    snd_pcm_resume(ad->pcmHandle) :
-		    snd_pcm_prepare(ad->pcmHandle);
-		break;
+		if ((err = E(snd_pcm_resume, ad->pcmHandle)) == -EAGAIN)
+			return 0;
+		/* fall-through to snd_pcm_prepare: */
 	case SND_PCM_STATE_SETUP:
 	case SND_PCM_STATE_XRUN:
-		err = snd_pcm_prepare(ad->pcmHandle);
+		err = E(snd_pcm_prepare, ad->pcmHandle);
 		break;
 	case SND_PCM_STATE_DISCONNECTED:
 		/* so alsa_closeDevice won't try to drain: */
@@ -349,13 +326,20 @@ static int alsa_errorRecovery(AlsaData * ad, int err)
 		break;
 	/* this is no error, so just keep running */
 	case SND_PCM_STATE_RUNNING:
-		err = 0;
+		if (mpd_unlikely(err)) {
+			DEBUG("ALSA(%s) ignoring possible error: %s\n",
+			      ad->device, snd_strerror(-err));
+			err = 0;
+		}
 		break;
 	default:
-		/* unknown state, do nothing */
+		DEBUG("ALSA device \"%s\" in unknown state: %s\n",
+		      ad->device, snd_pcm_state_name(state));
 		break;
 	}
-
+	if (err && err_cmd)
+		ERROR("ALSA error on device \"%s\" (%s): %s\n",
+		      ad->device, err_cmd, snd_strerror(-err));
 	return err;
 }
 
@@ -397,7 +381,7 @@ static int alsa_playAudio(AudioOutput * audioOutput,
 
 		if (ret < 0) {
 			if (alsa_errorRecovery(ad, ret) < 0) {
-				ERROR("closing alsa device \"%s\" due to write "
+				ERROR("closing ALSA device \"%s\" due to write "
 				      "error: %s\n", ad->device,
 				      snd_strerror(-errno));
 				alsa_closeDevice(audioOutput);