Contents of /trunk/amarok/patches/amarok-2.2.2-kde220532.patch
Parent Directory | Revision Log
Revision 987 -
(show 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 | 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 |