From f185b35088cc5b1025c3e60b5f72030627e79878 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Oct 2011 22:07:01 +0200 Subject: decoder_api: clear initial_seek_running on error Fixes possible assertion failure. --- src/decoder_api.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/decoder_api.c b/src/decoder_api.c index 99c02db87..f0ba3b01f 100644 --- a/src/decoder_api.c +++ b/src/decoder_api.c @@ -180,10 +180,12 @@ void decoder_seek_error(struct decoder * decoder) assert(dc->pipe != NULL); - if (decoder->initial_seek_running) + if (decoder->initial_seek_running) { /* d'oh, we can't seek to the sub-song start position, what now? - no idea, ignoring the problem for now. */ + decoder->initial_seek_running = false; return; + } assert(dc->command == DECODE_COMMAND_SEEK); -- cgit v1.2.3 From 99d4ae0c1ae5522202d71381b40f55418e7d531a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 5 Oct 2011 22:53:36 +0200 Subject: decoder_api: don't copy tag to pipe during initial seek Fixes one more assertion failure. --- src/decoder_api.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/decoder_api.c b/src/decoder_api.c index f0ba3b01f..bbe93eef0 100644 --- a/src/decoder_api.c +++ b/src/decoder_api.c @@ -438,6 +438,14 @@ decoder_tag(G_GNUC_UNUSED struct decoder *decoder, struct input_stream *is, update_stream_tag(decoder, is); + /* check if we're seeking */ + + if (decoder->initial_seek_pending) + /* during initial seek, no music chunk must be created + until seeking is finished; skip the rest of the + function here */ + return DECODE_COMMAND_SEEK; + /* send tag to music pipe */ if (decoder->stream_tag != NULL) { @@ -451,9 +459,6 @@ decoder_tag(G_GNUC_UNUSED struct decoder *decoder, struct input_stream *is, /* send only the decoder tag */ cmd = do_send_tag(decoder, is, tag); - if (cmd == DECODE_COMMAND_NONE) - cmd = decoder_get_virtual_command(decoder); - return cmd; } -- cgit v1.2.3 From 64b0ba6da7975fdde774f188b1647ab6c9024cfa Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 5 Oct 2011 22:37:59 +0200 Subject: decoder_control: add attributes start_ms, end_ms Don't read song.start_ms and song.end_ms, let the player thread manage this logic instead. --- src/decoder_api.c | 8 ++++---- src/decoder_control.c | 3 +++ src/decoder_control.h | 20 ++++++++++++++++++++ src/decoder_thread.c | 2 +- src/player_thread.c | 4 +++- 5 files changed, 31 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/decoder_api.c b/src/decoder_api.c index bbe93eef0..bec271179 100644 --- a/src/decoder_api.c +++ b/src/decoder_api.c @@ -132,7 +132,7 @@ decoder_command_finished(struct decoder *decoder) assert(music_pipe_empty(dc->pipe)); decoder->initial_seek_running = false; - decoder->timestamp = dc->song->start_ms / 1000.; + decoder->timestamp = dc->start_ms / 1000.; decoder_unlock(dc); return; } @@ -165,7 +165,7 @@ double decoder_seek_where(G_GNUC_UNUSED struct decoder * decoder) assert(dc->pipe != NULL); if (decoder->initial_seek_running) - return dc->song->start_ms / 1000.; + return dc->start_ms / 1000.; assert(dc->command == DECODE_COMMAND_SEEK); @@ -407,8 +407,8 @@ decoder_data(struct decoder *decoder, decoder->timestamp += (double)nbytes / audio_format_time_to_size(&dc->out_audio_format); - if (dc->song->end_ms > 0 && - decoder->timestamp >= dc->song->end_ms / 1000.0) + if (dc->end_ms > 0 && + decoder->timestamp >= dc->end_ms / 1000.0) /* the end of this range has been reached: stop decoding */ return DECODE_COMMAND_STOP; diff --git a/src/decoder_control.c b/src/decoder_control.c index a5e6e4ad3..85c2e5ba8 100644 --- a/src/decoder_control.c +++ b/src/decoder_control.c @@ -102,6 +102,7 @@ dc_command_async(struct decoder_control *dc, enum decoder_command cmd) void dc_start(struct decoder_control *dc, struct song *song, + unsigned start_ms, unsigned end_ms, struct music_buffer *buffer, struct music_pipe *pipe) { assert(song != NULL); @@ -110,6 +111,8 @@ dc_start(struct decoder_control *dc, struct song *song, assert(music_pipe_empty(pipe)); dc->song = song; + dc->start_ms = start_ms; + dc->end_ms = end_ms; dc->buffer = buffer; dc->pipe = pipe; dc_command(dc, DECODE_COMMAND_START); diff --git a/src/decoder_control.h b/src/decoder_control.h index 449e974b7..64c7c302e 100644 --- a/src/decoder_control.h +++ b/src/decoder_control.h @@ -79,6 +79,23 @@ struct decoder_control { */ const struct song *song; + /** + * The initial seek position (in milliseconds), e.g. to the + * start of a sub-track described by a CUE file. + * + * This attribute is set by dc_start(). + */ + unsigned start_ms; + + /** + * The decoder will stop when it reaches this position (in + * milliseconds). 0 means don't stop before the end of the + * file. + * + * This attribute is set by dc_start(). + */ + unsigned end_ms; + float total_time; /** the #music_chunk allocator */ @@ -225,11 +242,14 @@ dc_command_wait(struct decoder_control *dc); * * @param the decoder * @param song the song to be decoded + * @param start_ms see #decoder_control + * @param end_ms see #decoder_control * @param pipe the pipe which receives the decoded chunks (owned by * the caller) */ void dc_start(struct decoder_control *dc, struct song *song, + unsigned start_ms, unsigned end_ms, struct music_buffer *buffer, struct music_pipe *pipe); void diff --git a/src/decoder_thread.c b/src/decoder_thread.c index ce849df47..201cd5acd 100644 --- a/src/decoder_thread.c +++ b/src/decoder_thread.c @@ -369,7 +369,7 @@ decoder_run_song(struct decoder_control *dc, { struct decoder decoder = { .dc = dc, - .initial_seek_pending = song->start_ms > 0, + .initial_seek_pending = dc->start_ms > 0, .initial_seek_running = false, }; int ret; diff --git a/src/player_thread.c b/src/player_thread.c index be08aa32d..b63758544 100644 --- a/src/player_thread.c +++ b/src/player_thread.c @@ -145,7 +145,9 @@ player_dc_start(struct player *player, struct music_pipe *pipe) assert(player->queued || pc.command == PLAYER_COMMAND_SEEK); assert(pc.next_song != NULL); - dc_start(dc, pc.next_song, player_buffer, pipe); + dc_start(dc, pc.next_song, + pc.next_song->start_ms, pc.next_song->end_ms, + player_buffer, pipe); } /** -- cgit v1.2.3 From e07073ff286ed22d4859aba3584285586c7781c8 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 6 Oct 2011 00:25:44 +0200 Subject: decoder_api: move code to _prepare_initial_seek() .. and add a few code comments. --- src/decoder_api.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/decoder_api.c b/src/decoder_api.c index bec271179..3e4917508 100644 --- a/src/decoder_api.c +++ b/src/decoder_api.c @@ -79,30 +79,54 @@ decoder_initialized(struct decoder *decoder, } /** - * Returns the current decoder command. May return a "virtual" - * synthesized command, e.g. to seek to the beginning of the CUE - * track. + * Checks if we need an "initial seek". If so, then the initial seek + * is prepared, and the function returns true. */ G_GNUC_PURE -static enum decoder_command -decoder_get_virtual_command(struct decoder *decoder) +static bool +decoder_prepare_initial_seek(struct decoder *decoder) { const struct decoder_control *dc = decoder->dc; assert(dc->pipe != NULL); if (decoder->initial_seek_running) - return DECODE_COMMAND_SEEK; + /* initial seek has already begun - override any other + command */ + return true; if (decoder->initial_seek_pending) { if (dc->command == DECODE_COMMAND_NONE) { + /* begin initial seek */ + decoder->initial_seek_pending = false; decoder->initial_seek_running = true; - return DECODE_COMMAND_SEEK; + return true; } + /* skip initial seek when there's another command + (e.g. STOP) */ + decoder->initial_seek_pending = false; } + return false; +} + +/** + * Returns the current decoder command. May return a "virtual" + * synthesized command, e.g. to seek to the beginning of the CUE + * track. + */ +G_GNUC_PURE +static enum decoder_command +decoder_get_virtual_command(struct decoder *decoder) +{ + const struct decoder_control *dc = decoder->dc; + assert(dc->pipe != NULL); + + if (decoder_prepare_initial_seek(decoder)) + return DECODE_COMMAND_SEEK; + return dc->command; } -- cgit v1.2.3 From f67136df1951031a0561383b4421afc0328031d0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 6 Oct 2011 00:35:45 +0200 Subject: decoder_api: call _prepare_initial_seek() in decoder_tag() This checks both conditions: pending and running. Fixes yet another assertion failure! --- src/decoder_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/decoder_api.c b/src/decoder_api.c index 3e4917508..6dcca32c2 100644 --- a/src/decoder_api.c +++ b/src/decoder_api.c @@ -464,7 +464,7 @@ decoder_tag(G_GNUC_UNUSED struct decoder *decoder, struct input_stream *is, /* check if we're seeking */ - if (decoder->initial_seek_pending) + if (decoder_prepare_initial_seek(decoder)) /* during initial seek, no music chunk must be created until seeking is finished; skip the rest of the function here */ -- cgit v1.2.3 From 37f026a0a67b9014754e4ab1fcc22229e384fee3 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 5 Oct 2011 22:13:13 +0200 Subject: player_thread: handle SEEK while not playing --- src/player_thread.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/player_thread.c b/src/player_thread.c index b63758544..31bb3d50d 100644 --- a/src/player_thread.c +++ b/src/player_thread.c @@ -145,8 +145,12 @@ player_dc_start(struct player *player, struct music_pipe *pipe) assert(player->queued || pc.command == PLAYER_COMMAND_SEEK); assert(pc.next_song != NULL); + unsigned start_ms = pc.next_song->start_ms; + if (pc.command == PLAYER_COMMAND_SEEK) + start_ms += (unsigned)(pc.seek_where * 1000); + dc_start(dc, pc.next_song, - pc.next_song->start_ms, pc.next_song->end_ms, + start_ms, pc.next_song->end_ms, player_buffer, pipe); } @@ -835,6 +839,10 @@ static void do_play(struct decoder_control *dc) } player_lock(); + + if (pc.command == PLAYER_COMMAND_SEEK) + player.elapsed_time = pc.seek_where; + pc.state = PLAYER_STATE_PLAY; player_command_finished_locked(); @@ -1013,6 +1021,7 @@ static gpointer player_task(G_GNUC_UNUSED gpointer arg) while (1) { switch (pc.command) { + case PLAYER_COMMAND_SEEK: case PLAYER_COMMAND_QUEUE: assert(pc.next_song != NULL); @@ -1026,7 +1035,6 @@ static gpointer player_task(G_GNUC_UNUSED gpointer arg) /* fall through */ - case PLAYER_COMMAND_SEEK: case PLAYER_COMMAND_PAUSE: pc.next_song = NULL; player_command_finished_locked(); -- cgit v1.2.3 From 8ea6c113b57c9a8b577e7048a31f46cc868c99b7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 5 Oct 2011 22:24:53 +0200 Subject: player_control: auto-start playback when seeking is requested Now that the player thread can handle SEEK commands while not (yet) playing, we can remove the "pc.state" check from pc_seek(). --- src/player_control.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'src') diff --git a/src/player_control.c b/src/player_control.c index a190bbd8b..e3e6b7739 100644 --- a/src/player_control.c +++ b/src/player_control.c @@ -309,9 +309,6 @@ pc_seek(struct song *song, float seek_time) { assert(song != NULL); - if (pc.state == PLAYER_STATE_STOP) - return false; - player_lock(); pc.next_song = song; pc.seek_where = seek_time; -- cgit v1.2.3 From 23670795db6a1ef9f16862513b2efa6fe82928c4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 6 Oct 2011 19:51:37 +0200 Subject: output_control: remove unused prototype _close_locked() --- src/output_control.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'src') diff --git a/src/output_control.c b/src/output_control.c index f8c5cd873..24485f1c2 100644 --- a/src/output_control.c +++ b/src/output_control.c @@ -102,9 +102,6 @@ audio_output_disable(struct audio_output *ao) g_mutex_unlock(ao->mutex); } -static void -audio_output_close_locked(struct audio_output *ao); - /** * Object must be locked (and unlocked) by the caller. */ -- cgit v1.2.3 From 63b33b6ec584a6730174c45a119a27b4add76777 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 6 Oct 2011 20:55:52 +0200 Subject: player_thread: move code to player_open_output() Common function that manages "player" attributes after audio_output_all_open() has returned. --- src/player_thread.c | 59 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/player_thread.c b/src/player_thread.c index 31bb3d50d..9a7c917f4 100644 --- a/src/player_thread.c +++ b/src/player_thread.c @@ -276,6 +276,41 @@ real_song_duration(const struct song *song, double decoder_duration) return decoder_duration - song->start_ms / 1000.0; } +/** + * Wrapper for audio_output_all_open(). Upon failure, it pauses the + * player. + * + * @return true on success + */ +static bool +player_open_output(struct player *player) +{ + assert(audio_format_defined(&player->play_audio_format)); + assert(pc.state == PLAYER_STATE_PLAY || + pc.state == PLAYER_STATE_PAUSE); + + if (audio_output_all_open(&player->play_audio_format, player_buffer)) { + player->paused = false; + + player_lock(); + pc.state = PLAYER_STATE_PLAY; + player_unlock(); + + return true; + } else { + /* pause: the user may resume playback as soon as an + audio output becomes available */ + player->paused = true; + + player_lock(); + pc.error = PLAYER_ERROR_AUDIO; + pc.state = PLAYER_STATE_PAUSE; + player_unlock(); + + return false; + } +} + /** * The decoder has acknowledged the "START" command (see * player_wait_for_decoder()). This function checks if the decoder @@ -321,23 +356,12 @@ player_check_decoder_startup(struct player *player) player->play_audio_format = dc->out_audio_format; player->decoder_starting = false; - if (!player->paused && - !audio_output_all_open(&dc->out_audio_format, - player_buffer)) { + if (!player->paused && !player_open_output(player)) { char *uri = song_get_uri(dc->song); g_warning("problems opening audio device " "while playing \"%s\"", uri); g_free(uri); - player_lock(); - pc.error = PLAYER_ERROR_AUDIO; - - /* pause: the user may resume playback as soon - as an audio output becomes available */ - pc.state = PLAYER_STATE_PAUSE; - player_unlock(); - - player->paused = true; return true; } @@ -516,18 +540,9 @@ static void player_process_command(struct player *player) yet - don't open the audio device yet */ player_lock(); - pc.state = PLAYER_STATE_PLAY; - } else if (audio_output_all_open(&player->play_audio_format, player_buffer)) { - /* unpaused, continue playing */ - player_lock(); - pc.state = PLAYER_STATE_PLAY; } else { - /* the audio device has failed - rollback to - pause mode */ - pc.error = PLAYER_ERROR_AUDIO; - - player->paused = true; + player_open_output(player); player_lock(); } -- cgit v1.2.3 From b2f03e76ffd3af918d8eda0968f54c0b81bbff54 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 6 Oct 2011 20:30:46 +0200 Subject: player_thread: add flag "output_open", fixes assertion failure Previously, the condition "defined(play_audio_format)" was used to see if an output device has been opened, but if the device had failed on startup, an assertion failure could occur. This patch adds a separate flag. --- src/player_thread.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/player_thread.c b/src/player_thread.c index 9a7c917f4..52788e518 100644 --- a/src/player_thread.c +++ b/src/player_thread.c @@ -73,6 +73,14 @@ struct player { */ bool queued; + /** + * Was any audio output opened successfully? It might have + * failed meanwhile, but was not explicitly closed by the + * player thread. When this flag is unset, some output + * methods must not be called. + */ + bool output_open; + /** * the song currently being played */ @@ -290,6 +298,7 @@ player_open_output(struct player *player) pc.state == PLAYER_STATE_PAUSE); if (audio_output_all_open(&player->play_audio_format, player_buffer)) { + player->output_open = true; player->paused = false; player_lock(); @@ -298,6 +307,8 @@ player_open_output(struct player *player) return true; } else { + player->output_open = false; + /* pause: the user may resume playback as soon as an audio output becomes available */ player->paused = true; @@ -342,7 +353,7 @@ player_check_decoder_startup(struct player *player) decoder_unlock(dc); - if (audio_format_defined(&player->play_audio_format) && + if (player->output_open && !audio_output_all_wait(1)) /* the output devices havn't finished playing all chunks yet - wait for that */ @@ -386,6 +397,7 @@ player_check_decoder_startup(struct player *player) static bool player_send_silence(struct player *player) { + assert(player->output_open); assert(audio_format_defined(&player->play_audio_format)); struct music_chunk *chunk = music_buffer_allocate(player_buffer); @@ -579,8 +591,7 @@ static void player_process_command(struct player *player) break; case PLAYER_COMMAND_REFRESH: - if (audio_format_defined(&player->play_audio_format) && - !player->paused) { + if (player->output_open && !player->paused) { player_unlock(); audio_output_all_check(); player_lock(); @@ -831,6 +842,7 @@ static void do_play(struct decoder_control *dc) .decoder_starting = false, .paused = false, .queued = true, + .output_open = false, .song = NULL, .xfade = XFADE_UNKNOWN, .cross_fading = false, @@ -883,7 +895,7 @@ static void do_play(struct decoder_control *dc) /* not enough decoded buffer space yet */ if (!player.paused && - audio_format_defined(&player.play_audio_format) && + player.output_open && audio_output_all_check() < 4 && !player_send_silence(&player)) break; @@ -988,7 +1000,7 @@ static void do_play(struct decoder_control *dc) audio_output_all_drain(); break; } - } else { + } else if (player.output_open) { /* the decoder is too busy and hasn't provided new PCM data in time: send silence (if the output pipe is empty) */ -- cgit v1.2.3 From 039b3544902fe479fa2ce31f06de2c08377e0fc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Bravo=20=C3=81lvarez?= Date: Thu, 6 Oct 2011 22:21:24 +0200 Subject: playlist_song: fix absolute path support in playlists Right now, a playlist with absolute pathnames can only add songs that are in the same the directory of the playlist or under it. If uri is an absolute pathname and base_uri is set, playlist_check_translate_song() will check that base_uri is a prefix of uri, excluding every other song in the music directory outside base_uri. I think in this case base_uri should be completely ignored (and made NULL) and uri should just be checked against music root directory. --- src/playlist_song.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/playlist_song.c b/src/playlist_song.c index 1a543a0b8..827098655 100644 --- a/src/playlist_song.c +++ b/src/playlist_song.c @@ -114,9 +114,7 @@ playlist_check_translate_song(struct song *song, const char *base_uri) if (g_path_is_absolute(uri)) { /* XXX fs_charset vs utf8? */ - char *prefix = base_uri != NULL - ? map_uri_fs(base_uri) - : map_directory_fs(db_get_root()); + char *prefix = map_directory_fs(db_get_root()); if (prefix == NULL || !g_str_has_prefix(uri, prefix) || uri[strlen(prefix)] != '/') { @@ -127,6 +125,7 @@ playlist_check_translate_song(struct song *song, const char *base_uri) return NULL; } + base_uri = NULL; uri += strlen(prefix) + 1; g_free(prefix); } -- cgit v1.2.3