From 374cc51f770f73feef4c24b35c6c522ccc865f02 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 17:37:30 +0200 Subject: [PATCH 01/11] decoder/Bridge: add flag to make initial seek errors fatal When the client wants to seek, but the decoder has already finished decoding the current song, the player restarts the decoder with an initial seek at the new position. When this initial seek fails, MPD pretends nothing has happened and plays this song from the start. With this new flag, a restarted decoder marks the initial seek as "essential" and fails the decoder if that seek fails. Closes https://github.com/MusicPlayerDaemon/MPD/issues/895 --- NEWS | 2 ++ src/decoder/Bridge.cxx | 6 ++++++ src/decoder/Bridge.hxx | 7 +++++++ src/decoder/Control.cxx | 2 ++ src/decoder/Control.hxx | 9 +++++++++ src/decoder/Thread.cxx | 1 + src/player/Thread.cxx | 15 +++++++++------ 7 files changed, 36 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 34057bbc2..c6924f5aa 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ ver 0.21.24 (not yet released) - modplug: fix Windows build failure - wildmidi: attempt to detect WildMidi using pkg-config - wildmidi: fix Windows build failure +* player + - don't restart current song if seeking beyond end * Android - enable the decoder plugins ModPlug and WildMidi - fix build failure with Android NDK r21 diff --git a/src/decoder/Bridge.cxx b/src/decoder/Bridge.cxx index 59d80cc48..e11a0a45a 100644 --- a/src/decoder/Bridge.cxx +++ b/src/decoder/Bridge.cxx @@ -33,6 +33,8 @@ #include "util/ConstBuffer.hxx" #include "util/StringBuffer.hxx" +#include + #include #include #include @@ -344,6 +346,10 @@ DecoderBridge::SeekError() /* d'oh, we can't seek to the sub-song start position, what now? - no idea, ignoring the problem for now. */ initial_seek_running = false; + + if (initial_seek_essential) + error = std::make_exception_ptr(std::runtime_error("Decoder failed to seek")); + return; } diff --git a/src/decoder/Bridge.hxx b/src/decoder/Bridge.hxx index f94909f31..eb005ea79 100644 --- a/src/decoder/Bridge.hxx +++ b/src/decoder/Bridge.hxx @@ -62,6 +62,11 @@ public: */ bool initial_seek_pending; + /** + * Are initial seek failures fatal? + */ + const bool initial_seek_essential; + /** * Is the initial seek currently running? During this time, * the decoder command is SEEK. This flag is set by @@ -107,9 +112,11 @@ public: std::exception_ptr error; DecoderBridge(DecoderControl &_dc, bool _initial_seek_pending, + bool _initial_seek_essential, std::unique_ptr _tag) :dc(_dc), initial_seek_pending(_initial_seek_pending), + initial_seek_essential(_initial_seek_essential), song_tag(std::move(_tag)) {} ~DecoderBridge(); diff --git a/src/decoder/Control.cxx b/src/decoder/Control.cxx index cf8a74385..4962e194b 100644 --- a/src/decoder/Control.cxx +++ b/src/decoder/Control.cxx @@ -90,6 +90,7 @@ DecoderControl::IsCurrentSong(const DetachedSong &_song) const noexcept void DecoderControl::Start(std::unique_ptr _song, SongTime _start_time, SongTime _end_time, + bool _initial_seek_essential, MusicBuffer &_buffer, std::shared_ptr _pipe) noexcept { @@ -99,6 +100,7 @@ DecoderControl::Start(std::unique_ptr _song, song = std::move(_song); start_time = _start_time; end_time = _end_time; + initial_seek_essential = _initial_seek_essential; buffer = &_buffer; pipe = std::move(_pipe); diff --git a/src/decoder/Control.hxx b/src/decoder/Control.hxx index 385516a62..b630a7283 100644 --- a/src/decoder/Control.hxx +++ b/src/decoder/Control.hxx @@ -117,6 +117,12 @@ public: bool seek_error; bool seekable; + + /** + * @see #DecoderBridge::initial_seek_essential + */ + bool initial_seek_essential; + SongTime seek_time; private: @@ -398,11 +404,14 @@ public: * owned and freed by the decoder * @param start_time see #DecoderControl * @param end_time see #DecoderControl + * @param initial_seek_essential see + * #DecoderBridge::initial_seek_essential * @param pipe the pipe which receives the decoded chunks (owned by * the caller) */ void Start(std::unique_ptr song, SongTime start_time, SongTime end_time, + bool initial_seek_essential, MusicBuffer &buffer, std::shared_ptr pipe) noexcept; diff --git a/src/decoder/Thread.cxx b/src/decoder/Thread.cxx index 45129a206..b2670de65 100644 --- a/src/decoder/Thread.cxx +++ b/src/decoder/Thread.cxx @@ -461,6 +461,7 @@ decoder_run_song(DecoderControl &dc, dc.start_time = dc.seek_time; DecoderBridge bridge(dc, dc.start_time.IsPositive(), + dc.initial_seek_essential, /* pass the song tag only if it's authoritative, i.e. if it's a local file - tags on "stream" songs are just diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx index a10429472..286855136 100644 --- a/src/player/Thread.cxx +++ b/src/player/Thread.cxx @@ -223,7 +223,8 @@ private: * * Caller must lock the mutex. */ - void StartDecoder(std::shared_ptr pipe) noexcept; + void StartDecoder(std::shared_ptr pipe, + bool initial_seek_essential) noexcept; /** * The decoder has acknowledged the "START" command (see @@ -364,7 +365,8 @@ public: }; void -Player::StartDecoder(std::shared_ptr _pipe) noexcept +Player::StartDecoder(std::shared_ptr _pipe, + bool initial_seek_essential) noexcept { assert(queued || pc.command == PlayerCommand::SEEK); assert(pc.next_song != nullptr); @@ -376,6 +378,7 @@ Player::StartDecoder(std::shared_ptr _pipe) noexcept dc.Start(std::make_unique(*pc.next_song), start_time, pc.next_song->GetEndTime(), + initial_seek_essential, buffer, std::move(_pipe)); } @@ -633,7 +636,7 @@ Player::SeekDecoder() noexcept pipe->Clear(); /* re-start the decoder */ - StartDecoder(pipe); + StartDecoder(pipe, true); ActivateDecoder(); pc.seeking = true; @@ -711,7 +714,7 @@ Player::ProcessCommand() noexcept pc.CommandFinished(); if (dc.IsIdle()) - StartDecoder(std::make_shared()); + StartDecoder(std::make_shared(), false); break; @@ -982,7 +985,7 @@ Player::Run() noexcept const std::lock_guard lock(pc.mutex); - StartDecoder(pipe); + StartDecoder(pipe, true); ActivateDecoder(); pc.state = PlayerState::PLAY; @@ -1022,7 +1025,7 @@ Player::Run() noexcept assert(dc.pipe == nullptr || dc.pipe == pipe); - StartDecoder(std::make_shared()); + StartDecoder(std::make_shared(), false); } if (/* no cross-fading if MPD is going to pause at the From e92129f449f00039c9139b9762df6ef446a806b6 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 20:41:23 +0200 Subject: [PATCH 02/11] doc/protocol.rst: clarify the term "UNIX time" Closes https://github.com/MusicPlayerDaemon/MPD/issues/893 --- doc/protocol.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/protocol.rst b/doc/protocol.rst index 421b3492a..fe60d9c61 100644 --- a/doc/protocol.rst +++ b/doc/protocol.rst @@ -464,7 +464,8 @@ Querying :program:`MPD`'s status - ``songs``: number of songs - ``uptime``: daemon uptime in seconds - ``db_playtime``: sum of all song times in the database in seconds - - ``db_update``: last db update in UNIX time + - ``db_update``: last db update in UNIX time (seconds since + 1970-01-01 UTC) - ``playtime``: time length of music played Playback options From 563c7318f9305419053a876bb7bf4d1eae2517ce Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 21:00:35 +0200 Subject: [PATCH 03/11] fs/AllocatedPath: add method GetSuffix() --- src/fs/AllocatedPath.hxx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/fs/AllocatedPath.hxx b/src/fs/AllocatedPath.hxx index d8bab299c..8c792a2cf 100644 --- a/src/fs/AllocatedPath.hxx +++ b/src/fs/AllocatedPath.hxx @@ -285,6 +285,11 @@ public: bool IsAbsolute() const noexcept { return Traits::IsAbsolute(c_str()); } + + gcc_pure + const_pointer_type GetSuffix() const noexcept { + return ((Path)*this).GetSuffix(); + } }; #endif From c5cc256bf2e40cc4b92ad331101bcfff525fda0c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 20:59:52 +0200 Subject: [PATCH 04/11] decoder/gme: use Path::GetSuffix() --- src/decoder/plugins/GmeDecoderPlugin.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/decoder/plugins/GmeDecoderPlugin.cxx b/src/decoder/plugins/GmeDecoderPlugin.cxx index 82f47fdd3..9d952890a 100644 --- a/src/decoder/plugins/GmeDecoderPlugin.cxx +++ b/src/decoder/plugins/GmeDecoderPlugin.cxx @@ -30,7 +30,6 @@ #include "util/ScopeExit.hxx" #include "util/StringCompare.hxx" #include "util/StringFormat.hxx" -#include "util/UriUtil.hxx" #include "util/Domain.hxx" #include "Log.hxx" @@ -109,7 +108,7 @@ static Music_Emu* LoadGmeAndM3u(GmeContainerPath container) { const char *path = container.path.c_str(); - const char *suffix = uri_get_suffix(path); + const auto *suffix = container.path.GetSuffix(); Music_Emu *emu; const char *gme_err = @@ -303,7 +302,7 @@ gme_container_scan(Path path_fs) if (num_songs < 2) return list; - const char *subtune_suffix = uri_get_suffix(path_fs.c_str()); + const auto *subtune_suffix = path_fs.GetSuffix(); TagBuilder tag_builder; From 14412c867f00e54eb770398c9793f33415a5bf14 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 21:10:28 +0200 Subject: [PATCH 05/11] add a few IWYU pragmas --- src/AudioFormat.hxx | 2 +- src/decoder/DecoderPlugin.hxx | 2 +- src/util/StringFormat.hxx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AudioFormat.hxx b/src/AudioFormat.hxx index 38d0aa9a6..7e411b12a 100644 --- a/src/AudioFormat.hxx +++ b/src/AudioFormat.hxx @@ -20,7 +20,7 @@ #ifndef MPD_AUDIO_FORMAT_HXX #define MPD_AUDIO_FORMAT_HXX -#include "pcm/SampleFormat.hxx" +#include "pcm/SampleFormat.hxx" // IWYU pragma: export #include "util/Compiler.h" #include diff --git a/src/decoder/DecoderPlugin.hxx b/src/decoder/DecoderPlugin.hxx index 322d81157..c4b023018 100644 --- a/src/decoder/DecoderPlugin.hxx +++ b/src/decoder/DecoderPlugin.hxx @@ -22,7 +22,7 @@ #include "util/Compiler.h" -#include +#include // IWYU pragma: export struct ConfigBlock; class InputStream; diff --git a/src/util/StringFormat.hxx b/src/util/StringFormat.hxx index 723591c69..c761e8806 100644 --- a/src/util/StringFormat.hxx +++ b/src/util/StringFormat.hxx @@ -30,7 +30,7 @@ #ifndef STRING_FORMAT_HXX #define STRING_FORMAT_HXX -#include "StringBuffer.hxx" +#include "StringBuffer.hxx" // IWYU pragma: export #include From 8925cc17d8436b6793930d11dc2168a3f66d8c94 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 21:02:28 +0200 Subject: [PATCH 06/11] decoder/gme: use StringAfterPrefix() --- src/decoder/plugins/GmeDecoderPlugin.cxx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/decoder/plugins/GmeDecoderPlugin.cxx b/src/decoder/plugins/GmeDecoderPlugin.cxx index 9d952890a..b194fc57d 100644 --- a/src/decoder/plugins/GmeDecoderPlugin.cxx +++ b/src/decoder/plugins/GmeDecoderPlugin.cxx @@ -37,7 +37,6 @@ #include #include -#include #define SUBTUNE_PREFIX "tune_" @@ -75,11 +74,10 @@ gcc_pure static unsigned ParseSubtuneName(const char *base) noexcept { - if (memcmp(base, SUBTUNE_PREFIX, sizeof(SUBTUNE_PREFIX) - 1) != 0) + base = StringAfterPrefix(base, SUBTUNE_PREFIX); + if (base == nullptr) return 0; - base += sizeof(SUBTUNE_PREFIX) - 1; - char *endptr; auto track = strtoul(base, &endptr, 10); if (endptr == base || *endptr != '.') From fea326530b9b0fda6d4047511b7f455c5637a83b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 21:18:45 +0200 Subject: [PATCH 07/11] decoder/gme: simplify LoadGmeAndM3u() by moving code to ReplaceSuffix() --- src/decoder/plugins/GmeDecoderPlugin.cxx | 32 +++++++++++++----------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/decoder/plugins/GmeDecoderPlugin.cxx b/src/decoder/plugins/GmeDecoderPlugin.cxx index b194fc57d..3327e1b51 100644 --- a/src/decoder/plugins/GmeDecoderPlugin.cxx +++ b/src/decoder/plugins/GmeDecoderPlugin.cxx @@ -102,35 +102,39 @@ ParseContainerPath(Path path_fs) return { path_fs.GetDirectoryName(), track - 1 }; } +static AllocatedPath +ReplaceSuffix(Path src, + const PathTraitsFS::const_pointer_type new_suffix) noexcept +{ + const auto *old_suffix = src.GetSuffix(); + if (old_suffix == nullptr) + return nullptr; + + PathTraitsFS::string s(src.c_str(), old_suffix); + s += new_suffix; + return AllocatedPath::FromFS(std::move(s)); +} + static Music_Emu* LoadGmeAndM3u(GmeContainerPath container) { - const char *path = container.path.c_str(); - const auto *suffix = container.path.GetSuffix(); - Music_Emu *emu; const char *gme_err = - gme_open_file(path, &emu, GME_SAMPLE_RATE); + gme_open_file(container.path.c_str(), &emu, GME_SAMPLE_RATE); if (gme_err != nullptr) { LogWarning(gme_domain, gme_err); return nullptr; } - if(suffix == nullptr) { - return emu; - } - - std::string m3u_path(path,suffix); - m3u_path += "m3u"; - + const auto m3u_path = ReplaceSuffix(container.path, "m3u"); /* * Some GME formats lose metadata if you attempt to * load a non-existant M3U file, so check that one * exists before loading. */ - if(FileExists(Path::FromFS(m3u_path.c_str()))) { - gme_load_m3u(emu,m3u_path.c_str()); - } + if (!m3u_path.IsNull() && FileExists(m3u_path)) + gme_load_m3u(emu, m3u_path.c_str()); + return emu; } From aafc9ce75bbbb8b3195298eba6c457af01f16ee0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 20:58:46 +0200 Subject: [PATCH 08/11] decoder/gme: use class NarrowPath() for Windows compatibility --- src/decoder/plugins/GmeDecoderPlugin.cxx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/decoder/plugins/GmeDecoderPlugin.cxx b/src/decoder/plugins/GmeDecoderPlugin.cxx index 3327e1b51..98d47fb3f 100644 --- a/src/decoder/plugins/GmeDecoderPlugin.cxx +++ b/src/decoder/plugins/GmeDecoderPlugin.cxx @@ -27,6 +27,7 @@ #include "fs/Path.hxx" #include "fs/AllocatedPath.hxx" #include "fs/FileSystem.hxx" +#include "fs/NarrowPath.hxx" #include "util/ScopeExit.hxx" #include "util/StringCompare.hxx" #include "util/StringFormat.hxx" @@ -96,7 +97,7 @@ ParseContainerPath(Path path_fs) const Path base = path_fs.GetBase(); unsigned track; if (base.IsNull() || - (track = ParseSubtuneName(base.c_str())) < 1) + (track = ParseSubtuneName(NarrowPath(base))) < 1) return { AllocatedPath(path_fs), 0 }; return { path_fs.GetDirectoryName(), track - 1 }; @@ -120,20 +121,21 @@ LoadGmeAndM3u(GmeContainerPath container) { Music_Emu *emu; const char *gme_err = - gme_open_file(container.path.c_str(), &emu, GME_SAMPLE_RATE); + gme_open_file(NarrowPath(container.path), &emu, GME_SAMPLE_RATE); if (gme_err != nullptr) { LogWarning(gme_domain, gme_err); return nullptr; } - const auto m3u_path = ReplaceSuffix(container.path, "m3u"); + const auto m3u_path = ReplaceSuffix(container.path, + PATH_LITERAL("m3u")); /* * Some GME formats lose metadata if you attempt to * load a non-existant M3U file, so check that one * exists before loading. */ if (!m3u_path.IsNull() && FileExists(m3u_path)) - gme_load_m3u(emu, m3u_path.c_str()); + gme_load_m3u(emu, NarrowPath(m3u_path)); return emu; } From 7583cfe9b7628a912b3b19c88ff518bb75f700ad Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 20:58:31 +0200 Subject: [PATCH 09/11] {android,win32}/build.py: enable the GME decoder plugin Closes https://github.com/MusicPlayerDaemon/MPD/issues/891 --- NEWS | 4 ++-- android/build.py | 1 + python/build/libs.py | 12 ++++++++++++ win32/build.py | 1 + 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index c6924f5aa..bda78362f 100644 --- a/NEWS +++ b/NEWS @@ -10,10 +10,10 @@ ver 0.21.24 (not yet released) * player - don't restart current song if seeking beyond end * Android - - enable the decoder plugins ModPlug and WildMidi + - enable the decoder plugins GME, ModPlug and WildMidi - fix build failure with Android NDK r21 * Windows - - enable the decoder plugins ModPlug and WildMidi + - enable the decoder plugins GME, ModPlug and WildMidi - work around Meson bug breaking the Windows build with GCC 10 * fix unit test failure diff --git a/android/build.py b/android/build.py index 8716fc1a9..5bee46304 100755 --- a/android/build.py +++ b/android/build.py @@ -170,6 +170,7 @@ thirdparty_libs = [ libid3tag, libmodplug, wildmidi, + gme, ffmpeg, curl, libexpat, diff --git a/python/build/libs.py b/python/build/libs.py index c7b2250cd..e211a0107 100644 --- a/python/build/libs.py +++ b/python/build/libs.py @@ -135,6 +135,18 @@ wildmidi = CmakeProject( version='0.4.3', ) +gme = CmakeProject( + 'https://bitbucket.org/mpyne/game-music-emu/downloads/game-music-emu-0.6.3.tar.xz', + 'aba34e53ef0ec6a34b58b84e28bf8cfbccee6585cebca25333604c35db3e051d', + 'lib/libgme.a', + [ + '-DBUILD_SHARED_LIBS=OFF', + '-DENABLE_UBSAN=OFF', + '-DZLIB_INCLUDE_DIR=OFF', + '-DSDL2_DIR=OFF', + ], +) + ffmpeg = FfmpegProject( 'http://ffmpeg.org/releases/ffmpeg-4.2.3.tar.xz', '9df6c90aed1337634c1fb026fb01c154c29c82a64ea71291ff2da9aacb9aad31', diff --git a/win32/build.py b/win32/build.py index 9e1b6e9e4..fb636ded5 100755 --- a/win32/build.py +++ b/win32/build.py @@ -98,6 +98,7 @@ thirdparty_libs = [ liblame, libmodplug, wildmidi, + gme, ffmpeg, curl, libexpat, From 6b3a282db4e536d62d5f16f7c75c4e3368fc819c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 22:29:47 +0200 Subject: [PATCH 10/11] lib/curl/Request: don't enable CURLOPT_NETRC on Windows Our Windows build is built with `--disable-netrc`, and that makes CURLOPT_NETRC fail, causing failures with all streams. D'oh! Closes https://github.com/MusicPlayerDaemon/MPD/issues/886 --- NEWS | 1 + src/lib/curl/Request.cxx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index bda78362f..914b73705 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,7 @@ ver 0.21.24 (not yet released) - enable the decoder plugins GME, ModPlug and WildMidi - fix build failure with Android NDK r21 * Windows + - fix stream playback - enable the decoder plugins GME, ModPlug and WildMidi - work around Meson bug breaking the Windows build with GCC 10 * fix unit test failure diff --git a/src/lib/curl/Request.cxx b/src/lib/curl/Request.cxx index 75ce11eed..774066877 100644 --- a/src/lib/curl/Request.cxx +++ b/src/lib/curl/Request.cxx @@ -56,7 +56,7 @@ CurlRequest::CurlRequest(CurlGlobal &_global, easy.SetUserAgent("Music Player Daemon " VERSION); easy.SetHeaderFunction(_HeaderFunction, this); easy.SetWriteFunction(WriteFunction, this); -#ifndef ANDROID +#if !defined(ANDROID) && !defined(_WIN32) easy.SetOption(CURLOPT_NETRC, 1L); #endif easy.SetErrorBuffer(error_buffer); From 24741c5d065e1f0b575f4ba5472ce8066e5e141d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 22:48:50 +0200 Subject: [PATCH 11/11] release v0.21.24 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 914b73705..203b1fef2 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.21.24 (not yet released) +ver 0.21.24 (2020/06/10) * protocol - "tagtypes" requires no permissions * database