From d1eeed6a5ba0ac35f9dcad6355fc2d18c1860a9f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 06:54:51 +0200 Subject: [PATCH 01/12] output/alsa: fix SIGFPE when alsa announces a period size of 0 --- NEWS | 2 ++ src/output/alsa_plugin.c | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/NEWS b/NEWS index 4756b64f6..ddba638c7 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ ver 0.16.4 (2011/??/??) * decoder: - ffmpeg: workaround for semantic API change in recent ffmpeg versions - flac: validate the sample rate when scanning the tag +* output: + - alsa: fix SIGFPE when alsa announces a period size of 0 ver 0.16.3 (2011/06/04) diff --git a/src/output/alsa_plugin.c b/src/output/alsa_plugin.c index 9177fabe4..422264f53 100644 --- a/src/output/alsa_plugin.c +++ b/src/output/alsa_plugin.c @@ -508,6 +508,14 @@ configure_hw: g_debug("buffer_size=%u period_size=%u", (unsigned)alsa_buffer_size, (unsigned)alsa_period_size); + if (alsa_period_size == 0) + /* this works around a SIGFPE bug that occurred when + an ALSA driver indicated period_size==0; this + caused a division by zero in alsa_play(). By using + the fallback "1", we make sure that this won't + happen again. */ + alsa_period_size = 1; + ad->period_frames = alsa_period_size; ad->period_position = 0; From d7d717f2ce096015527593f8e163dc3d59cf91fc Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 11:33:51 +0200 Subject: [PATCH 02/12] playlist_control: don't resume playback when seeking to another song while paused Use a shortcut in playlist_seek_song(), don't call playlist_play_order() because that would reset the "paused" state. --- NEWS | 1 + src/playlist_control.c | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index ddba638c7..26a4c1134 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ ver 0.16.4 (2011/??/??) * fix memory leaks +* don't resume playback when seeking to another song while paused * decoder: - ffmpeg: workaround for semantic API change in recent ffmpeg versions - flac: validate the sample rate when scanning the tag diff --git a/src/playlist_control.c b/src/playlist_control.c index ce9bc8442..76066d274 100644 --- a/src/playlist_control.c +++ b/src/playlist_control.c @@ -222,10 +222,12 @@ playlist_seek_song(struct playlist *playlist, unsigned song, float seek_time) playlist->error_count = 0; if (!playlist->playing || (unsigned)playlist->current != i) { - /* seeking is not within the current song - first - start playing the new song */ + /* seeking is not within the current song - prepare + song change */ + + playlist->playing = true; + playlist->current = i; - playlist_play_order(playlist, i); queued = NULL; } From e464be5f3922d87942a3afd0f7e9239b89901f2e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Jul 2011 22:32:48 +0200 Subject: [PATCH 03/12] decoder/wavpack: simplify the WavpackUnpackSamples()==0 check .. and remove one indent level. --- src/decoder/wavpack_decoder_plugin.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/decoder/wavpack_decoder_plugin.c b/src/decoder/wavpack_decoder_plugin.c index efed98851..601f05065 100644 --- a/src/decoder/wavpack_decoder_plugin.c +++ b/src/decoder/wavpack_decoder_plugin.c @@ -197,7 +197,7 @@ wavpack_decode(struct decoder *decoder, WavpackContext *wpc, bool can_seek) decoder_initialized(decoder, &audio_format, can_seek, total_time); - do { + while (true) { if (decoder_get_command(decoder) == DECODE_COMMAND_SEEK) { if (can_seek) { unsigned where = decoder_seek_where(decoder) * @@ -220,22 +220,19 @@ wavpack_decode(struct decoder *decoder, WavpackContext *wpc, bool can_seek) samples_got = WavpackUnpackSamples( wpc, (int32_t *)chunk, samples_requested ); - if (samples_got > 0) { - int bitrate = (int)(WavpackGetInstantBitrate(wpc) / - 1000 + 0.5); - format_samples( - bytes_per_sample, chunk, - samples_got * audio_format.channels - ); + if (samples_got == 0) + break; - decoder_data( - decoder, NULL, chunk, - samples_got * output_sample_size, - bitrate - ); - } - } while (samples_got > 0); + int bitrate = (int)(WavpackGetInstantBitrate(wpc) / 1000 + + 0.5); + format_samples(bytes_per_sample, chunk, + samples_got * audio_format.channels); + + decoder_data(decoder, NULL, chunk, + samples_got * output_sample_size, + bitrate); + } } /** From 4c4f8bf02a6315ce988fcacab9cfd890c67ca5d0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Jul 2011 22:47:12 +0200 Subject: [PATCH 04/12] decoder/wavpack: use the correct integer types libwavpack provides int32_t samples, and wants uin32_t for sample counts. --- src/decoder/wavpack_decoder_plugin.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/decoder/wavpack_decoder_plugin.c b/src/decoder/wavpack_decoder_plugin.c index 601f05065..c8f0b7152 100644 --- a/src/decoder/wavpack_decoder_plugin.c +++ b/src/decoder/wavpack_decoder_plugin.c @@ -34,9 +34,6 @@ #undef G_LOG_DOMAIN #define G_LOG_DOMAIN "wavpack" -/* pick 1020 since its devisible for 8,16,24, and 32-bit audio */ -#define CHUNK_SIZE 1020 - #define ERRORLEN 80 static struct { @@ -162,8 +159,6 @@ wavpack_decode(struct decoder *decoder, WavpackContext *wpc, bool can_seek) enum sample_format sample_format; struct audio_format audio_format; format_samples_t format_samples; - char chunk[CHUNK_SIZE]; - int samples_requested, samples_got; float total_time; int bytes_per_sample, output_sample_size; @@ -193,7 +188,9 @@ wavpack_decode(struct decoder *decoder, WavpackContext *wpc, bool can_seek) output_sample_size = audio_format_frame_size(&audio_format); /* wavpack gives us all kind of samples in a 32-bit space */ - samples_requested = sizeof(chunk) / (4 * audio_format.channels); + int32_t chunk[1024]; + const uint32_t samples_requested = G_N_ELEMENTS(chunk) / + audio_format.channels; decoder_initialized(decoder, &audio_format, can_seek, total_time); @@ -217,10 +214,8 @@ wavpack_decode(struct decoder *decoder, WavpackContext *wpc, bool can_seek) break; } - samples_got = WavpackUnpackSamples( - wpc, (int32_t *)chunk, samples_requested - ); - + uint32_t samples_got = WavpackUnpackSamples(wpc, chunk, + samples_requested); if (samples_got == 0) break; From 2e28ed8f81630cdadf9e3a22d0cd460f6afffe1a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 12:30:56 +0200 Subject: [PATCH 05/12] wavpack: obey all decoder commands, stop at CUE track border It used to ignore the decoder_data() return value. --- NEWS | 1 + src/decoder/wavpack_decoder_plugin.c | 15 ++++++--------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 26a4c1134..e6580c5f0 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ ver 0.16.4 (2011/??/??) * decoder: - ffmpeg: workaround for semantic API change in recent ffmpeg versions - flac: validate the sample rate when scanning the tag + - wavpack: obey all decoder commands, stop at CUE track border * output: - alsa: fix SIGFPE when alsa announces a period size of 0 diff --git a/src/decoder/wavpack_decoder_plugin.c b/src/decoder/wavpack_decoder_plugin.c index c8f0b7152..24d0c1703 100644 --- a/src/decoder/wavpack_decoder_plugin.c +++ b/src/decoder/wavpack_decoder_plugin.c @@ -194,8 +194,9 @@ wavpack_decode(struct decoder *decoder, WavpackContext *wpc, bool can_seek) decoder_initialized(decoder, &audio_format, can_seek, total_time); - while (true) { - if (decoder_get_command(decoder) == DECODE_COMMAND_SEEK) { + enum decoder_command cmd = decoder_get_command(decoder); + while (cmd != DECODE_COMMAND_STOP) { + if (cmd == DECODE_COMMAND_SEEK) { if (can_seek) { unsigned where = decoder_seek_where(decoder) * audio_format.sample_rate; @@ -210,10 +211,6 @@ wavpack_decode(struct decoder *decoder, WavpackContext *wpc, bool can_seek) } } - if (decoder_get_command(decoder) == DECODE_COMMAND_STOP) { - break; - } - uint32_t samples_got = WavpackUnpackSamples(wpc, chunk, samples_requested); if (samples_got == 0) @@ -224,9 +221,9 @@ wavpack_decode(struct decoder *decoder, WavpackContext *wpc, bool can_seek) format_samples(bytes_per_sample, chunk, samples_got * audio_format.channels); - decoder_data(decoder, NULL, chunk, - samples_got * output_sample_size, - bitrate); + cmd = decoder_data(decoder, NULL, chunk, + samples_got * output_sample_size, + bitrate); } } From b2175629fde13ce58ccb7cd1c4c39b9707225b84 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 07:05:10 +0200 Subject: [PATCH 06/12] update_walk: apply follow_inside_symlinks to absolute symlinks --- NEWS | 1 + src/update_walk.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index e6580c5f0..b642c820b 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.16.4 (2011/??/??) * fix memory leaks * don't resume playback when seeking to another song while paused +* apply follow_inside_symlinks to absolute symlinks * decoder: - ffmpeg: workaround for semantic API change in recent ffmpeg versions - flac: validate the sample rate when scanning the tag diff --git a/src/update_walk.c b/src/update_walk.c index 845f152eb..bf3c8f54b 100644 --- a/src/update_walk.c +++ b/src/update_walk.c @@ -714,8 +714,14 @@ skip_symlink(const struct directory *directory, const char *utf8_name) return false; } - if (buffer[0] == '/') - return !follow_outside_symlinks; + if (g_path_is_absolute(buffer)) { + /* if the symlink points to an absolute path, see if + that path is inside the music directory */ + const char *relative = map_to_relative_path(buffer); + return relative > buffer + ? !follow_inside_symlinks + : !follow_outside_symlinks; + } p = buffer; while (*p == '.') { From 8fa51faa389426f1edd285e20c3ddbdf4aebce71 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 18:52:00 +0200 Subject: [PATCH 07/12] player_thread: lock the player while setting the bite_rate --- src/player_thread.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/player_thread.c b/src/player_thread.c index cce51c1a7..a89e59908 100644 --- a/src/player_thread.c +++ b/src/player_thread.c @@ -618,7 +618,9 @@ play_chunk(struct song *song, struct music_chunk *chunk, return true; } + player_lock(); pc.bit_rate = chunk->bit_rate; + player_unlock(); /* send the chunk to the audio outputs */ From 2b6542467c5b14be21e2319bac3b88f42696ddcb Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 18:35:24 +0200 Subject: [PATCH 08/12] output_thread: unlock the mutex while calling cancel() The method may take longer, and we shouldn't be holding the lock. --- src/output_thread.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/output_thread.c b/src/output_thread.c index 4e0446791..2c2b8d116 100644 --- a/src/output_thread.c +++ b/src/output_thread.c @@ -640,8 +640,13 @@ static gpointer audio_output_task(gpointer arg) case AO_COMMAND_CANCEL: ao->chunk = NULL; - if (ao->open) + + if (ao->open) { + g_mutex_unlock(ao->mutex); ao_plugin_cancel(ao->plugin, ao->data); + g_mutex_lock(ao->mutex); + } + ao_command_finished(ao); /* the player thread will now clear our music From d97c46bcdc60e91d3ac4700bfebc39092d65452e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 18:47:56 +0200 Subject: [PATCH 09/12] pipe: make read-only functions "pure" Enable gcc optimizations. --- src/pipe.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pipe.h b/src/pipe.h index f9540a30e..efa7a84f0 100644 --- a/src/pipe.h +++ b/src/pipe.h @@ -20,6 +20,7 @@ #ifndef MPD_PIPE_H #define MPD_PIPE_H +#include #include #ifndef NDEBUG @@ -38,6 +39,7 @@ struct music_pipe; /** * Creates a new #music_pipe object. It is empty. */ +G_GNUC_MALLOC struct music_pipe * music_pipe_new(void); @@ -70,6 +72,7 @@ music_pipe_contains(const struct music_pipe *mp, * Returns the first #music_chunk from the pipe. Returns NULL if the * pipe is empty. */ +G_GNUC_PURE const struct music_chunk * music_pipe_peek(const struct music_pipe *mp); @@ -96,9 +99,11 @@ music_pipe_push(struct music_pipe *mp, struct music_chunk *chunk); /** * Returns the number of chunks currently in this pipe. */ +G_GNUC_PURE unsigned music_pipe_size(const struct music_pipe *mp); +G_GNUC_PURE static inline bool music_pipe_empty(const struct music_pipe *mp) { From a26f2ef17df9a741ba759b31a3d6f5a8f7068b6d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 18:50:30 +0200 Subject: [PATCH 10/12] pipe: lock the mutex in music_pipe_size() --- src/pipe.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pipe.c b/src/pipe.c index 7e4b0d081..2f5f70e43 100644 --- a/src/pipe.c +++ b/src/pipe.c @@ -187,5 +187,8 @@ music_pipe_push(struct music_pipe *mp, struct music_chunk *chunk) unsigned music_pipe_size(const struct music_pipe *mp) { - return mp->size; + g_mutex_lock(mp->mutex); + unsigned size = mp->size; + g_mutex_unlock(mp->mutex); + return size; } From 13539961b25940ca6d28c28ab7972c244681e3b5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 19:16:47 +0200 Subject: [PATCH 11/12] output/httpd: explicitly convert size_t to bool in pause() --- src/output/httpd_output_plugin.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/output/httpd_output_plugin.c b/src/output/httpd_output_plugin.c index b82dc0599..0137965a6 100644 --- a/src/output/httpd_output_plugin.c +++ b/src/output/httpd_output_plugin.c @@ -493,7 +493,8 @@ httpd_output_pause(void *data) if (has_clients) { static const char silence[1020]; - return httpd_output_play(data, silence, sizeof(silence), NULL); + return httpd_output_play(data, silence, sizeof(silence), + NULL) > 0; } else { g_usleep(100000); return true; From 838f7cd210dbd3f071d48d54c168c123c3d20c58 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jul 2011 20:54:34 +0200 Subject: [PATCH 12/12] encoder_plugin: add method pre_tag() In the "vorbis" plugin, this is a copy of the old flush() method, while flush() gets a lot of code remove, it just sets the "flush" flag and nothing else. It doesn't start a new stream now, which should fix a few problems in some players. --- NEWS | 2 ++ src/encoder/vorbis_encoder.c | 11 +++++++++++ src/encoder_plugin.h | 24 ++++++++++++++++++++++++ src/output/httpd_output_plugin.c | 2 +- src/output/shout_plugin.c | 2 +- 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index b642c820b..3cc704ef0 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ ver 0.16.4 (2011/??/??) - ffmpeg: workaround for semantic API change in recent ffmpeg versions - flac: validate the sample rate when scanning the tag - wavpack: obey all decoder commands, stop at CUE track border +* encoder: + - vorbis: don't send end-of-stream on flush * output: - alsa: fix SIGFPE when alsa announces a period size of 0 diff --git a/src/encoder/vorbis_encoder.c b/src/encoder/vorbis_encoder.c index 08147be1c..38a998bd2 100644 --- a/src/encoder/vorbis_encoder.c +++ b/src/encoder/vorbis_encoder.c @@ -266,6 +266,15 @@ vorbis_encoder_flush(struct encoder *_encoder, G_GNUC_UNUSED GError **error) { struct vorbis_encoder *encoder = (struct vorbis_encoder *)_encoder; + encoder->flush = true; + return true; +} + +static bool +vorbis_encoder_pre_tag(struct encoder *_encoder, G_GNUC_UNUSED GError **error) +{ + struct vorbis_encoder *encoder = (struct vorbis_encoder *)_encoder; + vorbis_analysis_wrote(&encoder->vd, 0); vorbis_encoder_blockout(encoder); @@ -366,6 +375,7 @@ vorbis_encoder_read(struct encoder *_encoder, void *_dest, size_t length) if (ret == 0 && encoder->flush) { encoder->flush = false; ret = ogg_stream_flush(&encoder->os, &page); + } if (ret == 0) @@ -398,6 +408,7 @@ const struct encoder_plugin vorbis_encoder_plugin = { .open = vorbis_encoder_open, .close = vorbis_encoder_close, .flush = vorbis_encoder_flush, + .pre_tag = vorbis_encoder_pre_tag, .tag = vorbis_encoder_tag, .write = vorbis_encoder_write, .read = vorbis_encoder_read, diff --git a/src/encoder_plugin.h b/src/encoder_plugin.h index 13fb231f4..fb00413e6 100644 --- a/src/encoder_plugin.h +++ b/src/encoder_plugin.h @@ -50,6 +50,8 @@ struct encoder_plugin { bool (*flush)(struct encoder *encoder, GError **error); + bool (*pre_tag)(struct encoder *encoder, GError **error); + bool (*tag)(struct encoder *encoder, const struct tag *tag, GError **error); @@ -147,9 +149,31 @@ encoder_flush(struct encoder *encoder, GError **error) : true; } +/** + * Prepare for sending a tag to the encoder. This is used by some + * encoders to flush the previous sub-stream, in preparation to begin + * a new one. + * + * @param encoder the encoder + * @param tag the tag object + * @param error location to store the error occuring, or NULL to ignore errors. + * @return true on success + */ +static inline bool +encoder_pre_tag(struct encoder *encoder, GError **error) +{ + /* this method is optional */ + return encoder->plugin->pre_tag != NULL + ? encoder->plugin->pre_tag(encoder, error) + : true; +} + /** * Sends a tag to the encoder. * + * Instructions: call encoder_pre_tag(); then obtain flushed data with + * encoder_read(); finally call encoder_tag(). + * * @param encoder the encoder * @param tag the tag object * @param error location to store the error occuring, or NULL to ignore errors. diff --git a/src/output/httpd_output_plugin.c b/src/output/httpd_output_plugin.c index 0137965a6..40ad05c3d 100644 --- a/src/output/httpd_output_plugin.c +++ b/src/output/httpd_output_plugin.c @@ -523,7 +523,7 @@ httpd_output_tag(void *data, const struct tag *tag) /* flush the current stream, and end it */ - encoder_flush(httpd->encoder, NULL); + encoder_pre_tag(httpd->encoder, NULL); httpd_output_encoder_to_clients(httpd); /* send the tag to the encoder - which starts a new diff --git a/src/output/shout_plugin.c b/src/output/shout_plugin.c index 5e1ef762a..35efd9fc7 100644 --- a/src/output/shout_plugin.c +++ b/src/output/shout_plugin.c @@ -511,7 +511,7 @@ static void my_shout_set_tag(void *data, if (sd->encoder->plugin->tag != NULL) { /* encoder plugin supports stream tags */ - ret = encoder_flush(sd->encoder, &error); + ret = encoder_pre_tag(sd->encoder, &error); if (!ret) { g_warning("%s", error->message); g_error_free(error);