Merge branch 'v0.17.x'

8 files changed, 130 insertions, 145 deletions

diff --git a/NEWS b/NEWS
index 2407c155..e3b7041f 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,12 @@ ver 0.18 (2012/??/??)
   - new option "tags" may be used to disable sending tags to output
 * improved decoder/output error reporting
 
+ver 0.17.3 (2012/??/??)
+* output:
+  - recorder: fix I/O error check
+  - shout: fix memory leak in error handler
+  - recorder, shout: support Ogg packets that span more than one page
+
 ver 0.17.2 (2012/09/30)
 * protocol:
   - fix crash in local file check
diff --git a/src/encoder/vorbis_encoder.c b/src/encoder/vorbis_encoder.c
index fcf2b513..468cf38e 100644
--- a/src/encoder/vorbis_encoder.c
+++ b/src/encoder/vorbis_encoder.c
@@ -65,13 +65,11 @@ static bool
 vorbis_encoder_configure(struct vorbis_encoder *encoder,
 			 const struct config_param *param, GError **error)
 {
-	const char *value;
-	char *endptr;
-
-	value = config_get_block_string(param, "quality", NULL);
+	const char *value = config_get_block_string(param, "quality", NULL);
 	if (value != NULL) {
 		/* a quality was configured (VBR) */
 
+		char *endptr;
 		encoder->quality = g_ascii_strtod(value, &endptr);
 
 		if (*endptr != '\0' || encoder->quality < -1.0 ||
@@ -103,8 +101,9 @@ vorbis_encoder_configure(struct vorbis_encoder *encoder,
 		}
 
 		encoder->quality = -2.0;
-		encoder->bitrate = g_ascii_strtoll(value, &endptr, 10);
 
+		char *endptr;
+		encoder->bitrate = g_ascii_strtoll(value, &endptr, 10);
 		if (*endptr != '\0' || encoder->bitrate <= 0) {
 			g_set_error(error, vorbis_encoder_quark(), 0,
 				    "bitrate at line %i should be a positive integer",
@@ -119,9 +118,7 @@ vorbis_encoder_configure(struct vorbis_encoder *encoder,
 static struct encoder *
 vorbis_encoder_init(const struct config_param *param, GError **error)
 {
-	struct vorbis_encoder *encoder;
-
-	encoder = g_new(struct vorbis_encoder, 1);
+	struct vorbis_encoder *encoder = g_new(struct vorbis_encoder, 1);
 	encoder_struct_init(&encoder->encoder, &vorbis_encoder_plugin);
 
 	/* load configuration from "param" */
@@ -211,14 +208,12 @@ vorbis_encoder_open(struct encoder *_encoder,
 		    GError **error)
 {
 	struct vorbis_encoder *encoder = (struct vorbis_encoder *)_encoder;
-	bool ret;
 
 	audio_format->format = SAMPLE_FORMAT_S16;
 
 	encoder->audio_format = *audio_format;
 
-	ret = vorbis_encoder_reinit(encoder, error);
-	if (!ret)
+	if (!vorbis_encoder_reinit(encoder, error))
 		return false;
 
 	vorbis_encoder_send_header(encoder);
@@ -251,11 +246,10 @@ static void
 vorbis_encoder_blockout(struct vorbis_encoder *encoder)
 {
 	while (vorbis_analysis_blockout(&encoder->vd, &encoder->vb) == 1) {
-		ogg_packet packet;
-
 		vorbis_analysis(&encoder->vb, NULL);
 		vorbis_bitrate_addblock(&encoder->vb);
 
+		ogg_packet packet;
 		while (vorbis_bitrate_flushpacket(&encoder->vd, &packet))
 			ogg_stream_packetin(&encoder->os, &packet);
 	}
@@ -344,9 +338,9 @@ vorbis_encoder_write(struct encoder *_encoder,
 		     G_GNUC_UNUSED GError **error)
 {
 	struct vorbis_encoder *encoder = (struct vorbis_encoder *)_encoder;
-	unsigned num_frames;
 
-	num_frames = length / audio_format_frame_size(&encoder->audio_format);
+	unsigned num_frames = length
+		/ audio_format_frame_size(&encoder->audio_format);
 
 	/* this is for only 16-bit audio */
 
@@ -364,12 +358,10 @@ static size_t
 vorbis_encoder_read(struct encoder *_encoder, void *_dest, size_t length)
 {
 	struct vorbis_encoder *encoder = (struct vorbis_encoder *)_encoder;
-	ogg_page page;
-	int ret;
 	unsigned char *dest = _dest;
-	size_t nbytes;
 
-	ret = ogg_stream_pageout(&encoder->os, &page);
+	ogg_page page;
+	int ret = ogg_stream_pageout(&encoder->os, &page);
 	if (ret == 0 && encoder->flush) {
 		encoder->flush = false;
 		ret = ogg_stream_flush(&encoder->os, &page);
@@ -381,7 +373,7 @@ vorbis_encoder_read(struct encoder *_encoder, void *_dest, size_t length)
 
 	assert(page.header_len > 0 || page.body_len > 0);
 
-	nbytes = (size_t)page.header_len + (size_t)page.body_len;
+	size_t nbytes = (size_t)page.header_len + (size_t)page.body_len;
 
 	if (nbytes > length)
 		/* XXX better error handling */
diff --git a/src/encoder_plugin.h b/src/encoder_plugin.h
index 68a89d57..e0748a13 100644
--- a/src/encoder_plugin.h
+++ b/src/encoder_plugin.h
@@ -119,6 +119,10 @@ encoder_finish(struct encoder *encoder)
  * Before you free it, you must call encoder_close().  You may open
  * and close (reuse) one encoder any number of times.
  *
+ * After this function returns successfully and before the first
+ * encoder_write() call, you should invoke encoder_read() to obtain
+ * the file header.
+ *
  * @param encoder the encoder
  * @param audio_format the encoder's input audio format; the plugin
  * may modify the struct to adapt it to its abilities
@@ -291,6 +295,8 @@ encoder_write(struct encoder *encoder, const void *data, size_t length,
 /**
  * Reads encoded data from the encoder.
  *
+ * Call this repeatedly until no more data is returned.
+ *
  * @param encoder the encoder
  * @param dest the destination buffer to copy to
  * @param length the maximum length of the destination buffer
diff --git a/src/output/httpd_output_plugin.c b/src/output/httpd_output_plugin.c
index abef826b..1d730df7 100644
--- a/src/output/httpd_output_plugin.c
+++ b/src/output/httpd_output_plugin.c
@@ -114,10 +114,6 @@ httpd_output_init(const struct config_param *param,
 		return NULL;
 	}
 
-	const char *encoder_name, *bind_to_address;
-	const struct encoder_plugin *encoder_plugin;
-	guint port;
-
 	/* read configuration */
 	httpd->name =
 		config_get_block_string(param, "name", "Set name in config");
@@ -126,10 +122,12 @@ httpd_output_init(const struct config_param *param,
 	httpd->website =
 		config_get_block_string(param, "website", "Set website in config");
 
-	port = config_get_block_unsigned(param, "port", 8000);
+	guint port = config_get_block_unsigned(param, "port", 8000);
 
-	encoder_name = config_get_block_string(param, "encoder", "vorbis");
-	encoder_plugin = encoder_plugin_get(encoder_name);
+	const char *encoder_name =
+		config_get_block_string(param, "encoder", "vorbis");
+	const struct encoder_plugin *encoder_plugin =
+		encoder_plugin_get(encoder_name);
 	if (encoder_plugin == NULL) {
 		g_set_error(error, httpd_output_quark(), 0,
 			    "No such encoder: %s", encoder_name);
@@ -144,7 +142,7 @@ httpd_output_init(const struct config_param *param,
 
 	httpd->server_socket = server_socket_new(httpd_listen_in_event, httpd);
 
-	bind_to_address =
+	const char *bind_to_address =
 		config_get_block_string(param, "bind_to_address", NULL);
 	bool success = bind_to_address != NULL &&
 		strcmp(bind_to_address, "any") != 0
@@ -275,8 +273,6 @@ httpd_listen_in_event(int fd, const struct sockaddr *address,
 static struct page *
 httpd_output_read_page(struct httpd_output *httpd)
 {
-	size_t size = 0, nbytes;
-
 	if (httpd->unflushed_input >= 65536) {
 		/* we have fed a lot of input into the encoder, but it
 		   didn't give anything back yet - flush now to avoid
@@ -285,9 +281,11 @@ httpd_output_read_page(struct httpd_output *httpd)
 		httpd->unflushed_input = 0;
 	}
 
+	size_t size = 0;
 	do {
-		nbytes = encoder_read(httpd->encoder, httpd->buffer + size,
-				      sizeof(httpd->buffer) - size);
+		size_t nbytes = encoder_read(httpd->encoder,
+					     httpd->buffer + size,
+					     sizeof(httpd->buffer) - size);
 		if (nbytes == 0)
 			break;
 
@@ -307,10 +305,7 @@ httpd_output_encoder_open(struct httpd_output *httpd,
 			  struct audio_format *audio_format,
 			  GError **error)
 {
-	bool success;
-
-	success = encoder_open(httpd->encoder, audio_format, error);
-	if (!success)
+	if (!encoder_open(httpd->encoder, audio_format, error))
 		return false;
 
 	/* we have to remember the encoder header, i.e. the first
@@ -344,14 +339,12 @@ httpd_output_open(struct audio_output *ao, struct audio_format *audio_format,
 		  GError **error)
 {
 	struct httpd_output *httpd = (struct httpd_output *)ao;
-	bool success;
 
 	g_mutex_lock(httpd->mutex);
 
 	/* open the encoder */
 
-	success = httpd_output_encoder_open(httpd, audio_format, error);
-	if (!success) {
+	if (!httpd_output_encoder_open(httpd, audio_format, error)) {
 		g_mutex_unlock(httpd->mutex);
 		return false;
 	}
@@ -495,10 +488,7 @@ static bool
 httpd_output_encode_and_play(struct httpd_output *httpd,
 			     const void *chunk, size_t size, GError **error)
 {
-	bool success;
-
-	success = encoder_write(httpd->encoder, chunk, size, error);
-	if (!success)
+	if (!encoder_write(httpd->encoder, chunk, size, error))
 		return false;
 
 	httpd->unflushed_input += size;
@@ -510,16 +500,12 @@ httpd_output_encode_and_play(struct httpd_output *httpd,
 
 static size_t
 httpd_output_play(struct audio_output *ao, const void *chunk, size_t size,
-		  GError **error)
+		  GError **error_r)
 {
 	struct httpd_output *httpd = (struct httpd_output *)ao;
 
 	if (httpd_output_lock_has_clients(httpd)) {
-		bool success;
-
-		success = httpd_output_encode_and_play(httpd, chunk, size,
-						       error);
-		if (!success)
+		if (!httpd_output_encode_and_play(httpd, chunk, size, error_r))
 			return 0;
 	}
 
@@ -562,7 +548,6 @@ httpd_output_tag(struct audio_output *ao, const struct tag *tag)
 
 	if (httpd->encoder->plugin->tag != NULL) {
 		/* embed encoder tags */
-		struct page *page;
 
 		/* flush the current stream, and end it */
 
@@ -578,7 +563,7 @@ httpd_output_tag(struct audio_output *ao, const struct tag *tag)
 		   used as the new "header" page, which is sent to all
 		   new clients */
 
-		page = httpd_output_read_page(httpd);
+		struct page *page = httpd_output_read_page(httpd);
 		if (page != NULL) {
 			if (httpd->header != NULL)
 				page_unref(httpd->header);
diff --git a/src/output/recorder_output_plugin.c b/src/output/recorder_output_plugin.c
index ea299468..b84cb244 100644
--- a/src/output/recorder_output_plugin.c
+++ b/src/output/recorder_output_plugin.c
@@ -77,13 +77,12 @@ recorder_output_init(const struct config_param *param, GError **error_r)
 		return NULL;
 	}
 
-	const char *encoder_name;
-	const struct encoder_plugin *encoder_plugin;
-
 	/* read configuration */
 
-	encoder_name = config_get_block_string(param, "encoder", "vorbis");
-	encoder_plugin = encoder_plugin_get(encoder_name);
+	const char *encoder_name =
+		config_get_block_string(param, "encoder", "vorbis");
+	const struct encoder_plugin *encoder_plugin =
+		encoder_plugin_get(encoder_name);
 	if (encoder_plugin == NULL) {
 		g_set_error(error_r, recorder_output_quark(), 0,
 			    "No such encoder: %s", encoder_name);
@@ -121,33 +120,22 @@ recorder_output_finish(struct audio_output *ao)
 	g_free(recorder);
 }
 
-/**
- * Writes pending data from the encoder to the output file.
- */
 static bool
-recorder_output_encoder_to_file(struct recorder_output *recorder,
-			      GError **error_r)
+recorder_write_to_file(struct recorder_output *recorder,
+		       const void *_data, size_t length,
+		       GError **error_r)
 {
-	size_t size = 0, position, nbytes;
-
-	assert(recorder->fd >= 0);
-
-	/* read from the encoder */
+	assert(length > 0);
 
-	size = encoder_read(recorder->encoder, recorder->buffer,
-			    sizeof(recorder->buffer));
-	if (size == 0)
-		return true;
+	const int fd = recorder->fd;
 
-	/* write everything into the file */
+	const uint8_t *data = (const uint8_t *)_data, *end = data + length;
 
-	position = 0;
 	while (true) {
-		nbytes = write(recorder->fd, recorder->buffer + position,
-			       size - position);
+		ssize_t nbytes = write(fd, data, end - data);
 		if (nbytes > 0) {
-			position += (size_t)nbytes;
-			if (position >= size)
+			data += nbytes;
+			if (data == end)
 				return true;
 		} else if (nbytes == 0) {
 			/* shouldn't happen for files */
@@ -163,13 +151,37 @@ recorder_output_encoder_to_file(struct recorder_output *recorder,
 	}
 }
 
+/**
+ * Writes pending data from the encoder to the output file.
+ */
+static bool
+recorder_output_encoder_to_file(struct recorder_output *recorder,
+				GError **error_r)
+{
+	assert(recorder->fd >= 0);
+
+	while (true) {
+		/* read from the encoder */
+
+		size_t size = encoder_read(recorder->encoder, recorder->buffer,
+					   sizeof(recorder->buffer));
+		if (size == 0)
+			return true;
+
+		/* write everything into the file */
+
+		if (!recorder_write_to_file(recorder, recorder->buffer, size,
+					    error_r))
+			return false;
+	}
+}
+
 static bool
 recorder_output_open(struct audio_output *ao,
 		     struct audio_format *audio_format,
 		     GError **error_r)
 {
 	struct recorder_output *recorder = (struct recorder_output *)ao;
-	bool success;
 
 	/* create the output file */
 
@@ -185,8 +197,14 @@ recorder_output_open(struct audio_output *ao,
 
 	/* open the encoder */
 
-	success = encoder_open(recorder->encoder, audio_format, error_r);
-	if (!success) {
+	if (!encoder_open(recorder->encoder, audio_format, error_r)) {
+		close(recorder->fd);
+		unlink(recorder->path);
+		return false;
+	}
+
+	if (!recorder_output_encoder_to_file(recorder, error_r)) {
+		encoder_close(recorder->encoder);
 		close(recorder->fd);
 		unlink(recorder->path);
 		return false;
diff --git a/src/output/shout_output_plugin.c b/src/output/shout_output_plugin.c
index 7867ae63..56456a0e 100644
--- a/src/output/shout_output_plugin.c
+++ b/src/output/shout_output_plugin.c
@@ -36,11 +36,6 @@
 
 #define DEFAULT_CONN_TIMEOUT  2
 
-struct shout_buffer {
-	unsigned char data[32768];
-	size_t len;
-};
-
 struct shout_data {
 	struct audio_output base;
 
@@ -54,7 +49,7 @@ struct shout_data {
 
 	int timeout;
 
-	struct shout_buffer buf;
+	uint8_t buffer[32768];
 };
 
 static int shout_init_count;
@@ -114,24 +109,7 @@ static struct audio_output *
 my_shout_init_driver(const struct config_param *param,
 		     GError **error)
 {
-	struct shout_data *sd;
-	char *test;
-	unsigned port;
-	char *host;
-	char *mount;
-	char *passwd;
-	const char *encoding;
-	const struct encoder_plugin *encoder_plugin;
-	unsigned shout_format;
-	unsigned protocol;
-	const char *user;
-	char *name;
-	const char *value;
-	const struct block_param *block_param;
-	int public;
-
-	sd = new_shout_data();
-
+	struct shout_data *sd = new_shout_data();
 	if (!ao_base_init(&sd->base, &shout_output_plugin, param, error)) {
 		free_shout_data(sd);
 		return NULL;
@@ -152,13 +130,14 @@ my_shout_init_driver(const struct config_param *param,
 
 	shout_init_count++;
 
+	const struct block_param *block_param;
 	check_block_param("host");
-	host = block_param->value;
+	char *host = block_param->value;
 
 	check_block_param("mount");
-	mount = block_param->value;
+	char *mount = block_param->value;
 
-	port = config_get_block_unsigned(param, "port", 0);
+	unsigned port = config_get_block_unsigned(param, "port", 0);
 	if (port == 0) {
 		g_set_error(error, shout_output_quark(), 0,
 			    "shout port must be configured");
@@ -166,17 +145,18 @@ my_shout_init_driver(const struct config_param *param,
 	}
 
 	check_block_param("password");
-	passwd = block_param->value;
+	const char *passwd = block_param->value;
 
 	check_block_param("name");
-	name = block_param->value;
+	const char *name = block_param->value;
 
-	public = config_get_block_bool(param, "public", false);
+	bool public = config_get_block_bool(param, "public", false);
 
-	user = config_get_block_string(param, "user", "source");
+	const char *user = config_get_block_string(param, "user", "source");
 
-	value = config_get_block_string(param, "quality", NULL);
+	const char *value = config_get_block_string(param, "quality", NULL);
 	if (value != NULL) {
+		char *test;
 		sd->quality = strtod(value, &test);
 
 		if (*test != '\0' || sd->quality < -1.0 || sd->quality > 10.0) {
@@ -201,6 +181,7 @@ my_shout_init_driver(const struct config_param *param,
 			goto failure;
 		}
 
+		char *test;
 		sd->bitrate = strtol(value, &test, 10);
 
 		if (*test != '\0' || sd->bitrate <= 0) {
@@ -210,8 +191,10 @@ my_shout_init_driver(const struct config_param *param,
 		}
 	}
 
-	encoding = config_get_block_string(param, "encoding", "ogg");
-	encoder_plugin = shout_encoder_plugin_get(encoding);
+	const char *encoding = config_get_block_string(param, "encoding",
+						       "ogg");
+	const struct encoder_plugin *encoder_plugin =
+		shout_encoder_plugin_get(encoding);
 	if (encoder_plugin == NULL) {
 		g_set_error(error, shout_output_quark(), 0,
 			    "couldn't find shout encoder plugin \"%s\"",
@@ -223,11 +206,13 @@ my_shout_init_driver(const struct config_param *param,
 	if (sd->encoder == NULL)
 		goto failure;
 
+	unsigned shout_format;
 	if (strcmp(encoding, "mp3") == 0 || strcmp(encoding, "lame") == 0)
 		shout_format = SHOUT_FORMAT_MP3;
 	else
 		shout_format = SHOUT_FORMAT_OGG;
 
+	unsigned protocol;
 	value = config_get_block_string(param, "protocol", NULL);
 	if (value != NULL) {
 		if (0 == strcmp(value, "shoutcast") &&
@@ -355,26 +340,24 @@ handle_shout_error(struct shout_data *sd, int err, GError **error)
 static bool
 write_page(struct shout_data *sd, GError **error)
 {
-	int err;
-
 	assert(sd->encoder != NULL);
 
-	sd->buf.len = encoder_read(sd->encoder,
-				   sd->buf.data, sizeof(sd->buf.data));
-	if (sd->buf.len == 0)
-		return true;
+	while (true) {
+		size_t nbytes = encoder_read(sd->encoder,
+					     sd->buffer, sizeof(sd->buffer));
+		if (nbytes == 0)
+			return true;
 
-	err = shout_send(sd->shout_conn, sd->buf.data, sd->buf.len);
-	if (!handle_shout_error(sd, err, error))
-		return false;
+		int err = shout_send(sd->shout_conn, sd->buffer, nbytes);
+		if (!handle_shout_error(sd, err, error))
+			return false;
+	}
 
 	return true;
 }
 
 static void close_shout_conn(struct shout_data * sd)
 {
-	sd->buf.len = 0;
-
 	if (sd->encoder != NULL) {
 		if (encoder_end(sd->encoder, NULL))
 			write_page(sd, NULL);
@@ -425,10 +408,7 @@ my_shout_close_device(struct audio_output *ao)
 static bool
 shout_connect(struct shout_data *sd, GError **error)
 {
-	int state;
-
-	state = shout_open(sd->shout_conn);
-	switch (state) {
+	switch (shout_open(sd->shout_conn)) {
 	case SHOUTERR_SUCCESS:
 	case SHOUTERR_CONNECTED:
 		return true;
@@ -448,17 +428,17 @@ my_shout_open_device(struct audio_output *ao, struct audio_format *audio_format,
 		     GError **error)
 {
 	struct shout_data *sd = (struct shout_data *)ao;
-	bool ret;
 
-	ret = shout_connect(sd, error);
-	if (!ret)
+	if (!shout_connect(sd, error))
 		return false;
 
-	sd->buf.len = 0;
+	if (!encoder_open(sd->encoder, audio_format, error)) {
+		shout_close(sd->shout_conn);
+		return false;
+	}
 
-	ret = encoder_open(sd->encoder, audio_format, error) &&
-		write_page(sd, error);
-	if (!ret) {
+	if (!write_page(sd, error)) {
+		encoder_close(sd->encoder);
 		shout_close(sd->shout_conn);
 		return false;
 	}
@@ -528,32 +508,27 @@ static void my_shout_set_tag(struct audio_output *ao,
 			     const struct tag *tag)
 {
 	struct shout_data *sd = (struct shout_data *)ao;
-	bool ret;
 	GError *error = NULL;
 
 	if (sd->encoder->plugin->tag != NULL) {
 		/* encoder plugin supports stream tags */
 
-		ret = encoder_pre_tag(sd->encoder, &error);
-		if (!ret) {
+		if (!encoder_pre_tag(sd->encoder, &error)) {
 			g_warning("%s", error->message);
 			g_error_free(error);
 			return;
 		}
 
-		ret = write_page(sd, NULL);
-		if (!ret)
+		if (!write_page(sd, NULL))
 			return;
 
-		ret = encoder_tag(sd->encoder, tag, &error);
-		if (!ret) {
+		if (!encoder_tag(sd->encoder, tag, &error)) {
 			g_warning("%s", error->message);
 			g_error_free(error);
 		}
 	} else {
 		/* no stream tag support: fall back to icy-metadata */
 		char song[1024];
-
 		shout_tag_to_metadata(tag, song, sizeof(song));
 
 		shout_metadata_add(sd->shout_meta, "song", song);
diff --git a/test/run_encoder.c b/test/run_encoder.c
index 6a410862..db4d3af9 100644
--- a/test/run_encoder.c
+++ b/test/run_encoder.c
@@ -99,14 +99,15 @@ int main(int argc, char **argv)
 		}
 	}
 
-	ret = encoder_open(encoder, &audio_format, &error);
-	if (encoder == NULL) {
+	if (!encoder_open(encoder, &audio_format, &error)) {
 		g_printerr("Failed to open encoder: %s\n",
 			   error->message);
 		g_error_free(error);
 		return 1;
 	}
 
+	encoder_to_stdout(encoder);
+
 	/* do it */
 
 	while ((nbytes = read(0, buffer, sizeof(buffer))) > 0) {
diff --git a/test/test_vorbis_encoder.c b/test/test_vorbis_encoder.c
index 048d26f0..61939915 100644
--- a/test/test_vorbis_encoder.c
+++ b/test/test_vorbis_encoder.c
@@ -67,6 +67,8 @@ main(G_GNUC_UNUSED int argc, G_GNUC_UNUSED char **argv)
 	success = encoder_open(encoder, &audio_format, NULL);
 	assert(success);
 
+	encoder_to_stdout(encoder);
+
 	/* write a block of data */
 
 	success = encoder_write(encoder, zero, sizeof(zero), NULL);