release v0.21.14

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

@@ -1,3 +1,14 @@
+ver 0.21.14 (2019/08/21)
+* decoder
+  - 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
+  - fix seek position after restarting the decoder
+* protocol
+  - include command name in error responses
+
 ver 0.21.13 (2019/08/06)
 * input
   - cdio_paranoia: require libcdio-paranoia 10.2+0.93+1

android/AndroidManifest.xml

@@ -2,8 +2,8 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
           package="org.musicpd"
           android:installLocation="auto"
-          android:versionCode="36"
-          android:versionName="0.21.13">
+          android:versionCode="37"
+          android:versionName="0.21.14">
 
   <uses-sdk android:minSdkVersion="21" android:targetSdkVersion="26"/>
 

doc/conf.py

@@ -38,7 +38,7 @@ author = 'Max Kellermann'
 # built documents.
 #
 # The short X.Y version.
-version = '0.21.13'
+version = '0.21.14'
 # The full version, including alpha/beta/rc tags.
 release = version
 

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

meson.build

@@ -1,7 +1,7 @@
 project(
   'mpd',
   ['c', 'cpp'],
-  version: '0.21.13',
+  version: '0.21.14',
   meson_version: '>= 0.49.0',
   default_options: [
     'c_std=c99',

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

src/client/ClientProcess.cxx

@@ -30,7 +30,7 @@
 
 static CommandResult
 client_process_command_list(Client &client, bool list_ok,
-			    std::list<std::string> &&list)
+			    std::list<std::string> &&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;
 

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,8 +362,8 @@ command_checked_lookup(Response &r, unsigned permission,
 }
 
 CommandResult
-command_process(Client &client, unsigned num, char *line)
-try {
+command_process(Client &client, unsigned num, char *line) noexcept
+{
 	Response r(client, num);
 
 	/* get the command name (first word on the line) */
@@ -389,34 +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;
+
+		return cmd->handler(client, args, r);
+	} catch (...) {
+		PrintError(r, std::current_exception());
+		return CommandResult::ERROR;
 	}
-
-	/* look up and invoke the command handler */
-
-	const struct command *cmd =
-		command_checked_lookup(r, client.GetPermission(),
-				       cmd_name, args);
-
-	CommandResult ret = cmd
-		? cmd->handler(client, args, r)
-		: CommandResult::ERROR;
-
-	return ret;
-} catch (const std::exception &e) {
-	Response r(client, num);
-	PrintError(r, std::current_exception());
-	return CommandResult::ERROR;
 }

src/command/AllCommands.hxx

@@ -25,12 +25,9 @@
 class Client;
 
 void
-command_init();
-
-void
-command_finish();
+command_init() noexcept;
 
 CommandResult
-command_process(Client &client, unsigned num, char *line);
+command_process(Client &client, unsigned num, char *line) noexcept;
 
 #endif

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");
 }

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

src/decoder/plugins/SidplayDecoderPlugin.cxx

@@ -25,6 +25,7 @@
 #include "song/DetachedSong.hxx"
 #include "fs/Path.hxx"
 #include "fs/AllocatedPath.hxx"
+#include "lib/icu/Converter.hxx"
 #ifdef HAVE_SIDPLAYFP
 #include "fs/io/FileReader.hxx"
 #include "util/RuntimeError.hxx"
@@ -32,6 +33,8 @@
 #include "util/Macros.hxx"
 #include "util/StringFormat.hxx"
 #include "util/Domain.hxx"
+#include "util/AllocatedString.hxx"
+#include "util/CharUtil.hxx"
 #include "system/ByteOrder.hxx"
 #include "Log.hxx"
 
@@ -432,19 +435,70 @@ sidplay_file_decode(DecoderClient &client, Path path_fs)
 	} while (cmd != DecoderCommand::STOP);
 }
 
+static AllocatedString<char>
+Windows1252ToUTF8(const char *s) noexcept
+{
+#ifdef HAVE_ICU_CONVERTER
+	try {
+		std::unique_ptr<IcuConverter>
+			converter(IcuConverter::Create("windows-1252"));
+
+		return converter->ToUTF8(s);
+	} catch (...) { }
+#endif
+
+	/*
+	 * Fallback to not transcoding windows-1252 to utf-8, that may result
+	 * in invalid utf-8 unless nonprintable characters are replaced.
+	 */
+	auto t = AllocatedString<char>::Duplicate(s);
+
+	for (size_t i = 0; t[i] != AllocatedString<char>::SENTINEL; i++)
+		if (!IsPrintableASCII(t[i]))
+			t[i] = '?';
+
+	return t;
+}
+
 gcc_pure
-static const char *
+static AllocatedString<char>
 GetInfoString(const SidTuneInfo &info, unsigned i) noexcept
 {
 #ifdef HAVE_SIDPLAYFP
-	return info.numberOfInfoStrings() > i
+	const char *s = info.numberOfInfoStrings() > i
 		? info.infoString(i)
-		: nullptr;
+		: "";
 #else
-	return info.numberOfInfoStrings > i
+	const char *s = info.numberOfInfoStrings > i
 		? info.infoString[i]
-		: nullptr;
+		: "";
 #endif
+
+	return Windows1252ToUTF8(s);
+}
+
+gcc_pure
+static AllocatedString<char>
+GetDateString(const SidTuneInfo &info) noexcept
+{
+	/*
+	 * Field 2 is called <released>, previously used as <copyright>.
+	 * It is formatted <year><space><company or author or group>,
+	 * where <year> may be <YYYY>, <YYY?>, <YY??> or <YYYY-YY>, for
+	 * example "1987", "199?", "19??" or "1985-87". The <company or
+	 * author or group> may be for example Rob Hubbard. A full field
+	 * may be for example "1987 Rob Hubbard".
+	 */
+	AllocatedString<char> release = GetInfoString(info, 2);
+
+	/* Keep the <year> part only for the date. */
+	for (size_t i = 0; release[i] != AllocatedString<char>::SENTINEL; i++)
+		if (std::isspace(release[i])) {
+			release[i] = AllocatedString<char>::SENTINEL;
+			break;
+		}
+
+	return release;
 }
 
 static void
@@ -452,27 +506,25 @@ ScanSidTuneInfo(const SidTuneInfo &info, unsigned track, unsigned n_tracks,
 		TagHandler &handler) noexcept
 {
 	/* title */
-	const char *title = GetInfoString(info, 0);
-	if (title == nullptr)
-		title = "";
+	const auto title = GetInfoString(info, 0);
 
 	if (n_tracks > 1) {
 		const auto tag_title =
 			StringFormat<1024>("%s (%u/%u)",
-					   title, track, n_tracks);
-		handler.OnTag(TAG_TITLE, tag_title);
+					   title.c_str(), track, n_tracks);
+		handler.OnTag(TAG_TITLE, tag_title.c_str());
 	} else
-		handler.OnTag(TAG_TITLE, title);
+		handler.OnTag(TAG_TITLE, title.c_str());
 
 	/* artist */
-	const char *artist = GetInfoString(info, 1);
-	if (artist != nullptr)
-		handler.OnTag(TAG_ARTIST, artist);
+	const auto artist = GetInfoString(info, 1);
+	if (!artist.empty())
+		handler.OnTag(TAG_ARTIST, artist.c_str());
 
 	/* date */
-	const char *date = GetInfoString(info, 2);
-	if (date != nullptr)
-		handler.OnTag(TAG_DATE, date);
+	const auto date = GetDateString(info);
+	if (!date.empty())
+		handler.OnTag(TAG_DATE, date.c_str());
 
 	/* track */
 	handler.OnTag(TAG_TRACK, StringFormat<16>("%u", track));
@@ -547,6 +599,10 @@ sidplay_container_scan(Path path_fs)
 		AddTagHandler h(tag_builder);
 		ScanSidTuneInfo(info, i, n_tracks, h);
 
+		const SignedSongTime duration = get_song_length(tune);
+		if (!duration.IsNegative())
+			h.OnDuration(SongTime(duration));
+
 		char track_name[32];
 		/* Construct container/tune path names, eg.
 		   Delta.sid/tune_001.sid */

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)
@@ -458,7 +459,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;