From 995aafe9cc511430bff7a7a690df70998f4bb025 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 21 Jan 2021 16:27:52 +0100 Subject: [PATCH] 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);