diff options
70 files changed, 817 insertions, 785 deletions
diff --git a/Makefile.am b/Makefile.am index 20cc603ca..a63c9d1d7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -192,6 +192,7 @@ src_mpd_SOURCES = \ src/ReplayGainConfig.cxx src/ReplayGainConfig.hxx \ src/ReplayGainInfo.cxx src/ReplayGainInfo.hxx \ src/SignalHandlers.cxx src/SignalHandlers.hxx \ + src/DetachedSong.cxx src/DetachedSong.hxx \ src/Song.cxx src/Song.hxx \ src/SongUpdate.cxx \ src/SongPrint.cxx src/SongPrint.hxx \ @@ -206,7 +207,6 @@ src_mpd_SOURCES = \ src/TextInputStream.cxx \ src/Volume.cxx src/Volume.hxx \ src/SongFilter.cxx src/SongFilter.hxx \ - src/SongPointer.hxx \ src/PlaylistFile.cxx src/PlaylistFile.hxx \ src/Timer.cxx @@ -1271,7 +1271,7 @@ test_dump_playlist_SOURCES = test/dump_playlist.cxx \ $(DECODER_SRC) \ src/Log.cxx src/LogBackend.cxx \ src/IOThread.cxx \ - src/Song.cxx src/TagSave.cxx \ + src/TagSave.cxx \ src/TagFile.cxx \ src/CheckAudioFormat.cxx \ src/TextInputStream.cxx \ diff --git a/src/DatabasePlaylist.cxx b/src/DatabasePlaylist.cxx index b0cb19589..a29c9f2bc 100644 --- a/src/DatabasePlaylist.cxx +++ b/src/DatabasePlaylist.cxx @@ -23,6 +23,7 @@ #include "PlaylistFile.hxx" #include "DatabaseGlue.hxx" #include "DatabasePlugin.hxx" +#include "DetachedSong.hxx" #include <functional> @@ -30,7 +31,7 @@ static bool AddSong(const char *playlist_path_utf8, Song &song, Error &error) { - return spl_append_song(playlist_path_utf8, song, error); + return spl_append_song(playlist_path_utf8, DetachedSong(song), error); } bool diff --git a/src/DatabasePrint.cxx b/src/DatabasePrint.cxx index e500ee0c8..feb58a263 100644 --- a/src/DatabasePrint.cxx +++ b/src/DatabasePrint.cxx @@ -55,26 +55,24 @@ PrintDirectoryFull(Client &client, const Directory &directory) static void print_playlist_in_directory(Client &client, - const Directory &directory, + const Directory *directory, const char *name_utf8) { - if (directory.IsRoot()) + if (directory == nullptr || directory->IsRoot()) client_printf(client, "playlist: %s\n", name_utf8); else client_printf(client, "playlist: %s/%s\n", - directory.GetPath(), name_utf8); + directory->GetPath(), name_utf8); } static bool PrintSongBrief(Client &client, const Song &song) { - assert(song.parent != nullptr); - song_print_uri(client, song); if (song.tag != nullptr && song.tag->has_playlist) /* this song file has an embedded CUE sheet */ - print_playlist_in_directory(client, *song.parent, song.uri); + print_playlist_in_directory(client, song.parent, song.uri); return true; } @@ -82,13 +80,11 @@ PrintSongBrief(Client &client, const Song &song) static bool PrintSongFull(Client &client, const Song &song) { - assert(song.parent != nullptr); - song_print_info(client, song); if (song.tag != nullptr && song.tag->has_playlist) /* this song file has an embedded CUE sheet */ - print_playlist_in_directory(client, *song.parent, song.uri); + print_playlist_in_directory(client, song.parent, song.uri); return true; } @@ -98,7 +94,7 @@ PrintPlaylistBrief(Client &client, const PlaylistInfo &playlist, const Directory &directory) { - print_playlist_in_directory(client, directory, playlist.name.c_str()); + print_playlist_in_directory(client, &directory, playlist.name.c_str()); return true; } @@ -107,7 +103,7 @@ PrintPlaylistFull(Client &client, const PlaylistInfo &playlist, const Directory &directory) { - print_playlist_in_directory(client, directory, playlist.name.c_str()); + print_playlist_in_directory(client, &directory, playlist.name.c_str()); if (playlist.mtime > 0) time_print(client, "Last-Modified", playlist.mtime); diff --git a/src/DatabaseQueue.cxx b/src/DatabaseQueue.cxx index b093ce073..3dac54630 100644 --- a/src/DatabaseQueue.cxx +++ b/src/DatabaseQueue.cxx @@ -23,14 +23,16 @@ #include "DatabasePlugin.hxx" #include "Partition.hxx" #include "util/Error.hxx" +#include "DetachedSong.hxx" #include <functional> static bool -AddToQueue(Partition &partition, Song &song, Error &error) +AddToQueue(Partition &partition, const Song &song, Error &error) { PlaylistResult result = - partition.playlist.AppendSong(partition.pc, &song, nullptr); + partition.playlist.AppendSong(partition.pc, DetachedSong(song), + nullptr); if (result != PlaylistResult::SUCCESS) { error.Set(playlist_domain, int(result), "Playlist error"); return false; diff --git a/src/DecoderAPI.cxx b/src/DecoderAPI.cxx index a3f2fc3e0..148d20005 100644 --- a/src/DecoderAPI.cxx +++ b/src/DecoderAPI.cxx @@ -28,7 +28,7 @@ #include "MusicPipe.hxx" #include "DecoderControl.hxx" #include "DecoderInternal.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "InputStream.hxx" #include "util/Error.hxx" #include "Log.hxx" @@ -467,7 +467,7 @@ decoder_data(Decoder &decoder, const auto dest = chunk->Write(dc.out_audio_format, decoder.timestamp - - dc.song->start_ms / 1000.0, + dc.song->GetStartMS() / 1000.0, kbit_rate); if (dest.IsNull()) { /* the chunk is full, flush it */ diff --git a/src/DecoderControl.cxx b/src/DecoderControl.cxx index b63dba3a0..4e5e894e3 100644 --- a/src/DecoderControl.cxx +++ b/src/DecoderControl.cxx @@ -20,7 +20,7 @@ #include "config.h" #include "DecoderControl.hxx" #include "MusicPipe.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include <assert.h> @@ -36,8 +36,7 @@ DecoderControl::~DecoderControl() { ClearError(); - if (song != nullptr) - song->Free(); + delete song; } void @@ -53,7 +52,7 @@ DecoderControl::WaitForDecoder() } bool -DecoderControl::IsCurrentSong(const Song &_song) const +DecoderControl::IsCurrentSong(const DetachedSong &_song) const { switch (state) { case DecoderState::STOP: @@ -62,7 +61,7 @@ DecoderControl::IsCurrentSong(const Song &_song) const case DecoderState::START: case DecoderState::DECODE: - return SongEquals(*song, _song); + return song->IsSame(_song); } assert(false); @@ -70,16 +69,14 @@ DecoderControl::IsCurrentSong(const Song &_song) const } void -DecoderControl::Start(Song *_song, +DecoderControl::Start(DetachedSong *_song, unsigned _start_ms, unsigned _end_ms, MusicBuffer &_buffer, MusicPipe &_pipe) { assert(_song != nullptr); assert(_pipe.IsEmpty()); - if (song != nullptr) - song->Free(); - + delete song; song = _song; start_ms = _start_ms; end_ms = _end_ms; diff --git a/src/DecoderControl.hxx b/src/DecoderControl.hxx index 863398dca..bacdad347 100644 --- a/src/DecoderControl.hxx +++ b/src/DecoderControl.hxx @@ -36,7 +36,7 @@ #undef ERROR #endif -struct Song; +class DetachedSong; class MusicBuffer; class MusicPipe; @@ -123,7 +123,7 @@ struct DecoderControl { * This is a duplicate, and must be freed when this attribute * is cleared. */ - Song *song; + DetachedSong *song; /** * The initial seek position (in milliseconds), e.g. to the @@ -293,10 +293,10 @@ struct DecoderControl { * Caller must lock the object. */ gcc_pure - bool IsCurrentSong(const Song &_song) const; + bool IsCurrentSong(const DetachedSong &_song) const; gcc_pure - bool LockIsCurrentSong(const Song &_song) const { + bool LockIsCurrentSong(const DetachedSong &_song) const { Lock(); const bool result = IsCurrentSong(_song); Unlock(); @@ -360,7 +360,7 @@ public: * @param pipe the pipe which receives the decoded chunks (owned by * the caller) */ - void Start(Song *song, unsigned start_ms, unsigned end_ms, + void Start(DetachedSong *song, unsigned start_ms, unsigned end_ms, MusicBuffer &buffer, MusicPipe &pipe); void Stop(); diff --git a/src/DecoderThread.cxx b/src/DecoderThread.cxx index f71462bdf..7099b3bd4 100644 --- a/src/DecoderThread.cxx +++ b/src/DecoderThread.cxx @@ -23,7 +23,7 @@ #include "DecoderInternal.hxx" #include "DecoderError.hxx" #include "DecoderPlugin.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "system/FatalError.hxx" #include "Mapper.hxx" #include "fs/Traits.hxx" @@ -347,11 +347,10 @@ decoder_run_file(Decoder &decoder, const char *path_fs) static void decoder_run_song(DecoderControl &dc, - const Song &song, const char *uri) + const DetachedSong &song, const char *uri) { Decoder decoder(dc, dc.start_ms > 0, - song.tag != nullptr && song.IsFile() - ? new Tag(*song.tag) : nullptr); + new Tag(song.GetTag())); int ret; dc.state = DecoderState::START; @@ -381,7 +380,7 @@ decoder_run_song(DecoderControl &dc, else { dc.state = DecoderState::ERROR; - const char *error_uri = song.uri; + const char *error_uri = song.GetURI(); const std::string allocated = uri_remove_auth(error_uri); if (!allocated.empty()) error_uri = allocated.c_str(); @@ -399,10 +398,10 @@ decoder_run(DecoderControl &dc) dc.ClearError(); assert(dc.song != nullptr); - const Song &song = *dc.song; + const DetachedSong &song = *dc.song; const std::string uri = song.IsFile() - ? std::string(map_song_fs(song).c_str()) + ? map_song_fs(song).c_str() : song.GetURI(); if (uri.empty()) { diff --git a/src/SongPointer.hxx b/src/DetachedSong.cxx index ded3b3e1d..4b1d51a41 100644 --- a/src/SongPointer.hxx +++ b/src/DetachedSong.cxx @@ -1,5 +1,5 @@ /* - * Copyright (C) 2003-2013 The Music Player Daemon Project + * Copyright (C) 2003-2014 The Music Player Daemon Project * http://www.musicpd.org * * This program is free software; you can redistribute it and/or modify @@ -17,47 +17,35 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#ifndef MPD_SONG_POINTER_HXX -#define MPD_SONG_POINTER_HXX - +#include "config.h" +#include "DetachedSong.hxx" #include "Song.hxx" - -#include <utility> - -class SongPointer { - Song *song; - -public: - explicit SongPointer(Song *_song) - :song(_song) {} - - SongPointer(const SongPointer &) = delete; - - SongPointer(SongPointer &&other):song(other.song) { - other.song = nullptr; - } - - ~SongPointer() { - if (song != nullptr) - song->Free(); - } - - SongPointer &operator=(const SongPointer &) = delete; - - SongPointer &operator=(SongPointer &&other) { - std::swap(song, other.song); - return *this; - } - - operator const Song *() const { - return song; - } - - Song *Steal() { - auto result = song; - song = nullptr; - return result; - } -}; - -#endif +#include "util/UriUtil.hxx" +#include "fs/Traits.hxx" + +DetachedSong::DetachedSong(const Song &other) + :uri(other.GetURI().c_str()), + tag(other.tag != nullptr ? *other.tag : Tag()), + mtime(other.mtime), + start_ms(other.start_ms), end_ms(other.end_ms) {} + +bool +DetachedSong::IsRemote() const +{ + return uri_has_scheme(uri.c_str()); +} + +bool +DetachedSong::IsAbsoluteFile() const +{ + return PathTraitsUTF8::IsAbsolute(uri.c_str()); +} + +double +DetachedSong::GetDuration() const +{ + if (end_ms > 0) + return (end_ms - start_ms) / 1000.0; + + return tag.time - start_ms / 1000.0; +} diff --git a/src/DetachedSong.hxx b/src/DetachedSong.hxx new file mode 100644 index 000000000..bee6a73a4 --- /dev/null +++ b/src/DetachedSong.hxx @@ -0,0 +1,184 @@ +/* + * Copyright (C) 2003-2014 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_DETACHED_SONG_HXX +#define MPD_DETACHED_SONG_HXX + +#include "check.h" +#include "tag/Tag.hxx" +#include "Compiler.h" + +#include <string> +#include <utility> + +#include <time.h> + +struct Song; + +class DetachedSong { + /** + * An UTF-8-encoded URI referring to the song file. This can + * be one of: + * + * - an absolute URL with a scheme + * (e.g. "http://example.com/foo.mp3") + * + * - an absolute file name + * + * - a file name relative to the music directory + */ + std::string uri; + + Tag tag; + + time_t mtime; + + /** + * Start of this sub-song within the file in milliseconds. + */ + unsigned start_ms; + + /** + * End of this sub-song within the file in milliseconds. + * Unused if zero. + */ + unsigned end_ms; + +public: + explicit DetachedSong(const DetachedSong &other) + :uri(other.uri), + tag(other.tag), + mtime(other.mtime), + start_ms(other.start_ms), end_ms(other.end_ms) {} + + explicit DetachedSong(const Song &other); + + explicit DetachedSong(const char *_uri) + :uri(_uri), + mtime(0), start_ms(0), end_ms(0) {} + + explicit DetachedSong(const std::string &_uri) + :uri(std::move(_uri)), + mtime(0), start_ms(0), end_ms(0) {} + + explicit DetachedSong(std::string &&_uri) + :uri(std::move(_uri)), + mtime(0), start_ms(0), end_ms(0) {} + + template<typename U> + DetachedSong(U &&_uri, Tag &&_tag) + :uri(std::forward<U>(_uri)), + tag(std::move(_tag)), + mtime(0), start_ms(0), end_ms(0) {} + + DetachedSong(DetachedSong &&other) + :uri(std::move(other.uri)), + tag(std::move(other.tag)), + mtime(other.mtime), + start_ms(other.start_ms), end_ms(other.end_ms) {} + + gcc_pure + const char *GetURI() const { + return uri.c_str(); + } + + template<typename T> + void SetURI(T &&_uri) { + uri = std::forward<T>(_uri); + } + + /** + * Returns true if both objects refer to the same physical + * song. + */ + gcc_pure + bool IsSame(const DetachedSong &other) const { + return uri == other.uri; + } + + gcc_pure gcc_nonnull_all + bool IsURI(const char *other_uri) const { + return uri == other_uri; + } + + gcc_pure + bool IsRemote() const; + + gcc_pure + bool IsFile() const { + return !IsRemote(); + } + + gcc_pure + bool IsAbsoluteFile() const; + + gcc_pure + bool IsInDatabase() const { + return IsFile() && !IsAbsoluteFile(); + } + + const Tag &GetTag() const { + return tag; + } + + Tag &WritableTag() { + return tag; + } + + void SetTag(const Tag &_tag) { + tag = Tag(_tag); + } + + void SetTag(Tag &&_tag) { + tag = std::move(_tag); + } + + void MoveTagFrom(DetachedSong &&other) { + tag = std::move(other.tag); + } + + time_t GetLastModified() const { + return mtime; + } + + void SetLastModified(time_t _value) { + mtime = _value; + } + + unsigned GetStartMS() const { + return start_ms; + } + + void SetStartMS(unsigned _value) { + start_ms = _value; + } + + unsigned GetEndMS() const { + return end_ms; + } + + void SetEndMS(unsigned _value) { + end_ms = _value; + } + + gcc_pure + double GetDuration() const; +}; + +#endif diff --git a/src/Directory.cxx b/src/Directory.cxx index 750fee896..9180427f9 100644 --- a/src/Directory.cxx +++ b/src/Directory.cxx @@ -48,14 +48,6 @@ Directory::Allocate(const char *path) path); } -Directory::Directory() -{ - INIT_LIST_HEAD(&children); - INIT_LIST_HEAD(&songs); - - path[0] = 0; -} - Directory::Directory(const char *_path) :mtime(0), have_stat(false) { diff --git a/src/Directory.hxx b/src/Directory.hxx index 1ef4693e2..24ba685e6 100644 --- a/src/Directory.hxx +++ b/src/Directory.hxx @@ -95,10 +95,6 @@ protected: static Directory *Allocate(const char *path); public: - /** - * Default constructor, needed for #detached_root. - */ - Directory(); ~Directory(); /** diff --git a/src/DirectorySave.cxx b/src/DirectorySave.cxx index 709184289..f8ee5b0bd 100644 --- a/src/DirectorySave.cxx +++ b/src/DirectorySave.cxx @@ -22,6 +22,7 @@ #include "Directory.hxx" #include "Song.hxx" #include "SongSave.hxx" +#include "DetachedSong.hxx" #include "PlaylistDatabase.hxx" #include "fs/TextFile.hxx" #include "util/StringUtil.hxx" @@ -132,7 +133,6 @@ directory_load(TextFile &file, Directory &directory, Error &error) return false; } else if (StringStartsWith(line, SONG_BEGIN)) { const char *name = line + sizeof(SONG_BEGIN) - 1; - Song *song; if (directory.FindSong(name) != nullptr) { error.Format(directory_domain, @@ -140,11 +140,13 @@ directory_load(TextFile &file, Directory &directory, Error &error) return false; } - song = song_load(file, &directory, name, error); + DetachedSong *song = song_load(file, name, error); if (song == nullptr) return false; - directory.AddSong(song); + directory.AddSong(Song::NewFrom(std::move(*song), + &directory)); + delete song; } else if (StringStartsWith(line, PLAYLIST_META_BEGIN)) { const char *name = line + sizeof(PLAYLIST_META_BEGIN) - 1; if (!playlist_metadata_load(file, directory.playlists, diff --git a/src/Instance.cxx b/src/Instance.cxx index daad94212..a033ed82f 100644 --- a/src/Instance.cxx +++ b/src/Instance.cxx @@ -23,9 +23,9 @@ #include "Idle.hxx" void -Instance::DeleteSong(const Song &song) +Instance::DeleteSong(const char *uri) { - partition->DeleteSong(song); + partition->DeleteSong(uri); } void diff --git a/src/Instance.hxx b/src/Instance.hxx index a0dfd1b94..45b1e44f1 100644 --- a/src/Instance.hxx +++ b/src/Instance.hxx @@ -24,14 +24,13 @@ class ClientList; struct Partition; -struct Song; struct Instance { ClientList *client_list; Partition *partition; - void DeleteSong(const Song &song); + void DeleteSong(const char *uri); /** * The database has been modified. Propagate the change to diff --git a/src/Mapper.cxx b/src/Mapper.cxx index fbe1e8c34..6910b4983 100644 --- a/src/Mapper.cxx +++ b/src/Mapper.cxx @@ -25,6 +25,7 @@ #include "Mapper.hxx" #include "Directory.hxx" #include "Song.hxx" +#include "DetachedSong.hxx" #include "fs/AllocatedPath.hxx" #include "fs/Traits.hxx" #include "fs/Charset.hxx" @@ -219,14 +220,18 @@ map_detached_song_fs(const char *uri_utf8) AllocatedPath map_song_fs(const Song &song) { - assert(song.IsFile()); + return song.parent == nullptr + ? map_detached_song_fs(song.uri) + : map_directory_child_fs(*song.parent, song.uri); +} - if (song.IsInDatabase()) - return song.IsDetached() - ? map_detached_song_fs(song.uri) - : map_directory_child_fs(*song.parent, song.uri); +AllocatedPath +map_song_fs(const DetachedSong &song) +{ + if (song.IsAbsoluteFile()) + return AllocatedPath::FromUTF8(song.GetURI()); else - return AllocatedPath::FromUTF8(song.uri); + return map_uri_fs(song.GetURI()); } std::string diff --git a/src/Mapper.hxx b/src/Mapper.hxx index c757c421e..be61ab43a 100644 --- a/src/Mapper.hxx +++ b/src/Mapper.hxx @@ -33,6 +33,7 @@ class AllocatedPath; struct Directory; struct Song; +class DetachedSong; void mapper_init(AllocatedPath &&music_dir, AllocatedPath &&playlist_dir); @@ -116,6 +117,10 @@ gcc_pure AllocatedPath map_song_fs(const Song &song); +gcc_pure +AllocatedPath +map_song_fs(const DetachedSong &song); + /** * Maps a file system path (relative to the music directory or * absolute) to a relative path in UTF-8 encoding. diff --git a/src/MemorySongEnumerator.cxx b/src/MemorySongEnumerator.cxx index 7c9d05daa..3bb17083c 100644 --- a/src/MemorySongEnumerator.cxx +++ b/src/MemorySongEnumerator.cxx @@ -20,13 +20,13 @@ #include "config.h" #include "MemorySongEnumerator.hxx" -Song * +DetachedSong * MemorySongEnumerator::NextSong() { if (songs.empty()) return nullptr; - auto result = songs.front().Steal(); + auto result = new DetachedSong(std::move(songs.front())); songs.pop_front(); return result; } diff --git a/src/MemorySongEnumerator.hxx b/src/MemorySongEnumerator.hxx index 46086a064..085e16bc6 100644 --- a/src/MemorySongEnumerator.hxx +++ b/src/MemorySongEnumerator.hxx @@ -21,18 +21,18 @@ #define MPD_MEMORY_PLAYLIST_PROVIDER_HXX #include "SongEnumerator.hxx" -#include "SongPointer.hxx" +#include "DetachedSong.hxx" #include <forward_list> class MemorySongEnumerator final : public SongEnumerator { - std::forward_list<SongPointer> songs; + std::forward_list<DetachedSong> songs; public: - MemorySongEnumerator(std::forward_list<SongPointer> &&_songs) + MemorySongEnumerator(std::forward_list<DetachedSong> &&_songs) :songs(std::move(_songs)) {} - virtual Song *NextSong() override; + virtual DetachedSong *NextSong() override; }; #endif diff --git a/src/Partition.cxx b/src/Partition.cxx index 55750cfad..9c30ce0a7 100644 --- a/src/Partition.cxx +++ b/src/Partition.cxx @@ -19,7 +19,7 @@ #include "config.h" #include "Partition.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" void Partition::DatabaseModified() @@ -30,10 +30,10 @@ Partition::DatabaseModified() void Partition::TagModified() { - Song *song = pc.LockReadTaggedSong(); + DetachedSong *song = pc.LockReadTaggedSong(); if (song != nullptr) { playlist.TagModified(std::move(*song)); - song->Free(); + delete song; } } diff --git a/src/Partition.hxx b/src/Partition.hxx index 512ba3bca..69c6f0175 100644 --- a/src/Partition.hxx +++ b/src/Partition.hxx @@ -76,8 +76,8 @@ struct Partition { return playlist.DeleteRange(pc, start, end); } - void DeleteSong(const Song &song) { - playlist.DeleteSong(pc, song); + void DeleteSong(const char *uri) { + playlist.DeleteSong(pc, uri); } void Shuffle(unsigned start, unsigned end) { diff --git a/src/PlayerControl.cxx b/src/PlayerControl.cxx index 74b4673bc..baaf7bc3b 100644 --- a/src/PlayerControl.cxx +++ b/src/PlayerControl.cxx @@ -20,7 +20,7 @@ #include "config.h" #include "PlayerControl.hxx" #include "Idle.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include <algorithm> @@ -42,15 +42,12 @@ PlayerControl::PlayerControl(unsigned _buffer_chunks, PlayerControl::~PlayerControl() { - if (next_song != nullptr) - next_song->Free(); - - if (tagged_song != nullptr) - tagged_song->Free(); + delete next_song; + delete tagged_song; } void -PlayerControl::Play(Song *song) +PlayerControl::Play(DetachedSong *song) { assert(song != nullptr); @@ -195,26 +192,23 @@ PlayerControl::ClearError() } void -PlayerControl::LockSetTaggedSong(const Song &song) +PlayerControl::LockSetTaggedSong(const DetachedSong &song) { Lock(); - if (tagged_song != nullptr) - tagged_song->Free(); - tagged_song = song.DupDetached(); + delete tagged_song; + tagged_song = new DetachedSong(song); Unlock(); } void PlayerControl::ClearTaggedSong() { - if (tagged_song != nullptr) { - tagged_song->Free(); - tagged_song = nullptr; - } + delete tagged_song; + tagged_song = nullptr; } void -PlayerControl::EnqueueSong(Song *song) +PlayerControl::EnqueueSong(DetachedSong *song) { assert(song != nullptr); @@ -224,15 +218,13 @@ PlayerControl::EnqueueSong(Song *song) } bool -PlayerControl::Seek(Song *song, float seek_time) +PlayerControl::Seek(DetachedSong *song, float seek_time) { assert(song != nullptr); Lock(); - if (next_song != nullptr) - next_song->Free(); - + delete next_song; next_song = song; seek_where = seek_time; SynchronousCommand(PlayerCommand::SEEK); diff --git a/src/PlayerControl.hxx b/src/PlayerControl.hxx index 61bb408d2..6919d2cb1 100644 --- a/src/PlayerControl.hxx +++ b/src/PlayerControl.hxx @@ -29,7 +29,7 @@ #include <stdint.h> -struct Song; +class DetachedSong; enum class PlayerState : uint8_t { STOP, @@ -131,16 +131,16 @@ struct PlayerControl { Error error; /** - * A copy of the current #Song after its tags have been - * updated by the decoder (for example, a radio stream that - * has sent a new tag after switching to the next song). This - * shall be used by the GlobalEvents::TAG handler to update - * the current #Song in the queue. + * A copy of the current #DetachedSong after its tags have + * been updated by the decoder (for example, a radio stream + * that has sent a new tag after switching to the next song). + * This shall be used by the GlobalEvents::TAG handler to + * update the current #DetachedSong in the queue. * * Protected by #mutex. Set by the PlayerThread and consumed * by the main thread. */ - Song *tagged_song; + DetachedSong *tagged_song; uint16_t bit_rate; AudioFormat audio_format; @@ -153,7 +153,7 @@ struct PlayerControl { * This is a duplicate, and must be freed when this attribute * is cleared. */ - Song *next_song; + DetachedSong *next_song; double seek_where; @@ -299,7 +299,7 @@ public: * @param song the song to be queued; the given instance will * be owned and freed by the player */ - void Play(Song *song); + void Play(DetachedSong *song); /** * see PlayerCommand::CANCEL @@ -371,9 +371,9 @@ public: /** * Set the #tagged_song attribute to a newly allocated copy of - * the given #Song. Locks and unlocks the object. + * the given #DetachedSong. Locks and unlocks the object. */ - void LockSetTaggedSong(const Song &song); + void LockSetTaggedSong(const DetachedSong &song); void ClearTaggedSong(); @@ -382,8 +382,8 @@ public: * * Caller must lock the object. */ - Song *ReadTaggedSong() { - Song *result = tagged_song; + DetachedSong *ReadTaggedSong() { + DetachedSong *result = tagged_song; tagged_song = nullptr; return result; } @@ -391,9 +391,9 @@ public: /** * Like ReadTaggedSong(), but locks and unlocks the object. */ - Song *LockReadTaggedSong() { + DetachedSong *LockReadTaggedSong() { Lock(); - Song *result = ReadTaggedSong(); + DetachedSong *result = ReadTaggedSong(); Unlock(); return result; } @@ -403,7 +403,7 @@ public: void UpdateAudio(); private: - void EnqueueSongLocked(Song *song) { + void EnqueueSongLocked(DetachedSong *song) { assert(song != nullptr); assert(next_song == nullptr); @@ -416,7 +416,7 @@ public: * @param song the song to be queued; the given instance will be owned * and freed by the player */ - void EnqueueSong(Song *song); + void EnqueueSong(DetachedSong *song); /** * Makes the player thread seek the specified song to a position. @@ -426,7 +426,7 @@ public: * @return true on success, false on failure (e.g. if MPD isn't * playing currently) */ - bool Seek(Song *song, float seek_time); + bool Seek(DetachedSong *song, float seek_time); void SetCrossFade(float cross_fade_seconds); diff --git a/src/PlayerThread.cxx b/src/PlayerThread.cxx index 84d248d7b..814269b50 100644 --- a/src/PlayerThread.cxx +++ b/src/PlayerThread.cxx @@ -24,7 +24,7 @@ #include "MusicPipe.hxx" #include "MusicBuffer.hxx" #include "MusicChunk.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "system/FatalError.hxx" #include "CrossFade.hxx" #include "PlayerControl.hxx" @@ -92,7 +92,7 @@ class Player { /** * the song currently being played */ - Song *song; + DetachedSong *song; /** * is cross fading enabled? @@ -290,12 +290,12 @@ Player::StartDecoder(MusicPipe &_pipe) assert(queued || pc.command == PlayerCommand::SEEK); assert(pc.next_song != nullptr); - unsigned start_ms = pc.next_song->start_ms; + unsigned start_ms = pc.next_song->GetStartMS(); if (pc.command == PlayerCommand::SEEK) start_ms += (unsigned)(pc.seek_where * 1000); - dc.Start(pc.next_song->DupDetached(), - start_ms, pc.next_song->end_ms, + dc.Start(new DetachedSong(*pc.next_song), + start_ms, pc.next_song->GetEndMS(), buffer, _pipe); } @@ -329,7 +329,7 @@ Player::WaitForDecoder() if (error.IsDefined()) { pc.SetError(PlayerError::DECODER, std::move(error)); - pc.next_song->Free(); + delete pc.next_song; pc.next_song = nullptr; pc.Unlock(); @@ -339,9 +339,7 @@ Player::WaitForDecoder() pc.ClearTaggedSong(); - if (song != nullptr) - song->Free(); - + delete song; song = pc.next_song; elapsed_time = 0.0; @@ -370,17 +368,20 @@ Player::WaitForDecoder() * indicated by the decoder plugin. */ static double -real_song_duration(const Song &song, double decoder_duration) +real_song_duration(const DetachedSong &song, double decoder_duration) { if (decoder_duration <= 0.0) /* the decoder plugin didn't provide information; fall back to Song::GetDuration() */ return song.GetDuration(); - if (song.end_ms > 0 && song.end_ms / 1000.0 < decoder_duration) - return (song.end_ms - song.start_ms) / 1000.0; + const unsigned start_ms = song.GetStartMS(); + const unsigned end_ms = song.GetEndMS(); + + if (end_ms > 0 && end_ms / 1000.0 < decoder_duration) + return (end_ms - start_ms) / 1000.0; - return decoder_duration - song.start_ms / 1000.0; + return decoder_duration - start_ms / 1000.0; } bool @@ -458,10 +459,10 @@ Player::CheckDecoderStartup() decoder_starting = false; if (!paused && !OpenOutput()) { - const auto uri = dc.song->GetURI(); FormatError(player_domain, "problems opening audio device " - "while playing \"%s\"", uri.c_str()); + "while playing \"%s\"", + dc.song->GetURI()); return true; } @@ -516,7 +517,7 @@ Player::SeekDecoder() { assert(pc.next_song != nullptr); - const unsigned start_ms = pc.next_song->start_ms; + const unsigned start_ms = pc.next_song->GetStartMS(); if (!dc.LockIsCurrentSong(*pc.next_song)) { /* the decoder is already decoding the "next" song - @@ -542,7 +543,7 @@ Player::SeekDecoder() ClearAndReplacePipe(dc.pipe); } - pc.next_song->Free(); + delete pc.next_song; pc.next_song = nullptr; queued = false; } @@ -658,7 +659,7 @@ Player::ProcessCommand() pc.Lock(); } - pc.next_song->Free(); + delete pc.next_song; pc.next_song = nullptr; queued = false; pc.CommandFinished(); @@ -681,17 +682,14 @@ Player::ProcessCommand() } static void -update_song_tag(PlayerControl &pc, Song &song, const Tag &new_tag) +update_song_tag(PlayerControl &pc, DetachedSong &song, const Tag &new_tag) { if (song.IsFile()) /* don't update tags of local files, only remote streams may change tags dynamically */ return; - Tag *old_tag = song.tag; - song.tag = new Tag(new_tag); - - delete old_tag; + song.SetTag(new_tag); pc.LockSetTaggedSong(song); @@ -713,7 +711,7 @@ update_song_tag(PlayerControl &pc, Song &song, const Tag &new_tag) */ static bool play_chunk(PlayerControl &pc, - Song &song, struct music_chunk *chunk, + DetachedSong &song, struct music_chunk *chunk, MusicBuffer &buffer, const AudioFormat format, Error &error) @@ -880,10 +878,7 @@ Player::SongBorder() { xfade_state = CrossFadeState::UNKNOWN; - { - const auto uri = song->GetURI(); - FormatDefault(player_domain, "played \"%s\"", uri.c_str()); - } + FormatDefault(player_domain, "played \"%s\"", song->GetURI()); ReplacePipe(dc.pipe); @@ -1079,9 +1074,8 @@ Player::Run() delete cross_fade_tag; if (song != nullptr) { - const auto uri = song->GetURI(); - FormatDefault(player_domain, "played \"%s\"", uri.c_str()); - song->Free(); + FormatDefault(player_domain, "played \"%s\"", song->GetURI()); + delete song; } pc.Lock(); @@ -1090,7 +1084,7 @@ Player::Run() if (queued) { assert(pc.next_song != nullptr); - pc.next_song->Free(); + delete pc.next_song; pc.next_song = nullptr; } @@ -1139,10 +1133,8 @@ player_task(void *arg) /* fall through */ case PlayerCommand::PAUSE: - if (pc.next_song != nullptr) { - pc.next_song->Free(); - pc.next_song = nullptr; - } + delete pc.next_song; + pc.next_song = nullptr; pc.CommandFinished(); break; @@ -1177,10 +1169,8 @@ player_task(void *arg) return; case PlayerCommand::CANCEL: - if (pc.next_song != nullptr) { - pc.next_song->Free(); - pc.next_song = nullptr; - } + delete pc.next_song; + pc.next_song = nullptr; pc.CommandFinished(); break; diff --git a/src/Playlist.cxx b/src/Playlist.cxx index f566527ac..0720e490d 100644 --- a/src/Playlist.cxx +++ b/src/Playlist.cxx @@ -21,23 +21,23 @@ #include "Playlist.hxx" #include "PlaylistError.hxx" #include "PlayerControl.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "Idle.hxx" #include "Log.hxx" #include <assert.h> void -playlist::TagModified(Song &&song) +playlist::TagModified(DetachedSong &&song) { - if (!playing || song.tag == nullptr) + if (!playing) return; assert(current >= 0); - Song ¤t_song = queue.GetOrder(current); - if (SongEquals(song, current_song)) - current_song.ReplaceTag(std::move(*song.tag)); + DetachedSong ¤t_song = queue.GetOrder(current); + if (song.IsSame(current_song)) + current_song.MoveTagFrom(std::move(song)); queue.ModifyAtOrder(current); queue.IncrementVersion(); @@ -55,15 +55,12 @@ playlist_queue_song_order(playlist &playlist, PlayerControl &pc, playlist.queued = order; - Song *song = playlist.queue.GetOrder(order).DupDetached(); + const DetachedSong &song = playlist.queue.GetOrder(order); - { - const auto uri = song->GetURI(); - FormatDebug(playlist_domain, "queue song %i:\"%s\"", - playlist.queued, uri.c_str()); - } + FormatDebug(playlist_domain, "queue song %i:\"%s\"", + playlist.queued, song.GetURI()); - pc.EnqueueSong(song); + pc.EnqueueSong(new DetachedSong(song)); } /** @@ -88,7 +85,7 @@ playlist_song_started(playlist &playlist, PlayerControl &pc) idle_add(IDLE_PLAYER); } -const Song * +const DetachedSong * playlist::GetQueuedSong() const { return playing && queued >= 0 @@ -97,7 +94,7 @@ playlist::GetQueuedSong() const } void -playlist::UpdateQueuedSong(PlayerControl &pc, const Song *prev) +playlist::UpdateQueuedSong(PlayerControl &pc, const DetachedSong *prev) { if (!playing) return; @@ -124,7 +121,7 @@ playlist::UpdateQueuedSong(PlayerControl &pc, const Song *prev) current = queue.PositionToOrder(current_position); } - const Song *const next_song = next_order >= 0 + const DetachedSong *const next_song = next_order >= 0 ? &queue.GetOrder(next_order) : nullptr; @@ -148,15 +145,11 @@ playlist::PlayOrder(PlayerControl &pc, int order) playing = true; queued = -1; - Song *song = queue.GetOrder(order).DupDetached(); + const DetachedSong &song = queue.GetOrder(order); - { - const auto uri = song->GetURI(); - FormatDebug(playlist_domain, "play %i:\"%s\"", - order, uri.c_str()); - } + FormatDebug(playlist_domain, "play %i:\"%s\"", order, song.GetURI()); - pc.Play(song); + pc.Play(new DetachedSong(song)); current = order; } @@ -173,7 +166,7 @@ playlist::SyncWithPlayer(PlayerControl &pc) pc.Lock(); const PlayerState pc_state = pc.GetState(); - const Song *pc_next_song = pc.next_song; + const DetachedSong *pc_next_song = pc.next_song; pc.Unlock(); if (pc_state == PlayerState::STOP) @@ -286,7 +279,7 @@ playlist::SetRandom(PlayerControl &pc, bool status) if (status == queue.random) return; - const Song *const queued_song = GetQueuedSong(); + const DetachedSong *const queued_song = GetQueuedSong(); queue.random = status; diff --git a/src/Playlist.hxx b/src/Playlist.hxx index 2116abe7c..e578bfcee 100644 --- a/src/Playlist.hxx +++ b/src/Playlist.hxx @@ -25,7 +25,7 @@ enum TagType : uint8_t; struct PlayerControl; -struct Song; +class DetachedSong; class Error; struct playlist { @@ -100,7 +100,7 @@ struct playlist { * none if there is none (yet?) or if MPD isn't playing. */ gcc_pure - const Song *GetQueuedSong() const; + const DetachedSong *GetQueuedSong() const; /** * This is the "PLAYLIST" event handler. It is invoked by the @@ -125,7 +125,7 @@ protected: * @param prev the song which was previously queued, as * determined by playlist_get_queued_song() */ - void UpdateQueuedSong(PlayerControl &pc, const Song *prev); + void UpdateQueuedSong(PlayerControl &pc, const DetachedSong *prev); public: void Clear(PlayerControl &pc); @@ -135,7 +135,7 @@ public: * thread. Apply the given song's tag to the current song if * the song matches. */ - void TagModified(Song &&song); + void TagModified(DetachedSong &&song); /** * The database has been modified. Pull all updates. @@ -143,7 +143,7 @@ public: void DatabaseModified(); PlaylistResult AppendSong(PlayerControl &pc, - Song *song, + DetachedSong &&song, unsigned *added_id=nullptr); /** @@ -162,7 +162,7 @@ public: protected: void DeleteInternal(PlayerControl &pc, - unsigned song, const Song **queued_p); + unsigned song, const DetachedSong **queued_p); public: PlaylistResult DeletePosition(PlayerControl &pc, @@ -184,7 +184,7 @@ public: PlaylistResult DeleteRange(PlayerControl &pc, unsigned start, unsigned end); - void DeleteSong(PlayerControl &pc, const Song &song); + void DeleteSong(PlayerControl &pc, const char *uri); void Shuffle(PlayerControl &pc, unsigned start, unsigned end); diff --git a/src/PlaylistControl.cxx b/src/PlaylistControl.cxx index 2dbd75d6e..9ba7f7485 100644 --- a/src/PlaylistControl.cxx +++ b/src/PlaylistControl.cxx @@ -26,7 +26,7 @@ #include "Playlist.hxx" #include "PlaylistError.hxx" #include "PlayerControl.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "Log.hxx" void @@ -195,7 +195,7 @@ playlist::SeekSongPosition(PlayerControl &pc, unsigned song, float seek_time) if (!queue.IsValidPosition(song)) return PlaylistResult::BAD_RANGE; - const Song *queued_song = GetQueuedSong(); + const DetachedSong *queued_song = GetQueuedSong(); unsigned i = queue.random ? queue.PositionToOrder(song) @@ -215,8 +215,7 @@ playlist::SeekSongPosition(PlayerControl &pc, unsigned song, float seek_time) queued_song = nullptr; } - Song *the_song = queue.GetOrder(i).DupDetached(); - if (!pc.Seek(the_song, seek_time)) { + if (!pc.Seek(new DetachedSong(queue.GetOrder(i)), seek_time)) { UpdateQueuedSong(pc, queued_song); return PlaylistResult::NOT_PLAYING; diff --git a/src/PlaylistEdit.cxx b/src/PlaylistEdit.cxx index 668612a1a..a55f9b151 100644 --- a/src/PlaylistEdit.cxx +++ b/src/PlaylistEdit.cxx @@ -30,6 +30,7 @@ #include "util/UriUtil.hxx" #include "util/Error.hxx" #include "Song.hxx" +#include "DetachedSong.hxx" #include "Idle.hxx" #include "DatabaseGlue.hxx" #include "DatabasePlugin.hxx" @@ -64,23 +65,23 @@ playlist::AppendFile(PlayerControl &pc, if (song == nullptr) return PlaylistResult::NO_SUCH_SONG; - const auto result = AppendSong(pc, song, added_id); + const auto result = AppendSong(pc, DetachedSong(*song), added_id); song->Free(); return result; } PlaylistResult playlist::AppendSong(PlayerControl &pc, - Song *song, unsigned *added_id) + DetachedSong &&song, unsigned *added_id) { unsigned id; if (queue.IsFull()) return PlaylistResult::TOO_LARGE; - const Song *const queued_song = GetQueuedSong(); + const DetachedSong *const queued_song = GetQueuedSong(); - id = queue.Append(song, 0); + id = queue.Append(std::move(song), 0); if (queue.random) { /* shuffle the new song into the list of remaining @@ -110,25 +111,24 @@ playlist::AppendURI(PlayerControl &pc, { FormatDebug(playlist_domain, "add to playlist: %s", uri); - const Database *db = nullptr; - Song *song; + DetachedSong *song; if (uri_has_scheme(uri)) { - song = Song::NewRemote(uri); + song = new DetachedSong(uri); } else { - db = GetDatabase(); + const Database *db = GetDatabase(); if (db == nullptr) return PlaylistResult::NO_SUCH_SONG; - song = db->GetSong(uri, IgnoreError()); - if (song == nullptr) + Song *tmp = db->GetSong(uri, IgnoreError()); + if (tmp == nullptr) return PlaylistResult::NO_SUCH_SONG; + + song = new DetachedSong(*tmp); + db->ReturnSong(tmp); } - PlaylistResult result = AppendSong(pc, song, added_id); - if (db != nullptr) - db->ReturnSong(song); - else - song->Free(); + PlaylistResult result = AppendSong(pc, std::move(*song), added_id); + delete song; return result; } @@ -139,7 +139,7 @@ playlist::SwapPositions(PlayerControl &pc, unsigned song1, unsigned song2) if (!queue.IsValidPosition(song1) || !queue.IsValidPosition(song2)) return PlaylistResult::BAD_RANGE; - const Song *const queued_song = GetQueuedSong(); + const DetachedSong *const queued_song = GetQueuedSong(); queue.SwapPositions(song1, song2); @@ -193,7 +193,7 @@ playlist::SetPriorityRange(PlayerControl &pc, /* remember "current" and "queued" */ const int current_position = GetCurrentPosition(); - const Song *const queued_song = GetQueuedSong(); + const DetachedSong *const queued_song = GetQueuedSong(); /* apply the priority changes */ @@ -225,7 +225,7 @@ playlist::SetPriorityId(PlayerControl &pc, void playlist::DeleteInternal(PlayerControl &pc, - unsigned song, const Song **queued_p) + unsigned song, const DetachedSong **queued_p) { assert(song < GetLength()); @@ -275,7 +275,7 @@ playlist::DeletePosition(PlayerControl &pc, unsigned song) if (song >= queue.GetLength()) return PlaylistResult::BAD_RANGE; - const Song *queued_song = GetQueuedSong(); + const DetachedSong *queued_song = GetQueuedSong(); DeleteInternal(pc, song, &queued_song); @@ -297,7 +297,7 @@ playlist::DeleteRange(PlayerControl &pc, unsigned start, unsigned end) if (start >= end) return PlaylistResult::SUCCESS; - const Song *queued_song = GetQueuedSong(); + const DetachedSong *queued_song = GetQueuedSong(); do { DeleteInternal(pc, --end, &queued_song); @@ -320,10 +320,10 @@ playlist::DeleteId(PlayerControl &pc, unsigned id) } void -playlist::DeleteSong(PlayerControl &pc, const struct Song &song) +playlist::DeleteSong(PlayerControl &pc, const char *uri) { for (int i = queue.GetLength() - 1; i >= 0; --i) - if (SongEquals(song, queue.Get(i))) + if (queue.Get(i).IsURI(uri)) DeletePosition(pc, i); } @@ -341,7 +341,7 @@ playlist::MoveRange(PlayerControl &pc, unsigned start, unsigned end, int to) /* nothing happens */ return PlaylistResult::SUCCESS; - const Song *const queued_song = GetQueuedSong(); + const DetachedSong *const queued_song = GetQueuedSong(); /* * (to < 0) => move to offset from current song @@ -401,7 +401,7 @@ playlist::Shuffle(PlayerControl &pc, unsigned start, unsigned end) /* needs at least two entries. */ return; - const Song *const queued_song = GetQueuedSong(); + const DetachedSong *const queued_song = GetQueuedSong(); if (playing && current >= 0) { unsigned current_position = queue.OrderToPosition(current); diff --git a/src/PlaylistFile.cxx b/src/PlaylistFile.cxx index 3f9845166..0f84cc4eb 100644 --- a/src/PlaylistFile.cxx +++ b/src/PlaylistFile.cxx @@ -24,7 +24,7 @@ #include "PlaylistVector.hxx" #include "DatabasePlugin.hxx" #include "DatabaseGlue.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "Mapper.hxx" #include "fs/TextFile.hxx" #include "ConfigGlobal.hxx" @@ -361,7 +361,7 @@ spl_remove_index(const char *utf8path, unsigned pos, Error &error) } bool -spl_append_song(const char *utf8path, const Song &song, Error &error) +spl_append_song(const char *utf8path, const DetachedSong &song, Error &error) { if (spl_map(error).IsNull()) return false; @@ -402,22 +402,21 @@ bool spl_append_uri(const char *url, const char *utf8file, Error &error) { if (uri_has_scheme(url)) { - Song *song = Song::NewRemote(url); - bool success = spl_append_song(utf8file, *song, error); - song->Free(); - return success; + return spl_append_song(utf8file, DetachedSong(url), + error); } else { const Database *db = GetDatabase(error); if (db == nullptr) return false; - Song *song = db->GetSong(url, error); - if (song == nullptr) + Song *tmp = db->GetSong(url, error); + if (tmp == nullptr) return false; - bool success = spl_append_song(utf8file, *song, error); - db->ReturnSong(song); - return success; + const DetachedSong song(*tmp); + db->ReturnSong(tmp); + + return spl_append_song(utf8file, song, error); } } diff --git a/src/PlaylistFile.hxx b/src/PlaylistFile.hxx index 92030b1eb..33fe00eb7 100644 --- a/src/PlaylistFile.hxx +++ b/src/PlaylistFile.hxx @@ -23,7 +23,7 @@ #include <vector> #include <string> -struct Song; +class DetachedSong; class PlaylistVector; class Error; @@ -68,7 +68,7 @@ bool spl_remove_index(const char *utf8path, unsigned pos, Error &error); bool -spl_append_song(const char *utf8path, const Song &song, Error &error); +spl_append_song(const char *utf8path, const DetachedSong &song, Error &error); bool spl_append_uri(const char *file, const char *utf8file, Error &error); diff --git a/src/PlaylistPrint.cxx b/src/PlaylistPrint.cxx index e92d6e18a..cee64f859 100644 --- a/src/PlaylistPrint.cxx +++ b/src/PlaylistPrint.cxx @@ -31,6 +31,7 @@ #include "Client.hxx" #include "InputStream.hxx" #include "Song.hxx" +#include "DetachedSong.hxx" #include "fs/Traits.hxx" #include "util/Error.hxx" #include "thread/Cond.hxx" @@ -151,7 +152,7 @@ playlist_provider_print(Client &client, const char *uri, ? PathTraitsUTF8::GetParent(uri) : std::string("."); - Song *song; + DetachedSong *song; while ((song = e.NextSong()) != nullptr) { song = playlist_check_translate_song(song, base_uri.c_str(), false); @@ -163,7 +164,7 @@ playlist_provider_print(Client &client, const char *uri, else song_print_uri(client, *song); - song->Free(); + delete song; } } diff --git a/src/PlaylistQueue.cxx b/src/PlaylistQueue.cxx index 3a7660134..d2dce977a 100644 --- a/src/PlaylistQueue.cxx +++ b/src/PlaylistQueue.cxx @@ -24,7 +24,7 @@ #include "Playlist.hxx" #include "InputStream.hxx" #include "SongEnumerator.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "thread/Cond.hxx" #include "fs/Traits.hxx" @@ -38,13 +38,13 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e, ? PathTraitsUTF8::GetParent(uri) : std::string("."); - Song *song; + DetachedSong *song; for (unsigned i = 0; i < end_index && (song = e.NextSong()) != nullptr; ++i) { if (i < start_index) { /* skip songs before the start index */ - song->Free(); + delete song; continue; } @@ -53,8 +53,8 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e, if (song == nullptr) continue; - PlaylistResult result = dest.AppendSong(pc, song); - song->Free(); + PlaylistResult result = dest.AppendSong(pc, std::move(*song)); + delete song; if (result != PlaylistResult::SUCCESS) return result; } diff --git a/src/PlaylistSave.cxx b/src/PlaylistSave.cxx index 02195bd15..3bc66d16e 100644 --- a/src/PlaylistSave.cxx +++ b/src/PlaylistSave.cxx @@ -23,6 +23,7 @@ #include "PlaylistError.hxx" #include "Playlist.hxx" #include "Song.hxx" +#include "DetachedSong.hxx" #include "Mapper.hxx" #include "Idle.hxx" #include "fs/AllocatedPath.hxx" @@ -36,7 +37,7 @@ #include <string.h> void -playlist_print_song(FILE *file, const Song &song) +playlist_print_song(FILE *file, const DetachedSong &song) { if (playlist_saveAbsolutePaths && song.IsInDatabase()) { const auto path = map_song_fs(song); @@ -44,7 +45,7 @@ playlist_print_song(FILE *file, const Song &song) fprintf(file, "%s\n", path.c_str()); } else { const auto uri_utf8 = song.GetURI(); - const auto uri_fs = AllocatedPath::FromUTF8(uri_utf8.c_str()); + const auto uri_fs = AllocatedPath::FromUTF8(uri_utf8); if (!uri_fs.IsNull()) fprintf(file, "%s\n", uri_fs.c_str()); diff --git a/src/PlaylistSave.hxx b/src/PlaylistSave.hxx index 70b40f3fa..d9126aed2 100644 --- a/src/PlaylistSave.hxx +++ b/src/PlaylistSave.hxx @@ -24,14 +24,14 @@ #include <stdio.h> -struct Song; struct queue; struct playlist; struct PlayerControl; +class DetachedSong; class Error; void -playlist_print_song(FILE *fp, const Song &song); +playlist_print_song(FILE *file, const DetachedSong &song); void playlist_print_uri(FILE *fp, const char *uri); diff --git a/src/PlaylistSong.cxx b/src/PlaylistSong.cxx index f016e46b6..2da644bc2 100644 --- a/src/PlaylistSong.cxx +++ b/src/PlaylistSong.cxx @@ -24,41 +24,42 @@ #include "DatabaseGlue.hxx" #include "ls.hxx" #include "tag/Tag.hxx" +#include "tag/TagBuilder.hxx" #include "fs/AllocatedPath.hxx" #include "fs/Traits.hxx" #include "util/UriUtil.hxx" #include "util/Error.hxx" +#include "DetachedSong.hxx" #include "Song.hxx" #include <assert.h> #include <string.h> static void -merge_song_metadata(Song *dest, const Song *base, - const Song *add) +merge_song_metadata(DetachedSong *dest, const DetachedSong *base, + const DetachedSong *add) { - dest->tag = base->tag != nullptr - ? (add->tag != nullptr - ? Tag::Merge(*base->tag, *add->tag) - : new Tag(*base->tag)) - : (add->tag != nullptr - ? new Tag(*add->tag) - : nullptr); - - dest->mtime = base->mtime; - dest->start_ms = add->start_ms; - dest->end_ms = add->end_ms; + { + TagBuilder builder(add->GetTag()); + builder.Complement(base->GetTag()); + dest->SetTag(builder.Commit()); + } + + dest->SetLastModified(base->GetLastModified()); + dest->SetStartMS(add->GetStartMS()); + dest->SetEndMS(add->GetEndMS()); } -static Song * -apply_song_metadata(Song *dest, const Song *src) +static DetachedSong * +apply_song_metadata(DetachedSong *dest, const DetachedSong *src) { - Song *tmp; + DetachedSong *tmp; assert(dest != nullptr); assert(src != nullptr); - if (src->tag == nullptr && src->start_ms == 0 && src->end_ms == 0) + if (!src->GetTag().IsDefined() && + src->GetStartMS() == 0 && src->GetEndMS() == 0) return dest; if (dest->IsInDatabase()) { @@ -70,37 +71,41 @@ apply_song_metadata(Song *dest, const Song *src) if (path_utf8.empty()) path_utf8 = path_fs.c_str(); - tmp = Song::NewFile(path_utf8.c_str(), nullptr); + tmp = new DetachedSong(std::move(path_utf8)); merge_song_metadata(tmp, dest, src); } else { - tmp = Song::NewFile(dest->uri, nullptr); + tmp = new DetachedSong(dest->GetURI()); merge_song_metadata(tmp, dest, src); } - if (dest->tag != nullptr && dest->tag->time > 0 && - src->start_ms > 0 && src->end_ms == 0 && - src->start_ms / 1000 < (unsigned)dest->tag->time) + if (dest->GetTag().IsDefined() && dest->GetTag().time > 0 && + src->GetStartMS() > 0 && src->GetEndMS() == 0 && + src->GetStartMS() / 1000 < (unsigned)dest->GetTag().time) /* the range is open-ended, and the playlist plugin did not know the total length of the song file (e.g. last track on a CUE file); fix it up here */ - tmp->tag->time = dest->tag->time - src->start_ms / 1000; + tmp->WritableTag().time = + dest->GetTag().time - src->GetStartMS() / 1000; - dest->Free(); + delete dest; return tmp; } -static Song * -playlist_check_load_song(const Song *song, const char *uri, bool secure) +static DetachedSong * +playlist_check_load_song(const DetachedSong *song, const char *uri, bool secure) { - Song *dest; + DetachedSong *dest; if (uri_has_scheme(uri)) { - dest = Song::NewRemote(uri); + dest = new DetachedSong(uri); } else if (PathTraitsUTF8::IsAbsolute(uri) && secure) { - dest = Song::LoadFile(uri, nullptr); - if (dest == nullptr) + Song *tmp = Song::LoadFile(uri, nullptr); + if (tmp == nullptr) return nullptr; + + dest = new DetachedSong(*tmp); + delete tmp; } else { const Database *db = GetDatabase(); if (db == nullptr) @@ -111,22 +116,22 @@ playlist_check_load_song(const Song *song, const char *uri, bool secure) /* not found in database */ return nullptr; - dest = tmp->DupDetached(); + dest = new DetachedSong(*tmp); db->ReturnSong(tmp); } return apply_song_metadata(dest, song); } -Song * -playlist_check_translate_song(Song *song, const char *base_uri, +DetachedSong * +playlist_check_translate_song(DetachedSong *song, const char *base_uri, bool secure) { if (song->IsInDatabase()) /* already ok */ return song; - const char *uri = song->uri; + const char *uri = song->GetURI(); if (uri_has_scheme(uri)) { if (uri_supported_scheme(uri)) @@ -134,7 +139,7 @@ playlist_check_translate_song(Song *song, const char *base_uri, return song; else { /* unsupported remote song */ - song->Free(); + delete song; return nullptr; } } @@ -156,7 +161,7 @@ playlist_check_translate_song(Song *song, const char *base_uri, else if (!secure) { /* local files must be relative to the music directory when "secure" is enabled */ - song->Free(); + delete song; return nullptr; } @@ -169,8 +174,8 @@ playlist_check_translate_song(Song *song, const char *base_uri, uri = full_uri.c_str(); } - Song *dest = playlist_check_load_song(song, uri, secure); - song->Free(); + DetachedSong *dest = playlist_check_load_song(song, uri, secure); + delete song; return dest; } diff --git a/src/PlaylistSong.hxx b/src/PlaylistSong.hxx index d0db99868..9cfb09b24 100644 --- a/src/PlaylistSong.hxx +++ b/src/PlaylistSong.hxx @@ -20,7 +20,7 @@ #ifndef MPD_PLAYLIST_SONG_HXX #define MPD_PLAYLIST_SONG_HXX -struct Song; +class DetachedSong; /** * Verifies the song, returns nullptr if it is unsafe. Translate the @@ -30,8 +30,8 @@ struct Song; * @param secure if true, then local files are only allowed if they * are relative to base_uri */ -Song * -playlist_check_translate_song(Song *song, const char *base_uri, +DetachedSong * +playlist_check_translate_song(DetachedSong *song, const char *base_uri, bool secure); #endif diff --git a/src/PlaylistTag.cxx b/src/PlaylistTag.cxx index 2897f3252..672111ea5 100644 --- a/src/PlaylistTag.cxx +++ b/src/PlaylistTag.cxx @@ -26,7 +26,7 @@ #include "config.h" #include "Playlist.hxx" #include "PlaylistError.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "tag/Tag.hxx" #include "tag/TagBuilder.hxx" #include "util/Error.hxx" @@ -42,22 +42,19 @@ playlist::AddSongIdTag(unsigned id, TagType tag_type, const char *value, return false; } - Song &song = queue.Get(position); + DetachedSong &song = queue.Get(position); if (song.IsFile()) { error.Set(playlist_domain, int(PlaylistResult::DENIED), "Cannot edit tags of local file"); return false; } - TagBuilder tag; - if (song.tag != nullptr) { - tag = std::move(*song.tag); - delete song.tag; + { + TagBuilder tag(std::move(song.WritableTag())); + tag.AddItem(tag_type, value); + song.SetTag(tag.Commit()); } - tag.AddItem(tag_type, value); - song.tag = tag.CommitNew(); - queue.ModifyAtPosition(position); OnModified(); return true; @@ -74,24 +71,21 @@ playlist::ClearSongIdTag(unsigned id, TagType tag_type, return false; } - Song &song = queue.Get(position); + DetachedSong &song = queue.Get(position); if (song.IsFile()) { error.Set(playlist_domain, int(PlaylistResult::DENIED), "Cannot edit tags of local file"); return false; } - if (song.tag == nullptr) - return true; - - TagBuilder tag(std::move(*song.tag)); - delete song.tag; - - if (tag_type == TAG_NUM_OF_ITEM_TYPES) - tag.RemoveAll(); - else - tag.RemoveType(tag_type); - song.tag = tag.CommitNew(); + { + TagBuilder tag(std::move(song.WritableTag())); + if (tag_type == TAG_NUM_OF_ITEM_TYPES) + tag.RemoveAll(); + else + tag.RemoveType(tag_type); + song.SetTag(tag.Commit()); + } queue.ModifyAtPosition(position); OnModified(); diff --git a/src/PlaylistUpdate.cxx b/src/PlaylistUpdate.cxx index 0e72ef671..91adc01b7 100644 --- a/src/PlaylistUpdate.cxx +++ b/src/PlaylistUpdate.cxx @@ -22,35 +22,36 @@ #include "DatabaseGlue.hxx" #include "DatabasePlugin.hxx" #include "Song.hxx" +#include "DetachedSong.hxx" #include "tag/Tag.hxx" #include "Idle.hxx" #include "util/Error.hxx" static bool -UpdatePlaylistSong(const Database &db, Song &song) +UpdatePlaylistSong(const Database &db, DetachedSong &song) { - if (!song.IsInDatabase() || !song.IsDetached()) + if (!song.IsInDatabase()) /* only update Songs instances that are "detached" from the Database */ return false; - Song *original = db.GetSong(song.uri, IgnoreError()); + Song *original = db.GetSong(song.GetURI(), IgnoreError()); if (original == nullptr) /* not found - shouldn't happen, because the update thread should ensure that all stale Song instances have been purged */ return false; - if (original->mtime == song.mtime) { + if (original->mtime == song.GetLastModified()) { /* not modified */ db.ReturnSong(original); return false; } - song.mtime = original->mtime; + song.SetLastModified(original->mtime); if (original->tag != nullptr) - song.ReplaceTag(Tag(*original->tag)); + song.SetTag(*original->tag); db.ReturnSong(original); return true; diff --git a/src/Queue.cxx b/src/Queue.cxx index 92f92f77c..d4d6531f9 100644 --- a/src/Queue.cxx +++ b/src/Queue.cxx @@ -19,7 +19,7 @@ #include "config.h" #include "Queue.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" queue::queue(unsigned _max_length) :max_length(_max_length), length(0), @@ -84,7 +84,7 @@ queue::ModifyAtOrder(unsigned _order) } unsigned -queue::Append(Song *song, uint8_t priority) +queue::Append(DetachedSong &&song, uint8_t priority) { assert(!IsFull()); @@ -92,7 +92,7 @@ queue::Append(Song *song, uint8_t priority) const unsigned id = id_table.Insert(position); auto &item = items[position]; - item.song = song->DupDetached(); + item.song = new DetachedSong(std::move(song)); item.id = id; item.version = version; item.priority = priority; @@ -219,11 +219,7 @@ queue::DeletePosition(unsigned position) { assert(position < length); - { - Song &song = Get(position); - assert(!song.IsInDatabase() || song.IsDetached()); - song.Free(); - } + delete items[position].song; const unsigned id = PositionToId(position); const unsigned _order = PositionToOrder(position); @@ -257,9 +253,7 @@ queue::Clear() for (unsigned i = 0; i < length; i++) { Item *item = &items[i]; - assert(!item->song->IsInDatabase() || - item->song->IsDetached()); - item->song->Free(); + delete item->song; id_table.Erase(item->id); } diff --git a/src/Queue.hxx b/src/Queue.hxx index da90d4a3d..8f893dbfa 100644 --- a/src/Queue.hxx +++ b/src/Queue.hxx @@ -29,7 +29,7 @@ #include <assert.h> #include <stdint.h> -struct Song; +class DetachedSong; /** * A queue of songs. This is the backend of the playlist: it contains @@ -53,7 +53,7 @@ struct queue { * information attached. */ struct Item { - Song *song; + DetachedSong *song; /** the unique id of this item in the queue */ unsigned id; @@ -200,7 +200,7 @@ struct queue { /** * Returns the song at the specified position. */ - Song &Get(unsigned position) const { + DetachedSong &Get(unsigned position) const { assert(position < length); return *items[position].song; @@ -209,7 +209,7 @@ struct queue { /** * Returns the song at the specified order number. */ - Song &GetOrder(unsigned _order) const { + DetachedSong &GetOrder(unsigned _order) const { return Get(OrderToPosition(_order)); } @@ -268,7 +268,7 @@ struct queue { * * @param priority the priority of this new queue item */ - unsigned Append(Song *song, uint8_t priority); + unsigned Append(DetachedSong &&song, uint8_t priority); /** * Swaps two songs, addressed by their position. diff --git a/src/QueuePrint.cxx b/src/QueuePrint.cxx index fec4a36fb..ab00331b0 100644 --- a/src/QueuePrint.cxx +++ b/src/QueuePrint.cxx @@ -94,7 +94,7 @@ queue_find(Client &client, const queue &queue, const SongFilter &filter) { for (unsigned i = 0; i < queue.GetLength(); i++) { - const Song &song = queue.Get(i); + const DetachedSong &song = queue.Get(i); if (filter.Match(song)) queue_print_song_info(client, queue, i); diff --git a/src/QueueSave.cxx b/src/QueueSave.cxx index f3fc7c6d1..b84c051bb 100644 --- a/src/QueueSave.cxx +++ b/src/QueueSave.cxx @@ -21,7 +21,7 @@ #include "QueueSave.hxx" #include "Queue.hxx" #include "PlaylistError.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "SongSave.hxx" #include "DatabasePlugin.hxx" #include "DatabaseGlue.hxx" @@ -37,20 +37,19 @@ #define PRIO_LABEL "Prio: " static void -queue_save_database_song(FILE *fp, int idx, const Song &song) +queue_save_database_song(FILE *fp, int idx, const DetachedSong &song) { - const auto uri = song.GetURI(); - fprintf(fp, "%i:%s\n", idx, uri.c_str()); + fprintf(fp, "%i:%s\n", idx, song.GetURI()); } static void -queue_save_full_song(FILE *fp, const Song &song) +queue_save_full_song(FILE *fp, const DetachedSong &song) { song_save(fp, song); } static void -queue_save_song(FILE *fp, int idx, const Song &song) +queue_save_song(FILE *fp, int idx, const DetachedSong &song) { if (song.IsInDatabase()) queue_save_database_song(fp, idx, song); @@ -85,8 +84,7 @@ queue_load_song(TextFile &file, const char *line, queue &queue) return; } - const Database *db = nullptr; - Song *song; + DetachedSong *song; if (StringStartsWith(line, SONG_BEGIN)) { const char *uri = line + sizeof(SONG_BEGIN) - 1; @@ -94,7 +92,7 @@ queue_load_song(TextFile &file, const char *line, queue &queue) return; Error error; - song = song_load(file, nullptr, uri, error); + song = song_load(file, uri, error); if (song == nullptr) { LogError(error); return; @@ -111,22 +109,21 @@ queue_load_song(TextFile &file, const char *line, queue &queue) const char *uri = endptr + 1; if (uri_has_scheme(uri)) { - song = Song::NewRemote(uri); + song = new DetachedSong(uri); } else { - db = GetDatabase(); + const Database *db = GetDatabase(); if (db == nullptr) return; - song = db->GetSong(uri, IgnoreError()); - if (song == nullptr) + Song *tmp = db->GetSong(uri, IgnoreError()); + if (tmp == nullptr) return; + + song = new DetachedSong(*tmp); + db->ReturnSong(tmp); } } - queue.Append(song, priority); - - if (db != nullptr) - db->ReturnSong(song); - else - song->Free(); + queue.Append(std::move(*song), priority); + delete song; } diff --git a/src/Song.cxx b/src/Song.cxx index ae0c70ab9..bb74a6951 100644 --- a/src/Song.cxx +++ b/src/Song.cxx @@ -22,13 +22,12 @@ #include "Directory.hxx" #include "tag/Tag.hxx" #include "util/Alloc.hxx" +#include "DetachedSong.hxx" #include <assert.h> #include <string.h> #include <stdlib.h> -Directory detached_root; - static Song * song_alloc(const char *uri, Directory *parent) { @@ -51,57 +50,22 @@ song_alloc(const char *uri, Directory *parent) } Song * -Song::NewRemote(const char *uri) +Song::NewFrom(DetachedSong &&other, Directory *parent) { - return song_alloc(uri, nullptr); + Song *song = song_alloc(other.GetURI(), parent); + song->tag = new Tag(std::move(other.WritableTag())); + song->mtime = other.GetLastModified(); + song->start_ms = other.GetStartMS(); + song->end_ms = other.GetEndMS(); + return song; } Song * Song::NewFile(const char *path, Directory *parent) { - assert((parent == nullptr) == (*path == '/')); - return song_alloc(path, parent); } -Song * -Song::ReplaceURI(const char *new_uri) -{ - Song *new_song = song_alloc(new_uri, parent); - new_song->tag = tag; - new_song->mtime = mtime; - new_song->start_ms = start_ms; - new_song->end_ms = end_ms; - free(this); - return new_song; -} - -Song * -Song::NewDetached(const char *uri) -{ - assert(uri != nullptr); - - return song_alloc(uri, &detached_root); -} - -Song * -Song::DupDetached() const -{ - Song *song; - if (IsInDatabase()) { - const auto new_uri = GetURI(); - song = NewDetached(new_uri.c_str()); - } else - song = song_alloc(uri, nullptr); - - song->tag = tag != nullptr ? new Tag(*tag) : nullptr; - song->mtime = mtime; - song->start_ms = start_ms; - song->end_ms = end_ms; - - return song; -} - void Song::Free() { @@ -109,54 +73,12 @@ Song::Free() free(this); } -void -Song::ReplaceTag(Tag &&_tag) -{ - if (tag == nullptr) - tag = new Tag(); - *tag = std::move(_tag); -} - -gcc_pure -static inline bool -directory_equals(const Directory &a, const Directory &b) -{ - return strcmp(a.path, b.path) == 0; -} - -gcc_pure -static inline bool -directory_is_same(const Directory *a, const Directory *b) -{ - return a == b || - (a != nullptr && b != nullptr && - directory_equals(*a, *b)); - -} - -bool -SongEquals(const Song &a, const Song &b) -{ - if (a.parent != nullptr && b.parent != nullptr && - !directory_equals(*a.parent, *b.parent) && - (a.parent == &detached_root || b.parent == &detached_root)) { - /* must compare the full URI if one of the objects is - "detached" */ - const auto au = a.GetURI(); - const auto bu = b.GetURI(); - return au == bu; - } - - return directory_is_same(a.parent, b.parent) && - strcmp(a.uri, b.uri) == 0; -} - std::string Song::GetURI() const { assert(*uri); - if (!IsInDatabase() || parent->IsRoot()) + if (parent == nullptr || parent->IsRoot()) return std::string(uri); else { const char *path = parent->GetPath(); diff --git a/src/Song.hxx b/src/Song.hxx index 21e158560..c9719edd7 100644 --- a/src/Song.hxx +++ b/src/Song.hxx @@ -32,13 +32,12 @@ #define SONG_TIME "Time: " struct Tag; +struct Directory; +class DetachedSong; /** - * A dummy #directory instance that is used for "detached" song - * copies. + * A song file inside the configured music directory. */ -extern struct Directory detached_root; - struct Song { /** * Pointers to the siblings of this directory within the @@ -51,7 +50,14 @@ struct Song { struct list_head siblings; Tag *tag; + + /** + * The #Directory that contains this song. May be nullptr if + * the current database plugin does not manage the parent + * directory this way. + */ Directory *parent; + time_t mtime; /** @@ -65,11 +71,14 @@ struct Song { */ unsigned end_ms; + /** + * The file name. If #parent is nullptr, then this is the URI + * relative to the music directory. + */ char uri[sizeof(int)]; - /** allocate a new song with a remote URL */ gcc_malloc - static Song *NewRemote(const char *uri); + static Song *NewFrom(DetachedSong &&other, Directory *parent); /** allocate a new song with a local file name */ gcc_malloc @@ -87,47 +96,8 @@ struct Song { return LoadFile(path_utf8, &parent); } - /** - * Replaces the URI of a song object. The given song object - * is destroyed, and a newly allocated one is returned. It - * does not update the reference within the parent directory; - * the caller is responsible for doing that. - */ - gcc_malloc - Song *ReplaceURI(const char *uri); - - /** - * Creates a "detached" song object. - */ - gcc_malloc - static Song *NewDetached(const char *uri); - - /** - * Creates a duplicate of the song object. If the object is - * in the database, it creates a "detached" copy of this song, - * see Song::IsDetached(). - */ - gcc_malloc - Song *DupDetached() const; - void Free(); - bool IsInDatabase() const { - return parent != nullptr; - } - - bool IsFile() const { - return IsInDatabase() || uri[0] == '/'; - } - - bool IsDetached() const { - assert(IsInDatabase()); - - return parent == &detached_root; - } - - void ReplaceTag(Tag &&tag); - bool UpdateFile(); bool UpdateFileInArchive(); @@ -142,11 +112,4 @@ struct Song { double GetDuration() const; }; -/** - * Returns true if both objects refer to the same physical song. - */ -gcc_pure -bool -SongEquals(const Song &a, const Song &b); - #endif diff --git a/src/SongEnumerator.hxx b/src/SongEnumerator.hxx index 0e268a31a..2515647d1 100644 --- a/src/SongEnumerator.hxx +++ b/src/SongEnumerator.hxx @@ -20,7 +20,7 @@ #ifndef MPD_SONG_ENUMERATOR_HXX #define MPD_SONG_ENUMERATOR_HXX -struct Song; +class DetachedSong; /** * An object which provides serial access to a number of #Song @@ -35,7 +35,7 @@ public: * freeing the returned #Song object. Returns nullptr if * there are no more songs. */ - virtual Song *NextSong() = 0; + virtual DetachedSong *NextSong() = 0; }; #endif diff --git a/src/SongFilter.cxx b/src/SongFilter.cxx index 8d90c5fc8..53ab784c2 100644 --- a/src/SongFilter.cxx +++ b/src/SongFilter.cxx @@ -20,6 +20,7 @@ #include "config.h" #include "SongFilter.hxx" #include "Song.hxx" +#include "DetachedSong.hxx" #include "tag/Tag.hxx" #include "util/ASCII.hxx" #include "util/UriUtil.hxx" @@ -151,6 +152,18 @@ SongFilter::Item::Match(const Song &song) const return song.tag != NULL && Match(*song.tag); } +bool +SongFilter::Item::Match(const DetachedSong &song) const +{ + if (tag == LOCATE_TAG_BASE_TYPE) + return uri_is_child_or_same(value.c_str(), song.GetURI()); + + if (tag == LOCATE_TAG_FILE_TYPE) + return StringMatch(song.GetURI()); + + return Match(song.GetTag()); +} + SongFilter::SongFilter(unsigned tag, const char *value, bool fold_case) { items.push_back(Item(tag, value, fold_case)); @@ -203,6 +216,16 @@ SongFilter::Match(const Song &song) const return true; } +bool +SongFilter::Match(const DetachedSong &song) const +{ + for (const auto &i : items) + if (!i.Match(song)) + return false; + + return true; +} + std::string SongFilter::GetBase() const { diff --git a/src/SongFilter.hxx b/src/SongFilter.hxx index b15127c07..16970c350 100644 --- a/src/SongFilter.hxx +++ b/src/SongFilter.hxx @@ -38,6 +38,7 @@ struct Tag; struct TagItem; struct Song; +class DetachedSong; class SongFilter { public: @@ -80,6 +81,9 @@ public: gcc_pure bool Match(const Song &song) const; + + gcc_pure + bool Match(const DetachedSong &song) const; }; private: @@ -105,6 +109,9 @@ public: gcc_pure bool Match(const Song &song) const; + gcc_pure + bool Match(const DetachedSong &song) const; + const std::list<Item> &GetItems() const { return items; } diff --git a/src/SongPrint.cxx b/src/SongPrint.cxx index ea164d02b..b3e0c7b58 100644 --- a/src/SongPrint.cxx +++ b/src/SongPrint.cxx @@ -20,6 +20,7 @@ #include "config.h" #include "SongPrint.hxx" #include "Song.hxx" +#include "DetachedSong.hxx" #include "Directory.hxx" #include "TimePrint.hxx" #include "TagPrint.hxx" @@ -27,21 +28,31 @@ #include "Client.hxx" #include "util/UriUtil.hxx" +static void +song_print_uri(Client &client, const char *uri) +{ + const std::string allocated = uri_remove_auth(uri); + if (!allocated.empty()) + uri = allocated.c_str(); + + client_printf(client, "%s%s\n", SONG_FILE, + map_to_relative_path(uri)); +} + void song_print_uri(Client &client, const Song &song) { - if (song.IsInDatabase() && !song.parent->IsRoot()) { + if (song.parent != nullptr && !song.parent->IsRoot()) { client_printf(client, "%s%s/%s\n", SONG_FILE, song.parent->GetPath(), song.uri); - } else { - const char *uri = song.uri; - const std::string allocated = uri_remove_auth(uri); - if (!allocated.empty()) - uri = allocated.c_str(); + } else + song_print_uri(client, song.uri); +} - client_printf(client, "%s%s\n", SONG_FILE, - map_to_relative_path(uri)); - } +void +song_print_uri(Client &client, const DetachedSong &song) +{ + song_print_uri(client, song.GetURI()); } void @@ -66,3 +77,28 @@ song_print_info(Client &client, const Song &song) if (song.tag != nullptr) tag_print(client, *song.tag); } + +void +song_print_info(Client &client, const DetachedSong &song) +{ + song_print_uri(client, song); + + const unsigned start_ms = song.GetStartMS(); + const unsigned end_ms = song.GetEndMS(); + + if (end_ms > 0) + client_printf(client, "Range: %u.%03u-%u.%03u\n", + start_ms / 1000, + start_ms % 1000, + end_ms / 1000, + end_ms % 1000); + else if (start_ms > 0) + client_printf(client, "Range: %u.%03u-\n", + start_ms / 1000, + start_ms % 1000); + + if (song.GetLastModified() > 0) + time_print(client, "Last-Modified", song.GetLastModified()); + + tag_print(client, song.GetTag()); +} diff --git a/src/SongPrint.hxx b/src/SongPrint.hxx index f8df89d38..5779af4c3 100644 --- a/src/SongPrint.hxx +++ b/src/SongPrint.hxx @@ -21,12 +21,19 @@ #define MPD_SONG_PRINT_HXX struct Song; +class DetachedSong; class Client; void +song_print_info(Client &client, const DetachedSong &song); + +void song_print_info(Client &client, const Song &song); void song_print_uri(Client &client, const Song &song); +void +song_print_uri(Client &client, const DetachedSong &song); + #endif diff --git a/src/SongSave.cxx b/src/SongSave.cxx index 18ac63d12..d524f327e 100644 --- a/src/SongSave.cxx +++ b/src/SongSave.cxx @@ -20,6 +20,7 @@ #include "config.h" #include "SongSave.hxx" #include "Song.hxx" +#include "DetachedSong.hxx" #include "TagSave.hxx" #include "fs/TextFile.hxx" #include "tag/Tag.hxx" @@ -36,15 +37,21 @@ static constexpr Domain song_save_domain("song_save"); +static void +range_save(FILE *file, unsigned start_ms, unsigned end_ms) +{ + if (end_ms > 0) + fprintf(file, "Range: %u-%u\n", start_ms, end_ms); + else if (start_ms > 0) + fprintf(file, "Range: %u-\n", start_ms); +} + void song_save(FILE *fp, const Song &song) { fprintf(fp, SONG_BEGIN "%s\n", song.uri); - if (song.end_ms > 0) - fprintf(fp, "Range: %u-%u\n", song.start_ms, song.end_ms); - else if (song.start_ms > 0) - fprintf(fp, "Range: %u-\n", song.start_ms); + range_save(fp, song.start_ms, song.end_ms); if (song.tag != nullptr) tag_save(fp, *song.tag); @@ -53,13 +60,24 @@ song_save(FILE *fp, const Song &song) fprintf(fp, SONG_END "\n"); } -Song * -song_load(TextFile &file, Directory *parent, const char *uri, +void +song_save(FILE *fp, const DetachedSong &song) +{ + fprintf(fp, SONG_BEGIN "%s\n", song.GetURI()); + + range_save(fp, song.GetStartMS(), song.GetEndMS()); + + tag_save(fp, song.GetTag()); + + fprintf(fp, SONG_MTIME ": %li\n", (long)song.GetLastModified()); + fprintf(fp, SONG_END "\n"); +} + +DetachedSong * +song_load(TextFile &file, const char *uri, Error &error) { - Song *song = parent != nullptr - ? Song::NewFile(uri, parent) - : Song::NewRemote(uri); + DetachedSong *song = new DetachedSong(uri); TagBuilder tag; @@ -68,7 +86,7 @@ song_load(TextFile &file, Directory *parent, const char *uri, strcmp(line, SONG_END) != 0) { char *colon = strchr(line, ':'); if (colon == nullptr || colon == line) { - song->Free(); + delete song; error.Format(song_save_domain, "unknown line in db: %s", line); @@ -86,15 +104,19 @@ song_load(TextFile &file, Directory *parent, const char *uri, } else if (strcmp(line, "Playlist") == 0) { tag.SetHasPlaylist(strcmp(value, "yes") == 0); } else if (strcmp(line, SONG_MTIME) == 0) { - song->mtime = atoi(value); + song->SetLastModified(atoi(value)); } else if (strcmp(line, "Range") == 0) { char *endptr; - song->start_ms = strtoul(value, &endptr, 10); - if (*endptr == '-') - song->end_ms = strtoul(endptr + 1, nullptr, 10); + unsigned start_ms = strtoul(value, &endptr, 10); + unsigned end_ms = *endptr == '-' + ? strtoul(endptr + 1, nullptr, 10) + : 0; + + song->SetStartMS(start_ms); + song->SetEndMS(end_ms); } else { - song->Free(); + delete song; error.Format(song_save_domain, "unknown line in db: %s", line); @@ -102,8 +124,6 @@ song_load(TextFile &file, Directory *parent, const char *uri, } } - if (tag.IsDefined()) - song->tag = tag.CommitNew(); - + song->SetTag(tag.Commit()); return song; } diff --git a/src/SongSave.hxx b/src/SongSave.hxx index 40fb4abf7..6b458679a 100644 --- a/src/SongSave.hxx +++ b/src/SongSave.hxx @@ -26,12 +26,16 @@ struct Song; struct Directory; +class DetachedSong; class TextFile; class Error; void song_save(FILE *fp, const Song &song); +void +song_save(FILE *fp, const DetachedSong &song); + /** * Loads a song from the input file. Reading stops after the * "song_end" line. @@ -39,8 +43,8 @@ song_save(FILE *fp, const Song &song); * @param error location to store the error occurring * @return true on success, false on error */ -Song * -song_load(TextFile &file, Directory *parent, const char *uri, +DetachedSong * +song_load(TextFile &file, const char *uri, Error &error); #endif diff --git a/src/SongSticker.cxx b/src/SongSticker.cxx index 88bea1501..b17820fd5 100644 --- a/src/SongSticker.cxx +++ b/src/SongSticker.cxx @@ -31,8 +31,6 @@ std::string sticker_song_get_value(const Song &song, const char *name) { - assert(song.IsInDatabase()); - const auto uri = song.GetURI(); return sticker_load_value("song", uri.c_str(), name); } @@ -41,8 +39,6 @@ bool sticker_song_set_value(const Song &song, const char *name, const char *value) { - assert(song.IsInDatabase()); - const auto uri = song.GetURI(); return sticker_store_value("song", uri.c_str(), name, value); } @@ -50,8 +46,6 @@ sticker_song_set_value(const Song &song, bool sticker_song_delete(const Song &song) { - assert(song.IsInDatabase()); - const auto uri = song.GetURI(); return sticker_delete("song", uri.c_str()); } @@ -59,8 +53,6 @@ sticker_song_delete(const Song &song) bool sticker_song_delete_value(const Song &song, const char *name) { - assert(song.IsInDatabase()); - const auto uri = song.GetURI(); return sticker_delete_value("song", uri.c_str(), name); } @@ -68,8 +60,6 @@ sticker_song_delete_value(const Song &song, const char *name) struct sticker * sticker_song_get(const Song &song) { - assert(song.IsInDatabase()); - const auto uri = song.GetURI(); return sticker_load("song", uri.c_str()); } diff --git a/src/SongUpdate.cxx b/src/SongUpdate.cxx index 953c23961..cba3c6957 100644 --- a/src/SongUpdate.cxx +++ b/src/SongUpdate.cxx @@ -78,8 +78,6 @@ tag_scan_fallback(Path path, bool Song::UpdateFile() { - assert(IsFile()); - const auto path_fs = map_song_fs(*this); if (path_fs.IsNull()) return false; @@ -107,13 +105,9 @@ Song::UpdateFile() bool Song::UpdateFileInArchive() { - const char *suffix; - - assert(IsFile()); - /* check if there's a suffix and a plugin */ - suffix = uri_get_suffix(uri); + const char *suffix = uri_get_suffix(uri); if (suffix == nullptr) return false; diff --git a/src/UpdateRemove.cxx b/src/UpdateRemove.cxx index 91588330a..374a767b8 100644 --- a/src/UpdateRemove.cxx +++ b/src/UpdateRemove.cxx @@ -60,7 +60,10 @@ song_remove_event(void) sticker_song_delete(*removed_song); #endif - instance->DeleteSong(*removed_song); + { + const auto uri = removed_song->GetURI(); + instance->DeleteSong(uri.c_str()); + } /* clear "removed_song" and send signal to update thread */ remove_mutex.lock(); diff --git a/src/cue/CueParser.cxx b/src/cue/CueParser.cxx index 4a4f823ab..f05a69ae3 100644 --- a/src/cue/CueParser.cxx +++ b/src/cue/CueParser.cxx @@ -22,7 +22,7 @@ #include "util/Alloc.hxx" #include "util/StringUtil.hxx" #include "util/CharUtil.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "tag/Tag.hxx" #include <assert.h> @@ -38,14 +38,9 @@ CueParser::CueParser() CueParser::~CueParser() { - if (current != nullptr) - current->Free(); - - if (previous != nullptr) - previous->Free(); - - if (finished != nullptr) - finished->Free(); + delete current; + delete previous; + delete finished; } static const char * @@ -169,8 +164,8 @@ CueParser::Commit() if (current == nullptr) return; - assert(current->tag == nullptr); - current->tag = song_tag.CommitNew(); + assert(!current->GetTag().IsDefined()); + current->SetTag(song_tag.Commit()); finished = previous; previous = current; @@ -249,8 +244,8 @@ CueParser::Feed2(char *p) } state = TRACK; - current = Song::NewRemote(filename.c_str()); - assert(current->tag == nullptr); + current = new DetachedSong(std::move(filename)); + assert(!current->GetTag().IsDefined()); song_tag = header_tag; song_tag.AddItem(TAG_TRACK, nr); @@ -272,14 +267,14 @@ CueParser::Feed2(char *p) return; if (!last_updated && previous != nullptr && - previous->start_ms < (unsigned)position_ms) { + previous->GetStartMS() < (unsigned)position_ms) { last_updated = true; - previous->end_ms = position_ms; - previous->tag->time = - (previous->end_ms - previous->start_ms + 500) / 1000; + previous->SetEndMS(position_ms); + previous->WritableTag().time = + (previous->GetEndMS() - previous->GetStartMS() + 500) / 1000; } - current->start_ms = position_ms; + current->SetStartMS(position_ms); } } @@ -305,7 +300,7 @@ CueParser::Finish() end = true; } -Song * +DetachedSong * CueParser::Get() { if (finished == nullptr && end) { @@ -317,7 +312,7 @@ CueParser::Get() previous = nullptr; } - Song *song = finished; + DetachedSong *song = finished; finished = nullptr; return song; } diff --git a/src/cue/CueParser.hxx b/src/cue/CueParser.hxx index bcc759c37..9a32a33c4 100644 --- a/src/cue/CueParser.hxx +++ b/src/cue/CueParser.hxx @@ -26,7 +26,7 @@ #include <string> -struct Song; +class DetachedSong; struct Tag; class CueParser { @@ -74,19 +74,19 @@ class CueParser { /** * The song currently being edited. */ - Song *current; + DetachedSong *current; /** * The previous song. It is remembered because its end_time * will be set to the current song's start time. */ - Song *previous; + DetachedSong *previous; /** * A song that is completely finished and can be returned to * the caller via cue_parser_get(). */ - Song *finished; + DetachedSong *finished; /** * Set to true after previous.end_time has been updated to the @@ -125,7 +125,7 @@ public: * @return a song object that must be freed by the caller, or NULL if * no song was finished at this time */ - Song *Get(); + DetachedSong *Get(); private: gcc_pure diff --git a/src/db/ProxyDatabasePlugin.cxx b/src/db/ProxyDatabasePlugin.cxx index 3b0f4b4f0..a9da1717d 100644 --- a/src/db/ProxyDatabasePlugin.cxx +++ b/src/db/ProxyDatabasePlugin.cxx @@ -340,20 +340,19 @@ void ProxyDatabase::ReturnSong(Song *song) const { assert(song != nullptr); - assert(song->IsInDatabase()); - assert(song->IsDetached()); + assert(song->parent == nullptr); song->Free(); } static bool -Visit(struct mpd_connection *connection, const char *uri, +Visit(struct mpd_connection *connection, Directory &root, const char *uri, bool recursive, const SongFilter *filter, VisitDirectory visit_directory, VisitSong visit_song, VisitPlaylist visit_playlist, Error &error); static bool -Visit(struct mpd_connection *connection, +Visit(struct mpd_connection *connection, Directory &root, bool recursive, const SongFilter *filter, const struct mpd_directory *directory, VisitDirectory visit_directory, VisitSong visit_song, @@ -362,7 +361,7 @@ Visit(struct mpd_connection *connection, const char *path = mpd_directory_get_path(directory); if (visit_directory) { - Directory *d = Directory::NewGeneric(path, &detached_root); + Directory *d = Directory::NewGeneric(path, &root); bool success = visit_directory(*d, error); d->Free(); if (!success) @@ -370,7 +369,7 @@ Visit(struct mpd_connection *connection, } if (recursive && - !Visit(connection, path, recursive, filter, + !Visit(connection, root, path, recursive, filter, visit_directory, visit_song, visit_playlist, error)) return false; @@ -394,7 +393,7 @@ Copy(TagBuilder &tag, TagType d_tag, static Song * Convert(const struct mpd_song *song) { - Song *s = Song::NewDetached(mpd_song_get_uri(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; @@ -434,7 +433,7 @@ Visit(const SongFilter *filter, } static bool -Visit(const struct mpd_playlist *playlist, +Visit(const struct mpd_playlist *playlist, Directory &root, VisitPlaylist visit_playlist, Error &error) { if (!visit_playlist) @@ -443,7 +442,7 @@ Visit(const struct mpd_playlist *playlist, PlaylistInfo p(mpd_playlist_get_path(playlist), mpd_playlist_get_last_modified(playlist)); - return visit_playlist(p, detached_root, error); + return visit_playlist(p, root, error); } class ProxyEntity { @@ -485,7 +484,7 @@ ReceiveEntities(struct mpd_connection *connection) } static bool -Visit(struct mpd_connection *connection, const char *uri, +Visit(struct mpd_connection *connection, Directory &root, const char *uri, bool recursive, const SongFilter *filter, VisitDirectory visit_directory, VisitSong visit_song, VisitPlaylist visit_playlist, Error &error) @@ -503,7 +502,7 @@ Visit(struct mpd_connection *connection, const char *uri, break; case MPD_ENTITY_TYPE_DIRECTORY: - if (!Visit(connection, recursive, filter, + if (!Visit(connection, root, recursive, filter, mpd_entity_get_directory(entity), visit_directory, visit_song, visit_playlist, error)) @@ -518,7 +517,7 @@ Visit(struct mpd_connection *connection, const char *uri, break; case MPD_ENTITY_TYPE_PLAYLIST: - if (!Visit(mpd_entity_get_playlist(entity), + if (!Visit(mpd_entity_get_playlist(entity), root, visit_playlist, error)) return false; break; @@ -577,7 +576,7 @@ ProxyDatabase::Visit(const DatabaseSelection &selection, return ::SearchSongs(connection, selection, visit_song, error); /* fall back to recursive walk (slow!) */ - return ::Visit(connection, selection.uri.c_str(), + return ::Visit(connection, *root, selection.uri.c_str(), selection.recursive, selection.filter, visit_directory, visit_song, visit_playlist, error); diff --git a/src/playlist/AsxPlaylistPlugin.cxx b/src/playlist/AsxPlaylistPlugin.cxx index ccd98a711..11bdc93d6 100644 --- a/src/playlist/AsxPlaylistPlugin.cxx +++ b/src/playlist/AsxPlaylistPlugin.cxx @@ -43,7 +43,7 @@ struct AsxParser { * The list of songs (in reverse order because that's faster * while adding). */ - std::forward_list<SongPointer> songs; + std::forward_list<DetachedSong> songs; /** * The current position in the XML file. @@ -60,10 +60,9 @@ struct AsxParser { TagType tag_type; /** - * The current song. It is allocated after the "location" - * element. + * The current song URI. It is set by the "ref" element. */ - Song *song; + std::string location; TagBuilder tag_builder; @@ -96,7 +95,7 @@ asx_start_element(gcc_unused GMarkupParseContext *context, case AsxParser::ROOT: if (StringEqualsCaseASCII(element_name, "entry")) { parser->state = AsxParser::ENTRY; - parser->song = Song::NewRemote("asx:"); + parser->location.clear(); parser->tag_type = TAG_NUM_OF_ITEM_TYPES; } @@ -107,17 +106,8 @@ asx_start_element(gcc_unused GMarkupParseContext *context, const gchar *href = get_attribute(attribute_names, attribute_values, "href"); - if (href != nullptr) { - /* create new song object; we cannot - replace the existing song's URI, - because that attribute is - immutable */ - Song *song = Song::NewRemote(href); - if (parser->song != nullptr) - parser->song->Free(); - - parser->song = song; - } + if (href != nullptr) + parser->location = href; } else if (StringEqualsCaseASCII(element_name, "author")) /* is that correct? or should it be COMPOSER or PERFORMER? */ @@ -142,12 +132,9 @@ asx_end_element(gcc_unused GMarkupParseContext *context, case AsxParser::ENTRY: if (StringEqualsCaseASCII(element_name, "entry")) { - if (strcmp(parser->song->uri, "asx:") != 0) { - assert(parser->song->tag == nullptr); - parser->song->tag = parser->tag_builder.CommitNew(); - parser->songs.emplace_front(parser->song); - } else - parser->song->Free(); + if (!parser->location.empty()) + parser->songs.emplace_front(std::move(parser->location), + parser->tag_builder.Commit()); parser->state = AsxParser::ROOT; } else @@ -186,15 +173,6 @@ static const GMarkupParser asx_parser = { nullptr, }; -static void -asx_parser_destroy(gpointer data) -{ - AsxParser *parser = (AsxParser *)data; - - if (parser->state >= AsxParser::ENTRY) - parser->song->Free(); -} - /* * The playlist object * @@ -215,7 +193,7 @@ asx_open_stream(InputStream &is) context = g_markup_parse_context_new(&asx_parser, G_MARKUP_TREAT_CDATA_AS_TEXT, - &parser, asx_parser_destroy); + &parser, nullptr); while (true) { nbytes = is.LockRead(buffer, sizeof(buffer), error2); diff --git a/src/playlist/CuePlaylistPlugin.cxx b/src/playlist/CuePlaylistPlugin.cxx index 00aa758b0..1da76d372 100644 --- a/src/playlist/CuePlaylistPlugin.cxx +++ b/src/playlist/CuePlaylistPlugin.cxx @@ -36,7 +36,7 @@ class CuePlaylist final : public SongEnumerator { :is(_is), tis(is) { } - virtual Song *NextSong() override; + virtual DetachedSong *NextSong() override; }; static SongEnumerator * @@ -45,10 +45,10 @@ cue_playlist_open_stream(InputStream &is) return new CuePlaylist(is); } -Song * +DetachedSong * CuePlaylist::NextSong() { - Song *song = parser.Get(); + DetachedSong *song = parser.Get(); if (song != nullptr) return song; diff --git a/src/playlist/DespotifyPlaylistPlugin.cxx b/src/playlist/DespotifyPlaylistPlugin.cxx index 67400e20a..d73c7fe72 100644 --- a/src/playlist/DespotifyPlaylistPlugin.cxx +++ b/src/playlist/DespotifyPlaylistPlugin.cxx @@ -23,7 +23,7 @@ #include "PlaylistPlugin.hxx" #include "MemorySongEnumerator.hxx" #include "tag/Tag.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "Log.hxx" extern "C" { @@ -34,10 +34,9 @@ extern "C" { #include <stdlib.h> static void -add_song(std::forward_list<SongPointer> &songs, ds_track &track) +add_song(std::forward_list<DetachedSong> &songs, ds_track &track) { const char *dsp_scheme = despotify_playlist_plugin.schemes[0]; - Song *song; char uri[128]; char *ds_uri; @@ -52,15 +51,12 @@ add_song(std::forward_list<SongPointer> &songs, ds_track &track) return; } - song = Song::NewRemote(uri); - song->tag = new Tag(mpd_despotify_tag_from_track(track)); - - songs.emplace_front(song); + songs.emplace_front(uri, mpd_despotify_tag_from_track(track)); } static bool parse_track(struct despotify_session *session, - std::forward_list<SongPointer> &songs, + std::forward_list<DetachedSong> &songs, struct ds_link *link) { struct ds_track *track = despotify_link_get_track(session, link); @@ -73,7 +69,7 @@ parse_track(struct despotify_session *session, static bool parse_playlist(struct despotify_session *session, - std::forward_list<SongPointer> &songs, + std::forward_list<DetachedSong> &songs, struct ds_link *link) { ds_playlist *playlist = despotify_link_get_playlist(session, link); @@ -103,7 +99,7 @@ despotify_playlist_open_uri(const char *url, return nullptr; } - std::forward_list<SongPointer> songs; + std::forward_list<DetachedSong> songs; bool parse_result; switch (link->type) { diff --git a/src/playlist/EmbeddedCuePlaylistPlugin.cxx b/src/playlist/EmbeddedCuePlaylistPlugin.cxx index 9dfecbf46..77df51778 100644 --- a/src/playlist/EmbeddedCuePlaylistPlugin.cxx +++ b/src/playlist/EmbeddedCuePlaylistPlugin.cxx @@ -30,7 +30,7 @@ #include "tag/TagHandler.hxx" #include "tag/TagId3.hxx" #include "tag/ApeTag.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "TagFile.hxx" #include "cue/CueParser.hxx" #include "fs/Traits.hxx" @@ -69,7 +69,7 @@ public: delete parser; } - virtual Song *NextSong() override; + virtual DetachedSong *NextSong() override; }; static void @@ -124,10 +124,10 @@ embcue_playlist_open_uri(const char *uri, return playlist; } -Song * +DetachedSong * EmbeddedCuePlaylist::NextSong() { - Song *song = parser->Get(); + DetachedSong *song = parser->Get(); if (song != nullptr) return song; @@ -145,14 +145,16 @@ EmbeddedCuePlaylist::NextSong() parser->Feed(line); song = parser->Get(); - if (song != nullptr) - return song->ReplaceURI(filename.c_str()); + if (song != nullptr) { + song->SetURI(filename); + return song; + } } parser->Finish(); song = parser->Get(); if (song != nullptr) - song = song->ReplaceURI(filename.c_str()); + song->SetURI(filename); return song; } diff --git a/src/playlist/ExtM3uPlaylistPlugin.cxx b/src/playlist/ExtM3uPlaylistPlugin.cxx index 4f0c111ad..51211988c 100644 --- a/src/playlist/ExtM3uPlaylistPlugin.cxx +++ b/src/playlist/ExtM3uPlaylistPlugin.cxx @@ -21,7 +21,7 @@ #include "ExtM3uPlaylistPlugin.hxx" #include "PlaylistPlugin.hxx" #include "SongEnumerator.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "tag/Tag.hxx" #include "tag/TagBuilder.hxx" #include "util/StringUtil.hxx" @@ -44,7 +44,7 @@ public: strcmp(line.c_str(), "#EXTM3U") == 0; } - virtual Song *NextSong() override; + virtual DetachedSong *NextSong() override; }; static SongEnumerator * @@ -101,20 +101,19 @@ extm3u_parse_tag(const char *line) return tag.CommitNew(); } -Song * +DetachedSong * ExtM3uPlaylist::NextSong() { Tag *tag = NULL; std::string line; const char *line_s; - Song *song; do { if (!tis.ReadLine(line)) { delete tag; return NULL; } - + line_s = line.c_str(); if (StringStartsWith(line_s, "#EXTINF:")) { @@ -126,8 +125,8 @@ ExtM3uPlaylist::NextSong() line_s = strchug_fast(line_s); } while (line_s[0] == '#' || *line_s == 0); - song = Song::NewRemote(line_s); - song->tag = tag; + DetachedSong *song = new DetachedSong(line_s, std::move(*tag)); + delete tag; return song; } diff --git a/src/playlist/M3uPlaylistPlugin.cxx b/src/playlist/M3uPlaylistPlugin.cxx index 3f99bdfdf..09f2c91cb 100644 --- a/src/playlist/M3uPlaylistPlugin.cxx +++ b/src/playlist/M3uPlaylistPlugin.cxx @@ -21,7 +21,7 @@ #include "M3uPlaylistPlugin.hxx" #include "PlaylistPlugin.hxx" #include "SongEnumerator.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "util/StringUtil.hxx" #include "TextInputStream.hxx" @@ -33,7 +33,7 @@ public: :tis(is) { } - virtual Song *NextSong() override; + virtual DetachedSong *NextSong() override; }; static SongEnumerator * @@ -42,7 +42,7 @@ m3u_open_stream(InputStream &is) return new M3uPlaylist(is); } -Song * +DetachedSong * M3uPlaylist::NextSong() { std::string line; @@ -56,7 +56,7 @@ M3uPlaylist::NextSong() line_s = strchug_fast(line_s); } while (line_s[0] == '#' || *line_s == 0); - return Song::NewRemote(line_s); + return new DetachedSong(line_s); } static const char *const m3u_suffixes[] = { diff --git a/src/playlist/PlsPlaylistPlugin.cxx b/src/playlist/PlsPlaylistPlugin.cxx index 3fd420d89..103451dfe 100644 --- a/src/playlist/PlsPlaylistPlugin.cxx +++ b/src/playlist/PlsPlaylistPlugin.cxx @@ -22,7 +22,7 @@ #include "PlaylistPlugin.hxx" #include "MemorySongEnumerator.hxx" #include "InputStream.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "tag/TagBuilder.hxx" #include "util/Error.hxx" #include "util/Domain.hxx" @@ -37,7 +37,7 @@ static constexpr Domain pls_domain("pls"); static void -pls_parser(GKeyFile *keyfile, std::forward_list<SongPointer> &songs) +pls_parser(GKeyFile *keyfile, std::forward_list<DetachedSong> &songs) { gchar *value; GError *error = nullptr; @@ -61,8 +61,8 @@ pls_parser(GKeyFile *keyfile, std::forward_list<SongPointer> &songs) for (; num_entries > 0; --num_entries) { char key[64]; sprintf(key, "File%u", num_entries); - value = g_key_file_get_string(keyfile, "playlist", key, - &error); + char *uri = g_key_file_get_string(keyfile, "playlist", key, + &error); if(error) { FormatError(pls_domain, "Invalid PLS entry %s: '%s'", key, error->message); @@ -70,9 +70,6 @@ pls_parser(GKeyFile *keyfile, std::forward_list<SongPointer> &songs) return; } - Song *song = Song::NewRemote(value); - g_free(value); - TagBuilder tag; sprintf(key, "Title%u", num_entries); @@ -89,8 +86,8 @@ pls_parser(GKeyFile *keyfile, std::forward_list<SongPointer> &songs) if (length > 0) tag.SetTime(length); - song->tag = tag.CommitNew(); - songs.emplace_front(song); + songs.emplace_front(uri, tag.Commit()); + g_free(uri); } } @@ -135,7 +132,7 @@ pls_open_stream(InputStream &is) return nullptr; } - std::forward_list<SongPointer> songs; + std::forward_list<DetachedSong> songs; pls_parser(keyfile, songs); g_key_file_free(keyfile); diff --git a/src/playlist/RssPlaylistPlugin.cxx b/src/playlist/RssPlaylistPlugin.cxx index c4dd2f8c3..558c74619 100644 --- a/src/playlist/RssPlaylistPlugin.cxx +++ b/src/playlist/RssPlaylistPlugin.cxx @@ -43,7 +43,7 @@ struct RssParser { * The list of songs (in reverse order because that's faster * while adding). */ - std::forward_list<SongPointer> songs; + std::forward_list<DetachedSong> songs; /** * The current position in the XML file. @@ -60,10 +60,10 @@ struct RssParser { TagType tag_type; /** - * The current song. It is allocated after the "location" + * The current song URI. It is set by the "enclosure" * element. */ - Song *song; + std::string location; TagBuilder tag_builder; @@ -95,7 +95,7 @@ rss_start_element(gcc_unused GMarkupParseContext *context, case RssParser::ROOT: if (StringEqualsCaseASCII(element_name, "item")) { parser->state = RssParser::ITEM; - parser->song = Song::NewRemote("rss:"); + parser->location.clear(); parser->tag_type = TAG_NUM_OF_ITEM_TYPES; } @@ -106,18 +106,8 @@ rss_start_element(gcc_unused GMarkupParseContext *context, const gchar *href = get_attribute(attribute_names, attribute_values, "url"); - if (href != nullptr) { - /* create new song object; we cannot - replace the existing song's URI, - because that attribute is - immutable */ - Song *song = Song::NewRemote(href); - - if (parser->song != nullptr) - parser->song->Free(); - - parser->song = song; - } + if (href != nullptr) + parser->location = href; } else if (StringEqualsCaseASCII(element_name, "title")) parser->tag_type = TAG_TITLE; else if (StringEqualsCaseASCII(element_name, "itunes:author")) @@ -140,12 +130,9 @@ rss_end_element(gcc_unused GMarkupParseContext *context, case RssParser::ITEM: if (StringEqualsCaseASCII(element_name, "item")) { - if (strcmp(parser->song->uri, "rss:") != 0) { - assert(parser->song->tag == nullptr); - parser->song->tag = parser->tag_builder.CommitNew(); - parser->songs.emplace_front(parser->song); - } else - parser->song->Free(); + if (!parser->location.empty()) + parser->songs.emplace_front(std::move(parser->location), + parser->tag_builder.Commit()); parser->state = RssParser::ROOT; } else @@ -183,15 +170,6 @@ static const GMarkupParser rss_parser = { nullptr, }; -static void -rss_parser_destroy(gpointer data) -{ - RssParser *parser = (RssParser *)data; - - if (parser->state >= RssParser::ITEM) - parser->song->Free(); -} - /* * The playlist object * @@ -212,7 +190,7 @@ rss_open_stream(InputStream &is) context = g_markup_parse_context_new(&rss_parser, G_MARKUP_TREAT_CDATA_AS_TEXT, - &parser, rss_parser_destroy); + &parser, nullptr); while (true) { nbytes = is.LockRead(buffer, sizeof(buffer), error2); diff --git a/src/playlist/SoundCloudPlaylistPlugin.cxx b/src/playlist/SoundCloudPlaylistPlugin.cxx index cfbd0e8b5..50a9cb214 100644 --- a/src/playlist/SoundCloudPlaylistPlugin.cxx +++ b/src/playlist/SoundCloudPlaylistPlugin.cxx @@ -108,7 +108,7 @@ struct parse_data { char* title; int got_url; /* nesting level of last stream_url */ - std::forward_list<SongPointer> songs; + std::forward_list<DetachedSong> songs; }; static int @@ -214,16 +214,14 @@ handle_end_map(void *ctx) char *u = g_strconcat(data->stream_url, "?client_id=", soundcloud_config.apikey.c_str(), nullptr); - Song *s = Song::NewRemote(u); - g_free(u); TagBuilder tag; tag.SetTime(data->duration / 1000); if (data->title != nullptr) tag.AddItem(TAG_NAME, data->title); - s->tag = tag.CommitNew(); - data->songs.emplace_front(s); + data->songs.emplace_front(u, tag.Commit()); + g_free(u); return 1; } diff --git a/src/playlist/XspfPlaylistPlugin.cxx b/src/playlist/XspfPlaylistPlugin.cxx index 08fe49191..7c20df57d 100644 --- a/src/playlist/XspfPlaylistPlugin.cxx +++ b/src/playlist/XspfPlaylistPlugin.cxx @@ -21,6 +21,7 @@ #include "XspfPlaylistPlugin.hxx" #include "PlaylistPlugin.hxx" #include "MemorySongEnumerator.hxx" +#include "DetachedSong.hxx" #include "InputStream.hxx" #include "tag/TagBuilder.hxx" #include "util/Error.hxx" @@ -41,7 +42,7 @@ struct XspfParser { * The list of songs (in reverse order because that's faster * while adding). */ - std::forward_list<SongPointer> songs; + std::forward_list<DetachedSong> songs; /** * The current position in the XML file. @@ -59,10 +60,9 @@ struct XspfParser { TagType tag_type; /** - * The current song. It is allocated after the "location" - * element. + * The current song URI. It is set by the "location" element. */ - Song *song; + std::string location; TagBuilder tag_builder; @@ -95,7 +95,7 @@ xspf_start_element(gcc_unused GMarkupParseContext *context, case XspfParser::TRACKLIST: if (strcmp(element_name, "track") == 0) { parser->state = XspfParser::TRACK; - parser->song = nullptr; + parser->location.clear(); parser->tag_type = TAG_NUM_OF_ITEM_TYPES; } @@ -149,11 +149,9 @@ xspf_end_element(gcc_unused GMarkupParseContext *context, case XspfParser::TRACK: if (strcmp(element_name, "track") == 0) { - if (parser->song != nullptr) { - assert(parser->song->tag == nullptr); - parser->song->tag = parser->tag_builder.CommitNew(); - parser->songs.emplace_front(parser->song); - } + if (!parser->location.empty()) + parser->songs.emplace_front(std::move(parser->location), + parser->tag_builder.Commit()); parser->state = XspfParser::TRACKLIST; } else @@ -181,7 +179,7 @@ xspf_text(gcc_unused GMarkupParseContext *context, break; case XspfParser::TRACK: - if (parser->song != nullptr && + if (!parser->location.empty() && parser->tag_type != TAG_NUM_OF_ITEM_TYPES) parser->tag_builder.AddItem(parser->tag_type, text, text_len); @@ -189,11 +187,7 @@ xspf_text(gcc_unused GMarkupParseContext *context, break; case XspfParser::LOCATION: - if (parser->song == nullptr) { - char *uri = g_strndup(text, text_len); - parser->song = Song::NewRemote(uri); - g_free(uri); - } + parser->location.assign(text, text_len); break; } @@ -207,15 +201,6 @@ static const GMarkupParser xspf_parser = { nullptr, }; -static void -xspf_parser_destroy(gpointer data) -{ - XspfParser *parser = (XspfParser *)data; - - if (parser->state >= XspfParser::TRACK && parser->song != nullptr) - parser->song->Free(); -} - /* * The playlist object * @@ -236,7 +221,7 @@ xspf_open_stream(InputStream &is) context = g_markup_parse_context_new(&xspf_parser, G_MARKUP_TREAT_CDATA_AS_TEXT, - &parser, xspf_parser_destroy); + &parser, nullptr); while (true) { nbytes = is.LockRead(buffer, sizeof(buffer), error2); diff --git a/test/DumpDatabase.cxx b/test/DumpDatabase.cxx index 14bb4ba14..6e9259238 100644 --- a/test/DumpDatabase.cxx +++ b/test/DumpDatabase.cxx @@ -49,7 +49,10 @@ DumpDirectory(const Directory &directory, Error &) static bool DumpSong(Song &song, Error &) { - cout << "S " << song.parent->path << "/" << song.uri << endl; + cout << "S "; + if (song.parent != nullptr && !song.parent->IsRoot()) + cout << song.parent->path << "/"; + cout << song.uri << endl; return true; } diff --git a/test/dump_playlist.cxx b/test/dump_playlist.cxx index 2ac855a84..b0702d3be 100644 --- a/test/dump_playlist.cxx +++ b/test/dump_playlist.cxx @@ -19,9 +19,8 @@ #include "config.h" #include "TagSave.hxx" -#include "Song.hxx" +#include "DetachedSong.hxx" #include "SongEnumerator.hxx" -#include "Directory.hxx" #include "InputStream.hxx" #include "ConfigGlobal.hxx" #include "DecoderList.hxx" @@ -39,14 +38,10 @@ #include <unistd.h> #include <stdlib.h> -Directory::Directory() {} -Directory::~Directory() {} - int main(int argc, char **argv) { const char *uri; InputStream *is = NULL; - Song *song; if (argc != 3) { fprintf(stderr, "Usage: dump_playlist CONFIG URI\n"); @@ -114,24 +109,27 @@ int main(int argc, char **argv) /* dump the playlist */ + DetachedSong *song; while ((song = playlist->NextSong()) != NULL) { - printf("%s\n", song->uri); + printf("%s\n", song->GetURI()); + + const unsigned start_ms = song->GetStartMS(); + const unsigned end_ms = song->GetEndMS(); - if (song->end_ms > 0) + if (end_ms > 0) printf("range: %u:%02u..%u:%02u\n", - song->start_ms / 60000, - (song->start_ms / 1000) % 60, - song->end_ms / 60000, - (song->end_ms / 1000) % 60); - else if (song->start_ms > 0) + start_ms / 60000, + (start_ms / 1000) % 60, + end_ms / 60000, + (end_ms / 1000) % 60); + else if (start_ms > 0) printf("range: %u:%02u..\n", - song->start_ms / 60000, - (song->start_ms / 1000) % 60); + start_ms / 60000, + (start_ms / 1000) % 60); - if (song->tag != NULL) - tag_save(stdout, *song->tag); + tag_save(stdout, song->GetTag()); - song->Free(); + delete song; } /* deinitialize everything */ diff --git a/test/test_queue_priority.cxx b/test/test_queue_priority.cxx index a1037798c..953103f9a 100644 --- a/test/test_queue_priority.cxx +++ b/test/test_queue_priority.cxx @@ -1,7 +1,6 @@ #include "config.h" #include "Queue.hxx" -#include "Song.hxx" -#include "Directory.hxx" +#include "DetachedSong.hxx" #include "util/Macros.hxx" #include <cppunit/TestFixture.h> @@ -9,21 +8,8 @@ #include <cppunit/ui/text/TestRunner.h> #include <cppunit/extensions/HelperMacros.h> -Directory detached_root; - -Directory::Directory() {} -Directory::~Directory() {} - -Song * -Song::DupDetached() const -{ - return const_cast<Song *>(this); -} - -void -Song::Free() -{ -} +Tag::Tag(const Tag &) {} +void Tag::Clear() {} static void check_descending_priority(const struct queue *queue, @@ -53,12 +39,29 @@ public: void QueuePriorityTest::TestPriority() { - static Song songs[16]; + DetachedSong songs[16] = { + DetachedSong("0.ogg"), + DetachedSong("1.ogg"), + DetachedSong("2.ogg"), + DetachedSong("3.ogg"), + DetachedSong("4.ogg"), + DetachedSong("5.ogg"), + DetachedSong("6.ogg"), + DetachedSong("7.ogg"), + DetachedSong("8.ogg"), + DetachedSong("9.ogg"), + DetachedSong("a.ogg"), + DetachedSong("b.ogg"), + DetachedSong("c.ogg"), + DetachedSong("d.ogg"), + DetachedSong("e.ogg"), + DetachedSong("f.ogg"), + }; struct queue queue(32); for (unsigned i = 0; i < ARRAY_SIZE(songs); ++i) - queue.Append(&songs[i], 0); + queue.Append(DetachedSong(songs[i]), 0); CPPUNIT_ASSERT_EQUAL(unsigned(ARRAY_SIZE(songs)), queue.GetLength()); |