Merge branch 'v0.22.x'

NEWS

@@ -15,6 +15,7 @@ ver 0.23 (not yet released)
 ver 0.22.7 (not yet released)
 * protocol
   - don't use glibc extension to parse time stamps
+  - optimize the "albumart" command
 * input
   - curl: send user/password in the first request, save one roundtrip
 * decoder

src/client/Client.hxx

@@ -23,6 +23,7 @@
 #include "Message.hxx"
 #include "command/CommandResult.hxx"
 #include "command/CommandListBuilder.hxx"
+#include "input/LastInputStream.hxx"
 #include "tag/Mask.hxx"
 #include "event/FullyBufferedSocket.hxx"
 #include "event/CoarseTimerEvent.hxx"
@@ -90,6 +91,13 @@ public:
 	 */
 	size_t binary_limit = 8192;
 
+	/**
+	 * This caches the last "albumart" InputStream instance, to
+	 * avoid repeating the search for each chunk requested by this
+	 * client.
+	 */
+	LastInputStream last_album_art;
+
 private:
 	static constexpr size_t MAX_SUBSCRIPTIONS = 16;
 

src/client/New.cxx

@@ -44,7 +44,8 @@ Client::Client(EventLoop &_loop, Partition &_partition,
 	 partition(&_partition),
 	 permission(_permission),
 	 uid(_uid),
-	 num(_num)
+	 num(_num),
+	 last_album_art(_loop)
 {
 	timeout_event.Schedule(client_timeout);
 }

src/command/FileCommands.cxx

@@ -188,9 +188,16 @@ read_stream_art(Response &r, const char *uri, size_t offset)
 {
 	const auto art_directory = PathTraitsUTF8::GetParent(uri);
 
-	Mutex mutex;
-
-	InputStreamPtr is = find_stream_art(art_directory, mutex);
+	// TODO: eliminate this const_cast
+	auto &client = const_cast<Client &>(r.GetClient());
+
+	/* to avoid repeating the search for each chunk request by the
+	   same client, use the #LastInputStream class to cache the
+	   #InputStream instance */
+	auto *is = client.last_album_art.Open(art_directory, [](std::string_view directory,
+								Mutex &mutex){
+		return find_stream_art(directory, mutex);
+	});
 
 	if (is == nullptr) {
 		r.Error(ACK_ERROR_NO_EXIST, "No file exists");
@@ -216,7 +223,7 @@ read_stream_art(Response &r, const char *uri, size_t offset)
 
 	std::size_t read_size = 0;
 	if (buffer_size > 0) {
-		std::unique_lock<Mutex> lock(mutex);
+		std::unique_lock<Mutex> lock(is->mutex);
 		is->Seek(lock, offset);
 		read_size = is->Read(lock, buffer.get(), buffer_size);
 	}

src/command/PlaylistCommands.cxx

@@ -92,7 +92,7 @@ handle_load(Client &client, Request args, [[maybe_unused]] Response &r)
 	auto &instance = client.GetInstance();
 	const unsigned new_size = playlist.GetLength();
 	for (unsigned i = old_size; i < new_size; ++i)
-		instance.LookupRemoteTag(playlist.queue.Get(i).GetURI());
+		instance.LookupRemoteTag(playlist.queue.Get(i).GetRealURI());
 
 	return CommandResult::OK;
 }

src/input/LastInputStream.cxx

+/*
+ * 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.
+ */
+
+#include "LastInputStream.hxx"
+#include "InputStream.hxx"
+
+#include <cassert>
+
+LastInputStream::LastInputStream(EventLoop &event_loop) noexcept
+	:close_timer(event_loop, BIND_THIS_METHOD(OnCloseTimer))
+{
+}
+
+LastInputStream::~LastInputStream() noexcept = default;
+
+void
+LastInputStream::Close() noexcept
+{
+	is.reset();
+	close_timer.Cancel();
+}
+
+void
+LastInputStream::OnCloseTimer() noexcept
+{
+	assert(is);
+
+	is.reset();
+}

src/input/LastInputStream.hxx

+/*
+ * 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_LAST_INPUT_STREAM_HXX
+#define MPD_LAST_INPUT_STREAM_HXX
+
+#include "Ptr.hxx"
+#include "thread/Mutex.hxx"
+#include "event/TimerEvent.hxx"
+
+#include <string>
+
+/**
+ * A helper class which maintains an #InputStream that is opened once
+ * and may be reused later for some time.  It will be closed
+ * automatically after some time.
+ *
+ * This class is not thread-safe.  All methods must be called on the
+ * thread which runs the #EventLoop.
+ */
+class LastInputStream {
+	std::string uri;
+
+	Mutex mutex;
+
+	InputStreamPtr is;
+
+	TimerEvent close_timer;
+
+public:
+	explicit LastInputStream(EventLoop &event_loop) noexcept;
+	~LastInputStream() noexcept;
+
+	/**
+	 * Open an #InputStream instance with the given opener
+	 * function, but returns the cached instance if it matches.
+	 *
+	 * This object keeps owning the #InputStream; the caller shall
+	 * not close it.
+	 */
+	template<typename U, typename O>
+	InputStream *Open(U &&new_uri, O &&open) {
+		if (new_uri == uri) {
+			if (is)
+				/* refresh the timeout */
+				ScheduleClose();
+
+			return is.get();
+		}
+
+		is.reset();
+		close_timer.Cancel();
+
+		is = open(new_uri, mutex);
+		uri = std::forward<U>(new_uri);
+		ScheduleClose();
+		return is.get();
+	}
+
+	void Close() noexcept;
+
+private:
+	void ScheduleClose() noexcept {
+		close_timer.Schedule(std::chrono::seconds(20));
+	}
+
+	void OnCloseTimer() noexcept;
+};
+
+#endif

src/input/meson.build

@@ -8,6 +8,7 @@ input_api = static_library(
   'ThreadInputStream.cxx',
   'AsyncInputStream.cxx',
   'ProxyInputStream.cxx',
+  'LastInputStream.cxx',
   include_directories: inc,
   dependencies: [
     boost_dep,

src/queue/Playlist.cxx

@@ -44,13 +44,13 @@ playlist::TagModified(DetachedSong &&song) noexcept
 }
 
 void
-playlist::TagModified(const char *uri, const Tag &tag) noexcept
+playlist::TagModified(const char *real_uri, const Tag &tag) noexcept
 {
 	bool modified = false;
 
 	for (unsigned i = 0; i < queue.length; ++i) {
 		auto &song = *queue.items[i].song;
-		if (song.IsURI(uri)) {
+		if (song.IsRealURI(real_uri)) {
 			song.SetTag(tag);
 			queue.ModifyAtPosition(i);
 			modified = true;

src/queue/Playlist.hxx

@@ -197,7 +197,12 @@ public:
 	 * the song matches.
 	 */
 	void TagModified(DetachedSong &&song) noexcept;
-	void TagModified(const char *uri, const Tag &tag) noexcept;
+
+	/**
+	 * @param real_uri the song's "real uri" (see
+	 * DetachedSong::GetRealURI(), DetachedSong::IsRealURI())
+	 */
+	void TagModified(const char *real_uri, const Tag &tag) noexcept;
 
 #ifdef ENABLE_DATABASE
 	/**

src/song/DetachedSong.hxx

@@ -169,6 +169,11 @@ public:
 		return uri == other_uri;
 	}
 
+	gcc_pure gcc_nonnull_all
+	bool IsRealURI(const char *other_uri) const noexcept {
+		return (HasRealURI() ? real_uri : uri) == other_uri;
+	}
+
 	gcc_pure
 	bool IsRemote() const noexcept;