From 4464cdcc67ed40eaa0c7ad27ab28f1a0ae95038b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 12 Aug 2019 20:20:17 +0200 Subject: [PATCH 01/11] doc/protocol.rst: add missing newline to "albumart" example This was missing in commit 0f488dcecfb0c43134f15cfb2e6368845dcf3d8d --- doc/protocol.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/protocol.rst b/doc/protocol.rst index eb8e30608..421b3492a 100644 --- a/doc/protocol.rst +++ b/doc/protocol.rst @@ -824,7 +824,8 @@ The music database albumart size: 1024768 binary: 8192 - <8192 bytes>OK + <8192 bytes> + OK :command:`count {FILTER} [group {GROUPTYPE}]` Count the number of songs and their total playtime in From f3d16f6d1bad64c6ef395997e68e6c682ada13e4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 13 Aug 2019 13:08:40 +0200 Subject: [PATCH 02/11] output/Thread: fix typo in comment --- src/output/Thread.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output/Thread.cxx b/src/output/Thread.cxx index 847a6642e..d9e30dce5 100644 --- a/src/output/Thread.cxx +++ b/src/output/Thread.cxx @@ -458,7 +458,7 @@ AudioOutputControl::Task() noexcept case Command::RELEASE: if (!open) { /* the output has failed after - the PAUSE command was submitted; bail + the RELEASE command was submitted; bail out */ CommandFinished(); break; From ca450663d09b0e467292aaa17f636db71b95d574 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 20 Aug 2019 20:01:53 +0200 Subject: [PATCH 03/11] decoder/Control: work around crash after SEEK was too late See code comment. Closes https://github.com/MusicPlayerDaemon/MPD/issues/629 --- NEWS | 2 ++ src/decoder/Control.cxx | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/NEWS b/NEWS index acc735d2e..56ce212c9 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ ver 0.21.14 (not yet released) - sidplay: show track durations in database - sidplay: convert tag values from Windows-1252 charset - sidplay: strip text from "Date" tag +* player + - fix crash after song change ver 0.21.13 (2019/08/06) * input diff --git a/src/decoder/Control.cxx b/src/decoder/Control.cxx index 5faa23fe5..cf8a74385 100644 --- a/src/decoder/Control.cxx +++ b/src/decoder/Control.cxx @@ -147,6 +147,18 @@ DecoderControl::Seek(SongTime t) seek_error = false; SynchronousCommandLocked(DecoderCommand::SEEK); + while (state == DecoderState::START) + /* If the decoder falls back to DecoderState::START, + this means that our SEEK command arrived too late, + and the decoder had meanwhile finished decoding and + went idle. Our SEEK command is finished, but that + means only that the decoder thread has launched the + decoder. To work around illegal states, we wait + until the decoder plugin has become ready. This is + a kludge, built on top of the "late seek" kludge. + Not exactly elegant, sorry. */ + WaitForDecoder(); + if (seek_error) throw std::runtime_error("Decoder failed to seek"); } From 44444e1b89389b4430633676e0b27c6f2f37fb41 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 20 Aug 2019 20:10:27 +0200 Subject: [PATCH 04/11] decoder/Thread: on late SEEK, start decoder at seek position Previously, a bogus value (whatever happened to be still in `start_time`) was used. --- NEWS | 1 + src/decoder/Thread.cxx | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/NEWS b/NEWS index 56ce212c9..d47a02aff 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ ver 0.21.14 (not yet released) - sidplay: strip text from "Date" tag * player - fix crash after song change + - fix seek position after restarting the decoder ver 0.21.13 (2019/08/06) * input diff --git a/src/decoder/Thread.cxx b/src/decoder/Thread.cxx index 9021af57a..45129a206 100644 --- a/src/decoder/Thread.cxx +++ b/src/decoder/Thread.cxx @@ -455,6 +455,11 @@ static void decoder_run_song(DecoderControl &dc, const DetachedSong &song, const char *uri, Path path_fs) { + if (dc.command == DecoderCommand::SEEK) + /* if the SEEK command arrived too late, start the + decoder at the seek position */ + dc.start_time = dc.seek_time; + DecoderBridge bridge(dc, dc.start_time.IsPositive(), /* pass the song tag only if it's authoritative, i.e. if it's a local From 21fa44c0d5e151a225fce3f81e0eeb2ab1929360 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 20 Aug 2019 20:23:54 +0200 Subject: [PATCH 05/11] command/all: catch all exceptions --- src/command/AllCommands.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command/AllCommands.cxx b/src/command/AllCommands.cxx index dc77538bc..38e75697f 100644 --- a/src/command/AllCommands.cxx +++ b/src/command/AllCommands.cxx @@ -415,7 +415,7 @@ try { : CommandResult::ERROR; return ret; -} catch (const std::exception &e) { +} catch (...) { Response r(client, num); PrintError(r, std::current_exception()); return CommandResult::ERROR; From e33b50d9c501bbea6accb10b5fa78105b8ea0e43 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 20 Aug 2019 20:26:07 +0200 Subject: [PATCH 06/11] command/all: simplify `return` from command_process() --- src/command/AllCommands.cxx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/command/AllCommands.cxx b/src/command/AllCommands.cxx index 38e75697f..90d616098 100644 --- a/src/command/AllCommands.cxx +++ b/src/command/AllCommands.cxx @@ -409,12 +409,10 @@ try { const struct command *cmd = command_checked_lookup(r, client.GetPermission(), cmd_name, args); + if (cmd == nullptr) + return CommandResult::ERROR; - CommandResult ret = cmd - ? cmd->handler(client, args, r) - : CommandResult::ERROR; - - return ret; + return cmd->handler(client, args, r); } catch (...) { Response r(client, num); PrintError(r, std::current_exception()); From 2bf26a2ff8ad65848b051151c40c6a34a600cf2c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 20 Aug 2019 20:28:10 +0200 Subject: [PATCH 07/11] command/all: remove obsolete prototype --- src/command/AllCommands.hxx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/command/AllCommands.hxx b/src/command/AllCommands.hxx index 35f559d8a..3a0220e55 100644 --- a/src/command/AllCommands.hxx +++ b/src/command/AllCommands.hxx @@ -27,9 +27,6 @@ class Client; void command_init(); -void -command_finish(); - CommandResult command_process(Client &client, unsigned num, char *line); From 9bff5f9e36a02d40f660d39f52bd9c4b63caa1a5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 20 Aug 2019 20:27:15 +0200 Subject: [PATCH 08/11] client/Process, command/all: add `noexcept` Clarify that those can't throw, preparing for the next commit. --- src/client/ClientInternal.hxx | 2 +- src/client/ClientProcess.cxx | 4 ++-- src/command/AllCommands.cxx | 18 ++++++++++-------- src/command/AllCommands.hxx | 4 ++-- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/client/ClientInternal.hxx b/src/client/ClientInternal.hxx index 87a1f403b..37b5925f2 100644 --- a/src/client/ClientInternal.hxx +++ b/src/client/ClientInternal.hxx @@ -35,6 +35,6 @@ extern size_t client_max_command_list_size; extern size_t client_max_output_buffer_size; CommandResult -client_process_line(Client &client, char *line); +client_process_line(Client &client, char *line) noexcept; #endif diff --git a/src/client/ClientProcess.cxx b/src/client/ClientProcess.cxx index 547aa8c71..eeb72d35a 100644 --- a/src/client/ClientProcess.cxx +++ b/src/client/ClientProcess.cxx @@ -30,7 +30,7 @@ static CommandResult client_process_command_list(Client &client, bool list_ok, - std::list &&list) + std::list &&list) noexcept { CommandResult ret = CommandResult::OK; unsigned num = 0; @@ -51,7 +51,7 @@ client_process_command_list(Client &client, bool list_ok, } CommandResult -client_process_line(Client &client, char *line) +client_process_line(Client &client, char *line) noexcept { CommandResult ret; diff --git a/src/command/AllCommands.cxx b/src/command/AllCommands.cxx index 90d616098..a9fac6919 100644 --- a/src/command/AllCommands.cxx +++ b/src/command/AllCommands.cxx @@ -206,9 +206,10 @@ static constexpr struct command commands[] = { static constexpr unsigned num_commands = ARRAY_SIZE(commands); +gcc_pure static bool command_available(gcc_unused const Partition &partition, - gcc_unused const struct command *cmd) + gcc_unused const struct command *cmd) noexcept { #ifdef ENABLE_SQLITE if (StringIsEqual(cmd->cmd, "sticker")) @@ -235,7 +236,7 @@ command_available(gcc_unused const Partition &partition, static CommandResult PrintAvailableCommands(Response &r, const Partition &partition, - unsigned permission) + unsigned permission) noexcept { for (unsigned i = 0; i < num_commands; ++i) { const struct command *cmd = &commands[i]; @@ -249,7 +250,7 @@ PrintAvailableCommands(Response &r, const Partition &partition, } static CommandResult -PrintUnavailableCommands(Response &r, unsigned permission) +PrintUnavailableCommands(Response &r, unsigned permission) noexcept { for (unsigned i = 0; i < num_commands; ++i) { const struct command *cmd = &commands[i]; @@ -276,7 +277,7 @@ handle_not_commands(Client &client, gcc_unused Request request, Response &r) } void -command_init() +command_init() noexcept { #ifndef NDEBUG /* ensure that the command list is sorted */ @@ -285,8 +286,9 @@ command_init() #endif } +gcc_pure static const struct command * -command_lookup(const char *name) +command_lookup(const char *name) noexcept { unsigned a = 0, b = num_commands, i; @@ -308,7 +310,7 @@ command_lookup(const char *name) static bool command_check_request(const struct command *cmd, Response &r, - unsigned permission, Request args) + unsigned permission, Request args) noexcept { if (cmd->permission != (permission & cmd->permission)) { r.FormatError(ACK_ERROR_PERMISSION, @@ -342,7 +344,7 @@ command_check_request(const struct command *cmd, Response &r, static const struct command * command_checked_lookup(Response &r, unsigned permission, - const char *cmd_name, Request args) + const char *cmd_name, Request args) noexcept { const struct command *cmd = command_lookup(cmd_name); if (cmd == nullptr) { @@ -360,7 +362,7 @@ command_checked_lookup(Response &r, unsigned permission, } CommandResult -command_process(Client &client, unsigned num, char *line) +command_process(Client &client, unsigned num, char *line) noexcept try { Response r(client, num); diff --git a/src/command/AllCommands.hxx b/src/command/AllCommands.hxx index 3a0220e55..0e685c118 100644 --- a/src/command/AllCommands.hxx +++ b/src/command/AllCommands.hxx @@ -25,9 +25,9 @@ class Client; void -command_init(); +command_init() noexcept; CommandResult -command_process(Client &client, unsigned num, char *line); +command_process(Client &client, unsigned num, char *line) noexcept; #endif From 6c9f9c136b72b4453c39eca4edb70b48f53b8d2c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 20 Aug 2019 20:31:36 +0200 Subject: [PATCH 09/11] command/all: don't create new Response instance in exception handler The new Response instance in the `catch` block didn't have the `command` attribute set, so the error response didn't indicate which command had failed, which however is required in the MPD protocol. Closes https://github.com/MusicPlayerDaemon/MPD/issues/628 --- NEWS | 2 ++ src/command/AllCommands.cxx | 47 +++++++++++++++++++------------------ 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/NEWS b/NEWS index d47a02aff..e1859419f 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ ver 0.21.14 (not yet released) * player - fix crash after song change - fix seek position after restarting the decoder +* protocol + - include command name in error responses ver 0.21.13 (2019/08/06) * input diff --git a/src/command/AllCommands.cxx b/src/command/AllCommands.cxx index a9fac6919..7926b7128 100644 --- a/src/command/AllCommands.cxx +++ b/src/command/AllCommands.cxx @@ -363,7 +363,7 @@ command_checked_lookup(Response &r, unsigned permission, CommandResult command_process(Client &client, unsigned num, char *line) noexcept -try { +{ Response r(client, num); /* get the command name (first word on the line) */ @@ -391,32 +391,33 @@ try { char *argv[COMMAND_ARGV_MAX]; Request args(argv, 0); - /* now parse the arguments (quoted or unquoted) */ + try { + /* now parse the arguments (quoted or unquoted) */ - while (true) { - if (args.size == COMMAND_ARGV_MAX) { - r.Error(ACK_ERROR_ARG, "Too many arguments"); - return CommandResult::ERROR; + while (true) { + if (args.size == COMMAND_ARGV_MAX) { + r.Error(ACK_ERROR_ARG, "Too many arguments"); + return CommandResult::ERROR; + } + + char *a = tokenizer.NextParam(); + if (a == nullptr) + break; + + argv[args.size++] = a; } - char *a = tokenizer.NextParam(); - if (a == nullptr) - break; + /* look up and invoke the command handler */ - argv[args.size++] = a; - } + const struct command *cmd = + command_checked_lookup(r, client.GetPermission(), + cmd_name, args); + if (cmd == nullptr) + return CommandResult::ERROR; - /* look up and invoke the command handler */ - - const struct command *cmd = - command_checked_lookup(r, client.GetPermission(), - cmd_name, args); - if (cmd == nullptr) + return cmd->handler(client, args, r); + } catch (...) { + PrintError(r, std::current_exception()); return CommandResult::ERROR; - - return cmd->handler(client, args, r); -} catch (...) { - Response r(client, num); - PrintError(r, std::current_exception()); - return CommandResult::ERROR; + } } From b968e1b6dedf6723caa55cfacf5c862667f8b1d9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 21 Aug 2019 10:20:17 +0200 Subject: [PATCH 10/11] output/Thread: add missing `return` in exception handler --- src/output/Thread.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/output/Thread.cxx b/src/output/Thread.cxx index d9e30dce5..295035665 100644 --- a/src/output/Thread.cxx +++ b/src/output/Thread.cxx @@ -159,6 +159,7 @@ AudioOutputControl::InternalOpen(const AudioFormat in_audio_format, } catch (...) { LogError(std::current_exception()); Failure(std::current_exception()); + return; } if (f != in_audio_format || f != output->out_audio_format) From bc89ca92b402a0e1fae29c0450298b7e6634f75b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 21 Aug 2019 10:47:53 +0200 Subject: [PATCH 11/11] release v0.21.14 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index e1859419f..fa91b1444 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.21.14 (not yet released) +ver 0.21.14 (2019/08/21) * decoder - sidplay: show track durations in database - sidplay: convert tag values from Windows-1252 charset