From ddd3f119d302ecb0b96e35af0f01b65fc1fcbb6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bara?= Date: Sat, 15 Jan 2011 18:46:23 +0000 Subject: [PATCH] CUE songs are now properly updated in library - you can delete a CUE sheet, add it, you can change section markers in it etc. and everything should work as expected Song now knows it's cue path (if any) --- data/data.qrc | 1 + data/schema/device-schema.sql | 4 +- data/schema/jamendo.sql | 4 +- data/schema/schema-25.sql | 3 + src/core/database.cpp | 2 +- src/core/song.cpp | 10 +- src/core/song.h | 7 + src/library/librarybackend.cpp | 17 ++ src/library/librarybackend.h | 4 + src/library/librarywatcher.cpp | 198 ++++++++++++++++-------- src/library/librarywatcher.h | 19 +++ src/playlistparsers/cueparser.cpp | 1 + src/scripting/python/librarybackend.sip | 1 + src/scripting/python/song.sip | 4 + tests/cueparser_test.cpp | 8 +- tests/mock_librarybackend.h | 1 + 16 files changed, 216 insertions(+), 68 deletions(-) create mode 100644 data/schema/schema-25.sql diff --git a/data/data.qrc b/data/data.qrc index 067db8f40..f485e9b10 100644 --- a/data/data.qrc +++ b/data/data.qrc @@ -294,6 +294,7 @@ schema/jamendo.sql schema/schema-23.sql schema/schema-24.sql + schema/schema-25.sql pythonstartup.py diff --git a/data/schema/device-schema.sql b/data/schema/device-schema.sql index 336783ae4..ff1705040 100644 --- a/data/schema/device-schema.sql +++ b/data/schema/device-schema.sql @@ -49,7 +49,9 @@ CREATE TABLE device_%deviceid_songs ( skipcount INTEGER NOT NULL DEFAULT 0, score INTEGER NOT NULL DEFAULT 0, - beginning NOT NULL DEFAULT 0 + beginning NOT NULL DEFAULT 0, + + cue_path TEXT ); CREATE INDEX idx_device_%deviceid_songs_album ON device_%deviceid_songs (album); diff --git a/data/schema/jamendo.sql b/data/schema/jamendo.sql index db23e703f..c5fa5aa20 100644 --- a/data/schema/jamendo.sql +++ b/data/schema/jamendo.sql @@ -36,7 +36,9 @@ CREATE TABLE jamendo.songs ( effective_compilation NOT NULL DEFAULT 0, skipcount NOT NULL DEFAULT 0, score NOT NULL DEFAULT 0, - beginning NOT NULL DEFAULT 0 + beginning NOT NULL DEFAULT 0, + + cue_path TEXT ); CREATE VIRTUAL TABLE jamendo.songs_fts USING fts3( diff --git a/data/schema/schema-25.sql b/data/schema/schema-25.sql new file mode 100644 index 000000000..fe1754960 --- /dev/null +++ b/data/schema/schema-25.sql @@ -0,0 +1,3 @@ +ALTER TABLE %allsongstables ADD COLUMN cue_path TEXT; + +UPDATE schema_version SET version=25; diff --git a/src/core/database.cpp b/src/core/database.cpp index 9f5981004..da61fb614 100644 --- a/src/core/database.cpp +++ b/src/core/database.cpp @@ -31,7 +31,7 @@ #include const char* Database::kDatabaseFilename = "clementine.db"; -const int Database::kSchemaVersion = 24; +const int Database::kSchemaVersion = 25; const char* Database::kMagicAllSongsTables = "%allsongstables"; int Database::sNextConnectionId = 1; diff --git a/src/core/song.cpp b/src/core/song.cpp index 8394855aa..bdd307a03 100644 --- a/src/core/song.cpp +++ b/src/core/song.cpp @@ -93,7 +93,8 @@ const QStringList Song::kColumns = QStringList() << "mtime" << "ctime" << "filesize" << "sampler" << "art_automatic" << "art_manual" << "filetype" << "playcount" << "lastplayed" << "rating" << "forced_compilation_on" << "forced_compilation_off" - << "effective_compilation" << "skipcount" << "score" << "beginning" << "length"; + << "effective_compilation" << "skipcount" << "score" << "beginning" << "length" + << "cue_path"; const QString Song::kColumnSpec = Song::kColumns.join(", "); const QString Song::kBindSpec = Prepend(":", Song::kColumns).join(", "); @@ -506,6 +507,8 @@ void Song::InitFromQuery(const SqlRow& q, int col) { d->beginning_ = q.value(col + 32).isNull() ? 0 : q.value(col + 32).toInt(); set_length(toint(col + 33)); + d->cue_path_ = tostr(col + 34); + #undef tostr #undef toint #undef tofloat @@ -964,6 +967,8 @@ void Song::BindToQuery(QSqlQuery *query) const { query->bindValue(":beginning", d->beginning_); query->bindValue(":length", intval(length())); + query->bindValue(":cue_path", d->cue_path_); + #undef intval #undef notnullintval } @@ -1057,7 +1062,8 @@ bool Song::IsMetadataEqual(const Song& other) const { d->samplerate_ == other.d->samplerate_ && d->sampler_ == other.d->sampler_ && d->art_automatic_ == other.d->art_automatic_ && - d->art_manual_ == other.d->art_manual_; + d->art_manual_ == other.d->art_manual_ && + d->cue_path_ == other.d->cue_path_; } void Song::SetTextFrame(const QString& id, const QString& value, diff --git a/src/core/song.h b/src/core/song.h index c6cb21175..b4bc0b206 100644 --- a/src/core/song.h +++ b/src/core/song.h @@ -171,6 +171,9 @@ class Song { int lastplayed() const { return d->lastplayed_; } int score() const { return d->score_; } + const QString& cue_path() const { return d->cue_path_; } + bool has_cue() const { return !d->cue_path_.isEmpty(); } + int beginning() const { return d->beginning_; } int end() const { return d->end_; } @@ -241,6 +244,7 @@ class Song { void set_skipcount(int v) { d->skipcount_ = v; } void set_lastplayed(int v) { d->lastplayed_ = v; } void set_score(int v) { d->score_ = qBound(0, v, 100); } + void set_cue_path(const QString& v) { d->cue_path_ = v; } // Setters that should only be used by tests void set_filename(const QString& v) { d->filename_ = v; } @@ -312,6 +316,9 @@ class Song { int filesize_; FileType filetype_; + // If the song has a CUE, this contains it's path. + QString cue_path_; + // Filenames to album art for this song. QString art_automatic_; // Guessed by LibraryWatcher QString art_manual_; // Set by the user - should take priority diff --git a/src/library/librarybackend.cpp b/src/library/librarybackend.cpp index 660319eb0..edc299b97 100644 --- a/src/library/librarybackend.cpp +++ b/src/library/librarybackend.cpp @@ -574,6 +574,23 @@ Song LibraryBackend::GetSongByFilename(const QString& filename, int beginning) { return song; } +SongList LibraryBackend::GetSongsByFilename(const QString& filename) { + LibraryQuery query; + query.SetColumnSpec("%songs_table.ROWID, " + Song::kColumnSpec); + query.AddWhere("filename", filename); + + SongList songlist; + if (ExecQuery(&query)) { + while(query.Next()) { + Song song; + song.InitFromQuery(query); + + songlist << song; + } + } + return songlist; +} + bool LibraryBackend::HasCompilations(const QueryOptions& opt) { LibraryQuery query(opt); query.SetColumnSpec("%songs_table.ROWID"); diff --git a/src/library/librarybackend.h b/src/library/librarybackend.h index e5884e51a..790a09218 100644 --- a/src/library/librarybackend.h +++ b/src/library/librarybackend.h @@ -82,6 +82,9 @@ class LibraryBackendInterface : public QObject { virtual Song GetSongById(int id) = 0; + // Returns all sections of a song with the given filename. If there's just one section + // the resulting list will have it's size equal to 1. + virtual SongList GetSongsByFilename(const QString& filename) = 0; // Returns a section of a song with the given filename and beginning. If the section // is not present in library, returns invalid song. // Using default beginning value is suitable when searching for single-section songs. @@ -140,6 +143,7 @@ class LibraryBackend : public LibraryBackendInterface { SongList GetSongsByForeignId(const QStringList& ids, const QString& table, const QString& column); + SongList GetSongsByFilename(const QString& filename); Song GetSongByFilename(const QString& filename, int beginning = 0); void AddDirectory(const QString& path); diff --git a/src/library/librarywatcher.cpp b/src/library/librarywatcher.cpp index cc0835b0a..da852292e 100644 --- a/src/library/librarywatcher.cpp +++ b/src/library/librarywatcher.cpp @@ -26,9 +26,10 @@ #include #include #include -#include -#include +#include #include +#include +#include #include #include @@ -286,15 +287,13 @@ void LibraryWatcher::ScanSubdirectory( foreach (const QString& file, files_on_disk) { if (stop_requested_) return; + // associated cue QString matching_cue = NoExtensionPart(file) + ".cue"; - QDateTime cue_last_modified = QFileInfo(matching_cue).lastModified(); - - uint cue_mtime = cue_last_modified.isValid() - ? cue_last_modified.toTime_t() - : 0; Song matching_song; if (FindSongByPath(songs_in_db, file, &matching_song)) { + uint matching_cue_mtime = GetMtimeForCue(matching_cue); + // The song is in the database and still on disk. // Check the mtime to see if it's been changed since it was added. QFileInfo file_info(file); @@ -306,8 +305,16 @@ void LibraryWatcher::ScanSubdirectory( continue; } + // cue sheet's path from library (if any) + QString song_cue = matching_song.cue_path(); + uint song_cue_mtime = GetMtimeForCue(song_cue); + + bool cue_deleted = song_cue_mtime == 0 && matching_song.has_cue(); + bool cue_added = matching_cue_mtime != 0 && !matching_song.has_cue(); + // watch out for cue songs which have their mtime equal to qMax(media_file_mtime, cue_sheet_mtime) - bool changed = matching_song.mtime() != qMax(file_info.lastModified().toTime_t(), cue_mtime); + bool changed = (matching_song.mtime() != qMax(file_info.lastModified().toTime_t(), song_cue_mtime)) + || cue_deleted || cue_added; // Also want to look to see whether the album art has changed QString image = ImageForSong(file, album_art); @@ -317,69 +324,23 @@ void LibraryWatcher::ScanSubdirectory( } // the song's changed - reread the metadata from file - // TODO: problem if cue gets deleted or added before an update if (changed) { qDebug() << file << "changed"; - // cue associated? - if(cue_mtime) { - QFile cue(matching_cue); - cue.open(QIODevice::ReadOnly); - - foreach(Song cue_song, cue_parser_->Load(&cue, matching_cue, path)) { - // update every song that's in the cue and library - Song matching_section = backend_->GetSongByFilename(cue_song.filename(), cue_song.beginning()); - if(matching_section.is_valid()) { - cue_song.set_directory_id(t->dir()); - PreserveUserSetData(file, image, matching_section, &cue_song, t); - } - } + // if cue associated... + if(!cue_deleted && (matching_song.has_cue() || cue_added)) { + UpdateCueAssociatedSongs(file, path, matching_cue, image, t); + // if no cue or it's about to lose it... } else { - Song song_on_disk; - song_on_disk.InitFromFile(file, t->dir()); - - if (!song_on_disk.is_valid()) - continue; - - PreserveUserSetData(file, image, matching_song, &song_on_disk, t); + UpdateNonCueAssociatedSong(file, matching_song, image, cue_deleted, t); } } } else { // The song is on disk but not in the DB - SongList song_list; + SongList song_list = ScanNewFile(file, path, matching_cue, &cues_processed); - // don't process the same cue many times - if(cues_processed.contains(matching_cue)) + if(song_list.isEmpty()) { continue; - - // it's a cue - create virtual tracks - if(cue_mtime) { - QFile cue(matching_cue); - cue.open(QIODevice::ReadOnly); - - // ignore FILEs pointing to other media files - foreach(const Song& cue_song, cue_parser_->Load(&cue, matching_cue, path)) { - if(cue_song.filename() == file) { - song_list << cue_song; - } - } - - if(song_list.isEmpty()) { - continue; - } - - cues_processed << matching_cue; - - // it's a normal media file - } else { - Song song; - song.InitFromFile(file, -1); - - if (!song.is_valid()) { - continue; - } - - song_list << song; } qDebug() << file << "created"; @@ -426,6 +387,108 @@ void LibraryWatcher::ScanSubdirectory( } } +void LibraryWatcher::UpdateCueAssociatedSongs(const QString& file, const QString& path, + const QString& matching_cue, const QString& image, + ScanTransaction* t) { + QFile cue(matching_cue); + cue.open(QIODevice::ReadOnly); + + SongList old_sections = backend_->GetSongsByFilename(file); + + QHash sections_map; + foreach(const Song& song, old_sections) { + if(song.is_valid()) { + sections_map[song.beginning()] = song; + } + } + + QSet used_ids; + + // update every song that's in the cue and library + foreach(Song cue_song, cue_parser_->Load(&cue, matching_cue, path)) { + cue_song.set_directory_id(t->dir()); + + Song matching = sections_map[cue_song.beginning()]; + // a new section + if(!matching.is_valid()) { + t->new_songs << cue_song; + // changed section + } else { + PreserveUserSetData(file, image, matching, &cue_song, t); + used_ids.insert(matching.id()); + } + } + + // sections that are now missing + foreach(const Song& matching, old_sections) { + if(!used_ids.contains(matching.id())) { + t->deleted_songs << matching; + } + } +} + + +void LibraryWatcher::UpdateNonCueAssociatedSong(const QString& file, const Song& matching_song, + const QString& image, bool cue_deleted, + ScanTransaction* t) { + // if a cue got deleted, we turn it's first section into the new + // 'raw' (cueless) song and we just remove the rest of the sections + // from the library + if(cue_deleted) { + foreach(const Song& song, backend_->GetSongsByFilename(file)) { + if(!song.IsMetadataEqual(matching_song) && song.is_valid()) { + t->deleted_songs << song; + } + } + } + + Song song_on_disk; + song_on_disk.InitFromFile(file, t->dir()); + + if(song_on_disk.is_valid()) { + PreserveUserSetData(file, image, matching_song, &song_on_disk, t); + } +} + +SongList LibraryWatcher::ScanNewFile(const QString& file, const QString& path, + const QString& matching_cue, QSet* cues_processed) { + SongList song_list; + + uint matching_cue_mtime = GetMtimeForCue(matching_cue); + // if it's a cue - create virtual tracks + if(matching_cue_mtime) { + // don't process the same cue many times + if(cues_processed->contains(matching_cue)) + return song_list; + + QFile cue(matching_cue); + cue.open(QIODevice::ReadOnly); + + // ignore FILEs pointing to other media files + foreach(const Song& cue_song, cue_parser_->Load(&cue, matching_cue, path)) { + if(cue_song.filename() == file) { + song_list << cue_song; + } + } + + if(!song_list.isEmpty()) { + *cues_processed << matching_cue; + } + + // it's a normal media file + } else { + Song song; + song.InitFromFile(file, -1); + + if (song.is_valid()) { + song_list << song; + } + + } + + return song_list; +} + void LibraryWatcher::PreserveUserSetData(const QString& file, const QString& image, const Song& matching_song, Song* out, ScanTransaction* t) { out->set_id(matching_song.id()); @@ -449,6 +512,19 @@ void LibraryWatcher::PreserveUserSetData(const QString& file, const QString& ima } } +uint LibraryWatcher::GetMtimeForCue(const QString& cue_path) { + // slight optimisation + if(cue_path.isEmpty()) { + return 0; + } + + QDateTime cue_last_modified = QFileInfo(cue_path).lastModified(); + + return cue_last_modified.isValid() + ? cue_last_modified.toTime_t() + : 0; +} + void LibraryWatcher::AddWatch(QFileSystemWatcher* w, const QString& path) { if (!QFile::exists(path)) return; diff --git a/src/library/librarywatcher.h b/src/library/librarywatcher.h index e8b9c9af6..f02327761 100644 --- a/src/library/librarywatcher.h +++ b/src/library/librarywatcher.h @@ -133,8 +133,27 @@ class LibraryWatcher : public QObject { QString PickBestImage(const QStringList& images); QString ImageForSong(const QString& path, QMap& album_art); void AddWatch(QFileSystemWatcher* w, const QString& path); + uint GetMtimeForCue(const QString& cue_path); + + // Updates the sections of a cue associated and altered (according to mtime) + // media file during a scan. + void UpdateCueAssociatedSongs(const QString& file, const QString& path, + const QString& matching_cue, const QString& image, + ScanTransaction* t); + // Updates a single non-cue associated and altered (according to mtime) song + // during a scan. + void UpdateNonCueAssociatedSong(const QString& file, const Song& matching_song, + const QString& image, bool cue_deleted, + ScanTransaction* t) ; + // Updates a new song with some metadata taken from it's equivalent old + // song (for example rating and score). void PreserveUserSetData(const QString& file, const QString& image, const Song& matching_song, Song* out, ScanTransaction* t); + // Scans a single media file that's present on the disk but not yet in the library. + // It may result in a multiple files added to the library when the media file + // has many sections (like a CUE related media file). + SongList ScanNewFile(const QString& file, const QString& path, + const QString& matching_cue, QSet* cues_processed); private: // One of these gets stored for each Directory we're watching diff --git a/src/playlistparsers/cueparser.cpp b/src/playlistparsers/cueparser.cpp index 2ee3b0527..54fbb0256 100644 --- a/src/playlistparsers/cueparser.cpp +++ b/src/playlistparsers/cueparser.cpp @@ -220,6 +220,7 @@ SongList CueParser::Load(QIODevice* device, const QString& playlist_path, const if(cue_mtime.isValid()) { song.set_mtime(qMax(cue_mtime.toTime_t(), song.mtime())); } + song.set_cue_path(playlist_path); // overwrite the stuff, we may have read from the file or library, using // the current .cue metadata diff --git a/src/scripting/python/librarybackend.sip b/src/scripting/python/librarybackend.sip index 926358804..4c3a12369 100644 --- a/src/scripting/python/librarybackend.sip +++ b/src/scripting/python/librarybackend.sip @@ -56,6 +56,7 @@ public: SongList GetSongsByForeignId(const QStringList& ids, const QString& table, const QString& column); + SongList GetSongsByFilename(const QString& filename); Song GetSongByFilename(const QString& filename, int beginning); void AddDirectory(const QString& path); diff --git a/src/scripting/python/song.sip b/src/scripting/python/song.sip index 5e956d1f4..894b5265b 100644 --- a/src/scripting/python/song.sip +++ b/src/scripting/python/song.sip @@ -55,6 +55,9 @@ public: int lastplayed() const; int score() const; + const QString& cue_path() const; + bool has_cue() const; + int beginning() const; int end() const; @@ -128,6 +131,7 @@ public: void set_filename(const QString& v); void set_basefilename(const QString& v); void set_directory_id(int v); + void set_cue_path(const QString& v); // Comparison functions bool IsMetadataEqual(const Song& other) const; diff --git a/tests/cueparser_test.cpp b/tests/cueparser_test.cpp index 0b95ed8d5..2b4e1d4ad 100644 --- a/tests/cueparser_test.cpp +++ b/tests/cueparser_test.cpp @@ -41,7 +41,7 @@ TEST_F(CueParserTest, ParsesASong) { QFile file(":testdata/onesong.cue"); file.open(QIODevice::ReadOnly); - SongList song_list = parser_.Load(&file, "", QDir("")); + SongList song_list = parser_.Load(&file, "CUEPATH", QDir("")); // one song ASSERT_EQ(1, song_list.size()); @@ -54,6 +54,7 @@ TEST_F(CueParserTest, ParsesASong) { ASSERT_EQ("", first_song.album()); ASSERT_EQ(1, first_song.beginning()); ASSERT_EQ(1, first_song.track()); + ASSERT_EQ("CUEPATH", first_song.cue_path()); } TEST_F(CueParserTest, ParsesTwoSongs) { @@ -153,7 +154,7 @@ TEST_F(CueParserTest, AcceptsMultipleFileBasedCues) { QFile file(":testdata/manyfiles.cue"); file.open(QIODevice::ReadOnly); - SongList song_list = parser_.Load(&file, "", QDir("")); + SongList song_list = parser_.Load(&file, "CUEPATH", QDir("")); // five songs ASSERT_EQ(5, song_list.size()); @@ -173,6 +174,7 @@ TEST_F(CueParserTest, AcceptsMultipleFileBasedCues) { ASSERT_EQ(1, first_song.beginning()); ASSERT_EQ(second_song.beginning() - first_song.beginning(), first_song.length()); ASSERT_EQ(-1, first_song.track()); + ASSERT_EQ("CUEPATH", first_song.cue_path()); ASSERT_TRUE(second_song.filename().endsWith("files/longer_one.mp3")); ASSERT_EQ("A1Song2", second_song.title()); @@ -190,6 +192,7 @@ TEST_F(CueParserTest, AcceptsMultipleFileBasedCues) { ASSERT_EQ(0, third_song.beginning()); ASSERT_EQ(fourth_song.beginning() - third_song.beginning(), third_song.length()); ASSERT_EQ(-1, third_song.track()); + ASSERT_EQ("CUEPATH", third_song.cue_path()); ASSERT_TRUE(fourth_song.filename().endsWith("files/longer_two_p1.mp3")); ASSERT_EQ("A2P1Song2", fourth_song.title()); @@ -206,6 +209,7 @@ TEST_F(CueParserTest, AcceptsMultipleFileBasedCues) { ASSERT_EQ("Artist Two", fifth_song.albumartist()); ASSERT_EQ(1, fifth_song.beginning()); ASSERT_EQ(-1, fifth_song.track()); + ASSERT_EQ("CUEPATH", fifth_song.cue_path()); } TEST_F(CueParserTest, SkipsBrokenSongsInMultipleFileBasedCues) { diff --git a/tests/mock_librarybackend.h b/tests/mock_librarybackend.h index d9ced8332..4b20dc371 100644 --- a/tests/mock_librarybackend.h +++ b/tests/mock_librarybackend.h @@ -51,6 +51,7 @@ class MockLibraryBackend : public LibraryBackendInterface { MOCK_METHOD1(GetSongById, Song(int)); + MOCK_METHOD1(GetSongsByFilename, SongList(const QString&)); MOCK_METHOD2(GetSongByFilename, Song(const QString&, int)); MOCK_METHOD1(AddDirectory, void(const QString&));