1
Fixes potential crashes during scanning discovered in 2.2.2 after tagging.
2
Jeff Mitchell <mitchell@kde.org>
5
diff --git a/src/collection/sqlcollection/ScanResultProcessor.cpp b/src/collection/sqlcollection/ScanResultProcessor.cpp
6
index b680623..68b31df 100644
7
--- a/src/collection/sqlcollection/ScanResultProcessor.cpp
8
+++ b/src/collection/sqlcollection/ScanResultProcessor.cpp
9
@@ -42,19 +42,59 @@ ScanResultProcessor::ScanResultProcessor( SqlCollection *collection )
10
ScanResultProcessor::~ScanResultProcessor()
12
//everything has a URL, so enough to just delete from here
13
+ QSet<QStringList*> currSet; //prevent double deletes
14
foreach( QStringList *list, m_urlsHashByUid )
19
+ if( !currSet.contains( list ) )
22
+ currSet.insert( list );
26
+ debug() << "GAAH! Tried to double-delete a value in m_urlsHashByUid";
28
foreach( QLinkedList<QStringList*> *list, m_albumsHashByName )
30
- foreach( QStringList *slist, *list )
35
+ foreach( QStringList *slist, *list )
39
+ if( !currSet.contains( slist ) )
42
+ currSet.insert( slist );
45
+ debug() << "GAAH! Tried to double-delete a value in m_albumsHashByName";
51
foreach( QLinkedList<QStringList*> *list, m_tracksHashByAlbum )
53
- foreach( QStringList *slist, *list )
58
+ foreach( QStringList *slist, *list )
62
+ if( !currSet.contains( slist ) )
65
+ currSet.insert( slist );
68
+ debug() << "GAAH! Tried to double-delete a value in m_tracksHashByAlbum";
76
@@ -67,11 +107,11 @@ ScanResultProcessor::setScanType( ScanType type )
78
ScanResultProcessor::addDirectory( const QString &dir, uint mtime )
81
- debug() << "SRP::addDirectory on " << dir << " with mtime " << mtime;
83
+ //debug() << "SRP::addDirectory on " << dir << " with mtime " << mtime;
86
- debug() << "got directory with no path from the scanner, not adding";
87
+ //debug() << "got directory with no path from the scanner, not adding";
91
@@ -254,7 +294,7 @@ ScanResultProcessor::rollback()
93
ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
98
//using the following heuristics:
99
//if more than one album is in the dir, use the artist of each track as albumartist
100
@@ -274,24 +314,54 @@ ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
101
if( row.value( Field::ALBUM ).toString() != album )
102
multipleAlbums = true;
105
if( multipleAlbums || album.isEmpty() || artists.size() == 1 )
107
foreach( const QVariantMap &row, data )
109
- int artist = genericId( &m_artists, row.value( Field::ARTIST ).toString(), &m_nextArtistNum );
110
- addTrack( row, artist );
111
+ QString uid = row.value( Field::UNIQUEID ).toString();
112
+ if( m_uidsSeenThisScan.contains( uid ) )
114
+ QString originalLocation = ( ( m_urlsHashByUid.contains( uid ) &&
115
+ m_urlsHashByUid[uid] != 0 ) ?
116
+ MountPointManager::instance()->getAbsolutePath( m_urlsHashByUid[uid]->at( 1 ).toInt(), m_urlsHashByUid[uid]->at( 2 ) ) : "(unknown)" );
117
+ debug() << "Skipping file with uniqueid " << uid << " as it was already seen this scan," <<
118
+ "file is at " << row.value( Field::URL ).toString() << ", original file is at " << originalLocation;
122
+ int artist = genericId( &m_artists, row.value( Field::ARTIST ).toString(), &m_nextArtistNum );
123
+ //debug() << "artist found = " << artist;
124
+ addTrack( row, artist );
125
+ m_uidsSeenThisScan.insert( uid );
131
QString albumArtist = findAlbumArtist( artists, data.count() );
132
+ //debug() << "albumArtist found = " << albumArtist;
133
//an empty string means that no albumartist was found
134
int artist = albumArtist.isEmpty() ? 0 : genericId( &m_artists, albumArtist, &m_nextArtistNum );
135
+ //debug() << "artist found = " << artist;
137
//debug() << "albumartist " << albumArtist << "for artists" << artists;
138
foreach( const QVariantMap &row, data )
140
- addTrack( row, artist );
141
+ QString uid = row.value( Field::UNIQUEID ).toString();
142
+ if( m_uidsSeenThisScan.contains( uid ) )
144
+ QString originalLocation = ( ( m_urlsHashByUid.contains( uid ) &&
145
+ m_urlsHashByUid[uid] != 0 ) ?
146
+ MountPointManager::instance()->getAbsolutePath( m_urlsHashByUid[uid]->at( 1 ).toInt(), m_urlsHashByUid[uid]->at( 2 ) ) : "(unknown)" );
147
+ debug() << "Skipping file with uniqueid " << uid << " as it was already seen this scan," <<
148
+ "file is at " << row.value( Field::URL ).toString() << ", original file is at " << originalLocation;
152
+ addTrack( row, artist );
153
+ m_uidsSeenThisScan.insert( uid );
158
@@ -299,6 +369,7 @@ ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
160
ScanResultProcessor::findAlbumArtist( const QSet<QString> &artists, int trackCount ) const
163
QMap<QString, int> artistCount;
165
QStringList trackArtists;
166
@@ -371,6 +442,7 @@ void
167
ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
170
+ //debug() << "albumArtistId = " << albumArtistId;
171
//amarok 1 stored all tracks of a compilation in different directories.
172
//when using its "Organize Collection" feature
173
//try to detect these cases
174
@@ -419,7 +491,15 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
176
//urlId will take care of the urls table part of AFT
177
int url = urlId( path, uid );
180
+ foreach( QString key, m_urlsHashByUid.keys() )
181
+ debug() << "Key: " << key << ", list: " << *m_urlsHashByUid[key];
182
+ foreach( int key, m_urlsHashById.keys() )
183
+ debug() << "Key: " << key << ", list: " << *m_urlsHashById[key];
184
+ typedef QPair<int, QString> blahType; //QFOREACH is stupid when it comes to QPairs
185
+ foreach( blahType key, m_urlsHashByLocation.keys() )
186
+ debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
188
QStringList *trackList = new QStringList();
189
int id = m_nextTrackNum;
190
//debug() << "Appending new track number with tracknum: " << id;
191
@@ -470,7 +550,7 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
193
if( m_tracksHashByUrl.contains( url ) && m_tracksHashByUrl[url] != 0 )
195
- //debug() << "m_tracksHashByUrl contains the url!";
196
+ //debug() << "m_tracksHashByUrl already contains url " << url;
197
//need to replace, not overwrite/add a new one
198
QStringList *oldValues = m_tracksHashByUrl[url];
199
QString oldId = oldValues->at( 0 );
200
@@ -490,8 +570,24 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
201
m_tracksHashById.insert( id, trackList );
204
+ //debug() << "album = " << album;
206
if( m_tracksHashByAlbum.contains( album ) && m_tracksHashByAlbum[album] != 0 )
207
- m_tracksHashByAlbum[album]->append( trackList );
209
+ //contains isn't the fastest on linked lists, but in reality this is on the order of maybe
210
+ //ten quick pointer comparisons per track on average...probably lower
211
+ //debug() << "trackList is " << trackList;
212
+ if( !m_tracksHashByAlbum[album]->contains( trackList ) )
214
+ //debug() << "appending trackList to m_tracksHashByAlbum";
215
+ m_tracksHashByAlbum[album]->append( trackList );
219
+ //debug() << "not appending trackList to m_tracksHashByAlbum";
225
QLinkedList<QStringList*> *list = new QLinkedList<QStringList*>();
226
@@ -595,6 +691,8 @@ ScanResultProcessor::albumId( const QString &album, int albumArtistId )
227
QLinkedList<QStringList*> *list = m_albumsHashByName[album];
228
foreach( QStringList *slist, *list )
230
+ //debug() << "albumArtistId = " << albumArtistId;
231
+ //debug() << "Checking list: " << *slist;
232
if( slist->at( 2 ).isEmpty() && albumArtistId == 0 )
234
//debug() << "artist is empty and albumArtistId = 0, returning " << slist->at( 0 );
235
@@ -631,7 +729,10 @@ ScanResultProcessor::albumInsert( const QString &album, int albumArtistId )
236
albumList->append( QString() );
237
m_albumsHashById[returnedNum] = albumList;
238
if( m_albumsHashByName.contains( album ) && m_albumsHashByName[album] != 0 )
239
- m_albumsHashByName[album]->append( albumList );
241
+ if( !m_albumsHashByName[album]->contains( albumList ) )
242
+ m_albumsHashByName[album]->append( albumList );
246
QLinkedList<QStringList*> *list = new QLinkedList<QStringList*>();
247
@@ -645,7 +746,7 @@ ScanResultProcessor::albumInsert( const QString &album, int albumArtistId )
249
ScanResultProcessor::urlId( const QString &url, const QString &uid )
254
foreach( QString key, m_urlsHashByUid.keys() )
255
debug() << "Key: " << key << ", list: " << *m_urlsHashByUid[key];
256
@@ -654,8 +755,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
257
typedef QPair<int, QString> blahType; //QFOREACH is stupid when it comes to QPairs
258
foreach( blahType key, m_urlsHashByLocation.keys() )
259
debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
263
QFileInfo fileInfo( url );
264
const QString dir = fileInfo.absoluteDir().absolutePath();
265
int dirId = directoryId( dir );
266
@@ -665,6 +765,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
267
QPair<int, QString> locationPair( deviceId, rpath );
268
//debug() << "in urlId with url = " << url << " and uid = " << uid;
269
//debug() << "checking locationPair " << locationPair;
271
if( m_urlsHashByLocation.contains( locationPair ) )
274
@@ -674,6 +775,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
276
//debug() << "m_urlsHashByLocation contains it! It is " << values;
279
QStringList currUrlIdValues;
280
if( m_urlsHashByUid.contains( uid ) && m_urlsHashByUid[uid] != 0 )
281
currUrlIdValues = *m_urlsHashByUid[uid];
282
@@ -717,6 +819,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
283
//debug() << "m_urlsHashByUid contains this UID, updating deviceId and path";
284
QStringList *list = m_urlsHashByUid[uid];
285
//debug() << "list from UID hash is " << list << " with values " << *list;
286
+ QPair<int, QString> oldLocationPair( list->at( 1 ).toInt(), list->at( 2 ) );
287
list->replace( 1, QString::number( deviceId ) );
288
list->replace( 2, rpath );
289
list->replace( 3, QString::number( dirId ) );
290
@@ -737,6 +840,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
293
m_urlsHashByLocation[locationPair] = list;
294
+ m_urlsHashByLocation.remove( oldLocationPair );
296
m_permanentTablesUrlUpdates.insert( uid, url );
297
m_changedUrls.insert( uid, QPair<QString, QString>( MountPointManager::instance()->getAbsolutePath( currUrlIdValues[1].toInt(), currUrlIdValues[2] ), url ) );
298
@@ -751,6 +855,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
300
QStringList *list = m_urlsHashByLocation[locationPair];
301
//debug() << "Replacing hash " << list->at( 4 ) << " with " << uid;
302
+ QString oldId = list->at( 4 );
303
list->replace( 4, uid );
304
if( m_urlsHashByUid.contains( uid )
305
&& m_urlsHashByUid[uid] != 0
306
@@ -762,6 +867,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
309
m_urlsHashByUid[uid] = list;
310
+ m_urlsHashByUid.remove( oldId );
312
m_permanentTablesUidUpdates.insert( url, uid );
313
m_changedUids.insert( currUrlIdValues[4], uid );
314
@@ -855,7 +961,8 @@ ScanResultProcessor::directoryId( const QString &dir )
316
ScanResultProcessor::checkExistingAlbums( const QString &album )
320
+ //debug() << "looking for album " << album;
321
// "Unknown" albums shouldn't be handled as compilations
322
if( album.isEmpty() )
324
@@ -865,7 +972,10 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
325
//it's probably a compilation.
326
//this handles A1 compilations that were automatically organized by Amarok
327
if( !m_albumsHashByName.contains( album ) || m_albumsHashByName[album] == 0 )
329
+ //debug() << "hashByName doesn't contain album, or it's zero";
333
QStringList trackIds;
334
QLinkedList<QStringList*> *llist = m_albumsHashByName[album];
335
@@ -915,8 +1025,10 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
339
+ //debug() << "trackIds = " << trackIds;
340
if( trackIds.isEmpty() )
342
+ //debug() << "trackIds empty, returning zero";
346
@@ -933,6 +1045,7 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
347
list->replace( 3, compilationString );
350
+ //debug() << "returning " << compilationId;
351
return compilationId;
354
@@ -1167,6 +1280,17 @@ ScanResultProcessor::copyHashesToTempTables()
355
foreach( blahType key, m_urlsHashByLocation.keys() )
356
debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
357
debug() << "Next album num: " << m_nextAlbumNum;
359
+ foreach( int key, m_tracksHashById.keys() )
360
+ debug() << "Key: " << key << ", list: " << *m_tracksHashById[key];
361
+ foreach( int key, m_tracksHashByUrl.keys() )
362
+ debug() << "Key: " << key << ", list: " << *m_tracksHashByUrl[key];
363
+ foreach( int key, m_tracksHashByAlbum.keys() )
365
+ debug() << "Key: " << key;
366
+ foreach( QStringList* item, *m_tracksHashByAlbum[key] )
367
+ debug() << "list: " << item << " is " << *item;
372
diff --git a/src/collection/sqlcollection/ScanResultProcessor.h b/src/collection/sqlcollection/ScanResultProcessor.h
373
index 71afc98..c7f200f 100644
374
--- a/src/collection/sqlcollection/ScanResultProcessor.h
375
+++ b/src/collection/sqlcollection/ScanResultProcessor.h
376
@@ -94,6 +94,7 @@ class ScanResultProcessor : public QObject
377
QMap<QString, int> m_directories;
378
QMap<QString, QList< QPair< QString, QString > > > m_imageMap;
380
+ QSet<QString> m_uidsSeenThisScan;
381
QHash<QString, uint> m_filesInDirs;
383
TrackUrls m_changedUids; //not really track urls