Annotation of /trunk/amarok/patches/amarok-2.2.2-kde220532.patch
Parent Directory | Revision Log
Revision 987 -
(hide annotations)
(download)
Fri Feb 19 16:00:40 2010 UTC (14 years, 7 months ago) by niro
File size: 16478 byte(s)
Fri Feb 19 16:00:40 2010 UTC (14 years, 7 months ago) by niro
File size: 16478 byte(s)
amarok 2.2.2 fixes
1 | niro | 987 | Fixes potential crashes during scanning discovered in 2.2.2 after tagging. |
2 | Jeff Mitchell <mitchell@kde.org> | ||
3 | |||
4 | |||
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() | ||
11 | { | ||
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 ) | ||
15 | - delete list; | ||
16 | + { | ||
17 | + if( list ) | ||
18 | + { | ||
19 | + if( !currSet.contains( list ) ) | ||
20 | + { | ||
21 | + delete list; | ||
22 | + currSet.insert( list ); | ||
23 | + } | ||
24 | + } | ||
25 | + else | ||
26 | + debug() << "GAAH! Tried to double-delete a value in m_urlsHashByUid"; | ||
27 | + } | ||
28 | foreach( QLinkedList<QStringList*> *list, m_albumsHashByName ) | ||
29 | { | ||
30 | - foreach( QStringList *slist, *list ) | ||
31 | - delete slist; | ||
32 | - delete list; | ||
33 | + if( list ) | ||
34 | + { | ||
35 | + foreach( QStringList *slist, *list ) | ||
36 | + { | ||
37 | + if( slist ) | ||
38 | + { | ||
39 | + if( !currSet.contains( slist ) ) | ||
40 | + { | ||
41 | + delete slist; | ||
42 | + currSet.insert( slist ); | ||
43 | + } | ||
44 | + else | ||
45 | + debug() << "GAAH! Tried to double-delete a value in m_albumsHashByName"; | ||
46 | + } | ||
47 | + } | ||
48 | + delete list; | ||
49 | + } | ||
50 | } | ||
51 | foreach( QLinkedList<QStringList*> *list, m_tracksHashByAlbum ) | ||
52 | { | ||
53 | - foreach( QStringList *slist, *list ) | ||
54 | - delete slist; | ||
55 | - delete list; | ||
56 | + if( list ) | ||
57 | + { | ||
58 | + foreach( QStringList *slist, *list ) | ||
59 | + { | ||
60 | + if( slist ) | ||
61 | + { | ||
62 | + if( !currSet.contains( slist ) ) | ||
63 | + { | ||
64 | + delete slist; | ||
65 | + currSet.insert( slist ); | ||
66 | + } | ||
67 | + else | ||
68 | + debug() << "GAAH! Tried to double-delete a value in m_tracksHashByAlbum"; | ||
69 | + } | ||
70 | + } | ||
71 | + delete list; | ||
72 | + } | ||
73 | } | ||
74 | } | ||
75 | |||
76 | @@ -67,11 +107,11 @@ ScanResultProcessor::setScanType( ScanType type ) | ||
77 | void | ||
78 | ScanResultProcessor::addDirectory( const QString &dir, uint mtime ) | ||
79 | { | ||
80 | - DEBUG_BLOCK | ||
81 | - debug() << "SRP::addDirectory on " << dir << " with mtime " << mtime; | ||
82 | + //DEBUG_BLOCK | ||
83 | + //debug() << "SRP::addDirectory on " << dir << " with mtime " << mtime; | ||
84 | if( dir.isEmpty() ) | ||
85 | { | ||
86 | - debug() << "got directory with no path from the scanner, not adding"; | ||
87 | + //debug() << "got directory with no path from the scanner, not adding"; | ||
88 | return; | ||
89 | } | ||
90 | setupDatabase(); | ||
91 | @@ -254,7 +294,7 @@ ScanResultProcessor::rollback() | ||
92 | void | ||
93 | ScanResultProcessor::processDirectory( const QList<QVariantMap > &data ) | ||
94 | { | ||
95 | -// DEBUG_BLOCK | ||
96 | + //DEBUG_BLOCK | ||
97 | setupDatabase(); | ||
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; | ||
103 | } | ||
104 | + | ||
105 | if( multipleAlbums || album.isEmpty() || artists.size() == 1 ) | ||
106 | { | ||
107 | foreach( const QVariantMap &row, data ) | ||
108 | { | ||
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 ) ) | ||
113 | + { | ||
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; | ||
119 | + } | ||
120 | + else | ||
121 | + { | ||
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 ); | ||
126 | + } | ||
127 | } | ||
128 | } | ||
129 | else | ||
130 | { | ||
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; | ||
136 | |||
137 | //debug() << "albumartist " << albumArtist << "for artists" << artists; | ||
138 | foreach( const QVariantMap &row, data ) | ||
139 | { | ||
140 | - addTrack( row, artist ); | ||
141 | + QString uid = row.value( Field::UNIQUEID ).toString(); | ||
142 | + if( m_uidsSeenThisScan.contains( uid ) ) | ||
143 | + { | ||
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; | ||
149 | + } | ||
150 | + else | ||
151 | + { | ||
152 | + addTrack( row, artist ); | ||
153 | + m_uidsSeenThisScan.insert( uid ); | ||
154 | + } | ||
155 | } | ||
156 | } | ||
157 | } | ||
158 | @@ -299,6 +369,7 @@ ScanResultProcessor::processDirectory( const QList<QVariantMap > &data ) | ||
159 | QString | ||
160 | ScanResultProcessor::findAlbumArtist( const QSet<QString> &artists, int trackCount ) const | ||
161 | { | ||
162 | + //DEBUG_BLOCK | ||
163 | QMap<QString, int> artistCount; | ||
164 | bool featuring; | ||
165 | QStringList trackArtists; | ||
166 | @@ -371,6 +442,7 @@ void | ||
167 | ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId ) | ||
168 | { | ||
169 | //DEBUG_BLOCK | ||
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 ) | ||
175 | |||
176 | //urlId will take care of the urls table part of AFT | ||
177 | int url = urlId( path, uid ); | ||
178 | - | ||
179 | +/* | ||
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]; | ||
187 | +*/ | ||
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 ) | ||
192 | //insert into hashes | ||
193 | if( m_tracksHashByUrl.contains( url ) && m_tracksHashByUrl[url] != 0 ) | ||
194 | { | ||
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 ); | ||
202 | } | ||
203 | |||
204 | + //debug() << "album = " << album; | ||
205 | + | ||
206 | if( m_tracksHashByAlbum.contains( album ) && m_tracksHashByAlbum[album] != 0 ) | ||
207 | - m_tracksHashByAlbum[album]->append( trackList ); | ||
208 | + { | ||
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 ) ) | ||
213 | + { | ||
214 | + //debug() << "appending trackList to m_tracksHashByAlbum"; | ||
215 | + m_tracksHashByAlbum[album]->append( trackList ); | ||
216 | + } | ||
217 | + else | ||
218 | + { | ||
219 | + //debug() << "not appending trackList to m_tracksHashByAlbum"; | ||
220 | + } | ||
221 | + | ||
222 | + } | ||
223 | else | ||
224 | { | ||
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 ) | ||
229 | { | ||
230 | + //debug() << "albumArtistId = " << albumArtistId; | ||
231 | + //debug() << "Checking list: " << *slist; | ||
232 | if( slist->at( 2 ).isEmpty() && albumArtistId == 0 ) | ||
233 | { | ||
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 ); | ||
240 | + { | ||
241 | + if( !m_albumsHashByName[album]->contains( albumList ) ) | ||
242 | + m_albumsHashByName[album]->append( albumList ); | ||
243 | + } | ||
244 | else | ||
245 | { | ||
246 | QLinkedList<QStringList*> *list = new QLinkedList<QStringList*>(); | ||
247 | @@ -645,7 +746,7 @@ ScanResultProcessor::albumInsert( const QString &album, int albumArtistId ) | ||
248 | int | ||
249 | ScanResultProcessor::urlId( const QString &url, const QString &uid ) | ||
250 | { | ||
251 | - /* | ||
252 | +/* | ||
253 | DEBUG_BLOCK | ||
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]; | ||
260 | - */ | ||
261 | - | ||
262 | +*/ | ||
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; | ||
270 | +/* | ||
271 | if( m_urlsHashByLocation.contains( locationPair ) ) | ||
272 | { | ||
273 | QStringList values; | ||
274 | @@ -674,6 +775,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid ) | ||
275 | values << "zero"; | ||
276 | //debug() << "m_urlsHashByLocation contains it! It is " << values; | ||
277 | } | ||
278 | +*/ | ||
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 ) | ||
291 | delete oldList; | ||
292 | } | ||
293 | m_urlsHashByLocation[locationPair] = list; | ||
294 | + m_urlsHashByLocation.remove( oldLocationPair ); | ||
295 | } | ||
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 ) | ||
299 | { | ||
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 ) | ||
307 | delete oldList; | ||
308 | } | ||
309 | m_urlsHashByUid[uid] = list; | ||
310 | + m_urlsHashByUid.remove( oldId ); | ||
311 | } | ||
312 | m_permanentTablesUidUpdates.insert( url, uid ); | ||
313 | m_changedUids.insert( currUrlIdValues[4], uid ); | ||
314 | @@ -855,7 +961,8 @@ ScanResultProcessor::directoryId( const QString &dir ) | ||
315 | int | ||
316 | ScanResultProcessor::checkExistingAlbums( const QString &album ) | ||
317 | { | ||
318 | -// DEBUG_BLOCK | ||
319 | + //DEBUG_BLOCK | ||
320 | + //debug() << "looking for album " << album; | ||
321 | // "Unknown" albums shouldn't be handled as compilations | ||
322 | if( album.isEmpty() ) | ||
323 | return 0; | ||
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 ) | ||
328 | + { | ||
329 | + //debug() << "hashByName doesn't contain album, or it's zero"; | ||
330 | return 0; | ||
331 | + } | ||
332 | |||
333 | QStringList trackIds; | ||
334 | QLinkedList<QStringList*> *llist = m_albumsHashByName[album]; | ||
335 | @@ -915,8 +1025,10 @@ ScanResultProcessor::checkExistingAlbums( const QString &album ) | ||
336 | } | ||
337 | } | ||
338 | |||
339 | + //debug() << "trackIds = " << trackIds; | ||
340 | if( trackIds.isEmpty() ) | ||
341 | { | ||
342 | + //debug() << "trackIds empty, returning zero"; | ||
343 | return 0; | ||
344 | } | ||
345 | else | ||
346 | @@ -933,6 +1045,7 @@ ScanResultProcessor::checkExistingAlbums( const QString &album ) | ||
347 | list->replace( 3, compilationString ); | ||
348 | } | ||
349 | } | ||
350 | + //debug() << "returning " << compilationId; | ||
351 | return compilationId; | ||
352 | } | ||
353 | } | ||
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; | ||
358 | + | ||
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() ) | ||
364 | + { | ||
365 | + debug() << "Key: " << key; | ||
366 | + foreach( QStringList* item, *m_tracksHashByAlbum[key] ) | ||
367 | + debug() << "list: " << item << " is " << *item; | ||
368 | + } | ||
369 | */ | ||
370 | |||
371 | DEBUG_BLOCK | ||
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; | ||
379 | |||
380 | + QSet<QString> m_uidsSeenThisScan; | ||
381 | QHash<QString, uint> m_filesInDirs; | ||
382 | |||
383 | TrackUrls m_changedUids; //not really track urls |