From ea80587ddb1905851623200b4cf5ef260f8c9631 Mon Sep 17 00:00:00 2001
From: John Regan <john@jrjrtech.com>
Date: Tue, 26 Sep 2017 08:42:53 -0500
Subject: [PATCH 01/10] GME Plugin: fix track numbering
GME starts all track indexes at zero, but subtune prefixes
start at one. This fixes an off-by-one error during track
enumeration.
---
NEWS | 1 +
src/decoder/plugins/GmeDecoderPlugin.cxx | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index ae617cdce..92593f54c 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,7 @@ ver 0.20.11 (not yet released)
- curl: support Content-Type application/xml
* decoder
- ffmpeg: more reliable song duration
+ - gme: fix track numbering
* fix case insensitive search without libicu
ver 0.20.10 (2017/08/24)
diff --git a/src/decoder/plugins/GmeDecoderPlugin.cxx b/src/decoder/plugins/GmeDecoderPlugin.cxx
index 403e62346..1b9d06163 100644
--- a/src/decoder/plugins/GmeDecoderPlugin.cxx
+++ b/src/decoder/plugins/GmeDecoderPlugin.cxx
@@ -293,13 +293,13 @@ gme_container_scan(Path path_fs)
TagBuilder tag_builder;
auto tail = list.before_begin();
- for (unsigned i = 1; i <= num_songs; ++i) {
+ for (unsigned i = 0; i < num_songs; ++i) {
ScanMusicEmu(emu, i,
add_tag_handler, &tag_builder);
char track_name[64];
snprintf(track_name, sizeof(track_name),
- SUBTUNE_PREFIX "%03u.%s", i, subtune_suffix);
+ SUBTUNE_PREFIX "%03u.%s", i+1, subtune_suffix);
tail = list.emplace_after(tail, track_name,
tag_builder.Commit());
}
From fa67c2548a75c3788feb12f494ad3f3627257b42 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Wed, 27 Sep 2017 17:08:16 +0200
Subject: [PATCH 02/10] decoder/Thread: clear the command after catching an
exception
If an early exception gets caught (e.g. from
AllocatedPath::FromUTF8Throw()) before
DecoderControl::CommandFinishedLocked() is called, the decoder thread
would go in an endless loop, because DecoderCommand::START is still
set.
Closes #118
---
NEWS | 1 +
src/decoder/DecoderThread.cxx | 1 +
2 files changed, 2 insertions(+)
diff --git a/NEWS b/NEWS
index 92593f54c..3ab0b453b 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,7 @@ ver 0.20.11 (not yet released)
- ffmpeg: more reliable song duration
- gme: fix track numbering
* fix case insensitive search without libicu
+* fix endless loop when accessing malformed file names in ZIP files
ver 0.20.10 (2017/08/24)
* decoder
diff --git a/src/decoder/DecoderThread.cxx b/src/decoder/DecoderThread.cxx
index e74ee0f84..7d4a91560 100644
--- a/src/decoder/DecoderThread.cxx
+++ b/src/decoder/DecoderThread.cxx
@@ -508,6 +508,7 @@ try {
decoder_run_song(dc, song, uri_utf8, path_fs);
} catch (...) {
dc.state = DecoderState::ERROR;
+ dc.command = DecoderCommand::NONE;
dc.error = std::current_exception();
dc.client_cond.signal();
}
From 81b7373637e4b0c5c961b31c64ada8ed2a84d3a2 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Wed, 18 Oct 2017 08:46:31 +0200
Subject: [PATCH 03/10] queue/Queue: MoveOrder() returns to_order
---
src/queue/Playlist.cxx | 3 +--
src/queue/Queue.cxx | 3 ++-
src/queue/Queue.hxx | 4 +++-
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/queue/Playlist.cxx b/src/queue/Playlist.cxx
index 667e3457f..dfeb0d163 100644
--- a/src/queue/Playlist.cxx
+++ b/src/queue/Playlist.cxx
@@ -310,8 +310,7 @@ playlist::SetRandom(PlayerControl &pc, bool status)
playlist is played after that */
unsigned current_order =
queue.PositionToOrder(current_position);
- queue.MoveOrder(current_order, 0);
- current = 0;
+ current = queue.MoveOrder(current_order, 0);
} else
current = -1;
} else
diff --git a/src/queue/Queue.cxx b/src/queue/Queue.cxx
index d248f306b..9ee9568e0 100644
--- a/src/queue/Queue.cxx
+++ b/src/queue/Queue.cxx
@@ -195,7 +195,7 @@ Queue::MoveRange(unsigned start, unsigned end, unsigned to) noexcept
}
}
-void
+unsigned
Queue::MoveOrder(unsigned from_order, unsigned to_order) noexcept
{
assert(from_order < length);
@@ -212,6 +212,7 @@ Queue::MoveOrder(unsigned from_order, unsigned to_order) noexcept
}
order[to_order] = from_position;
+ return to_order;
}
void
diff --git a/src/queue/Queue.hxx b/src/queue/Queue.hxx
index bd2f95c1a..b9ca341e4 100644
--- a/src/queue/Queue.hxx
+++ b/src/queue/Queue.hxx
@@ -284,8 +284,10 @@ struct Queue {
/**
* Moves a song to a new position in the "order" list.
+ *
+ * @return to_order
*/
- void MoveOrder(unsigned from_order, unsigned to_order) noexcept;
+ unsigned MoveOrder(unsigned from_order, unsigned to_order) noexcept;
/**
* Moves a song to a new position.
From f2fac77d8c6ae42ce2e9a67dcfde4085ec44cfdf Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Wed, 18 Oct 2017 08:50:01 +0200
Subject: [PATCH 04/10] queue/Queue: add methods MoveOrderBefore() and
MoveOrderAfter()
---
src/queue/Queue.cxx | 18 ++++++++++++++++++
src/queue/Queue.hxx | 18 ++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/src/queue/Queue.cxx b/src/queue/Queue.cxx
index 9ee9568e0..720778099 100644
--- a/src/queue/Queue.cxx
+++ b/src/queue/Queue.cxx
@@ -215,6 +215,24 @@ Queue::MoveOrder(unsigned from_order, unsigned to_order) noexcept
return to_order;
}
+unsigned
+Queue::MoveOrderBefore(unsigned from_order, unsigned to_order) noexcept
+{
+ /* if "from_order" comes before "to_order", then the new
+ position is "to_order-1"; otherwise the "to_order" song is
+ moved one ahead */
+ return MoveOrder(from_order, to_order - (from_order < to_order));
+}
+
+unsigned
+Queue::MoveOrderAfter(unsigned from_order, unsigned to_order) noexcept
+{
+ /* if "from_order" comes after "to_order", then the new
+ position is "to_order+1"; otherwise the "to_order" song is
+ moved one back */
+ return MoveOrder(from_order, to_order + (from_order > to_order));
+}
+
void
Queue::DeletePosition(unsigned position) noexcept
{
diff --git a/src/queue/Queue.hxx b/src/queue/Queue.hxx
index b9ca341e4..06e800479 100644
--- a/src/queue/Queue.hxx
+++ b/src/queue/Queue.hxx
@@ -289,6 +289,24 @@ struct Queue {
*/
unsigned MoveOrder(unsigned from_order, unsigned to_order) noexcept;
+ /**
+ * Moves a song to a new position in the "order" list before
+ * the given one.
+ *
+ * @return the new order number of the given "from" song
+ */
+ unsigned MoveOrderBefore(unsigned from_order,
+ unsigned to_order) noexcept;
+
+ /**
+ * Moves a song to a new position in the "order" list after
+ * the given one.
+ *
+ * @return the new order number of the given "from" song
+ */
+ unsigned MoveOrderAfter(unsigned from_order,
+ unsigned to_order) noexcept;
+
/**
* Moves a song to a new position.
*/
From 0f79287b04602ce92cb86c056f78d5f70ab71d42 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Tue, 17 Oct 2017 19:02:41 +0200
Subject: [PATCH 05/10] queue/Playlist: move code to MoveOrderToCurrent()
---
src/queue/Playlist.hxx | 12 ++++++++++++
src/queue/PlaylistControl.cxx | 26 +++++++++++++++++++-------
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/src/queue/Playlist.hxx b/src/queue/Playlist.hxx
index 32489e354..c04a0913a 100644
--- a/src/queue/Playlist.hxx
+++ b/src/queue/Playlist.hxx
@@ -356,6 +356,18 @@ public:
}
void SetConsume(bool new_value);
+
+private:
+ /**
+ * Prepare a manual song change: move the given song to the
+ * current playback order. This is done to avoid skipping
+ * upcoming songs in the order list. The newly selected song
+ * shall be inserted in the order list, and the rest shall be
+ * played after that as previously planned.
+ *
+ * @return the new order number of the given song
+ */
+ unsigned MoveOrderToCurrent(unsigned old_order);
};
#endif
diff --git a/src/queue/PlaylistControl.cxx b/src/queue/PlaylistControl.cxx
index 99012aedf..c2284b60c 100644
--- a/src/queue/PlaylistControl.cxx
+++ b/src/queue/PlaylistControl.cxx
@@ -56,6 +56,24 @@ playlist::Stop(PlayerControl &pc)
}
}
+unsigned
+playlist::MoveOrderToCurrent(unsigned old_order)
+{
+ if (!queue.random)
+ /* no-op because there is no order list */
+ return old_order;
+
+ const unsigned destination_order = playing
+ ? (unsigned)current
+ : 0;
+
+ /* swap the new song with the previous "current" one, so
+ playback continues as planned */
+ queue.SwapOrders(old_order, destination_order);
+
+ return destination_order;
+}
+
void
playlist::PlayPosition(PlayerControl &pc, int song)
{
@@ -90,13 +108,7 @@ playlist::PlayPosition(PlayerControl &pc, int song)
number, because random mode is enabled */
i = queue.PositionToOrder(song);
- if (!playing)
- current = 0;
-
- /* swap the new song with the previous "current" one,
- so playback continues as planned */
- queue.SwapOrders(i, current);
- i = current;
+ i = MoveOrderToCurrent(i);
}
stop_on_error = false;
From 91254e9211f6fa9864dd8f2d2bb7b43cccf7cb77 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Tue, 17 Oct 2017 19:44:16 +0200
Subject: [PATCH 06/10] queue/PlaylistControl: keep order list consistency in
MoveOrderToCurrent()
Our previous use of Queue::SwapOrders() could cause surprising
results:
- sometimes, the old "current" song would be played again (if the
newly selected song had not been played already)
- sometimes, the old "current" song would not be played again (if the
newly selected song had already been played)
This is inconsistent, because it should not depend on whether the
newly selected song had already been played.
So instead of Queue::SwapOrders() we now use Queue::MoveOrderAfter()
and Queue::MoveOrderBefore(), which is more expensive, but also more
consistent. It attempts to retain as much from the previous order
list as possible, and only moves the newly selected song around.
---
NEWS | 1 +
src/queue/PlaylistControl.cxx | 24 +++++++++++++++---------
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/NEWS b/NEWS
index 3ab0b453b..25bf749f5 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,7 @@ ver 0.20.11 (not yet released)
* decoder
- ffmpeg: more reliable song duration
- gme: fix track numbering
+* improve random song order when switching songs manually
* fix case insensitive search without libicu
* fix endless loop when accessing malformed file names in ZIP files
diff --git a/src/queue/PlaylistControl.cxx b/src/queue/PlaylistControl.cxx
index c2284b60c..2217756dd 100644
--- a/src/queue/PlaylistControl.cxx
+++ b/src/queue/PlaylistControl.cxx
@@ -63,15 +63,21 @@ playlist::MoveOrderToCurrent(unsigned old_order)
/* no-op because there is no order list */
return old_order;
- const unsigned destination_order = playing
- ? (unsigned)current
- : 0;
-
- /* swap the new song with the previous "current" one, so
- playback continues as planned */
- queue.SwapOrders(old_order, destination_order);
-
- return destination_order;
+ if (playing) {
+ /* already playing: move the specified song after the
+ current one (because the current one has already
+ been playing and shall not be played again) */
+ return queue.MoveOrderAfter(old_order, current);
+ } else if (current >= 0) {
+ /* not playing: move the specified song before the
+ current one, so it will be played eventually */
+ return queue.MoveOrderBefore(old_order, current);
+ } else {
+ /* not playing anything: move the specified song to
+ the front */
+ queue.SwapOrders(old_order, 0);
+ return 0;
+ }
}
void
From 10990a06846c1766e86fe6af227ceb81fbd3c12d Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Tue, 17 Oct 2017 19:33:56 +0200
Subject: [PATCH 07/10] queue/Playlist: call MoveOrderToCurrent() in
SeekSongOrder() on song change
Applies the improvements from the previous commit to the "seek"
commands, which are also capable of switching songs.
Closes #119
---
src/queue/PlaylistControl.cxx | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/queue/PlaylistControl.cxx b/src/queue/PlaylistControl.cxx
index 2217756dd..4b17386ae 100644
--- a/src/queue/PlaylistControl.cxx
+++ b/src/queue/PlaylistControl.cxx
@@ -223,6 +223,8 @@ playlist::SeekSongOrder(PlayerControl &pc, unsigned i, SongTime seek_time)
/* seeking is not within the current song - prepare
song change */
+ i = MoveOrderToCurrent(i);
+
playing = true;
current = i;
From 753a2aa4622b2a116b510ae1ea41b3898c425955 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Wed, 18 Oct 2017 09:51:04 +0200
Subject: [PATCH 08/10] PlaylistSave: move code to playlist_print_path()
---
src/PlaylistSave.cxx | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/PlaylistSave.cxx b/src/PlaylistSave.cxx
index e64daba8f..88d8cbe74 100644
--- a/src/PlaylistSave.cxx
+++ b/src/PlaylistSave.cxx
@@ -35,6 +35,12 @@
#include <stdexcept>
+static void
+playlist_print_path(BufferedOutputStream &os, const Path path)
+{
+ os.Format("%s\n", NarrowPath(path).c_str());
+}
+
void
playlist_print_song(BufferedOutputStream &os, const DetachedSong &song)
{
@@ -44,7 +50,7 @@ playlist_print_song(BufferedOutputStream &os, const DetachedSong &song)
try {
const auto uri_fs = AllocatedPath::FromUTF8Throw(uri_utf8);
- os.Format("%s\n", NarrowPath(uri_fs).c_str());
+ playlist_print_path(os, uri_fs);
} catch (const std::runtime_error &) {
}
}
@@ -63,7 +69,7 @@ playlist_print_uri(BufferedOutputStream &os, const char *uri)
AllocatedPath::FromUTF8Throw(uri);
if (!path.IsNull())
- os.Format("%s\n", NarrowPath(path).c_str());
+ playlist_print_path(os, path);
} catch (const std::runtime_error &) {
}
}
From 9703a401c5ac3f679503b845805fe24db22c458c Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Wed, 18 Oct 2017 10:05:26 +0200
Subject: [PATCH 09/10] Playlist{File,Save}: always use UTF-8 in playlists on
Windows
Turns out that using CP_ACP is a lousy idea, because only very few
Unicode characters can be represented by it. Instead, switch to UTF-8
(which every sane person on other operating system already uses).
Closes #102
---
NEWS | 1 +
src/PlaylistFile.cxx | 11 +++++------
src/PlaylistSave.cxx | 10 ++++++++--
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/NEWS b/NEWS
index 25bf749f5..2ad95f03d 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ ver 0.20.11 (not yet released)
- gme: fix track numbering
* improve random song order when switching songs manually
* fix case insensitive search without libicu
+* fix Unicode file names in playlists on Windows
* fix endless loop when accessing malformed file names in ZIP files
ver 0.20.10 (2017/08/24)
diff --git a/src/PlaylistFile.cxx b/src/PlaylistFile.cxx
index 2deae4321..c1fcdeb3c 100644
--- a/src/PlaylistFile.cxx
+++ b/src/PlaylistFile.cxx
@@ -207,13 +207,12 @@ try {
continue;
#ifdef _UNICODE
- wchar_t buffer[MAX_PATH];
- auto result = MultiByteToWideChar(CP_ACP, 0, s, -1,
- buffer, ARRAY_SIZE(buffer));
- if (result <= 0)
+ /* on Windows, playlists always contain UTF-8, because
+ its "narrow" charset (i.e. CP_ACP) is incapable of
+ storing all Unicode paths */
+ const auto path = AllocatedPath::FromUTF8(s);
+ if (path.IsNull())
continue;
-
- const Path path = Path::FromFS(buffer);
#else
const Path path = Path::FromFS(s);
#endif
diff --git a/src/PlaylistSave.cxx b/src/PlaylistSave.cxx
index 88d8cbe74..d3ec30bb2 100644
--- a/src/PlaylistSave.cxx
+++ b/src/PlaylistSave.cxx
@@ -28,7 +28,6 @@
#include "fs/AllocatedPath.hxx"
#include "fs/Traits.hxx"
#include "fs/FileSystem.hxx"
-#include "fs/NarrowPath.hxx"
#include "fs/io/FileOutputStream.hxx"
#include "fs/io/BufferedOutputStream.hxx"
#include "util/UriUtil.hxx"
@@ -38,7 +37,14 @@
static void
playlist_print_path(BufferedOutputStream &os, const Path path)
{
- os.Format("%s\n", NarrowPath(path).c_str());
+#ifdef _UNICODE
+ /* on Windows, playlists always contain UTF-8, because its
+ "narrow" charset (i.e. CP_ACP) is incapable of storing all
+ Unicode paths */
+ os.Format("%s\n", path.ToUTF8().c_str());
+#else
+ os.Format("%s\n", path.c_str());
+#endif
}
void
From a7fdfa08e138e5b18f84a9ad52489c6c61861d60 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Wed, 18 Oct 2017 10:14:46 +0200
Subject: [PATCH 10/10] release v0.20.11
---
NEWS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index 2ad95f03d..804d6d723 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,4 @@
-ver 0.20.11 (not yet released)
+ver 0.20.11 (2017/10/18)
* storage
- curl: support Content-Type application/xml
* decoder