From f5ae1ce00b85699291a7cdf9782574e70a8c28f5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 19 Jan 2014 10:51:34 +0100 Subject: 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. --- src/db/LazyDatabase.cxx | 4 +- src/db/LazyDatabase.hxx | 6 +- src/db/ProxyDatabasePlugin.cxx | 129 +++++++++++++++++++++------------------- src/db/SimpleDatabasePlugin.cxx | 30 ++++++---- src/db/SimpleDatabasePlugin.hxx | 12 +++- src/db/UpnpDatabasePlugin.cxx | 71 ++++++++++++---------- 6 files changed, 142 insertions(+), 110 deletions(-) (limited to 'src/db') 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 #include +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(_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 @@ -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(_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); } /** -- cgit v1.2.3