From ec0d3ac95db0eb74aa618374dd97665b936c3c9c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 8 Jan 2021 16:50:27 +0100 Subject: [PATCH 01/28] Remove relative path handling which was not needed The original base relative path was introduced due to an erroneous test where the URL started with three slashes: "https:///" instead of two, which led to implementing handling for such cases but broke the two slashes case. This fix removes the base relative path handling because with two slashes the path is anyway always relative to the host (aka absolute URI, without host). This reverts 216f62ea1468933f4a78f17885b27e37e1393d8c and part of 74b2fc7fdca9be13cbbe4cb52b2fab573b3cf82c Signed-off-by: Vincent Petry --- src/storage/plugins/CurlStorage.cxx | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/storage/plugins/CurlStorage.cxx b/src/storage/plugins/CurlStorage.cxx index d717a824c..c986a8a49 100644 --- a/src/storage/plugins/CurlStorage.cxx +++ b/src/storage/plugins/CurlStorage.cxx @@ -468,19 +468,11 @@ CurlStorage::GetInfo(std::string_view uri_utf8, [[maybe_unused]] bool follow) gcc_pure static std::string_view -UriPathOrSlash(const char *uri, bool relative) noexcept +UriPathOrSlash(const char *uri) noexcept { auto path = uri_get_path(uri); if (path.data() == nullptr) path = "/"; - else if (relative) { - // search after first slash - path = path.substr(1); - auto slash = path.find('/'); - if (slash != std::string_view::npos) - path = path.substr(slash); - } - return path; } @@ -489,15 +481,13 @@ UriPathOrSlash(const char *uri, bool relative) noexcept */ class HttpListDirectoryOperation final : public PropfindOperation { const std::string base_path; - const std::string base_path_relative; MemoryStorageDirectoryReader::List entries; public: HttpListDirectoryOperation(CurlGlobal &curl, const char *uri) :PropfindOperation(curl, uri, 1), - base_path(CurlUnescape(GetEasy(), UriPathOrSlash(uri, false))), - base_path_relative(CurlUnescape(GetEasy(), UriPathOrSlash(uri, true))) {} + base_path(CurlUnescape(GetEasy(), UriPathOrSlash(uri))) {} std::unique_ptr Perform() { DeferStart(); @@ -523,15 +513,9 @@ private: /* kludge: ignoring case in this comparison to avoid false negatives if the web server uses a different case */ - if (uri_has_scheme(path)) { - path = StringAfterPrefixIgnoreCase(path, base_path.c_str()); - } else { - path = StringAfterPrefixIgnoreCase(path, base_path_relative.c_str()); - } - - if (path == nullptr || path.empty()) { + path = StringAfterPrefixIgnoreCase(path, base_path.c_str()); + if (path == nullptr || path.empty()) return nullptr; - } const char *slash = path.Find('/'); if (slash == nullptr) From 83391e2bd99dd526f925044e5be0074e2ef7c1da Mon Sep 17 00:00:00 2001 From: "Itai Y. Efrat" Date: Fri, 15 Jan 2021 20:40:05 +0200 Subject: [PATCH 02/28] doc/protocol.rst: fix playlist(find|search) The `{TAG} {NEEDLE}` input format documented seems to be a holdover of pre-0.21 filters, and the commands support the new format. --- doc/protocol.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/protocol.rst b/doc/protocol.rst index 73f810210..8d2eea243 100644 --- a/doc/protocol.rst +++ b/doc/protocol.rst @@ -727,7 +727,7 @@ Whenever possible, ids should be used. .. _command_playlistfind: -:command:`playlistfind {TAG} {NEEDLE}` +:command:`playlistfind {FILTER}` Finds songs in the queue with strict matching. @@ -748,7 +748,7 @@ Whenever possible, ids should be used. .. _command_playlistsearch: -:command:`playlistsearch {TAG} {NEEDLE}` +:command:`playlistsearch {FILTER}` Searches case-insensitively for partial matches in the queue. From 153d464ce8162928a94aaa7649c316f47a5a12f9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Jan 2021 18:05:16 +0100 Subject: [PATCH 03/28] python/build/libs.py: update Boost to 1.75.0 --- python/build/libs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/build/libs.py b/python/build/libs.py index c9cfd1d25..e29d24009 100644 --- a/python/build/libs.py +++ b/python/build/libs.py @@ -434,7 +434,7 @@ libnfs = AutotoolsProject( ) boost = BoostProject( - 'https://dl.bintray.com/boostorg/release/1.74.0/source/boost_1_74_0.tar.bz2', - '83bfc1507731a0906e387fc28b7ef5417d591429e51e788417fe9ff025e116b1', + 'https://dl.bintray.com/boostorg/release/1.75.0/source/boost_1_75_0.tar.bz2', + '953db31e016db7bb207f11432bef7df100516eeb746843fa0486a222e3fd49cb', 'include/boost/version.hpp', ) From 85a5b7dec4f692667500967989491615a58b8c8a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Jan 2021 18:08:12 +0100 Subject: [PATCH 04/28] python/build/libs.py: update CURL to 7.74.0 --- python/build/libs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/build/libs.py b/python/build/libs.py index e29d24009..c17237c4b 100644 --- a/python/build/libs.py +++ b/python/build/libs.py @@ -377,8 +377,8 @@ ffmpeg = FfmpegProject( ) curl = AutotoolsProject( - 'http://curl.haxx.se/download/curl-7.73.0.tar.xz', - '7c4c7ca4ea88abe00fea4740dcf81075c031b1d0bb23aff2d5efde20a3c2408a', + 'http://curl.haxx.se/download/curl-7.74.0.tar.xz', + '999d5f2c403cf6e25d58319fdd596611e455dd195208746bc6e6d197a77e878b', 'lib/libcurl.a', [ '--disable-shared', '--enable-static', From a14ce4c7cbd15aecbf8291eab8fb396c81577b78 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Jan 2021 19:57:37 +0100 Subject: [PATCH 05/28] lib/pcre/RegexPointer: work around bogus -Wmaybe-uninitialized with GCC 11 --- src/lib/pcre/RegexPointer.hxx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/lib/pcre/RegexPointer.hxx b/src/lib/pcre/RegexPointer.hxx index 7311d0407..cb7e730ba 100644 --- a/src/lib/pcre/RegexPointer.hxx +++ b/src/lib/pcre/RegexPointer.hxx @@ -40,6 +40,13 @@ #include +#if GCC_CHECK_VERSION(11,0) +#pragma GCC diagnostic push +/* bogus GCC 11 warning "ovector may be used uninitialized" in the + ovector.size() call */ +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif + class RegexPointer { protected: pcre *re = nullptr; @@ -63,4 +70,8 @@ public: } }; +#if GCC_CHECK_VERSION(11,0) +#pragma GCC diagnostic pop +#endif + #endif From 4949cd98f30dbc0a15cf0af5e087e7799d3f291f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Jan 2021 20:17:08 +0100 Subject: [PATCH 06/28] output/sles: add missing include for assert() --- src/output/plugins/sles/SlesOutputPlugin.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/output/plugins/sles/SlesOutputPlugin.cxx b/src/output/plugins/sles/SlesOutputPlugin.cxx index 61a815554..6c3978a9d 100644 --- a/src/output/plugins/sles/SlesOutputPlugin.cxx +++ b/src/output/plugins/sles/SlesOutputPlugin.cxx @@ -33,6 +33,7 @@ #include #include +#include #include #include From 68f4be323c595f7aefcaae0cc4b5a692b9b3e6ff Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Jan 2021 20:14:07 +0100 Subject: [PATCH 07/28] doc/user.rst: require Android NDK r22 --- doc/user.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user.rst b/doc/user.rst index 694ce0cc5..e21b07d36 100644 --- a/doc/user.rst +++ b/doc/user.rst @@ -167,7 +167,7 @@ Compiling for Android You need: * Android SDK -* Android NDK +* `Android NDK r22 `_ Just like with the native build, unpack the :program:`MPD` source tarball and change into the directory. Then, instead of From 3825175bfc7bf5438bf2ef24be027dd43b320ad9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Jan 2021 20:11:41 +0100 Subject: [PATCH 08/28] python/build/ffmpeg.py: remove obsolete -no-integrated-as workaround --- android/build.py | 2 +- python/build/ffmpeg.py | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/android/build.py b/android/build.py index 5bee46304..982160783 100755 --- a/android/build.py +++ b/android/build.py @@ -103,7 +103,7 @@ class AndroidNdkToolchain: llvm_bin = os.path.join(llvm_path, 'bin') self.cc = os.path.join(llvm_bin, 'clang') self.cxx = os.path.join(llvm_bin, 'clang++') - common_flags += ' -target ' + llvm_triple + ' -integrated-as -gcc-toolchain ' + toolchain_path + common_flags += ' -target ' + llvm_triple + ' -gcc-toolchain ' + toolchain_path common_flags += ' -fvisibility=hidden -fdata-sections -ffunction-sections' diff --git a/python/build/ffmpeg.py b/python/build/ffmpeg.py index 6c0753618..ae2ff6b00 100644 --- a/python/build/ffmpeg.py +++ b/python/build/ffmpeg.py @@ -10,11 +10,6 @@ class FfmpegProject(Project): self.configure_args = configure_args self.cppflags = cppflags - def _filter_cflags(self, flags): - # FFmpeg expects the GNU as syntax - flags = flags.replace(' -integrated-as ', ' -no-integrated-as ') - return flags - def build(self, toolchain): src = self.unpack(toolchain) build = self.make_build_path(toolchain) @@ -36,8 +31,8 @@ class FfmpegProject(Project): '--cc=' + toolchain.cc, '--cxx=' + toolchain.cxx, '--nm=' + toolchain.nm, - '--extra-cflags=' + self._filter_cflags(toolchain.cflags) + ' ' + toolchain.cppflags + ' ' + self.cppflags, - '--extra-cxxflags=' + self._filter_cflags(toolchain.cxxflags) + ' ' + toolchain.cppflags + ' ' + self.cppflags, + '--extra-cflags=' + toolchain.cflags + ' ' + toolchain.cppflags + ' ' + self.cppflags, + '--extra-cxxflags=' + toolchain.cxxflags + ' ' + toolchain.cppflags + ' ' + self.cppflags, '--extra-ldflags=' + toolchain.ldflags, '--extra-libs=' + toolchain.libs, '--ar=' + toolchain.ar, From 97a2122f41d88dda1ddf43ee77dc954dd19d9a18 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jan 2021 14:58:15 +0100 Subject: [PATCH 09/28] doc/mpd.conf.5.rst: updated ReplayGain website links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "The documentation for mpd.conf still links to https://replaygain.org – according to archive.org, this domain has been defunct since 2014-09-14, shortly after got domain squatted and ever since hosts dubious content." Closes https://github.com/MusicPlayerDaemon/MPD/issues/1050 --- doc/mpd.conf.5.rst | 3 ++- doc/mpdconf.example | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/mpd.conf.5.rst b/doc/mpd.conf.5.rst index 6b364c513..90d515c16 100644 --- a/doc/mpd.conf.5.rst +++ b/doc/mpd.conf.5.rst @@ -130,7 +130,8 @@ audio_output replaygain If specified, mpd will adjust the volume of songs played using ReplayGain - tags (see http://www.replaygain.org/). Setting this to "album" will + tags (see https://wiki.hydrogenaud.io/index.php?title=Replaygain). + Setting this to "album" will adjust volume using the album's ReplayGain tags, while setting it to "track" will adjust it using the track ReplayGain tags. "auto" uses the track ReplayGain tags if random play is activated otherwise the album ReplayGain diff --git a/doc/mpdconf.example b/doc/mpdconf.example index a097fc652..6f36cf8b5 100644 --- a/doc/mpdconf.example +++ b/doc/mpdconf.example @@ -372,7 +372,8 @@ input { # the argument "off", "album", "track" or "auto". "auto" is a special mode that # chooses between "track" and "album" depending on the current state of # random playback. If random playback is enabled then "track" mode is used. -# See for more details about ReplayGain. +# See for +# more details about ReplayGain. # This setting is off by default. # #replaygain "album" From 2a8c420cff6844266bccc444af061027164f3403 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jan 2021 20:43:14 +0100 Subject: [PATCH 10/28] client/Response: add `printf` attribute --- src/client/Response.hxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client/Response.hxx b/src/client/Response.hxx index 1bf0e7c1e..00c1566fc 100644 --- a/src/client/Response.hxx +++ b/src/client/Response.hxx @@ -75,6 +75,8 @@ public: bool Write(const void *data, size_t length) noexcept; bool Write(const char *data) noexcept; bool FormatV(const char *fmt, std::va_list args) noexcept; + + gcc_printf(2,3) bool Format(const char *fmt, ...) noexcept; static constexpr size_t MAX_BINARY_SIZE = 8192; From 9551166f2795238cda1089dc7de2ff02ec1dcce4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jan 2021 20:41:51 +0100 Subject: [PATCH 11/28] command/file: use %zu to format a size_t `PRIoffset` was wrong, because it expects an `offset_type` (i.e. `uint64_t`). This broke on 32 bit machines where `size_t` has 32 bits. Closes https://github.com/MusicPlayerDaemon/MPD/issues/1058 --- NEWS | 2 ++ src/command/FileCommands.cxx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 05f56ef39..f6172c454 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.22.4 (not yet released) +* protocol + - fix "readpicture" on 32 bit machines * storage - curl: fix several WebDAV protocol bugs * decoder diff --git a/src/command/FileCommands.cxx b/src/command/FileCommands.cxx index e50e3324d..dc68463aa 100644 --- a/src/command/FileCommands.cxx +++ b/src/command/FileCommands.cxx @@ -296,7 +296,7 @@ public: return; } - response.Format("size: %" PRIoffset "\n", buffer.size); + response.Format("size: %zu\n", buffer.size); if (mime_type != nullptr) response.Format("type: %s\n", mime_type); From 07d2bc6898e9af5c343a36113154f497a7f65695 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 30 Nov 2020 22:29:32 +0100 Subject: [PATCH 12/28] util/StringView: add method SplitLast() --- src/util/StringView.hxx | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/util/StringView.hxx b/src/util/StringView.hxx index 990a6f27a..a56cd361e 100644 --- a/src/util/StringView.hxx +++ b/src/util/StringView.hxx @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 Max Kellermann + * Copyright 2013-2020 Max Kellermann * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -35,6 +35,7 @@ #include "Compiler.h" #include +#include #if __cplusplus >= 201703L && !GCC_OLDER_THAN(7,0) #include @@ -126,6 +127,20 @@ struct BasicStringView : ConstBuffer { return {{begin(), separator}, {separator + 1, end()}}; } + /** + * Split the string at the last occurrence of the given + * character. If the character is not found, then the first + * value is the whole string and the second value is nullptr. + */ + gcc_pure + std::pair, BasicStringView> SplitLast(value_type ch) const noexcept { + const auto separator = FindLast(ch); + if (separator == nullptr) + return {*this, nullptr}; + + return {{begin(), separator}, {separator + 1, end()}}; + } + gcc_pure bool StartsWith(BasicStringView needle) const noexcept { return this->size >= needle.size && From fee282f49c6a998cf37ee4050971c20239f98056 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 13:47:13 +0100 Subject: [PATCH 13/28] SongPrint: use LightSong::GetDuration() This properly prints the "Time"/"duration" values for songs in virtual CUE folders. This is loosely related to https://github.com/MusicPlayerDaemon/MPD/issues/1048 --- NEWS | 1 + src/SongPrint.cxx | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index f6172c454..16353791c 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.22.4 (not yet released) * protocol - fix "readpicture" on 32 bit machines + - show duration of songs in virtual playlist (CUE) folders * storage - curl: fix several WebDAV protocol bugs * decoder diff --git a/src/SongPrint.cxx b/src/SongPrint.cxx index 7e8729ad0..5ad300081 100644 --- a/src/SongPrint.cxx +++ b/src/SongPrint.cxx @@ -91,7 +91,14 @@ song_print_info(Response &r, const LightSong &song, bool base) noexcept if (song.audio_format.IsDefined()) r.Format("Format: %s\n", ToString(song.audio_format).c_str()); - tag_print(r, song.tag); + tag_print_values(r, song.tag); + + const auto duration = song.GetDuration(); + if (!duration.IsNegative()) + r.Format("Time: %i\n" + "duration: %1.3f\n", + duration.RoundS(), + duration.ToDoubleS()); } void From 6d08e761c8203ac8810e1f49c8ac4fd188c9d9ac Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 20 Jan 2021 20:55:39 +0100 Subject: [PATCH 14/28] db/simple/ExportedSong: new class --- src/db/plugins/simple/Directory.cxx | 4 +-- src/db/plugins/simple/ExportedSong.hxx | 35 +++++++++++++++++++ .../plugins/simple/SimpleDatabasePlugin.cxx | 10 +++--- .../plugins/simple/SimpleDatabasePlugin.hxx | 4 +-- src/db/plugins/simple/Song.cxx | 5 +-- src/db/plugins/simple/Song.hxx | 4 +-- 6 files changed, 49 insertions(+), 13 deletions(-) create mode 100644 src/db/plugins/simple/ExportedSong.hxx diff --git a/src/db/plugins/simple/Directory.cxx b/src/db/plugins/simple/Directory.cxx index 1b34655bd..1465922eb 100644 --- a/src/db/plugins/simple/Directory.cxx +++ b/src/db/plugins/simple/Directory.cxx @@ -18,11 +18,11 @@ */ #include "Directory.hxx" +#include "ExportedSong.hxx" #include "SongSort.hxx" #include "Song.hxx" #include "Mount.hxx" #include "db/LightDirectory.hxx" -#include "song/LightSong.hxx" #include "db/Uri.hxx" #include "db/DatabaseLock.hxx" #include "db/Interface.hxx" @@ -234,7 +234,7 @@ Directory::Walk(bool recursive, const SongFilter *filter, if (visit_song) { for (auto &song : songs){ - const LightSong song2 = song.Export(); + const auto song2 = song.Export(); if (filter == nullptr || filter->Match(song2)) visit_song(song2); } diff --git a/src/db/plugins/simple/ExportedSong.hxx b/src/db/plugins/simple/ExportedSong.hxx new file mode 100644 index 000000000..6f2f8bd84 --- /dev/null +++ b/src/db/plugins/simple/ExportedSong.hxx @@ -0,0 +1,35 @@ +/* + * Copyright 2003-2021 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef MPD_DB_SIMPLE_EXPORTED_SONG_HXX +#define MPD_DB_SIMPLE_EXPORTED_SONG_HXX + +#include "song/LightSong.hxx" + +/** + * The return type for Song::Export(). In addition to implementing + * #LightSong, it hold allocations necessary to represent the #Song as + * a #LightSong, e.g. a merged #Tag. + */ +class ExportedSong : public LightSong { +public: + using LightSong::LightSong; +}; + +#endif diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.cxx b/src/db/plugins/simple/SimpleDatabasePlugin.cxx index af01d4b45..1e126f3da 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.cxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.cxx @@ -238,20 +238,20 @@ SimpleDatabase::GetSong(std::string_view uri) const throw DatabaseError(DatabaseErrorCode::NOT_FOUND, "No such song"); - light_song.Construct(song->Export()); + exported_song.Construct(song->Export()); #ifndef NDEBUG ++borrowed_song_count; #endif - return &light_song.Get(); + return &exported_song.Get(); } void SimpleDatabase::ReturnSong([[maybe_unused]] const LightSong *song) const noexcept { assert(song != nullptr); - assert(song == prefixed_light_song || song == &light_song.Get()); + assert(song == prefixed_light_song || song == &exported_song.Get()); if (prefixed_light_song != nullptr) { delete prefixed_light_song; @@ -262,7 +262,7 @@ SimpleDatabase::ReturnSong([[maybe_unused]] const LightSong *song) const noexcep --borrowed_song_count; #endif - light_song.Destruct(); + exported_song.Destruct(); } } @@ -316,7 +316,7 @@ SimpleDatabase::Visit(const DatabaseSelection &selection, if (visit_song) { Song *song = r.directory->FindSong(r.rest); if (song != nullptr) { - const LightSong song2 = song->Export(); + const auto song2 = song->Export(); if (selection.Match(song2)) visit_song(song2); diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.hxx b/src/db/plugins/simple/SimpleDatabasePlugin.hxx index a2245faa1..4fe68014e 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.hxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.hxx @@ -20,10 +20,10 @@ #ifndef MPD_SIMPLE_DATABASE_PLUGIN_HXX #define MPD_SIMPLE_DATABASE_PLUGIN_HXX +#include "ExportedSong.hxx" #include "db/Interface.hxx" #include "db/Ptr.hxx" #include "fs/AllocatedPath.hxx" -#include "song/LightSong.hxx" #include "util/Manual.hxx" #include "util/Compiler.h" #include "config.h" @@ -63,7 +63,7 @@ class SimpleDatabase : public Database { /** * A buffer for GetSong(). */ - mutable Manual light_song; + mutable Manual exported_song; #ifndef NDEBUG mutable unsigned borrowed_song_count; diff --git a/src/db/plugins/simple/Song.cxx b/src/db/plugins/simple/Song.cxx index 03ec19f50..519a1a3d7 100644 --- a/src/db/plugins/simple/Song.cxx +++ b/src/db/plugins/simple/Song.cxx @@ -18,6 +18,7 @@ */ #include "Song.hxx" +#include "ExportedSong.hxx" #include "Directory.hxx" #include "tag/Tag.hxx" #include "song/DetachedSong.hxx" @@ -53,10 +54,10 @@ Song::GetURI() const noexcept } } -LightSong +ExportedSong Song::Export() const noexcept { - LightSong dest(filename.c_str(), tag); + ExportedSong dest(filename.c_str(), tag); if (!parent.IsRoot()) dest.directory = parent.GetPath(); if (!target.empty()) diff --git a/src/db/plugins/simple/Song.hxx b/src/db/plugins/simple/Song.hxx index 8750a91b3..eb425dc5c 100644 --- a/src/db/plugins/simple/Song.hxx +++ b/src/db/plugins/simple/Song.hxx @@ -32,8 +32,8 @@ #include struct StringView; -struct LightSong; struct Directory; +class ExportedSong; class DetachedSong; class Storage; class ArchiveFile; @@ -153,7 +153,7 @@ struct Song { std::string GetURI() const noexcept; gcc_pure - LightSong Export() const noexcept; + ExportedSong Export() const noexcept; }; typedef boost::intrusive::list Date: Wed, 20 Jan 2021 21:03:58 +0100 Subject: [PATCH 15/28] db/simple/ExportedSong: add option to own a Tag --- src/db/plugins/simple/ExportedSong.hxx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/db/plugins/simple/ExportedSong.hxx b/src/db/plugins/simple/ExportedSong.hxx index 6f2f8bd84..886be078b 100644 --- a/src/db/plugins/simple/ExportedSong.hxx +++ b/src/db/plugins/simple/ExportedSong.hxx @@ -21,6 +21,7 @@ #define MPD_DB_SIMPLE_EXPORTED_SONG_HXX #include "song/LightSong.hxx" +#include "tag/Tag.hxx" /** * The return type for Song::Export(). In addition to implementing @@ -28,8 +29,14 @@ * a #LightSong, e.g. a merged #Tag. */ class ExportedSong : public LightSong { + Tag tag_buffer; + public: using LightSong::LightSong; + + ExportedSong(const char *_uri, Tag &&_tag) noexcept + :LightSong(_uri, tag_buffer), + tag_buffer(std::move(_tag)) {} }; #endif From 1afa33c3c766af22c35b02ba58e84693243a4f3e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 13:14:27 +0100 Subject: [PATCH 16/28] db/simple/Song: Export() merges tags with "target" Closes https://github.com/MusicPlayerDaemon/MPD/issues/1048 --- NEWS | 2 +- .../plugins/simple/SimpleDatabasePlugin.cxx | 2 +- src/db/plugins/simple/Song.cxx | 83 +++++++++++++++++-- 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 16353791c..d24c90f8c 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,7 @@ ver 0.22.4 (not yet released) * protocol - fix "readpicture" on 32 bit machines - - show duration of songs in virtual playlist (CUE) folders + - show duration and tags of songs in virtual playlist (CUE) folders * storage - curl: fix several WebDAV protocol bugs * decoder diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.cxx b/src/db/plugins/simple/SimpleDatabasePlugin.cxx index 1e126f3da..bb8230723 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.cxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.cxx @@ -233,12 +233,12 @@ SimpleDatabase::GetSong(std::string_view uri) const "No such song"); const Song *song = r.directory->FindSong(r.rest); - protect.unlock(); if (song == nullptr) throw DatabaseError(DatabaseErrorCode::NOT_FOUND, "No such song"); exported_song.Construct(song->Export()); + protect.unlock(); #ifndef NDEBUG ++borrowed_song_count; diff --git a/src/db/plugins/simple/Song.cxx b/src/db/plugins/simple/Song.cxx index 519a1a3d7..8db47d03f 100644 --- a/src/db/plugins/simple/Song.cxx +++ b/src/db/plugins/simple/Song.cxx @@ -21,9 +21,12 @@ #include "ExportedSong.hxx" #include "Directory.hxx" #include "tag/Tag.hxx" +#include "tag/Builder.hxx" #include "song/DetachedSong.hxx" #include "song/LightSong.hxx" #include "fs/Traits.hxx" +#include "time/ChronoUtil.hxx" +#include "util/IterableSplitString.hxx" Song::Song(DetachedSong &&other, Directory &_parent) noexcept :tag(std::move(other.WritableTag())), @@ -54,17 +57,87 @@ Song::GetURI() const noexcept } } +/** + * Path name traversal of a #Directory. + */ +gcc_pure +static const Directory * +FindTargetDirectory(const Directory &base, StringView path) noexcept +{ + const auto *directory = &base; + for (const StringView name : IterableSplitString(path, '/')) { + if (name.empty() || name.Equals(".")) + continue; + + directory = name.Equals("..") + ? directory->parent + : directory->FindChild(name); + if (directory == nullptr) + break; + } + + return directory; +} + +/** + * Path name traversal of a #Song. + */ +gcc_pure +static const Song * +FindTargetSong(const Directory &_directory, StringView target) noexcept +{ + auto [path, last] = target.SplitLast('/'); + if (last == nullptr) { + last = path; + path = nullptr; + } + + if (last.empty()) + return nullptr; + + const auto *directory = FindTargetDirectory(_directory, path); + if (directory == nullptr) + return nullptr; + + return directory->FindSong(last); +} + ExportedSong Song::Export() const noexcept { - ExportedSong dest(filename.c_str(), tag); + const auto *target_song = !target.empty() + ? FindTargetSong(parent, (std::string_view)target) + : nullptr; + + Tag merged_tag; + if (target_song != nullptr) { + /* if we found the target song (which may be the + underlying song file of a CUE file), merge the tags + from that song with this song's tags (from the CUE + file) */ + TagBuilder builder(tag); + builder.Complement(target_song->tag); + merged_tag = builder.Commit(); + } + + ExportedSong dest = merged_tag.IsDefined() + ? ExportedSong(filename.c_str(), std::move(merged_tag)) + : ExportedSong(filename.c_str(), tag); if (!parent.IsRoot()) dest.directory = parent.GetPath(); if (!target.empty()) dest.real_uri = target.c_str(); - dest.mtime = mtime; - dest.start_time = start_time; - dest.end_time = end_time; - dest.audio_format = audio_format; + dest.mtime = IsNegative(mtime) && target_song != nullptr + ? target_song->mtime + : mtime; + dest.start_time = start_time.IsZero() && target_song != nullptr + ? target_song->start_time + : start_time; + dest.end_time = end_time.IsZero() && target_song != nullptr + ? target_song->end_time + : end_time; + dest.audio_format = audio_format.IsDefined() || target_song == nullptr + ? audio_format + : target_song->audio_format; return dest; } From 168d6257b41fe88fd1595cd91396b7b0f17a6766 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 14:13:59 +0100 Subject: [PATCH 17/28] python/build/libs.py: build CURL with OpenSSL support Closes https://github.com/MusicPlayerDaemon/MPD/issues/1059 --- NEWS | 2 ++ android/build.py | 1 + python/build/libs.py | 9 ++++++- python/build/openssl.py | 55 +++++++++++++++++++++++++++++++++++++++++ python/build/project.py | 2 +- 5 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 python/build/openssl.py diff --git a/NEWS b/NEWS index d24c90f8c..10c020fa8 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,8 @@ ver 0.22.4 (not yet released) - ffmpeg: detect the output sample format * output - moveoutput: fix always_on and tag lost on move +* Android + - enable https:// support (via OpenSSL) ver 0.22.3 (2020/11/06) * playlist diff --git a/android/build.py b/android/build.py index 982160783..eb6600111 100755 --- a/android/build.py +++ b/android/build.py @@ -172,6 +172,7 @@ thirdparty_libs = [ wildmidi, gme, ffmpeg, + openssl, curl, libexpat, libnfs, diff --git a/python/build/libs.py b/python/build/libs.py index c17237c4b..517e892b3 100644 --- a/python/build/libs.py +++ b/python/build/libs.py @@ -7,6 +7,7 @@ from build.meson import MesonProject from build.cmake import CmakeProject from build.autotools import AutotoolsProject from build.ffmpeg import FfmpegProject +from build.openssl import OpenSSLProject from build.boost import BoostProject libmpdclient = MesonProject( @@ -376,6 +377,12 @@ ffmpeg = FfmpegProject( ], ) +openssl = OpenSSLProject( + 'https://www.openssl.org/source/openssl-3.0.0-alpha10.tar.gz', + 'b1699acf2148db31f12edf5ebfdf12a92bfd3f0e60538d169710408a3cd3b138', + 'include/openssl/ossl_typ.h', +) + curl = AutotoolsProject( 'http://curl.haxx.se/download/curl-7.74.0.tar.xz', '999d5f2c403cf6e25d58319fdd596611e455dd195208746bc6e6d197a77e878b', @@ -399,7 +406,7 @@ curl = AutotoolsProject( '--disable-netrc', '--disable-progress-meter', '--disable-alt-svc', - '--without-ssl', '--without-gnutls', '--without-nss', '--without-libssh2', + '--without-gnutls', '--without-nss', '--without-libssh2', ], patches='src/lib/curl/patches', diff --git a/python/build/openssl.py b/python/build/openssl.py new file mode 100644 index 000000000..a7350e6ac --- /dev/null +++ b/python/build/openssl.py @@ -0,0 +1,55 @@ +import subprocess + +from build.makeproject import MakeProject + +class OpenSSLProject(MakeProject): + def __init__(self, url, md5, installed, + **kwargs): + MakeProject.__init__(self, url, md5, installed, install_target='install_dev', **kwargs) + + def get_make_args(self, toolchain): + return MakeProject.get_make_args(self, toolchain) + [ + 'CC=' + toolchain.cc, + 'CFLAGS=' + toolchain.cflags, + 'CPPFLAGS=' + toolchain.cppflags, + 'AR=' + toolchain.ar, + 'RANLIB=' + toolchain.ranlib, + 'build_libs', + ] + + def build(self, toolchain): + src = self.unpack(toolchain, out_of_tree=False) + + # OpenSSL has a weird target architecture scheme with lots of + # hard-coded architectures; this table translates between our + # "toolchain_arch" (HOST_TRIPLET) and the OpenSSL target + openssl_archs = { + # not using "android-*" because those OpenSSL targets want + # to know where the SDK is, but our own build scripts + # prepared everything already to look like a regular Linux + # build + 'arm-linux-androideabi': 'linux-generic32', + 'aarch64-linux-android': 'linux-aarch64', + 'i686-linux-android': 'linux-x86-clang', + 'x86_64-linux-android': 'linux-x86_64-clang', + + # Kobo + 'arm-linux-gnueabihf': 'linux-generic32', + + # Windows + 'i686-w64-mingw32': 'mingw', + 'x86_64-w64-mingw32': 'mingw64', + } + + openssl_arch = openssl_archs[toolchain.arch] + + subprocess.check_call(['./Configure', + 'no-shared', + 'no-module', 'no-engine', 'no-static-engine', + 'no-async', + 'no-tests', + 'no-asm', # "asm" causes build failures on Windows + openssl_arch, + '--prefix=' + toolchain.install_prefix], + cwd=src, env=toolchain.env) + MakeProject.build(self, toolchain, src) diff --git a/python/build/project.py b/python/build/project.py index b78c89238..a0cfa60ba 100644 --- a/python/build/project.py +++ b/python/build/project.py @@ -20,7 +20,7 @@ class Project: self.base = base if name is None or version is None: - m = re.match(r'^([-\w]+)-(\d[\d.]*[a-z]?[\d.]*)$', self.base) + m = re.match(r'^([-\w]+)-(\d[\d.]*[a-z]?[\d.]*(?:-alpha\d+)?)$', self.base) if name is None: name = m.group(1) if version is None: version = m.group(2) From 74396448dfd7d975b77e6bff36680757ee56ddb2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 14:53:38 +0100 Subject: [PATCH 18/28] input/curl: disable verify_peer on Android by default I havn't yet figured out how to use Android's system CA certificates with CURL/OpenSSL, so a temporary workaround is to disable verify_peer by default. The data MPD transfers isn't extremely important, so the servers's authenticity isn't extremely important either. --- src/input/plugins/CurlInputPlugin.cxx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/input/plugins/CurlInputPlugin.cxx b/src/input/plugins/CurlInputPlugin.cxx index 151b9ab96..fb6e7a892 100644 --- a/src/input/plugins/CurlInputPlugin.cxx +++ b/src/input/plugins/CurlInputPlugin.cxx @@ -369,8 +369,15 @@ input_curl_init(EventLoop &event_loop, const ConfigBlock &block) proxy_user = block.GetBlockValue("proxy_user"); proxy_password = block.GetBlockValue("proxy_password"); - verify_peer = block.GetBlockValue("verify_peer", true); - verify_host = block.GetBlockValue("verify_host", true); +#ifdef ANDROID + // TODO: figure out how to use Android's CA certificates and re-enable verify + constexpr bool default_verify = false; +#else + constexpr bool default_verify = true; +#endif + + verify_peer = block.GetBlockValue("verify_peer", default_verify); + verify_host = block.GetBlockValue("verify_host", default_verify); } static void From a5d382348e5e993baf1877643ea6b4f0b09d5345 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:33:10 +0100 Subject: [PATCH 19/28] command/Request: ParseUnsigned() returns unsigned Of course, it should do that! --- src/command/Request.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/command/Request.hxx b/src/command/Request.hxx index f6f0072a0..7a459bb0f 100644 --- a/src/command/Request.hxx +++ b/src/command/Request.hxx @@ -54,12 +54,12 @@ public: return ParseCommandArgInt(data[idx], min_value, max_value); } - int ParseUnsigned(unsigned idx) const { + unsigned ParseUnsigned(unsigned idx) const { assert(idx < size); return ParseCommandArgUnsigned(data[idx]); } - int ParseUnsigned(unsigned idx, unsigned max_value) const { + unsigned ParseUnsigned(unsigned idx, unsigned max_value) const { assert(idx < size); return ParseCommandArgUnsigned(data[idx], max_value); } From dafba203e7c3ac7e13f82648869b78566d8151ab Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:37:50 +0100 Subject: [PATCH 20/28] util/ForeignFifoBuffer: use `auto` --- src/util/ForeignFifoBuffer.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/ForeignFifoBuffer.hxx b/src/util/ForeignFifoBuffer.hxx index d6912d3c2..65f6bce94 100644 --- a/src/util/ForeignFifoBuffer.hxx +++ b/src/util/ForeignFifoBuffer.hxx @@ -235,7 +235,7 @@ public: w = Write(); } - size_t n = std::min(r.size, w.size); + const auto n = std::min(r.size, w.size); std::move(r.data, r.data + n, w.data); Append(n); From 6b555b70178ce3048875a52f8fb3ac02858e8c90 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:37:14 +0100 Subject: [PATCH 21/28] util/PeakBuffer: use std::size_t --- src/util/PeakBuffer.cxx | 14 +++++++------- src/util/PeakBuffer.hxx | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/util/PeakBuffer.cxx b/src/util/PeakBuffer.cxx index dcc6f92aa..1b93d1459 100644 --- a/src/util/PeakBuffer.cxx +++ b/src/util/PeakBuffer.cxx @@ -57,7 +57,7 @@ PeakBuffer::Read() const noexcept } void -PeakBuffer::Consume(size_t length) noexcept +PeakBuffer::Consume(std::size_t length) noexcept { if (normal_buffer != nullptr && !normal_buffer->empty()) { normal_buffer->Consume(length); @@ -75,21 +75,21 @@ PeakBuffer::Consume(size_t length) noexcept } } -static size_t +static std::size_t AppendTo(DynamicFifoBuffer &buffer, const void *data, size_t length) noexcept { assert(data != nullptr); assert(length > 0); - size_t total = 0; + std::size_t total = 0; do { const auto p = buffer.Write(); if (p.empty()) break; - const size_t nbytes = std::min(length, p.size); + const std::size_t nbytes = std::min(length, p.size); memcpy(p.data, data, nbytes); buffer.Append(nbytes); @@ -102,20 +102,20 @@ AppendTo(DynamicFifoBuffer &buffer, } bool -PeakBuffer::Append(const void *data, size_t length) +PeakBuffer::Append(const void *data, std::size_t length) { if (length == 0) return true; if (peak_buffer != nullptr && !peak_buffer->empty()) { - size_t nbytes = AppendTo(*peak_buffer, data, length); + std::size_t nbytes = AppendTo(*peak_buffer, data, length); return nbytes == length; } if (normal_buffer == nullptr) normal_buffer = new DynamicFifoBuffer(normal_size); - size_t nbytes = AppendTo(*normal_buffer, data, length); + std::size_t nbytes = AppendTo(*normal_buffer, data, length); if (nbytes > 0) { data = (const uint8_t *)data + nbytes; length -= nbytes; diff --git a/src/util/PeakBuffer.hxx b/src/util/PeakBuffer.hxx index 0684937b4..d40b31f4e 100644 --- a/src/util/PeakBuffer.hxx +++ b/src/util/PeakBuffer.hxx @@ -34,12 +34,12 @@ template class DynamicFifoBuffer; * kernel when it has been consumed. */ class PeakBuffer { - size_t normal_size, peak_size; + std::size_t normal_size, peak_size; DynamicFifoBuffer *normal_buffer, *peak_buffer; public: - PeakBuffer(size_t _normal_size, size_t _peak_size) + PeakBuffer(std::size_t _normal_size, std::size_t _peak_size) :normal_size(_normal_size), peak_size(_peak_size), normal_buffer(nullptr), peak_buffer(nullptr) {} @@ -62,9 +62,9 @@ public: gcc_pure WritableBuffer Read() const noexcept; - void Consume(size_t length) noexcept; + void Consume(std::size_t length) noexcept; - bool Append(const void *data, size_t length); + bool Append(const void *data, std::size_t length); }; #endif From fa82f558be629ade4f63733bb919972f76d28039 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:41:57 +0100 Subject: [PATCH 22/28] util/PeakBuffer: add `noexcept` --- src/util/PeakBuffer.cxx | 2 +- src/util/PeakBuffer.hxx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/PeakBuffer.cxx b/src/util/PeakBuffer.cxx index 1b93d1459..31e0292c1 100644 --- a/src/util/PeakBuffer.cxx +++ b/src/util/PeakBuffer.cxx @@ -25,7 +25,7 @@ #include -PeakBuffer::~PeakBuffer() +PeakBuffer::~PeakBuffer() noexcept { delete normal_buffer; delete peak_buffer; diff --git a/src/util/PeakBuffer.hxx b/src/util/PeakBuffer.hxx index d40b31f4e..dc7e79b1e 100644 --- a/src/util/PeakBuffer.hxx +++ b/src/util/PeakBuffer.hxx @@ -39,11 +39,11 @@ class PeakBuffer { DynamicFifoBuffer *normal_buffer, *peak_buffer; public: - PeakBuffer(std::size_t _normal_size, std::size_t _peak_size) + PeakBuffer(std::size_t _normal_size, std::size_t _peak_size) noexcept :normal_size(_normal_size), peak_size(_peak_size), normal_buffer(nullptr), peak_buffer(nullptr) {} - PeakBuffer(PeakBuffer &&other) + PeakBuffer(PeakBuffer &&other) noexcept :normal_size(other.normal_size), peak_size(other.peak_size), normal_buffer(other.normal_buffer), peak_buffer(other.peak_buffer) { @@ -51,7 +51,7 @@ public: other.peak_buffer = nullptr; } - ~PeakBuffer(); + ~PeakBuffer() noexcept; PeakBuffer(const PeakBuffer &) = delete; PeakBuffer &operator=(const PeakBuffer &) = delete; From eea0e084afec4049ba94c2745e3d2f9ec959f814 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:43:20 +0100 Subject: [PATCH 23/28] util/PeakBuffer: use std::byte instead of std::uint8_t --- src/util/PeakBuffer.cxx | 10 +++++----- src/util/PeakBuffer.hxx | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/util/PeakBuffer.cxx b/src/util/PeakBuffer.cxx index 31e0292c1..6af9df266 100644 --- a/src/util/PeakBuffer.cxx +++ b/src/util/PeakBuffer.cxx @@ -76,7 +76,7 @@ PeakBuffer::Consume(std::size_t length) noexcept } static std::size_t -AppendTo(DynamicFifoBuffer &buffer, +AppendTo(DynamicFifoBuffer &buffer, const void *data, size_t length) noexcept { assert(data != nullptr); @@ -93,7 +93,7 @@ AppendTo(DynamicFifoBuffer &buffer, memcpy(p.data, data, nbytes); buffer.Append(nbytes); - data = (const uint8_t *)data + nbytes; + data = (const std::byte *)data + nbytes; length -= nbytes; total += nbytes; } while (length > 0); @@ -113,11 +113,11 @@ PeakBuffer::Append(const void *data, std::size_t length) } if (normal_buffer == nullptr) - normal_buffer = new DynamicFifoBuffer(normal_size); + normal_buffer = new DynamicFifoBuffer(normal_size); std::size_t nbytes = AppendTo(*normal_buffer, data, length); if (nbytes > 0) { - data = (const uint8_t *)data + nbytes; + data = (const std::byte *)data + nbytes; length -= nbytes; if (length == 0) return true; @@ -125,7 +125,7 @@ PeakBuffer::Append(const void *data, std::size_t length) if (peak_buffer == nullptr) { if (peak_size > 0) - peak_buffer = new DynamicFifoBuffer(peak_size); + peak_buffer = new DynamicFifoBuffer(peak_size); if (peak_buffer == nullptr) return false; } diff --git a/src/util/PeakBuffer.hxx b/src/util/PeakBuffer.hxx index dc7e79b1e..c3d871035 100644 --- a/src/util/PeakBuffer.hxx +++ b/src/util/PeakBuffer.hxx @@ -23,7 +23,6 @@ #include "Compiler.h" #include -#include template struct WritableBuffer; template class DynamicFifoBuffer; @@ -36,7 +35,7 @@ template class DynamicFifoBuffer; class PeakBuffer { std::size_t normal_size, peak_size; - DynamicFifoBuffer *normal_buffer, *peak_buffer; + DynamicFifoBuffer *normal_buffer, *peak_buffer; public: PeakBuffer(std::size_t _normal_size, std::size_t _peak_size) noexcept From 056ab199abb302297529812834e4f36a1d91149c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:44:11 +0100 Subject: [PATCH 24/28] util/PeakBuffer: add method max_size() --- src/util/PeakBuffer.hxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/PeakBuffer.hxx b/src/util/PeakBuffer.hxx index c3d871035..febf8bd73 100644 --- a/src/util/PeakBuffer.hxx +++ b/src/util/PeakBuffer.hxx @@ -55,6 +55,10 @@ public: PeakBuffer(const PeakBuffer &) = delete; PeakBuffer &operator=(const PeakBuffer &) = delete; + std::size_t max_size() const noexcept { + return normal_size + peak_size; + } + gcc_pure bool empty() const noexcept; From 3b3c1d466d0d3de16697269b72c289d55935402b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:49:37 +0100 Subject: [PATCH 25/28] event/FullyBufferedSocket: add method GetOutputMaxSize() --- src/event/FullyBufferedSocket.hxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/event/FullyBufferedSocket.hxx b/src/event/FullyBufferedSocket.hxx index 1d6ee6046..995deb47e 100644 --- a/src/event/FullyBufferedSocket.hxx +++ b/src/event/FullyBufferedSocket.hxx @@ -45,6 +45,10 @@ public: BufferedSocket::Close(); } + std::size_t GetOutputMaxSize() const noexcept { + return output.max_size(); + } + private: /** * @return the number of bytes written to the socket, 0 if the From 6e33566ceeb7e6c4454e44efd203b2e2fcd10feb Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:55:47 +0100 Subject: [PATCH 26/28] client/FileCommands: validate the given offset --- src/command/FileCommands.cxx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/command/FileCommands.cxx b/src/command/FileCommands.cxx index dc68463aa..4ac239646 100644 --- a/src/command/FileCommands.cxx +++ b/src/command/FileCommands.cxx @@ -205,6 +205,17 @@ read_stream_art(Response &r, const char *uri, size_t offset) const offset_type art_file_size = is->GetSize(); + if (offset >= art_file_size) { + if (offset > art_file_size) { + r.Error(ACK_ERROR_ARG, "Offset too large"); + return CommandResult::ERROR; + } else { + r.Format("size: %" PRIoffset "\n", art_file_size); + r.WriteBinary(nullptr); + return CommandResult::OK; + } + } + uint8_t buffer[Response::MAX_BINARY_SIZE]; size_t read_size; From 995aafe9cc511430bff7a7a690df70998f4bb025 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:27:52 +0100 Subject: [PATCH 27/28] protocol: add command "binarylimit" Increasing the protocol version to 0.22.4 to allow clients to detect this feature. Closes https://github.com/MusicPlayerDaemon/MPD/issues/1038 --- NEWS | 1 + doc/protocol.rst | 25 ++++++++++++++++++++----- meson.build | 2 +- src/client/Client.hxx | 7 +++++++ src/client/Response.cxx | 2 +- src/client/Response.hxx | 2 -- src/command/AllCommands.cxx | 1 + src/command/ClientCommands.cxx | 15 +++++++++++++++ src/command/ClientCommands.hxx | 3 +++ src/command/FileCommands.cxx | 33 +++++++++++++++++---------------- 10 files changed, 66 insertions(+), 25 deletions(-) diff --git a/NEWS b/NEWS index 10c020fa8..99a4343f7 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ ver 0.22.4 (not yet released) * protocol + - add command "binarylimit" to allow larger chunk sizes - fix "readpicture" on 32 bit machines - show duration and tags of songs in virtual playlist (CUE) folders * storage diff --git a/doc/protocol.rst b/doc/protocol.rst index 8d2eea243..0c4c9ac6f 100644 --- a/doc/protocol.rst +++ b/doc/protocol.rst @@ -69,11 +69,14 @@ that, the specified number of bytes of binary data follows, then a newline, and finally the ``OK`` line. If the object to be transmitted is large, the server may choose a -reasonable chunk size and transmit only a portion. Usually, the -response also contains a ``size`` line which specifies the total -(uncropped) size, and the command usually has a way to specify an -offset into the object; this way, the client can copy the whole file -without blocking the connection for too long. +reasonable chunk size and transmit only a portion. The maximum chunk +size can be changed by clients with the :ref:`binarylimit +` command. + +Usually, the response also contains a ``size`` line which specifies +the total (uncropped) size, and the command usually has a way to +specify an offset into the object; this way, the client can copy the +whole file without blocking the connection for too long. Example:: @@ -1355,6 +1358,17 @@ Connection settings :command:`ping` Does nothing but return "OK". +.. _command_binarylimit: + +:command:`binarylimit SIZE` [#since_0_22_4]_ + + Set the maximum :ref:`binary response ` size for the + current connection to the specified number of bytes. + + A bigger value means less overhead for transmitting large + entities, but it also means that the connection is blocked for a + longer time. + .. _command_tagtypes: :command:`tagtypes` @@ -1583,3 +1597,4 @@ client-to-client messages are local to the current partition. .. [#since_0_19] Since :program:`MPD` 0.19 .. [#since_0_20] Since :program:`MPD` 0.20 .. [#since_0_21] Since :program:`MPD` 0.21 +.. [#since_0_22_4] Since :program:`MPD` 0.22.4 diff --git a/meson.build b/meson.build index eead7888f..313bc5d77 100644 --- a/meson.build +++ b/meson.build @@ -32,7 +32,7 @@ version_conf = configuration_data() version_conf.set_quoted('PACKAGE', meson.project_name()) version_conf.set_quoted('PACKAGE_NAME', meson.project_name()) version_conf.set_quoted('VERSION', meson.project_version()) -version_conf.set_quoted('PROTOCOL_VERSION', '0.22.0') +version_conf.set_quoted('PROTOCOL_VERSION', '0.22.4') configure_file(output: 'Version.h', configuration: version_conf) conf = configuration_data() diff --git a/src/client/Client.hxx b/src/client/Client.hxx index d19d08760..deb4b8caa 100644 --- a/src/client/Client.hxx +++ b/src/client/Client.hxx @@ -84,6 +84,12 @@ public: */ TagMask tag_mask = TagMask::All(); + /** + * The maximum number of bytes transmitted in a binary + * response. Can be changed with the "binarylimit" command. + */ + size_t binary_limit = 8192; + private: static constexpr size_t MAX_SUBSCRIPTIONS = 16; @@ -122,6 +128,7 @@ public: ~Client() noexcept; using FullyBufferedSocket::GetEventLoop; + using FullyBufferedSocket::GetOutputMaxSize; gcc_pure bool IsExpired() const noexcept { diff --git a/src/client/Response.cxx b/src/client/Response.cxx index 3900df75e..0c9145a50 100644 --- a/src/client/Response.cxx +++ b/src/client/Response.cxx @@ -59,7 +59,7 @@ Response::Format(const char *fmt, ...) noexcept bool Response::WriteBinary(ConstBuffer payload) noexcept { - assert(payload.size <= MAX_BINARY_SIZE); + assert(payload.size <= client.binary_limit); return Format("binary: %zu\n", payload.size) && Write(payload.data, payload.size) && diff --git a/src/client/Response.hxx b/src/client/Response.hxx index 00c1566fc..4c7e40e8a 100644 --- a/src/client/Response.hxx +++ b/src/client/Response.hxx @@ -79,8 +79,6 @@ public: gcc_printf(2,3) bool Format(const char *fmt, ...) noexcept; - static constexpr size_t MAX_BINARY_SIZE = 8192; - /** * Write a binary chunk; this writes the "binary" line, the * given chunk and the trailing newline. diff --git a/src/command/AllCommands.cxx b/src/command/AllCommands.cxx index 96ae21e42..4508fa92f 100644 --- a/src/command/AllCommands.cxx +++ b/src/command/AllCommands.cxx @@ -87,6 +87,7 @@ static constexpr struct command commands[] = { { "addid", PERMISSION_ADD, 1, 2, handle_addid }, { "addtagid", PERMISSION_ADD, 3, 3, handle_addtagid }, { "albumart", PERMISSION_READ, 2, 2, handle_album_art }, + { "binarylimit", PERMISSION_NONE, 1, 1, handle_binary_limit }, { "channels", PERMISSION_READ, 0, 0, handle_channels }, { "clear", PERMISSION_CONTROL, 0, 0, handle_clear }, { "clearerror", PERMISSION_CONTROL, 0, 0, handle_clearerror }, diff --git a/src/command/ClientCommands.cxx b/src/command/ClientCommands.cxx index f0e3470be..01bd462f2 100644 --- a/src/command/ClientCommands.cxx +++ b/src/command/ClientCommands.cxx @@ -40,6 +40,21 @@ handle_ping([[maybe_unused]] Client &client, [[maybe_unused]] Request args, return CommandResult::OK; } +CommandResult +handle_binary_limit(Client &client, Request args, + [[maybe_unused]] Response &r) +{ + size_t value = args.ParseUnsigned(0, client.GetOutputMaxSize() - 4096); + if (value < 64) { + r.Error(ACK_ERROR_ARG, "Value too small"); + return CommandResult::ERROR; + } + + client.binary_limit = value; + + return CommandResult::OK; +} + CommandResult handle_password(Client &client, Request args, Response &r) { diff --git a/src/command/ClientCommands.hxx b/src/command/ClientCommands.hxx index fc0dc42e1..de766e802 100644 --- a/src/command/ClientCommands.hxx +++ b/src/command/ClientCommands.hxx @@ -32,6 +32,9 @@ handle_close(Client &client, Request request, Response &response); CommandResult handle_ping(Client &client, Request request, Response &response); +CommandResult +handle_binary_limit(Client &client, Request request, Response &response); + CommandResult handle_password(Client &client, Request request, Response &response); diff --git a/src/command/FileCommands.cxx b/src/command/FileCommands.cxx index 4ac239646..7efbb211c 100644 --- a/src/command/FileCommands.cxx +++ b/src/command/FileCommands.cxx @@ -43,6 +43,7 @@ #include "thread/Mutex.hxx" #include "Log.hxx" +#include #include #include /* for PRIu64 */ @@ -205,28 +206,26 @@ read_stream_art(Response &r, const char *uri, size_t offset) const offset_type art_file_size = is->GetSize(); - if (offset >= art_file_size) { - if (offset > art_file_size) { - r.Error(ACK_ERROR_ARG, "Offset too large"); - return CommandResult::ERROR; - } else { - r.Format("size: %" PRIoffset "\n", art_file_size); - r.WriteBinary(nullptr); - return CommandResult::OK; - } + if (offset > art_file_size) { + r.Error(ACK_ERROR_ARG, "Offset too large"); + return CommandResult::ERROR; } - uint8_t buffer[Response::MAX_BINARY_SIZE]; - size_t read_size; + std::size_t buffer_size = + std::min(art_file_size - offset, + r.GetClient().binary_limit); - { + std::unique_ptr buffer(new std::byte[buffer_size]); + + std::size_t read_size = 0; + if (buffer_size > 0) { std::unique_lock lock(mutex); is->Seek(lock, offset); - read_size = is->Read(lock, &buffer, sizeof(buffer)); + read_size = is->Read(lock, buffer.get(), buffer_size); } r.Format("size: %" PRIoffset "\n", art_file_size); - r.WriteBinary({buffer, read_size}); + r.WriteBinary({buffer.get(), read_size}); return CommandResult::OK; } @@ -313,8 +312,10 @@ public: response.Format("type: %s\n", mime_type); buffer.size -= offset; - if (buffer.size > Response::MAX_BINARY_SIZE) - buffer.size = Response::MAX_BINARY_SIZE; + + const std::size_t binary_limit = response.GetClient().binary_limit; + if (buffer.size > binary_limit) + buffer.size = binary_limit; buffer.data = OffsetPointer(buffer.data, offset); response.WriteBinary(buffer); From a0d76c3be93738f2e21aeada3612a73bc05ca816 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 17:21:20 +0100 Subject: [PATCH 28/28] release v0.22.4 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 99a4343f7..14b0c4398 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.22.4 (not yet released) +ver 0.22.4 (2021/01/21) * protocol - add command "binarylimit" to allow larger chunk sizes - fix "readpicture" on 32 bit machines