pcm_convert: use GError for error handling

src/decoder_api.c

@@ -225,6 +225,7 @@ decoder_data(struct decoder *decoder,
 	     struct replay_gain_info *replay_gain_info)
 {
 	const char *data = _data;
+	GError *error = NULL;
 
 	assert(dc.state == DECODE_STATE_DECODE);
 	assert(dc.pipe != NULL);
@@ -259,14 +260,15 @@ decoder_data(struct decoder *decoder,
 	if (!audio_format_equals(&dc.in_audio_format, &dc.out_audio_format)) {
 		data = pcm_convert(&decoder->conv_state,
 				   &dc.in_audio_format, data, length,
-				   &dc.out_audio_format, &length);
-
-		/* under certain circumstances, pcm_convert() may
-		   return an empty buffer - this condition should be
-		   investigated further, but for now, do this check as
-		   a workaround: */
-		if (data == NULL)
-			return DECODE_COMMAND_NONE;
+				   &dc.out_audio_format, &length,
+				   &error);
+		if (data == NULL) {
+			/* the PCM conversion has failed - stop
+			   playback, since we have no better way to
+			   bail out */
+			g_warning("%s", error->message);
+			return DECODE_COMMAND_STOP;
+		}
 	}
 
 	while (length > 0) {

src/filter/convert_filter_plugin.c

@@ -53,12 +53,6 @@ struct convert_filter {
 	struct pcm_convert_state state;
 };
 
-static inline GQuark
-convert_quark(void)
-{
-	return g_quark_from_static_string("pcm_convert");
-}
-
 static struct filter *
 convert_filter_init(G_GNUC_UNUSED const struct config_param *param,
 		    G_GNUC_UNUSED GError **error_r)
@@ -119,12 +113,10 @@ convert_filter_filter(struct filter *_filter, const void *src, size_t src_size,
 
 	dest = pcm_convert(&filter->state, &filter->in_audio_format,
 			   src, src_size,
-			   &filter->out_audio_format, dest_size_r);
-	if (dest == NULL) {
-		g_set_error(error_r, convert_quark(), 0,
-			    "pcm_convert() has failed");
+			   &filter->out_audio_format, dest_size_r,
+			   error_r);
+	if (dest == NULL)
 		return NULL;
-	}
 
 	return dest;
 }

src/pcm_channels.c

@@ -20,13 +20,8 @@
 #include "pcm_channels.h"
 #include "pcm_buffer.h"
 
-#include <glib.h>
-
 #include <assert.h>
 
-#undef G_LOG_DOMAIN
-#define G_LOG_DOMAIN "pcm"
-
 static void
 pcm_convert_channels_16_1_to_2(int16_t *dest, const int16_t *src,
 			       unsigned num_frames)
@@ -92,11 +87,8 @@ pcm_convert_channels_16(struct pcm_buffer *buffer,
 	else if (dest_channels == 2)
 		pcm_convert_channels_16_n_to_2(dest, src_channels, src,
 					       num_frames);
-	else {
-		g_warning("conversion %u->%u channels is not supported",
-			  src_channels, dest_channels);
+	else
 		return NULL;
-	}
 
 	return dest;
 }
@@ -166,11 +158,8 @@ pcm_convert_channels_24(struct pcm_buffer *buffer,
 	else if (dest_channels == 2)
 		pcm_convert_channels_24_n_to_2(dest, src_channels, src,
 					       num_frames);
-	else {
-		g_warning("conversion %u->%u channels is not supported",
-			  src_channels, dest_channels);
+	else
 		return NULL;
-	}
 
 	return dest;
 }
@@ -235,11 +224,8 @@ pcm_convert_channels_32(struct pcm_buffer *buffer,
 	else if (dest_channels == 2)
 		pcm_convert_channels_32_n_to_2(dest, src_channels, src,
 					       num_frames);
-	else {
-		g_warning("conversion %u->%u channels is not supported",
-			  src_channels, dest_channels);
+	else
 		return NULL;
-	}
 
 	return dest;
 }

src/pcm_convert.c

@@ -56,8 +56,8 @@ static const int16_t *
 pcm_convert_16(struct pcm_convert_state *state,
 	       const struct audio_format *src_format,
 	       const void *src_buffer, size_t src_size,
-	       const struct audio_format *dest_format,
-	       size_t *dest_size_r)
+	       const struct audio_format *dest_format, size_t *dest_size_r,
+	       GError **error_r)
 {
 	const int16_t *buf;
 	size_t len;
@@ -67,24 +67,37 @@ pcm_convert_16(struct pcm_convert_state *state,
 	buf = pcm_convert_to_16(&state->format_buffer, &state->dither,
 				src_format->bits, src_buffer, src_size,
 				&len);
-	if (!buf)
-		g_error("pcm_convert_to_16() failed");
+	if (buf == NULL) {
+		g_set_error(error_r, pcm_convert_quark(), 0,
+			    "Conversion from %u to 16 bit is not implemented",
+			    src_format->bits);
+		return NULL;
+	}
 
 	if (src_format->channels != dest_format->channels) {
 		buf = pcm_convert_channels_16(&state->channels_buffer,
 					      dest_format->channels,
 					      src_format->channels,
 					      buf, len, &len);
-		if (!buf)
-			g_error("pcm_convert_channels_16() failed");
+		if (buf == NULL) {
+			g_set_error(error_r, pcm_convert_quark(), 0,
+				    "Conversion from %u to %u channels "
+				    "is not implemented",
+				    src_format->channels,
+				    dest_format->channels);
+			return NULL;
+		}
 	}
 
-	if (src_format->sample_rate != dest_format->sample_rate)
+	if (src_format->sample_rate != dest_format->sample_rate) {
 		buf = pcm_resample_16(&state->resample,
 				      dest_format->channels,
 				      src_format->sample_rate, buf, len,
-				      dest_format->sample_rate,
-				      &len);
+				      dest_format->sample_rate, &len,
+				      error_r);
+		if (buf == NULL)
+			return NULL;
+	}
 
 	if (dest_format->reverse_endian) {
 		buf = pcm_byteswap_16(&state->byteswap_buffer, buf, len);
@@ -99,8 +112,8 @@ static const int32_t *
 pcm_convert_24(struct pcm_convert_state *state,
 	       const struct audio_format *src_format,
 	       const void *src_buffer, size_t src_size,
-	       const struct audio_format *dest_format,
-	       size_t *dest_size_r)
+	       const struct audio_format *dest_format, size_t *dest_size_r,
+	       GError **error_r)
 {
 	const int32_t *buf;
 	size_t len;
@@ -109,24 +122,37 @@ pcm_convert_24(struct pcm_convert_state *state,
 
 	buf = pcm_convert_to_24(&state->format_buffer, src_format->bits,
 				src_buffer, src_size, &len);
-	if (!buf)
-		g_error("pcm_convert_to_24() failed");
+	if (buf == NULL) {
+		g_set_error(error_r, pcm_convert_quark(), 0,
+			    "Conversion from %u to 24 bit is not implemented",
+			    src_format->bits);
+		return NULL;
+	}
 
 	if (src_format->channels != dest_format->channels) {
 		buf = pcm_convert_channels_24(&state->channels_buffer,
 					      dest_format->channels,
 					      src_format->channels,
 					      buf, len, &len);
-		if (!buf)
-			g_error("pcm_convert_channels_24() failed");
+		if (buf == NULL) {
+			g_set_error(error_r, pcm_convert_quark(), 0,
+				    "Conversion from %u to %u channels "
+				    "is not implemented",
+				    src_format->channels,
+				    dest_format->channels);
+			return NULL;
+		}
 	}
 
-	if (src_format->sample_rate != dest_format->sample_rate)
+	if (src_format->sample_rate != dest_format->sample_rate) {
 		buf = pcm_resample_24(&state->resample,
 				      dest_format->channels,
 				      src_format->sample_rate, buf, len,
-				      dest_format->sample_rate,
-				      &len);
+				      dest_format->sample_rate, &len,
+				      error_r);
+		if (buf == NULL)
+			return NULL;
+	}
 
 	if (dest_format->reverse_endian) {
 		buf = pcm_byteswap_32(&state->byteswap_buffer, buf, len);
@@ -141,8 +167,8 @@ static const int32_t *
 pcm_convert_32(struct pcm_convert_state *state,
 	       const struct audio_format *src_format,
 	       const void *src_buffer, size_t src_size,
-	       const struct audio_format *dest_format,
-	       size_t *dest_size_r)
+	       const struct audio_format *dest_format, size_t *dest_size_r,
+	       GError **error_r)
 {
 	const int32_t *buf;
 	size_t len;
@@ -151,24 +177,37 @@ pcm_convert_32(struct pcm_convert_state *state,
 
 	buf = pcm_convert_to_32(&state->format_buffer, src_format->bits,
 				src_buffer, src_size, &len);
-	if (!buf)
-		g_error("pcm_convert_to_32() failed");
+	if (buf == NULL) {
+		g_set_error(error_r, pcm_convert_quark(), 0,
+			    "Conversion from %u to 24 bit is not implemented",
+			    src_format->bits);
+		return NULL;
+	}
 
 	if (src_format->channels != dest_format->channels) {
 		buf = pcm_convert_channels_32(&state->channels_buffer,
 					      dest_format->channels,
 					      src_format->channels,
 					      buf, len, &len);
-		if (!buf)
-			g_error("pcm_convert_channels_32() failed");
+		if (buf == NULL) {
+			g_set_error(error_r, pcm_convert_quark(), 0,
+				    "Conversion from %u to %u channels "
+				    "is not implemented",
+				    src_format->channels,
+				    dest_format->channels);
+			return NULL;
+		}
 	}
 
-	if (src_format->sample_rate != dest_format->sample_rate)
+	if (src_format->sample_rate != dest_format->sample_rate) {
 		buf = pcm_resample_32(&state->resample,
 				      dest_format->channels,
 				      src_format->sample_rate, buf, len,
-				      dest_format->sample_rate,
-				      &len);
+				      dest_format->sample_rate, &len,
+				      error_r);
+		if (buf == NULL)
+			return buf;
+	}
 
 	if (dest_format->reverse_endian) {
 		buf = pcm_byteswap_32(&state->byteswap_buffer, buf, len);
@@ -184,26 +223,32 @@ pcm_convert(struct pcm_convert_state *state,
 	    const struct audio_format *src_format,
 	    const void *src, size_t src_size,
 	    const struct audio_format *dest_format,
-	    size_t *dest_size_r)
+	    size_t *dest_size_r,
+	    GError **error_r)
 {
 	switch (dest_format->bits) {
 	case 16:
 		return pcm_convert_16(state,
 				      src_format, src, src_size,
-				      dest_format, dest_size_r);
+				      dest_format, dest_size_r,
+				      error_r);
 
 	case 24:
 		return pcm_convert_24(state,
 				      src_format, src, src_size,
-				      dest_format, dest_size_r);
+				      dest_format, dest_size_r,
+				      error_r);
 
 	case 32:
 		return pcm_convert_32(state,
 				      src_format, src, src_size,
-				      dest_format, dest_size_r);
+				      dest_format, dest_size_r,
+				      error_r);
 
 	default:
-		g_error("cannot convert to %u bit\n", dest_format->bits);
+		g_set_error(error_r, pcm_convert_quark(), 0,
+			    "PCM conversion to %u bit is not implemented",
+			    dest_format->bits);
 		return NULL;
 	}
 }

src/pcm_convert.h

@@ -46,6 +46,12 @@ struct pcm_convert_state {
 	struct pcm_buffer byteswap_buffer;
 };
 
+static inline GQuark
+pcm_convert_quark(void)
+{
+	return g_quark_from_static_string("pcm_convert");
+}
+
 /**
  * Initializes a pcm_convert_state object.
  */
@@ -66,13 +72,16 @@ void pcm_convert_deinit(struct pcm_convert_state *state);
  * @param src_size the size of #src in bytes
  * @param dest_format the requested destination audio format
  * @param dest_size_r returns the number of bytes of the destination buffer
- * @return the destination buffer
+ * @param error_r location to store the error occuring, or NULL to
+ * ignore errors
+ * @return the destination buffer, or NULL on error
  */
 const void *
 pcm_convert(struct pcm_convert_state *state,
 	    const struct audio_format *src_format,
 	    const void *src, size_t src_size,
 	    const struct audio_format *dest_format,
-	    size_t *dest_size_r);
+	    size_t *dest_size_r,
+	    GError **error_r);
 
 #endif

src/pcm_format.c

@@ -21,8 +21,6 @@
 #include "pcm_dither.h"
 #include "pcm_buffer.h"
 
-#include <glib.h>
-
 static void
 pcm_convert_8_to_16(int16_t *out, const int8_t *in,
 		    unsigned num_samples)
@@ -93,7 +91,6 @@ pcm_convert_to_16(struct pcm_buffer *buffer, struct pcm_dither *dither,
 		return dest;
 	}
 
-	g_warning("only 8 or 16 bits are supported for conversion!\n");
 	return NULL;
 }
 
@@ -168,7 +165,6 @@ pcm_convert_to_24(struct pcm_buffer *buffer,
 		return dest;
 	}
 
-	g_warning("only 8 or 24 bits are supported for conversion!\n");
 	return NULL;
 }
 
@@ -243,6 +239,5 @@ pcm_convert_to_32(struct pcm_buffer *buffer,
 		return src;
 	}
 
-	g_warning("only 8 or 32 bits are supported for conversion!\n");
 	return NULL;
 }

src/pcm_resample.c

@@ -63,14 +63,17 @@ const int16_t *
 pcm_resample_16(struct pcm_resample_state *state,
 		uint8_t channels,
 		unsigned src_rate, const int16_t *src_buffer, size_t src_size,
-		unsigned dest_rate,
-		size_t *dest_size_r)
+		unsigned dest_rate, size_t *dest_size_r,
+		GError **error_r)
 {
 #ifdef HAVE_LIBSAMPLERATE
 	if (pcm_resample_lsr_enabled())
 		return pcm_resample_lsr_16(state, channels,
 					   src_rate, src_buffer, src_size,
-					   dest_rate, dest_size_r);
+					   dest_rate, dest_size_r,
+					   error_r);
+#else
+	(void)error_r;
 #endif
 
 	return pcm_resample_fallback_16(state, channels,
@@ -82,14 +85,17 @@ const int32_t *
 pcm_resample_32(struct pcm_resample_state *state,
 		uint8_t channels,
 		unsigned src_rate, const int32_t *src_buffer, size_t src_size,
-		unsigned dest_rate,
-		size_t *dest_size_r)
+		unsigned dest_rate, size_t *dest_size_r,
+		GError **error_r)
 {
 #ifdef HAVE_LIBSAMPLERATE
 	if (pcm_resample_lsr_enabled())
 		return pcm_resample_lsr_32(state, channels,
 					   src_rate, src_buffer, src_size,
-					   dest_rate, dest_size_r);
+					   dest_rate, dest_size_r,
+					   error_r);
+#else
+	(void)error_r;
 #endif
 
 	return pcm_resample_fallback_32(state, channels,

src/pcm_resample.h

@@ -48,7 +48,7 @@ struct pcm_resample_state {
 		uint8_t channels;
 	} prev;
 
-	bool error;
+	int error;
 #endif
 
 	struct pcm_buffer buffer;
@@ -82,8 +82,8 @@ pcm_resample_16(struct pcm_resample_state *state,
 		uint8_t channels,
 		unsigned src_rate,
 		const int16_t *src_buffer, size_t src_size,
-		unsigned dest_rate,
-		size_t *dest_size_r);
+		unsigned dest_rate, size_t *dest_size_r,
+		GError **error_r);
 
 /**
  * Resamples 32 bit PCM data.
@@ -102,8 +102,8 @@ pcm_resample_32(struct pcm_resample_state *state,
 		uint8_t channels,
 		unsigned src_rate,
 		const int32_t *src_buffer, size_t src_size,
-		unsigned dest_rate,
-		size_t *dest_size_r);
+		unsigned dest_rate, size_t *dest_size_r,
+		GError **error_r);
 
 /**
  * Resamples 24 bit PCM data.
@@ -122,14 +122,14 @@ pcm_resample_24(struct pcm_resample_state *state,
 		uint8_t channels,
 		unsigned src_rate,
 		const int32_t *src_buffer, size_t src_size,
-		unsigned dest_rate,
-		size_t *dest_size_r)
+		unsigned dest_rate, size_t *dest_size_r,
+		GError **error_r)
 {
 	/* reuse the 32 bit code - the resampler code doesn't care if
 	   the upper 8 bits are actually used */
 	return pcm_resample_32(state, channels,
 			       src_rate, src_buffer, src_size,
-			       dest_rate, dest_size_r);
+			       dest_rate, dest_size_r, error_r);
 }
 
 #endif

src/pcm_resample_internal.h

@@ -40,8 +40,8 @@ pcm_resample_lsr_16(struct pcm_resample_state *state,
 		    uint8_t channels,
 		    unsigned src_rate,
 		    const int16_t *src_buffer, size_t src_size,
-		    unsigned dest_rate,
-		    size_t *dest_size_r);
+		    unsigned dest_rate, size_t *dest_size_r,
+		    GError **error_r);
 
 const int32_t *
 pcm_resample_lsr_32(struct pcm_resample_state *state,
@@ -49,8 +49,8 @@ pcm_resample_lsr_32(struct pcm_resample_state *state,
 		    unsigned src_rate,
 		    const int32_t *src_buffer,
 		    G_GNUC_UNUSED size_t src_size,
-		    unsigned dest_rate,
-		    size_t *dest_size_r);
+		    unsigned dest_rate, size_t *dest_size_r,
+		    GError **error_r);
 
 #endif
 

src/pcm_resample_libsamplerate.c

@@ -30,6 +30,12 @@
 #undef G_LOG_DOMAIN
 #define G_LOG_DOMAIN "pcm"
 
+static inline GQuark
+libsamplerate_quark(void)
+{
+	return g_quark_from_static_string("libsamplerate");
+}
+
 void
 pcm_resample_lsr_deinit(struct pcm_resample_state *state)
 {
@@ -77,9 +83,10 @@ out:
 	return convalgo;
 }
 
-static void
+static bool
 pcm_resample_set(struct pcm_resample_state *state,
-		 uint8_t channels, unsigned src_rate, unsigned dest_rate)
+		 uint8_t channels, unsigned src_rate, unsigned dest_rate,
+		 GError **error_r)
 {
 	static int convalgo = -1;
 	int error;
@@ -92,9 +99,9 @@ pcm_resample_set(struct pcm_resample_state *state,
 	if (channels == state->prev.channels &&
 	    src_rate == state->prev.src_rate &&
 	    dest_rate == state->prev.dest_rate)
-		return;
+		return true;
 
-	state->error = false;
+	state->error = 0;
 	state->prev.channels = channels;
 	state->prev.src_rate = src_rate;
 	state->prev.dest_rate = dest_rate;
@@ -104,16 +111,18 @@ pcm_resample_set(struct pcm_resample_state *state,
 
 	state->state = src_new(convalgo, channels, &error);
 	if (!state->state) {
-		g_warning("cannot create new libsamplerate state: %s",
-			  src_strerror(error));
-		state->error = true;
-		return;
+		g_set_error(error_r, libsamplerate_quark(), state->error,
+			    "libsamplerate initialization has failed: %s",
+			    src_strerror(error));
+		return false;
 	}
 
 	data->src_ratio = (double)dest_rate / (double)src_rate;
 	g_debug("setting samplerate conversion ratio to %.2lf",
 		data->src_ratio);
 	src_set_ratio(state->state, data->src_ratio);
+
+	return true;
 }
 
 const int16_t *
@@ -121,9 +130,10 @@ pcm_resample_lsr_16(struct pcm_resample_state *state,
 		    uint8_t channels,
 		    unsigned src_rate,
 		    const int16_t *src_buffer, size_t src_size,
-		    unsigned dest_rate,
-		    size_t *dest_size_r)
+		    unsigned dest_rate, size_t *dest_size_r,
+		    GError **error_r)
 {
+	bool success;
 	SRC_DATA *data = &state->data;
 	size_t data_in_size;
 	size_t data_out_size;
@@ -132,11 +142,18 @@ pcm_resample_lsr_16(struct pcm_resample_state *state,
 
 	assert((src_size % (sizeof(*src_buffer) * channels)) == 0);
 
-	pcm_resample_set(state, channels, src_rate, dest_rate);
+	success = pcm_resample_set(state, channels, src_rate, dest_rate,
+				   error_r);
+	if (!success)
+		return NULL;
 
 	/* there was an error previously, and nothing has changed */
-	if (state->error)
+	if (state->error) {
+		g_set_error(error_r, libsamplerate_quark(), state->error,
+			    "libsamplerate has failed: %s",
+			    src_strerror(state->error));
 		return NULL;
+	}
 
 	data->input_frames = src_size / sizeof(*src_buffer) / channels;
 	data_in_size = data->input_frames * sizeof(float) * channels;
@@ -151,9 +168,10 @@ pcm_resample_lsr_16(struct pcm_resample_state *state,
 
 	error = src_process(state->state, data);
 	if (error) {
-		g_warning("error processing samples with libsamplerate: %s",
-			  src_strerror(error));
-		state->error = true;
+		g_set_error(error_r, libsamplerate_quark(), error,
+			    "libsamplerate has failed: %s",
+			    src_strerror(error));
+		state->error = error;
 		return NULL;
 	}
 
@@ -191,9 +209,10 @@ pcm_resample_lsr_32(struct pcm_resample_state *state,
 		    uint8_t channels,
 		    unsigned src_rate,
 		    const int32_t *src_buffer, size_t src_size,
-		    unsigned dest_rate,
-		    size_t *dest_size_r)
+		    unsigned dest_rate, size_t *dest_size_r,
+		    GError **error_r)
 {
+	bool success;
 	SRC_DATA *data = &state->data;
 	size_t data_in_size;
 	size_t data_out_size;
@@ -202,11 +221,18 @@ pcm_resample_lsr_32(struct pcm_resample_state *state,
 
 	assert((src_size % (sizeof(*src_buffer) * channels)) == 0);
 
-	pcm_resample_set(state, channels, src_rate, dest_rate);
+	success = pcm_resample_set(state, channels, src_rate, dest_rate,
+				   error_r);
+	if (!success)
+		return NULL;
 
 	/* there was an error previously, and nothing has changed */
-	if (state->error)
+	if (state->error) {
+		g_set_error(error_r, libsamplerate_quark(), state->error,
+			    "libsamplerate has failed: %s",
+			    src_strerror(state->error));
 		return NULL;
+	}
 
 	data->input_frames = src_size / sizeof(*src_buffer) / channels;
 	data_in_size = data->input_frames * sizeof(float) * channels;
@@ -221,9 +247,10 @@ pcm_resample_lsr_32(struct pcm_resample_state *state,
 
 	error = src_process(state->state, data);
 	if (error) {
-		g_warning("error processing samples with libsamplerate: %s",
-			  src_strerror(error));
-		state->error = true;
+		g_set_error(error_r, libsamplerate_quark(), error,
+			    "libsamplerate has failed: %s",
+			    src_strerror(error));
+		state->error = error;
 		return NULL;
 	}
 

test/run_output.c

@@ -44,8 +44,11 @@ pcm_convert(G_GNUC_UNUSED struct pcm_convert_state *state,
 	    G_GNUC_UNUSED const struct audio_format *src_format,
 	    G_GNUC_UNUSED const void *src, G_GNUC_UNUSED size_t src_size,
 	    G_GNUC_UNUSED const struct audio_format *dest_format,
-	    G_GNUC_UNUSED size_t *dest_size_r)
+	    G_GNUC_UNUSED size_t *dest_size_r,
+	    GError **error_r)
 {
+	g_set_error(error_r, pcm_convert_quark(), 0,
+		    "Not implemented");
 	return NULL;
 }