LightSong: new class to be used by DatabasePlugin callbacks

Detach the Song class completely from the public API, only to be used
by SimpleDatabase and the update thread.

6 files changed, 142 insertions, 110 deletions

diff --git a/src/db/LazyDatabase.cxx b/src/db/LazyDatabase.cxx
index 0718c3dcd..6a01ffb82 100644
--- a/src/db/LazyDatabase.cxx
+++ b/src/db/LazyDatabase.cxx
@@ -51,7 +51,7 @@ LazyDatabase::Close()
 	}
 }
 
-Song *
+const LightSong *
 LazyDatabase::GetSong(const char *uri, Error &error) const
 {
 	return EnsureOpen(error)
@@ -60,7 +60,7 @@ LazyDatabase::GetSong(const char *uri, Error &error) const
 }
 
 void
-LazyDatabase::ReturnSong(Song *song) const
+LazyDatabase::ReturnSong(const LightSong *song) const
 {
 	assert(open);
 
diff --git a/src/db/LazyDatabase.hxx b/src/db/LazyDatabase.hxx
index 7f97aa40d..f718ecb3f 100644
--- a/src/db/LazyDatabase.hxx
+++ b/src/db/LazyDatabase.hxx
@@ -41,9 +41,9 @@ public:
 
 	virtual void Close() override;
 
-	virtual Song *GetSong(const char *uri_utf8,
-			      Error &error) const override;
-	virtual void ReturnSong(Song *song) const;
+	virtual const LightSong *GetSong(const char *uri_utf8,
+					 Error &error) const override;
+	virtual void ReturnSong(const LightSong *song) const;
 
 	virtual bool Visit(const DatabaseSelection &selection,
 			   VisitDirectory visit_directory,
diff --git a/src/db/ProxyDatabasePlugin.cxx b/src/db/ProxyDatabasePlugin.cxx
index e5e9ac76f..f65e4f3d0 100644
--- a/src/db/ProxyDatabasePlugin.cxx
+++ b/src/db/ProxyDatabasePlugin.cxx
@@ -24,11 +24,12 @@
 #include "DatabaseSelection.hxx"
 #include "DatabaseError.hxx"
 #include "Directory.hxx"
-#include "Song.hxx"
+#include "LightSong.hxx"
 #include "SongFilter.hxx"
 #include "Compiler.h"
 #include "ConfigData.hxx"
 #include "tag/TagBuilder.hxx"
+#include "tag/Tag.hxx"
 #include "util/Error.hxx"
 #include "util/Domain.hxx"
 #include "protocol/Ack.hxx"
@@ -44,6 +45,25 @@
 #include <string>
 #include <list>
 
+class ProxySong : public LightSong {
+	Tag tag2;
+
+public:
+	explicit ProxySong(const mpd_song *song);
+};
+
+class AllocatedProxySong : public ProxySong {
+	mpd_song *const song;
+
+public:
+	explicit AllocatedProxySong(mpd_song *_song)
+		:ProxySong(_song), song(_song) {}
+
+	~AllocatedProxySong() {
+		mpd_song_free(song);
+	}
+};
+
 class ProxyDatabase final : public Database, SocketMonitor, IdleMonitor {
 	DatabaseListener &listener;
 
@@ -79,9 +99,9 @@ public:
 
 	virtual bool Open(Error &error) override;
 	virtual void Close() override;
-	virtual Song *GetSong(const char *uri_utf8,
+	virtual const LightSong *GetSong(const char *uri_utf8,
 				     Error &error) const override;
-	virtual void ReturnSong(Song *song) const;
+	virtual void ReturnSong(const LightSong *song) const;
 
 	virtual bool Visit(const DatabaseSelection &selection,
 			   VisitDirectory visit_directory,
@@ -144,6 +164,38 @@ static constexpr struct {
 	{ TAG_NUM_OF_ITEM_TYPES, MPD_TAG_COUNT }
 };
 
+static void
+Copy(TagBuilder &tag, TagType d_tag,
+     const struct mpd_song *song, enum mpd_tag_type s_tag)
+{
+
+	for (unsigned i = 0;; ++i) {
+		const char *value = mpd_song_get_tag(song, s_tag, i);
+		if (value == nullptr)
+			break;
+
+		tag.AddItem(d_tag, value);
+	}
+}
+
+ProxySong::ProxySong(const mpd_song *song)
+{
+	directory = nullptr;
+	uri = mpd_song_get_uri(song);
+	tag = &tag2;
+	mtime = mpd_song_get_last_modified(song);
+	start_ms = mpd_song_get_start(song) * 1000;
+	end_ms = mpd_song_get_end(song) * 1000;
+
+	TagBuilder tag_builder;
+	tag_builder.SetTime(mpd_song_get_duration(song));
+
+	for (const auto *i = &tag_table[0]; i->d != TAG_NUM_OF_ITEM_TYPES; ++i)
+		Copy(tag_builder, i->d, song, i->s);
+
+	tag_builder.Commit(tag2);
+}
+
 gcc_const
 static enum mpd_tag_type
 Convert(TagType tag_type)
@@ -424,10 +476,7 @@ ProxyDatabase::OnIdle()
 	SocketMonitor::ScheduleRead();
 }
 
-static Song *
-Convert(const struct mpd_song *song);
-
-Song *
+const LightSong *
 ProxyDatabase::GetSong(const char *uri, Error &error) const
 {
 	// TODO: eliminate the const_cast
@@ -452,18 +501,17 @@ ProxyDatabase::GetSong(const char *uri, Error &error) const
 		return nullptr;
 	}
 
-	Song *song2 = Convert(song);
-	mpd_song_free(song);
-	return song2;
+	return new AllocatedProxySong(song);
 }
 
 void
-ProxyDatabase::ReturnSong(Song *song) const
+ProxyDatabase::ReturnSong(const LightSong *_song) const
 {
-	assert(song != nullptr);
-	assert(song->parent == nullptr);
+	assert(_song != nullptr);
 
-	song->Free();
+	AllocatedProxySong *song = (AllocatedProxySong *)
+		const_cast<LightSong *>(_song);
+	delete song;
 }
 
 static bool
@@ -493,60 +541,23 @@ Visit(struct mpd_connection *connection, Directory &root,
 	return true;
 }
 
-static void
-Copy(TagBuilder &tag, TagType d_tag,
-     const struct mpd_song *song, enum mpd_tag_type s_tag)
-{
-
-	for (unsigned i = 0;; ++i) {
-		const char *value = mpd_song_get_tag(song, s_tag, i);
-		if (value == nullptr)
-			break;
-
-		tag.AddItem(d_tag, value);
-	}
-}
-
-static Song *
-Convert(const struct mpd_song *song)
-{
-	Song *s = Song::NewFile(mpd_song_get_uri(song), nullptr);
-
-	s->mtime = mpd_song_get_last_modified(song);
-	s->start_ms = mpd_song_get_start(song) * 1000;
-	s->end_ms = mpd_song_get_end(song) * 1000;
-
-	TagBuilder tag;
-	tag.SetTime(mpd_song_get_duration(song));
-
-	for (const auto *i = &tag_table[0]; i->d != TAG_NUM_OF_ITEM_TYPES; ++i)
-		Copy(tag, i->d, song, i->s);
-
-	tag.Commit(s->tag);
-
-	return s;
-}
-
 gcc_pure
 static bool
-Match(const SongFilter *filter, const Song &song)
+Match(const SongFilter *filter, const LightSong &song)
 {
 	return filter == nullptr || filter->Match(song);
 }
 
 static bool
 Visit(const SongFilter *filter,
-      const struct mpd_song *song,
+      const mpd_song *_song,
       VisitSong visit_song, Error &error)
 {
 	if (!visit_song)
 		return true;
 
-	Song *s = Convert(song);
-	bool success = !Match(filter, *s) || visit_song(*s, error);
-	s->Free();
-
-	return success;
+	const ProxySong song(_song);
+	return !Match(filter, song) || visit_song(song, error);
 }
 
 static bool
@@ -664,12 +675,10 @@ SearchSongs(struct mpd_connection *connection,
 	bool result = true;
 	struct mpd_song *song;
 	while (result && (song = mpd_recv_song(connection)) != nullptr) {
-		Song *song2 = Convert(song);
-		mpd_song_free(song);
+		AllocatedProxySong song2(song);
 
-		result = !Match(selection.filter, *song2) ||
-			visit_song(*song2, error);
-		song2->Free();
+		result = !Match(selection.filter, song2) ||
+			visit_song(song2, error);
 	}
 
 	mpd_response_finish(connection);
diff --git a/src/db/SimpleDatabasePlugin.cxx b/src/db/SimpleDatabasePlugin.cxx
index c33db3831..3d947c042 100644
--- a/src/db/SimpleDatabasePlugin.cxx
+++ b/src/db/SimpleDatabasePlugin.cxx
@@ -22,6 +22,7 @@
 #include "DatabaseSelection.hxx"
 #include "DatabaseHelpers.hxx"
 #include "Directory.hxx"
+#include "Song.hxx"
 #include "SongFilter.hxx"
 #include "DatabaseSave.hxx"
 #include "DatabaseLock.hxx"
@@ -193,29 +194,34 @@ SimpleDatabase::Close()
 	delete root;
 }
 
-Song *
+const LightSong *
 SimpleDatabase::GetSong(const char *uri, Error &error) const
 {
 	assert(root != nullptr);
+	assert(borrowed_song_count == 0);
 
 	db_lock();
-	Song *song = root->LookupSong(uri);
+	const Song *song = root->LookupSong(uri);
 	db_unlock();
-	if (song == nullptr)
+	if (song == nullptr) {
 		error.Format(db_domain, DB_NOT_FOUND,
 			     "No such song: %s", uri);
+		return nullptr;
+	}
+
+	light_song = song->Export();
+
 #ifndef NDEBUG
-	else
-		++borrowed_song_count;
+	++borrowed_song_count;
 #endif
 
-	return song;
+	return &light_song;
 }
 
 void
-SimpleDatabase::ReturnSong(gcc_unused Song *song) const
+SimpleDatabase::ReturnSong(gcc_unused const LightSong *song) const
 {
-	assert(song != nullptr);
+	assert(song == &light_song);
 
 #ifndef NDEBUG
 	assert(borrowed_song_count > 0);
@@ -247,9 +253,11 @@ SimpleDatabase::Visit(const DatabaseSelection &selection,
 	if (directory == nullptr) {
 		if (visit_song) {
 			Song *song = root->LookupSong(selection.uri.c_str());
-			if (song != nullptr)
-				return !selection.Match(*song) ||
-					visit_song(*song, error);
+			if (song != nullptr) {
+				const LightSong song2 = song->Export();
+				return !selection.Match(song2) ||
+					visit_song(song2, error);
+			}
 		}
 
 		error.Set(db_domain, DB_NOT_FOUND, "No such directory");
diff --git a/src/db/SimpleDatabasePlugin.hxx b/src/db/SimpleDatabasePlugin.hxx
index d51174194..509b91e4e 100644
--- a/src/db/SimpleDatabasePlugin.hxx
+++ b/src/db/SimpleDatabasePlugin.hxx
@@ -22,6 +22,7 @@
 
 #include "DatabasePlugin.hxx"
 #include "fs/AllocatedPath.hxx"
+#include "LightSong.hxx"
 #include "Compiler.h"
 
 #include <cassert>
@@ -36,6 +37,11 @@ class SimpleDatabase : public Database {
 
 	time_t mtime;
 
+	/**
+	 * A buffer for GetSong().
+	 */
+	mutable LightSong light_song;
+
 #ifndef NDEBUG
 	mutable unsigned borrowed_song_count;
 #endif
@@ -60,9 +66,9 @@ public:
 	virtual bool Open(Error &error) override;
 	virtual void Close() override;
 
-	virtual Song *GetSong(const char *uri_utf8,
-			      Error &error) const override;
-	virtual void ReturnSong(Song *song) const;
+	virtual const LightSong *GetSong(const char *uri_utf8,
+					 Error &error) const override;
+	virtual void ReturnSong(const LightSong *song) const;
 
 	virtual bool Visit(const DatabaseSelection &selection,
 			   VisitDirectory visit_directory,
diff --git a/src/db/UpnpDatabasePlugin.cxx b/src/db/UpnpDatabasePlugin.cxx
index dbf04f818..0768488a3 100644
--- a/src/db/UpnpDatabasePlugin.cxx
+++ b/src/db/UpnpDatabasePlugin.cxx
@@ -31,7 +31,7 @@
 #include "DatabaseError.hxx"
 #include "PlaylistVector.hxx"
 #include "Directory.hxx"
-#include "Song.hxx"
+#include "LightSong.hxx"
 #include "ConfigData.hxx"
 #include "tag/TagBuilder.hxx"
 #include "tag/TagTable.hxx"
@@ -49,6 +49,31 @@
 
 static const char *const rootid = "0";
 
+class UpnpSong : public LightSong {
+	std::string uri2;
+
+	Tag tag2;
+
+public:
+	explicit UpnpSong(UPnPDirObject &&object)
+		:uri2(std::move(object.url)), tag2(std::move(object.tag)) {
+		directory = nullptr;
+		uri = uri2.c_str();
+		tag = &tag2;
+		mtime = 0;
+		start_ms = end_ms = 0;
+	}
+
+	UpnpSong(UPnPDirObject &&object, const char *_uri)
+		:uri2(_uri), tag2(std::move(object.tag)) {
+		directory = nullptr;
+		uri = uri2.c_str();
+		tag = &tag2;
+		mtime = 0;
+		start_ms = end_ms = 0;
+	}
+};
+
 class UpnpDatabase : public Database {
 	LibUPnP *m_lib;
 	UPnPDeviceDirectory *m_superdir;
@@ -61,9 +86,9 @@ public:
 
 	virtual bool Open(Error &error) override;
 	virtual void Close() override;
-	virtual Song *GetSong(const char *uri_utf8,
-			      Error &error) const override;
-	virtual void ReturnSong(Song *song) const;
+	virtual const LightSong *GetSong(const char *uri_utf8,
+					 Error &error) const override;
+	virtual void ReturnSong(const LightSong *song) const;
 
 	virtual bool Visit(const DatabaseSelection &selection,
 			   VisitDirectory visit_directory,
@@ -187,34 +212,20 @@ UpnpDatabase::Close()
 }
 
 void
-UpnpDatabase::ReturnSong(Song *song) const
-{
-	assert(song != nullptr);
-
-	song->Free();
-}
-
-// If uri is empty, we use the object's url instead. This happens
-// when the target of a Visit() is a song, which  only happens when
-// "add"ing AFAIK. Visit() calls us with a null uri so that the url
-// appropriate for fetching is used instead.
-static Song *
-upnpItemToSong(UPnPDirObject &&dirent, const char *uri)
+UpnpDatabase::ReturnSong(const LightSong *_song) const
 {
-	if (*uri == 0)
-		uri = dirent.url.c_str();
+	assert(_song != nullptr);
 
-	Song *s = Song::NewFile(uri, nullptr);
-	s->tag = std::move(dirent.tag);
-	return s;
+	UpnpSong *song = (UpnpSong *)const_cast<LightSong *>(_song);
+	delete song;
 }
 
 // Get song info by path. We can receive either the id path, or the titles
 // one
-Song *
+const LightSong *
 UpnpDatabase::GetSong(const char *uri, Error &error) const
 {
-	Song *song = nullptr;
+	UpnpSong *song = nullptr;
 	auto vpath = stringToTokens(uri, "/", true);
 	if (vpath.size() >= 2) {
 		ContentDirectoryService server;
@@ -232,7 +243,8 @@ UpnpDatabase::GetSong(const char *uri, Error &error) const
 				      error))
 				return nullptr;
 		}
-		song = upnpItemToSong(std::move(dirent), "");
+
+		song = new UpnpSong(std::move(dirent));
 	}
 	if (song == nullptr)
 		error.Format(db_domain, DB_NOT_FOUND, "No such song: %s", uri);
@@ -357,12 +369,9 @@ visitSong(UPnPDirObject &&meta, const char *path,
 {
 	if (!visit_song)
 		return true;
-	Song *s = upnpItemToSong(std::move(meta), path);
-	if (!selection.Match(*s))
-		return true;
-	bool success = visit_song(*s, error);
-	s->Free();
-	return success;
+
+	const UpnpSong song(std::move(meta), path);
+	return !selection.Match(song) || visit_song(song, error);
 }
 
 /**