From 887e045a63e8e15e67f3909cad7e40b835568428 Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Sun, 4 Mar 2018 20:13:05 +0100 Subject: [PATCH] Fix crash when loading CD playlists --- TODO | 5 ++++- src/core/song.cpp | 12 +++++------ src/core/song.h | 3 +-- src/device/cddasongloader.cpp | 2 ++ src/engine/gstengine.cpp | 7 ++++--- src/playlist/playlistbackend.cpp | 34 ++----------------------------- src/playlist/playlistitem.cpp | 2 +- src/playlist/songplaylistitem.cpp | 2 +- 8 files changed, 20 insertions(+), 47 deletions(-) diff --git a/TODO b/TODO index e0fdff3e8..52736827e 100644 --- a/TODO +++ b/TODO @@ -3,6 +3,9 @@ Strawberry Music Player TODO - Fix freeze on exit caused by GStreamer -- Fix crash when loading playlists from Audio CD +- Fix Audio CD playback - Improve status/context - Fix crash when switching backend + +. + diff --git a/src/core/song.cpp b/src/core/song.cpp index 09ae5cfc9..c9013ee49 100644 --- a/src/core/song.cpp +++ b/src/core/song.cpp @@ -530,14 +530,12 @@ void Song::InitFromQuery(const SqlRow &q, bool reliable_metadata, int col) { //qLog(Debug) << __PRETTY_FUNCTION__; //qLog(Debug) << "Song::kColumns.size():" << Song::kColumns.size() << "q.columns_.size():" << q.columns_.size() << "col:" << col; - int i = 0; int x = col; - d->id_ = toint(col); - - for (i = 0 ; i < Song::kColumns.size(); i++) { - x=i+col+1; - + + for (int i = 0 ; i < Song::kColumns.size(); i++) { + x++; + if (x >= q.columns_.size()) { qLog(Error) << "Skipping" << Song::kColumns.value(i); break; @@ -662,7 +660,7 @@ void Song::InitFromQuery(const SqlRow &q, bool reliable_metadata, int col) { } else if (Song::kColumns.value(i) == "cue_path") { - d->cue_path_ = tostr(col + x); + d->cue_path_ = tostr(x); } else { diff --git a/src/core/song.h b/src/core/song.h index 6265efa46..c7c4f7fab 100644 --- a/src/core/song.h +++ b/src/core/song.h @@ -112,8 +112,7 @@ class Song { void InitFromProtobuf(const pb::tagreader::SongMetadata &pb); void InitFromQuery(const SqlRow &query, bool reliable_metadata, int col = 0); void InitFromFilePartial(const QString &filename); // Just store the filename: incomplete but fast - void InitArtManual(); // Check if there is already a art in the cache and - // store the filename in art_manual + void InitArtManual(); // Check if there is already a art in the cache and store the filename in art_manual #ifdef HAVE_LIBLASTFM void InitFromLastFM(const lastfm::Track &track); #endif diff --git a/src/device/cddasongloader.cpp b/src/device/cddasongloader.cpp index 9deb50433..3b20aa995 100644 --- a/src/device/cddasongloader.cpp +++ b/src/device/cddasongloader.cpp @@ -40,6 +40,8 @@ CddaSongLoader::~CddaSongLoader() { QUrl CddaSongLoader::GetUrlFromTrack(int track_number) const { + qLog(Debug) << url_; + if (url_.isEmpty()) { return QUrl(QString("cdda://%1").arg(track_number)); } diff --git a/src/engine/gstengine.cpp b/src/engine/gstengine.cpp index 1a653bca2..753332ca9 100644 --- a/src/engine/gstengine.cpp +++ b/src/engine/gstengine.cpp @@ -695,9 +695,9 @@ void GstEngine::SeekNow() { } void GstEngine::SetEqualizerEnabled(bool enabled) { - + //qLog(Debug) << "equalizer ENABLED: " << enabled; - + equalizer_enabled_ = enabled; if (current_pipeline_) current_pipeline_->SetEqualizerEnabled(enabled); @@ -757,6 +757,7 @@ void GstEngine::timerEvent(QTimerEvent *e) { } } } + } void GstEngine::HandlePipelineError(int pipeline_id, const QString &message, int domain, int error_code) { @@ -766,7 +767,7 @@ void GstEngine::HandlePipelineError(int pipeline_id, const QString &message, int if (!current_pipeline_.get() || current_pipeline_->id() != pipeline_id) return; - qLog(Warning) << "Gstreamer error:" << message; + qLog(Warning) << "Gstreamer error:" << domain << error_code << message; current_pipeline_.reset(); diff --git a/src/playlist/playlistbackend.cpp b/src/playlist/playlistbackend.cpp index 8ef3a0528..99378f35d 100644 --- a/src/playlist/playlistbackend.cpp +++ b/src/playlist/playlistbackend.cpp @@ -107,7 +107,6 @@ PlaylistBackend::Playlist PlaylistBackend::GetPlaylist(int id) { QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); - QSqlQuery q(db); q.prepare("SELECT ROWID, name, last_played, special_type, ui_path, is_favorite FROM playlists WHERE ROWID=:id"); @@ -126,12 +125,11 @@ PlaylistBackend::Playlist PlaylistBackend::GetPlaylist(int id) { p.favorite = q.value(5).toBool(); return p; + } QSqlQuery PlaylistBackend::GetPlaylistRows(int playlist) { - - //qLog(Debug) << __PRETTY_FUNCTION__; - + QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); @@ -158,8 +156,6 @@ QSqlQuery PlaylistBackend::GetPlaylistRows(int playlist) { QList PlaylistBackend::GetPlaylistItems(int playlist) { - //qLog(Debug) << __PRETTY_FUNCTION__; - QSqlQuery q = GetPlaylistRows(playlist); // Note that as this only accesses the query, not the db, we don't need the // mutex. @@ -178,8 +174,6 @@ QList PlaylistBackend::GetPlaylistItems(int playlist) { QList PlaylistBackend::GetPlaylistSongs(int playlist) { - //qLog(Debug) << __PRETTY_FUNCTION__; - QSqlQuery q = GetPlaylistRows(playlist); // Note that as this only accesses the query, not the db, we don't need the // mutex. @@ -198,12 +192,8 @@ QList PlaylistBackend::GetPlaylistSongs(int playlist) { PlaylistItemPtr PlaylistBackend::NewPlaylistItemFromQuery(const SqlRow &row, std::shared_ptr state) { - //qLog(Debug) << __PRETTY_FUNCTION__; - // The song tables get joined first, plus one each for the song ROWIDs const int playlist_row = (Song::kColumns.count() + 1) * kSongTableJoins; - - //qLog(Debug) << row.value(playlist_row).toString(); PlaylistItemPtr item(PlaylistItem::NewFromType(row.value(playlist_row).toString())); if (item) { @@ -217,8 +207,6 @@ PlaylistItemPtr PlaylistBackend::NewPlaylistItemFromQuery(const SqlRow &row, std } Song PlaylistBackend::NewSongFromQuery(const SqlRow &row, std::shared_ptr state) { - - //qLog(Debug) << __PRETTY_FUNCTION__; return NewPlaylistItemFromQuery(row, state)->Metadata(); @@ -227,8 +215,6 @@ Song PlaylistBackend::NewSongFromQuery(const SqlRow &row, std::shared_ptr state) { - - //qLog(Debug) << __PRETTY_FUNCTION__; // we need collection to run a CueParser; also, this method applies only to file-type PlaylistItems if (item->type() != "File") return item; @@ -277,8 +263,6 @@ PlaylistItemPtr PlaylistBackend::RestoreCueData(PlaylistItemPtr item, std::share } void PlaylistBackend::SavePlaylistAsync(int playlist, const PlaylistItemList &items, int last_played) { - - //qLog(Debug) << __PRETTY_FUNCTION__; metaObject()->invokeMethod(this, "SavePlaylist", Qt::QueuedConnection, Q_ARG(int, playlist), Q_ARG(PlaylistItemList, items), Q_ARG(int, last_played)); @@ -286,8 +270,6 @@ void PlaylistBackend::SavePlaylistAsync(int playlist, const PlaylistItemList &it void PlaylistBackend::SavePlaylist(int playlist, const PlaylistItemList& items, int last_played) { - //qLog(Debug) << __PRETTY_FUNCTION__; - QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); @@ -327,8 +309,6 @@ void PlaylistBackend::SavePlaylist(int playlist, const PlaylistItemList& items, } int PlaylistBackend::CreatePlaylist(const QString& name, const QString& special_type) { - - //qLog(Debug) << __PRETTY_FUNCTION__; QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); @@ -345,8 +325,6 @@ int PlaylistBackend::CreatePlaylist(const QString& name, const QString& special_ } void PlaylistBackend::RemovePlaylist(int id) { - - //qLog(Debug) << __PRETTY_FUNCTION__; QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); @@ -371,8 +349,6 @@ void PlaylistBackend::RemovePlaylist(int id) { } void PlaylistBackend::RenamePlaylist(int id, const QString &new_name) { - - //qLog(Debug) << __PRETTY_FUNCTION__; QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); @@ -387,8 +363,6 @@ void PlaylistBackend::RenamePlaylist(int id, const QString &new_name) { } void PlaylistBackend::FavoritePlaylist(int id, bool is_favorite) { - - //qLog(Debug) << __PRETTY_FUNCTION__; QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); @@ -403,8 +377,6 @@ void PlaylistBackend::FavoritePlaylist(int id, bool is_favorite) { } void PlaylistBackend::SetPlaylistOrder(const QList &ids) { - - //qLog(Debug) << __PRETTY_FUNCTION__; QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); @@ -428,8 +400,6 @@ void PlaylistBackend::SetPlaylistOrder(const QList &ids) { } void PlaylistBackend::SetPlaylistUiPath(int id, const QString &path) { - - //qLog(Debug) << __PRETTY_FUNCTION__; QMutexLocker l(db_->Mutex()); QSqlDatabase db(db_->Connect()); diff --git a/src/playlist/playlistitem.cpp b/src/playlist/playlistitem.cpp index 760336c85..1b41e3195 100644 --- a/src/playlist/playlistitem.cpp +++ b/src/playlist/playlistitem.cpp @@ -56,7 +56,7 @@ PlaylistItem* PlaylistItem::NewFromSongsTable(const QString &table, const Song & } -void PlaylistItem::BindToQuery(QSqlQuery* query) const { +void PlaylistItem::BindToQuery(QSqlQuery *query) const { query->bindValue(":type", type()); query->bindValue(":collection_id", DatabaseValue(Column_CollectionId)); diff --git a/src/playlist/songplaylistitem.cpp b/src/playlist/songplaylistitem.cpp index 4bccebb00..96d47befc 100644 --- a/src/playlist/songplaylistitem.cpp +++ b/src/playlist/songplaylistitem.cpp @@ -37,7 +37,7 @@ SongPlaylistItem::SongPlaylistItem(const Song &song) bool SongPlaylistItem::InitFromQuery(const SqlRow &query) { - song_.InitFromQuery(query, false, (Song::kColumns.count() + 1) * 3); + song_.InitFromQuery(query, false, (Song::kColumns.count()+1)); return true; }