From 6080c3b4ba3afb918977d4354121ddac01c446c9 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Tue, 7 Apr 2020 20:14:00 -0700 Subject: [PATCH 01/24] Math.hxx: move cmath include out of define The _GLIBCXX_USE_C99_MATH macro is defined in glibcxx by c++config.h, which gets included by every header. Which means a header needs to be present. (cherry picked from commit 79e9aff3382d8b7521318c44835a6dd6b284e2c1) --- src/util/Math.hxx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/util/Math.hxx b/src/util/Math.hxx index 9784c0b64..ad5cffd82 100644 --- a/src/util/Math.hxx +++ b/src/util/Math.hxx @@ -30,11 +30,16 @@ #ifndef MATH_HXX #define MATH_HXX +#include + +/* + * C99 math can be optionally omitted with gcc's libstdc++. + * Use boost if unavailable. + */ #if (defined(__GLIBCPP__) || defined(__GLIBCXX__)) && !defined(_GLIBCXX_USE_C99_MATH) #include using boost::math::lround; #else -#include using std::lround; #endif From c331c75fde9a15dd94e187356db4e4b460faa224 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 14 Apr 2020 13:12:24 +0200 Subject: [PATCH 02/24] increment version number to 0.21.23 --- NEWS | 2 ++ android/AndroidManifest.xml | 4 ++-- doc/conf.py | 2 +- meson.build | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index fb8a7d89a..184d6566c 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,5 @@ +ver 0.21.23 (not yet released) + ver 0.21.22 (2020/04/02) * database - simple: optimize startup diff --git a/android/AndroidManifest.xml b/android/AndroidManifest.xml index 4dcdba4fd..64d0802b1 100644 --- a/android/AndroidManifest.xml +++ b/android/AndroidManifest.xml @@ -2,8 +2,8 @@ + android:versionCode="46" + android:versionName="0.21.23"> diff --git a/doc/conf.py b/doc/conf.py index f70f7b890..246aa98c3 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -38,7 +38,7 @@ author = 'Max Kellermann' # built documents. # # The short X.Y version. -version = '0.21.22' +version = '0.21.23' # The full version, including alpha/beta/rc tags. release = version diff --git a/meson.build b/meson.build index c0f859d78..800785b85 100644 --- a/meson.build +++ b/meson.build @@ -1,7 +1,7 @@ project( 'mpd', ['c', 'cpp'], - version: '0.21.22', + version: '0.21.23', meson_version: '>= 0.49.0', default_options: [ 'c_std=c99', From 32a5bf043b8be3195a718adfec425535c3608992 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 14 Apr 2020 16:03:49 +0200 Subject: [PATCH 03/24] player/Thread: drain outputs at end of song in "single" mode Without this, the Pause() call would drop the ring buffers and would skip a considerable portion of the end of the song. Closes https://github.com/MusicPlayerDaemon/MPD/issues/824 --- NEWS | 2 ++ src/player/Thread.cxx | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/NEWS b/NEWS index 184d6566c..d2f758909 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.21.23 (not yet released) +* player + - drain outputs at end of song in "single" mode ver 0.21.22 (2020/04/02) * database diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx index 64a502e6d..a10429472 100644 --- a/src/player/Thread.cxx +++ b/src/player/Thread.cxx @@ -964,6 +964,12 @@ Player::SongBorder() noexcept if (border_pause) { paused = true; pc.listener.OnBorderPause(); + + /* drain all outputs to guarantee the current song is + really being played to the end; without this, the + Pause() call would drop all ring buffers */ + pc.outputs.Drain(); + pc.outputs.Pause(); idle_add(IDLE_PLAYER); } From f6fe001fa99a2d1c1c5e22d3313ae98a188662b7 Mon Sep 17 00:00:00 2001 From: Florian Heese Date: Tue, 14 Apr 2020 21:42:02 +0200 Subject: [PATCH 04/24] Added missing channel order setups for ALSA --- NEWS | 2 ++ src/pcm/Order.cxx | 86 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/NEWS b/NEWS index d2f758909..e44865281 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.21.23 (not yet released) +* output + - alsa: implement channel mapping for 5.0 and 7.0 * player - drain outputs at end of song in "single" mode diff --git a/src/pcm/Order.cxx b/src/pcm/Order.cxx index e728f0c9f..e75cacb94 100644 --- a/src/pcm/Order.cxx +++ b/src/pcm/Order.cxx @@ -21,6 +21,28 @@ #include "PcmBuffer.hxx" #include "util/ConstBuffer.hxx" + +/* + * According to: + * - https://xiph.org/flac/format.html#frame_header + * - https://github.com/nu774/qaac/wiki/Multichannel--handling + * the source channel order (after decoding, e.g., flac, alac) is for + * - 1ch: mono + * - 2ch: left, right + * - 3ch: left, right, center + * - 4ch: front left, front right, back left, back right + * - 5ch: front left, front right, front center, back/surround left, back/surround right + * - 6ch (aka 5.1): front left, front right, front center, LFE, back/surround left, back/surround right + * - 7ch: front left, front right, front center, LFE, back center, side left, side right + * - 8ch: (aka 7.1): front left, front right, front center, LFE, back left, back right, side left, side right + * + * The ALSA default channel map is (see /usr/share/alsa/pcm/surround71.conf): + * - front left, front right, back left, back right, front center, LFE, side left, side right + * + * Hence, in case of the following source channel orders 3ch, 5ch, 6ch (aka + * 5.1), 7ch and 8ch the channel order has to be adapted + */ + template struct TwoPointers { V *dest; @@ -44,17 +66,57 @@ struct TwoPointers { return *this; } + TwoPointers &ToAlsa50() noexcept { + *dest++ = src[0]; // front left + *dest++ = src[1]; // front right + *dest++ = src[3]; // surround left + *dest++ = src[4]; // surround right + *dest++ = src[2]; // front center + src += 5; + return *this; + } + TwoPointers &ToAlsa51() noexcept { return CopyTwo() // left+right .SwapTwoPairs(); // center, LFE, surround left+right } + TwoPointers &ToAlsa70() noexcept { + *dest++ = src[0]; // front left + *dest++ = src[1]; // front right + *dest++ = src[5]; // side left + *dest++ = src[6]; // side right + *dest++ = src[2]; // front center + *dest++ = src[3]; // LFE + *dest++ = src[4]; // back center + src += 7; + return *this; + } + TwoPointers &ToAlsa71() noexcept { return ToAlsa51() .CopyTwo(); // side left+right } }; +template +static void +ToAlsaChannelOrder50(V *dest, const V *src, size_t n) noexcept +{ + TwoPointers p{dest, src}; + for (size_t i = 0; i != n; ++i) + p.ToAlsa50(); +} + +template +static inline ConstBuffer +ToAlsaChannelOrder50(PcmBuffer &buffer, ConstBuffer src) noexcept +{ + auto dest = buffer.GetT(src.size); + ToAlsaChannelOrder50(dest, src.data, src.size / 5); + return { dest, src.size }; +} + template static void ToAlsaChannelOrder51(V *dest, const V *src, size_t n) noexcept @@ -73,6 +135,24 @@ ToAlsaChannelOrder51(PcmBuffer &buffer, ConstBuffer src) noexcept return { dest, src.size }; } +template +static void +ToAlsaChannelOrder70(V *dest, const V *src, size_t n) noexcept +{ + TwoPointers p{dest, src}; + for (size_t i = 0; i != n; ++i) + p.ToAlsa70(); +} + +template +static inline ConstBuffer +ToAlsaChannelOrder70(PcmBuffer &buffer, ConstBuffer src) noexcept +{ + auto dest = buffer.GetT(src.size); + ToAlsaChannelOrder70(dest, src.data, src.size / 7); + return { dest, src.size }; +} + template static void ToAlsaChannelOrder71(V *dest, const V *src, size_t n) noexcept @@ -97,9 +177,15 @@ ToAlsaChannelOrderT(PcmBuffer &buffer, ConstBuffer src, unsigned channels) noexcept { switch (channels) { + case 5: // 5.0 + return ToAlsaChannelOrder50(buffer, src); + case 6: // 5.1 return ToAlsaChannelOrder51(buffer, src); + case 7: // 7.0 + return ToAlsaChannelOrder70(buffer, src); + case 8: // 7.1 return ToAlsaChannelOrder71(buffer, src); From 3d8067a041fae2c117772a498702b9f40fe24595 Mon Sep 17 00:00:00 2001 From: Thomas Guillem Date: Mon, 13 Apr 2020 23:49:06 +0200 Subject: [PATCH 05/24] storage/curl: fix href when file has a '&' char If the file name is "Hello & bye", 3 CharacterData events will be sent with the State::HREF state: - "Hello%20" - "&" - "%20bye" Reproduced with files hosted on an apache2 DAV server: 2.4.38-3+deb10u3. --- NEWS | 2 ++ src/storage/plugins/CurlStorage.cxx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index e44865281..15a4fb77a 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.21.23 (not yet released) +* storage + - curl: fix corrupt "href" values in the presence of XML entities * output - alsa: implement channel mapping for 5.0 and 7.0 * player diff --git a/src/storage/plugins/CurlStorage.cxx b/src/storage/plugins/CurlStorage.cxx index e2bc99bdf..9452f7e54 100644 --- a/src/storage/plugins/CurlStorage.cxx +++ b/src/storage/plugins/CurlStorage.cxx @@ -402,7 +402,7 @@ private: break; case State::HREF: - response.href.assign(s, len); + response.href.append(s, len); break; case State::STATUS: From b18074f8998f7aefc326e09932db37aae4f13062 Mon Sep 17 00:00:00 2001 From: Thomas Guillem Date: Tue, 14 Apr 2020 22:07:51 +0200 Subject: [PATCH 06/24] storage/curl: fix path comparison when the server escapes differently Unescape the base path and the path coming from the server (href) to fix the comparison when the server uses different escaped characters. The outputted name need to be unescaped. Doing that before or after the HrefToEscapedName() call should not change the current behavior. --- NEWS | 1 + src/storage/plugins/CurlStorage.cxx | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 15a4fb77a..ac5656a6b 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.21.23 (not yet released) * storage - curl: fix corrupt "href" values in the presence of XML entities + - curl: unescape "href" values * output - alsa: implement channel mapping for 5.0 and 7.0 * player diff --git a/src/storage/plugins/CurlStorage.cxx b/src/storage/plugins/CurlStorage.cxx index 9452f7e54..e47aa7ae9 100644 --- a/src/storage/plugins/CurlStorage.cxx +++ b/src/storage/plugins/CurlStorage.cxx @@ -482,7 +482,7 @@ class HttpListDirectoryOperation final : public PropfindOperation { public: HttpListDirectoryOperation(CurlGlobal &curl, const char *uri) :PropfindOperation(curl, uri, 1), - base_path(UriPathOrSlash(uri)) {} + base_path(CurlUnescape(GetEasy(), UriPathOrSlash(uri))) {} std::unique_ptr Perform() { DeferStart(); @@ -507,8 +507,7 @@ private: /* kludge: ignoring case in this comparison to avoid false negatives if the web server uses a different - case in hex digits in escaped characters; TODO: - implement properly */ + case */ path = StringAfterPrefixIgnoreCase(path, base_path.c_str()); if (path == nullptr || *path == 0) return nullptr; @@ -531,11 +530,12 @@ protected: if (r.status != 200) return; - const auto escaped_name = HrefToEscapedName(r.href.c_str()); - if (escaped_name.IsNull()) + std::string href = CurlUnescape(GetEasy(), r.href.c_str()); + const auto name = HrefToEscapedName(href.c_str()); + if (name.IsNull()) return; - entries.emplace_front(CurlUnescape(GetEasy(), escaped_name)); + entries.emplace_front(std::string(name.data, name.size)); auto &info = entries.front().info; info = StorageFileInfo(r.collection From a5273d699274c0a6274052bb80fcfdd5c7fac88b Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Wed, 8 Apr 2020 18:20:19 -0700 Subject: [PATCH 07/24] Fix unsafe float comparison. Switching == to >= should be safe here since the next if is the opposite. Signed-off-by: Rosen Penev --- src/player/CrossFade.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/player/CrossFade.cxx b/src/player/CrossFade.cxx index b31610e66..beb9b18a5 100644 --- a/src/player/CrossFade.cxx +++ b/src/player/CrossFade.cxx @@ -62,7 +62,7 @@ mixramp_interpolate(const char *ramp_list, float required_db) noexcept ++ramp_list; /* Check for exact match. */ - if (db == required_db) { + if (db >= required_db) { return duration; } From 3b0f8d551630cdc23f3ba003014e22376dbe8ff0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Apr 2020 19:30:16 +0200 Subject: [PATCH 08/24] lib/icu/CaseFold: remove Windows implementation Reverts commit fb3564fbe76a10a0825bd06c0ff19f481d94b835 LCMapStringEx() doesn't do what I imagined it would do 5 years ago. D'oh! Closes https://github.com/MusicPlayerDaemon/MPD/issues/820 --- NEWS | 2 ++ src/lib/icu/CaseFold.cxx | 24 ------------------------ src/lib/icu/CaseFold.hxx | 2 +- 3 files changed, 3 insertions(+), 25 deletions(-) diff --git a/NEWS b/NEWS index ac5656a6b..198302b20 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ ver 0.21.23 (not yet released) - alsa: implement channel mapping for 5.0 and 7.0 * player - drain outputs at end of song in "single" mode +* Windows + - fix case insensitive search ver 0.21.22 (2020/04/02) * database diff --git a/src/lib/icu/CaseFold.cxx b/src/lib/icu/CaseFold.cxx index 8ad8dadfa..e7cf5ba26 100644 --- a/src/lib/icu/CaseFold.cxx +++ b/src/lib/icu/CaseFold.cxx @@ -36,11 +36,6 @@ #include #endif -#ifdef _WIN32 -#include "Win32.hxx" -#include -#endif - #include #include @@ -72,25 +67,6 @@ try { folded.SetSize(folded_length); return UCharToUTF8({folded.begin(), folded.size()}); -#elif defined(_WIN32) - const auto u = MultiByteToWideChar(CP_UTF8, src); - - const int size = LCMapStringEx(LOCALE_NAME_INVARIANT, - LCMAP_SORTKEY|LINGUISTIC_IGNORECASE, - u.c_str(), -1, nullptr, 0, - nullptr, nullptr, 0); - if (size <= 0) - return AllocatedString<>::Duplicate(src); - - std::unique_ptr buffer(new wchar_t[size]); - if (LCMapStringEx(LOCALE_NAME_INVARIANT, - LCMAP_SORTKEY|LINGUISTIC_IGNORECASE, - u.c_str(), -1, buffer.get(), size, - nullptr, nullptr, 0) <= 0) - return AllocatedString<>::Duplicate(src); - - return WideCharToMultiByte(CP_UTF8, buffer.get()); - #else #error not implemented #endif diff --git a/src/lib/icu/CaseFold.hxx b/src/lib/icu/CaseFold.hxx index 828415b9b..13a517e0a 100644 --- a/src/lib/icu/CaseFold.hxx +++ b/src/lib/icu/CaseFold.hxx @@ -22,7 +22,7 @@ #include "config.h" -#if defined(HAVE_ICU) || defined(_WIN32) +#ifdef HAVE_ICU #define HAVE_ICU_CASE_FOLD #include "util/Compiler.h" From fc92db83cf52839423aa58438f992f86ff71ab70 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Apr 2020 21:38:22 +0200 Subject: [PATCH 09/24] lib/icu/Collate: use NORM_IGNORECASE instead of LINGUISTIC_IGNORECASE LINGUISTIC_IGNORECASE is unimplemented on Wine, but since we don't have any locale support (yet), and we're using LOCALE_NAME_INVARIANT, NORM_IGNORECASE should essentially be the same, so why bother. --- src/lib/icu/Collate.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/icu/Collate.cxx b/src/lib/icu/Collate.cxx index baff5137a..4b64e8668 100644 --- a/src/lib/icu/Collate.cxx +++ b/src/lib/icu/Collate.cxx @@ -109,7 +109,7 @@ IcuCollate(const char *a, const char *b) noexcept } auto result = CompareStringEx(LOCALE_NAME_INVARIANT, - LINGUISTIC_IGNORECASE, + NORM_IGNORECASE, wa.c_str(), -1, wb.c_str(), -1, nullptr, nullptr, 0); From f3fd2eb6189523fc91c9c0a761036d4d999438f3 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Apr 2020 19:42:04 +0200 Subject: [PATCH 10/24] lib/icu/Compare: use AllocatedString::Clone() --- src/lib/icu/Compare.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/icu/Compare.hxx b/src/lib/icu/Compare.hxx index 5504b5d51..ee079a56f 100644 --- a/src/lib/icu/Compare.hxx +++ b/src/lib/icu/Compare.hxx @@ -38,12 +38,12 @@ public: IcuCompare(const IcuCompare &src) noexcept :needle(src - ? AllocatedString<>::Duplicate(src.needle.c_str()) + ? src.needle.Clone() : nullptr) {} IcuCompare &operator=(const IcuCompare &src) noexcept { needle = src - ? AllocatedString<>::Duplicate(src.needle.c_str()) + ? src.needle.Clone() : nullptr; return *this; } From 8f00dbea45954a7fb0cac4a0b9e9b90117373b08 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Apr 2020 19:43:29 +0200 Subject: [PATCH 11/24] lib/icu/Compare: add Windows implementation Using CompareStringEx() and FindNLSStringEx(). Implements a missing piece for https://github.com/MusicPlayerDaemon/MPD/issues/820 --- src/lib/icu/Compare.cxx | 50 +++++++++++++++++++++++++++++++++++++++++ src/lib/icu/Compare.hxx | 10 +++++++++ 2 files changed, 60 insertions(+) diff --git a/src/lib/icu/Compare.cxx b/src/lib/icu/Compare.cxx index 5af297404..b0c817897 100644 --- a/src/lib/icu/Compare.cxx +++ b/src/lib/icu/Compare.cxx @@ -22,6 +22,11 @@ #include "util/StringAPI.hxx" #include "config.h" +#ifdef _WIN32 +#include "Win32.hxx" +#include +#endif + #include #ifdef HAVE_ICU_CASE_FOLD @@ -29,6 +34,17 @@ IcuCompare::IcuCompare(const char *_needle) noexcept :needle(IcuCaseFold(_needle)) {} +#elif defined(_WIN32) + +IcuCompare::IcuCompare(const char *_needle) noexcept + :needle(nullptr) +{ + try { + needle = MultiByteToWideChar(CP_UTF8, _needle); + } catch (...) { + } +} + #else IcuCompare::IcuCompare(const char *_needle) noexcept @@ -41,6 +57,22 @@ IcuCompare::operator==(const char *haystack) const noexcept { #ifdef HAVE_ICU_CASE_FOLD return StringIsEqual(IcuCaseFold(haystack).c_str(), needle.c_str()); +#elif defined(_WIN32) + if (needle.IsNull()) + /* the MultiByteToWideChar() call in the constructor + has failed, so let's always fail the comparison */ + return false; + + try { + auto w_haystack = MultiByteToWideChar(CP_UTF8, haystack); + return CompareStringEx(LOCALE_NAME_INVARIANT, + NORM_IGNORECASE, + w_haystack.c_str(), -1, + needle.c_str(), -1, + nullptr, nullptr, 0) == CSTR_EQUAL; + } catch (...) { + return false; + } #else return strcasecmp(haystack, needle.c_str()); #endif @@ -52,6 +84,24 @@ IcuCompare::IsIn(const char *haystack) const noexcept #ifdef HAVE_ICU_CASE_FOLD return StringFind(IcuCaseFold(haystack).c_str(), needle.c_str()) != nullptr; +#elif defined(_WIN32) + if (needle.IsNull()) + /* the MultiByteToWideChar() call in the constructor + has failed, so let's always fail the comparison */ + return false; + + try { + auto w_haystack = MultiByteToWideChar(CP_UTF8, haystack); + return FindNLSStringEx(LOCALE_NAME_INVARIANT, + FIND_FROMSTART|NORM_IGNORECASE, + w_haystack.c_str(), -1, + needle.c_str(), -1, + nullptr, + nullptr, nullptr, 0) >= 0; + } catch (...) { + /* MultiByteToWideChar() has failed */ + return false; + } #elif defined(HAVE_STRCASESTR) return strcasestr(haystack, needle.c_str()) != nullptr; #else diff --git a/src/lib/icu/Compare.hxx b/src/lib/icu/Compare.hxx index ee079a56f..39522b5d5 100644 --- a/src/lib/icu/Compare.hxx +++ b/src/lib/icu/Compare.hxx @@ -23,13 +23,23 @@ #include "util/Compiler.h" #include "util/AllocatedString.hxx" +#ifdef _WIN32 +#include +#endif + /** * This class can compare one string ("needle") with lots of other * strings ("haystacks") efficiently, ignoring case. With some * configurations, it can prepare a case-folded version of the needle. */ class IcuCompare { +#ifdef _WIN32 + /* Windows API functions work with wchar_t strings, so let's + cache the MultiByteToWideChar() result for performance */ + AllocatedString needle; +#else AllocatedString<> needle; +#endif public: IcuCompare():needle(nullptr) {} From 138c29320be50e212396735941094f555ce5ce0c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Apr 2020 21:49:10 +0200 Subject: [PATCH 12/24] gme: adapt to API change in the upcoming version 0.7.0 Closes https://github.com/MusicPlayerDaemon/MPD/issues/833 --- NEWS | 2 ++ src/decoder/plugins/GmeDecoderPlugin.cxx | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 198302b20..68b2e5558 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ ver 0.21.23 (not yet released) * storage - curl: fix corrupt "href" values in the presence of XML entities - curl: unescape "href" values +* decoder + - gme: adapt to API change in the upcoming version 0.7.0 * output - alsa: implement channel mapping for 5.0 and 7.0 * player diff --git a/src/decoder/plugins/GmeDecoderPlugin.cxx b/src/decoder/plugins/GmeDecoderPlugin.cxx index fcea526af..82f47fdd3 100644 --- a/src/decoder/plugins/GmeDecoderPlugin.cxx +++ b/src/decoder/plugins/GmeDecoderPlugin.cxx @@ -185,7 +185,11 @@ gme_file_decode(DecoderClient &client, Path path_fs) LogWarning(gme_domain, gme_err); if (length > 0) - gme_set_fade(emu, length); + gme_set_fade(emu, length +#if GME_VERSION >= 0x000700 + , 8000 +#endif + ); /* play */ DecoderCommand cmd; From 0a92fbc18e9ae84ca088f96078acd8705269bec9 Mon Sep 17 00:00:00 2001 From: geneticdrift Date: Wed, 22 Apr 2020 21:55:02 +0200 Subject: [PATCH 13/24] tag/Fallback: add tag fallback for AlbumSort Closes https://github.com/MusicPlayerDaemon/MPD/issues/832 --- NEWS | 2 ++ src/tag/Fallback.hxx | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/NEWS b/NEWS index 68b2e5558..264ac1737 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.21.23 (not yet released) +* protocol + - add tag fallback for AlbumSort * storage - curl: fix corrupt "href" values in the presence of XML entities - curl: unescape "href" values diff --git a/src/tag/Fallback.hxx b/src/tag/Fallback.hxx index ecbe0d439..fb07fc1e8 100644 --- a/src/tag/Fallback.hxx +++ b/src/tag/Fallback.hxx @@ -45,6 +45,10 @@ ApplyTagFallback(TagType type, F &&f) noexcept "AlbumArtist"/"ArtistSort" was found */ return f(TAG_ARTIST); + if (type == TAG_ALBUM_SORT) + /* fall back to "Album" if no "AlbumSort" was found */ + return f(TAG_ALBUM); + return false; } From 159389164a589bba0328dfdc661740aeb7c3fca0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 14:51:09 +0200 Subject: [PATCH 14/24] lib/nfs/FileReader: set `state=IDLE` before invoking callback Fixes assertion failure if the callback fails. --- NEWS | 2 ++ src/lib/nfs/FileReader.cxx | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 264ac1737..81a5e6b26 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,8 @@ ver 0.21.23 (not yet released) * storage - curl: fix corrupt "href" values in the presence of XML entities - curl: unescape "href" values +* input + - nfs: fix crash bug * decoder - gme: adapt to API change in the upcoming version 0.7.0 * output diff --git a/src/lib/nfs/FileReader.cxx b/src/lib/nfs/FileReader.cxx index 6fe4faaab..02900a059 100644 --- a/src/lib/nfs/FileReader.cxx +++ b/src/lib/nfs/FileReader.cxx @@ -186,6 +186,8 @@ NfsFileReader::OpenCallback(nfsfh *_fh) noexcept fh = _fh; + state = State::IDLE; + try { connection->Stat(fh, *this); } catch (...) { @@ -204,13 +206,13 @@ NfsFileReader::StatCallback(const struct stat *st) noexcept assert(fh != nullptr); assert(st != nullptr); + state = State::IDLE; + if (!S_ISREG(st->st_mode)) { OnNfsFileError(std::make_exception_ptr(std::runtime_error("Not a regular file"))); return; } - state = State::IDLE; - OnNfsFileOpen(st->st_size); } From e53a4d0a9ebea73f27fc6786a85439f9f8c00e42 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 14:51:58 +0200 Subject: [PATCH 15/24] lib/nfs/FileReader: reset `state` in OnNfsCallback() The object's state is `IDLE` when OnNfsCallback() gets invoked, so let's use the start of the method to reset the `state` field. --- src/lib/nfs/FileReader.cxx | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/lib/nfs/FileReader.cxx b/src/lib/nfs/FileReader.cxx index 02900a059..27dea109f 100644 --- a/src/lib/nfs/FileReader.cxx +++ b/src/lib/nfs/FileReader.cxx @@ -180,14 +180,11 @@ NfsFileReader::OnNfsConnectionDisconnected(std::exception_ptr e) noexcept inline void NfsFileReader::OpenCallback(nfsfh *_fh) noexcept { - assert(state == State::OPEN); assert(connection != nullptr); assert(_fh != nullptr); fh = _fh; - state = State::IDLE; - try { connection->Stat(fh, *this); } catch (...) { @@ -201,13 +198,10 @@ NfsFileReader::OpenCallback(nfsfh *_fh) noexcept inline void NfsFileReader::StatCallback(const struct stat *st) noexcept { - assert(state == State::STAT); assert(connection != nullptr); assert(fh != nullptr); assert(st != nullptr); - state = State::IDLE; - if (!S_ISREG(st->st_mode)) { OnNfsFileError(std::make_exception_ptr(std::runtime_error("Not a regular file"))); return; @@ -219,7 +213,7 @@ NfsFileReader::StatCallback(const struct stat *st) noexcept void NfsFileReader::OnNfsCallback(unsigned status, void *data) noexcept { - switch (state) { + switch (std::exchange(state, State::IDLE)) { case State::INITIAL: case State::DEFER: case State::MOUNT: @@ -236,7 +230,6 @@ NfsFileReader::OnNfsCallback(unsigned status, void *data) noexcept break; case State::READ: - state = State::IDLE; OnNfsFileRead(data, status); break; } From e71bd2a08b60292c4d5672501e0972c2a97fd398 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 15:10:56 +0200 Subject: [PATCH 16/24] event/PollGroupWinSelect: make EVENT_{READ,WRITE} `static` --- src/event/PollGroupWinSelect.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/event/PollGroupWinSelect.cxx b/src/event/PollGroupWinSelect.cxx index 7fac95ecc..4e80fa130 100644 --- a/src/event/PollGroupWinSelect.cxx +++ b/src/event/PollGroupWinSelect.cxx @@ -23,8 +23,8 @@ #include "PollGroupWinSelect.hxx" -constexpr int EVENT_READ = 0; -constexpr int EVENT_WRITE = 1; +static constexpr int EVENT_READ = 0; +static constexpr int EVENT_WRITE = 1; static constexpr bool HasEvent(unsigned events, int event_id) noexcept From 4242aee21ee15690b4eb5f83b726f02f64bc5c2a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 15:15:53 +0200 Subject: [PATCH 17/24] event/SocketMonitor: remove HANGUP|ERROR from ScheduleRead() These flags are output-only. Using them here is misleading. --- src/event/SocketMonitor.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/event/SocketMonitor.hxx b/src/event/SocketMonitor.hxx index e4a2087d6..8e1dbc0e9 100644 --- a/src/event/SocketMonitor.hxx +++ b/src/event/SocketMonitor.hxx @@ -109,7 +109,7 @@ public: } bool ScheduleRead() noexcept { - return Schedule(GetScheduledFlags() | READ | HANGUP | ERROR); + return Schedule(GetScheduledFlags() | READ); } bool ScheduleWrite() noexcept { @@ -117,7 +117,7 @@ public: } void CancelRead() noexcept { - Schedule(GetScheduledFlags() & ~(READ|HANGUP|ERROR)); + Schedule(GetScheduledFlags() & ~READ); } void CancelWrite() noexcept { From 905db05cf9951cf7d395146aa2f0a95106508356 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 15:19:12 +0200 Subject: [PATCH 18/24] zeroconf/AvahiPoll: don't pass ERROR|HANGUP to Schedule() These flags are output-only. --- src/zeroconf/AvahiPoll.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/zeroconf/AvahiPoll.cxx b/src/zeroconf/AvahiPoll.cxx index cc62e67e4..43f084f92 100644 --- a/src/zeroconf/AvahiPoll.cxx +++ b/src/zeroconf/AvahiPoll.cxx @@ -26,9 +26,7 @@ static unsigned FromAvahiWatchEvent(AvahiWatchEvent e) { return (e & AVAHI_WATCH_IN ? SocketMonitor::READ : 0) | - (e & AVAHI_WATCH_OUT ? SocketMonitor::WRITE : 0) | - (e & AVAHI_WATCH_ERR ? SocketMonitor::ERROR : 0) | - (e & AVAHI_WATCH_HUP ? SocketMonitor::HANGUP : 0); + (e & AVAHI_WATCH_OUT ? SocketMonitor::WRITE : 0); } static AvahiWatchEvent From a27580d0ccf106c08b3dd52b00d630c674605c59 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 15:20:48 +0200 Subject: [PATCH 19/24] lib/nfs/Connection: don't pass HANGUP to Schedule() This flag is output-only. --- src/lib/nfs/Connection.cxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/nfs/Connection.cxx b/src/lib/nfs/Connection.cxx index 35c84b7de..6b61f8c2b 100644 --- a/src/lib/nfs/Connection.cxx +++ b/src/lib/nfs/Connection.cxx @@ -450,8 +450,7 @@ NfsConnection::ScheduleSocket() noexcept SocketMonitor::Open(_fd); } - SocketMonitor::Schedule(libnfs_to_events(which_events) - | SocketMonitor::HANGUP); + SocketMonitor::Schedule(libnfs_to_events(which_events)); } inline int From 8ed533acf3e9afad102d30c8978e200ccab314e5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 16:48:10 +0200 Subject: [PATCH 20/24] event/SocketMonitor: handle epoll_ctl()=EBADF/ENOENT in Schedule() This fixes a freeze bug in the NFS input/storage plugins: when libnfs auto-reconnets after a failure, it installs the new socket on the same file descriptor number. MPD's attempt to unregister the old socket by calling SocketMonitor::Steal() from NfsConnection::ScheduleSocket() fails because the new/old socket number is not registered in epoll, so epoll_ctl() returns ENOENT. The problem is that it left `scheduled_flags`, and so subsequent Schedule() calls will use `EPOLL_CTL_MOD`, which will fail again and again. Instead, we need to use `EPOLL_CTL_ADD` to register the new socket. Closes https://github.com/MusicPlayerDaemon/MPD/issues/806 Closes https://github.com/MusicPlayerDaemon/MPD/issues/756 --- NEWS | 1 + src/event/SocketMonitor.cxx | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/NEWS b/NEWS index 81a5e6b26..2206a506f 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ ver 0.21.23 (not yet released) - curl: unescape "href" values * input - nfs: fix crash bug + - nfs: fix freeze bug on reconnect * decoder - gme: adapt to API change in the upcoming version 0.7.0 * output diff --git a/src/event/SocketMonitor.cxx b/src/event/SocketMonitor.cxx index f335c8fdc..00a2663b3 100644 --- a/src/event/SocketMonitor.cxx +++ b/src/event/SocketMonitor.cxx @@ -20,6 +20,10 @@ #include "SocketMonitor.hxx" #include "Loop.hxx" +#ifdef USE_EPOLL +#include +#endif + #include #ifdef _WIN32 @@ -86,6 +90,21 @@ SocketMonitor::Schedule(unsigned flags) noexcept if (success) scheduled_flags = flags; +#ifdef USE_EPOLL + else if (errno == EBADF || errno == ENOENT) + /* the socket was probably closed by somebody else + (EBADF) or a new file descriptor with the same + number was created but not registered already + (ENOENT) - we can assume that there are no + scheduled events */ + /* note that when this happens, we're actually lucky + that it has failed - imagine another thread may + meanwhile have created something on the same file + descriptor number, and has registered it; the + epoll_ctl() call above would then have succeeded, + but broke the other thread's epoll registration */ + scheduled_flags = 0; +#endif return success; } From 7ded244a6104cd52330118270e7dfbcfda931b10 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 15:06:35 +0200 Subject: [PATCH 21/24] lib/nfs/Connection: pass POLLHUP and POLLERR to nfs_service() --- src/lib/nfs/Connection.cxx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/nfs/Connection.cxx b/src/lib/nfs/Connection.cxx index 6b61f8c2b..05dbab516 100644 --- a/src/lib/nfs/Connection.cxx +++ b/src/lib/nfs/Connection.cxx @@ -191,7 +191,9 @@ static constexpr int events_to_libnfs(unsigned i) noexcept { return ((i & SocketMonitor::READ) ? POLLIN : 0) | - ((i & SocketMonitor::WRITE) ? POLLOUT : 0); + ((i & SocketMonitor::WRITE) ? POLLOUT : 0) | + ((i & SocketMonitor::HANGUP) ? POLLHUP : 0) | + ((i & SocketMonitor::ERROR) ? POLLERR : 0); } NfsConnection::~NfsConnection() noexcept From fdb28eb0c4190ceccf4372f7289b1cd9de444a2c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 17:10:12 +0200 Subject: [PATCH 22/24] fs/NarrowPath: preserve nullptr in Path operator Fixes Path::IsNull() checks on Windows. --- src/fs/NarrowPath.hxx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/fs/NarrowPath.hxx b/src/fs/NarrowPath.hxx index 48198ffbf..55756e983 100644 --- a/src/fs/NarrowPath.hxx +++ b/src/fs/NarrowPath.hxx @@ -90,6 +90,11 @@ public: constexpr #endif operator Path() const noexcept { +#ifdef _UNICODE + if (value.IsNull()) + return nullptr; +#endif + return value; } }; From 3040ddb5ec51c92c8f840f0500f64f0e0906301e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 17:30:58 +0200 Subject: [PATCH 23/24] lib/nfs/FileReader: use `struct stat64` on Windows 32-bit libnfs is compiled with `-D_FILE_OFFSET_BITS=64`, but Meson decides not to enable this mode. We could force this mode, but then again, these days, nobody should be using 32-bit Windows ... so this is a kludge only for debugging with 32-bit WINE. --- src/lib/nfs/FileReader.cxx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/lib/nfs/FileReader.cxx b/src/lib/nfs/FileReader.cxx index 27dea109f..c16faa11c 100644 --- a/src/lib/nfs/FileReader.cxx +++ b/src/lib/nfs/FileReader.cxx @@ -196,11 +196,20 @@ NfsFileReader::OpenCallback(nfsfh *_fh) noexcept } inline void -NfsFileReader::StatCallback(const struct stat *st) noexcept +NfsFileReader::StatCallback(const struct stat *_st) noexcept { assert(connection != nullptr); assert(fh != nullptr); - assert(st != nullptr); + assert(_st != nullptr); + +#if defined(_WIN32) && !defined(_WIN64) + /* on 32-bit Windows, libnfs enables -D_FILE_OFFSET_BITS=64, + but MPD (Meson) doesn't - to work around this mismatch, we + cast explicitly to "struct stat64" */ + const auto *st = (const struct stat64 *)_st; +#else + const auto *st = _st; +#endif if (!S_ISREG(st->st_mode)) { OnNfsFileError(std::make_exception_ptr(std::runtime_error("Not a regular file"))); From 6c240f667c391f678dd916d0a1d7e9b149557240 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Apr 2020 17:46:20 +0200 Subject: [PATCH 24/24] release v0.21.23 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 2206a506f..d64d9e50e 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.21.23 (not yet released) +ver 0.21.23 (2020/04/23) * protocol - add tag fallback for AlbumSort * storage