Bugzilla – Bug 1735
Scanning (corrupt) playlists adds albums, removes years
Last modified: 2008-08-18 10:54:16 UTC
I have a folder "Peter Gabriel" with the following content: 1 (Car)/ 2 (Scratch)/ 3 (Melt)/ 4 (Security)/ Birdy/ Ein deutsches Album [3]/ Ein deutsches Album [4]/ Growing Up (Summer 2003) - 06.26.03 Milwaukee/ Hit/ OVO Millennium Show/ Passion/ pg.m3u* Plays Live/ Secret World Live/ Shaking the Tree/ Sky Blue (Martin Bennet Remix).mp3* So/ Still Growing UP - Zurich 24.05.04/ Up/ Us/ pg.m3u contains a reference to "1 (car)" (yes, lowercase!). This is added as an empty album to the album list. The other albums contained in that playlist (Up, So) don't show their year in the album list. After removing the playlist and a rescan everything's fine again.
Created attachment 583 [details] The playlist. It was created under Windows using winamp, I guess.
Created attachment 584 [details] log while scanning "Peter Gabriel" folder Oh, it isn't reading any information from this playlist as it does not find the files (see slimserver.log). Might be due to using Windows style backslashes but running the server on Linux. Shouldn't playlist entries which aren't available be ignored?
Dan is fixing playlist scanning in the music folder...to make sure that they aren't.
Perhaps a related bug is reported in this post in the forums: http://forums.slimdevices.com/showpost.php?p=43189&postcount=3 It seems that when scanning a music folder that contains a *.m3u file such as the following: #EXTM3U #EXTINF:307,Mysterons Portishead - Mysterons.mp3 #EXTINF:254,Sour Times Portishead - Sour Times.mp3 #EXTINF:238,Strangers Portishead - Strangers.mp3 #EXTINF:260,It Could Be Sweet Portishead - It Could Be Sweet.mp3 #EXTINF:297,Wandering Star Portishead - Wandering Star.mp3 #EXTINF:238,Numb Portishead - Numb.mp3 #EXTINF:310,Roads Portishead - Roads.mp3 #EXTINF:221,Pedestal Portishead - Pedestal.mp3 #EXTINF:304,Biscuit Portishead - Biscuit.mp3 #EXTINF:309,Glory Box Portishead - Glory Box.mp3 6.1b1 will prefix the directory and song name with "C:/WINDOWS/system32". With the 12 June 2005 nightly this did not happen and I had 5113 songs in the library (which is correct) but now with 6.1b1 I have 7233 songs in my library (very incorrect!)
Created attachment 597 [details] Scan Log and M3U that results in files being erroneously indexed in "C:\WINDOWS\system32" This behaviour is seen with 6.1b1 on Windows 2003 Server. Never seen before this version (last version used 12 Jun 2005 nightly). Previous version (12 Jun 2005) uninstalled and 6.1b1 installed from scratch.
neil, could you attach a d_paths log for the same section as the d_scan? These playlists are relative paths, so the d_paths will hopefully show anything going wrong in converting the relative to absolute.
Created attachment 620 [details] d_path and d_scan logs for D:\Music\Test\MP3\Portishead\Dummy Hi kdf - sorry for the late reply, I didn't receive an email from bugzilla informing me this entry had been updated. To simply matters, I've created a new library consisting only of the Porishead album, as follows: D:\Music\Test\MP3\Portishead\Dummy The "Dummy" directory consists of 11 items - 10 songs and 1 m3u/playlist file. The playlist file is as previously attached to my earlier entry. The d_paths and d_scan log output for the single "Dummy" directory is attached to this entry. Having rescanned the library for the single Portishead album, when I access the "Home" page of the web interface (Browse Artists) Slim Server informs me that I now have "2 albums with 20 songs by 2 artists" and lists All Albums, No Artist and Portishead. If I drill into the Portishead, Slim Server informs me I have "1 album with 10 songs by 1 artist" and lists the album Dummy. Drilling into Dummy correctly lists the 10 tracks. However if I drill into "No Artist" from the Home page, Slim Server informs me I have "1 album with 10 songs by 1 artist" - an album called "No Album" is listed, but drilling into this album results in an empty track listing. Clicking on the "No Artist" in the navigation "Browse Artists / No Artist / No Album" results in the message "0 albums with 0 songs by 0 artists". Clicking on "Browse Artists" now results in "2 albums with 10 songs by 1 artist", and only Portishead is listed. Note that the above testing has been performed using 6.1b2 on Windows 2003 Server, standard install in "C:\Program Files\SlimServer" and running slim.exe as a service.
ok, something is wrong with parsing m3u and fixPath(). As I dont' run windows, I can't easily test with this setup myself. However, if you can provide the same section of scan with just d_paths and d_parse it might help narrow down the point where things go wrong.
Created attachment 621 [details] d_paths and d_parse logs for D:\Music\Test\MP3\Portishead\Dummy Hi kdf - log attached for the scan of the single Portishead/Dummy directory with d_paths and d_parse enabled. Let me know if I can be of any assistance. PS. Bugzilla emails still don't seem to be arriving... :(
Forget about the email issues - probably explained due to me not being the reporter, assignee, cc or voter! Adding myself to cc list... :)
a few notes: this seems to be the result of change 3507, which is the playlist-bmf merge. in the branch, this was change 3427. we used to do pathFromFileURL, but instead its now splitdir and catdir, which doesn't seem to construct a proper fileurl at the end. Cause is around Utils/Scan.pm line 638. adding: shift(@parts); at line 643 may help, but I can't be sure without a test on my windows machine. May work for Michael as well.
seems to get valid looking results on my linux setup, so I've committed to trunk at change 3661. This means it will make it to the July 10 nightly build. Please try scaning with that and see if you have improved results. Michael, I'm not sure yet why you are seeing metadata issues with this, but the paths should now parse correctly for your playlist. try updating svn and see if you see any improvement
OK, real thickie question - after adding the patch to line 640 of Utils/Scan.pm, what do I have to do for Slim Server to pick up the change? I've tried stopping and restarting (after saving the updated Scan.pm!) but am not seeing any difference. I added an extra $::d_scan message but am not seeing it output, so I don't think the server has picked up the change to Scan.pm. Being a newbie when it comes to Perl, is there anything else I need to do in addition to changing the source file?
Created attachment 622 [details] d_paths, d_parse and d_scan logs for D:\Music\Test\MP3\Portishead\Dummy Found out that running "perl slimserver.pl --d_paths --d_parse --d_scan" from the command line does the trick - lucky I had perl already installed :) I've attached the logs (d_paths, d_parse and d_scan enabled) for the single "Dummy" directory - it's improved but still not correct as it continues to load 20 files but no longer references C:\Windows\System32 as part of the filename (lots of \\\\ though). My Utils\Scan.pm now looks like the following: if ($playlist_filehandle) { my $playlist_base = undef; if (Slim::Music::Info::isFileURL($playlisturl)) { my @parts = splitdir($playlistpath); pop(@parts); shift(@parts); # Added 10 July 2005 $playlist_base = Slim::Utils::Misc::fileURLFromPath(catdir(@parts)); $::d_scan && msg("gonna scan $playlisturl, with path $playlistpath, for base: $playlist_base\n"); }
neil, can you add: use Data::Dumper; print Dumper(\@parts); before the pop(@parts) line and try that again. no debug flags needed this time.
With those two lines added and no debug options, I get the following in the console: C:\Program Files\SlimServer\server>perl slimserver.pl $VAR1 = [ 'file:', '', '', 'D:', 'Music', 'Test', 'MP3', 'Portishead', 'Dummy', 'Portishead%20-%20Dummy.m3u' ]; Any good? :)
thanks. that's just what I was expecting to see. The old method (pre r3427) would work, but I'll need to find out from Dan why the pathFromFileURL command was removed before simply dropping it back in.
I have now reverted change 3661, in favour of a reversion of change 3427 which should more cleanly handle the path conversion and other problems. Will await feedback from Neil and Michael from next nightly.
Just to confirm that the June 10 nightly is looking good with the patch from kdf.
Ermm... July 10. :)
I don't see any change, with or without the shift(@parts). What change would you have expected?
Oh, there's been a change - though not an improvement: I now have two artists. "Peter Gabriel" and "Peter%20Gabriel". The latter is the artist of all the albums in the playlist, but they contain no song.
don't use the shift patch (that leaves url escaping in). the july 10 nightly uses the pathFromFileURL conversion, which should clean up url escaping (bug 1717)
Thanks, Kevin, just finished a rescan without the shift(..). But my problems remain: empty albums and no year for those that exist AND are in the playlist. Does the album parsed from the playlist overwrite the album parsed from tags, thus removing the date info?
the patch changed nothing but the parsing of playlists for filenames. you'll need to look at the d_info, d_parse, d_scan logs to see what is going on with the metadata. I had simply hoped that having the urls correct would have matched up the song data properly. be sure you are using the latest nightly, with none of the tweaks previously mentioned for this bug.
When scanning playlists slimserver even stores information for files which do _not_ exist (or it does not find). As my playlist had been created on Windows but the server is running on linux it does not recognize the backslash as a path separator: 2005-07-11 09:58:16.6606 entry from file: #EXTINF:382,Peter Gabriel - Mercy Street 2005-07-11 09:58:16.6615 found title: Peter Gabriel - Mercy Street 2005-07-11 09:58:16.6621 entry from file: So\05 - Mercy Street.mp3 2005-07-11 09:58:16.6646 entry: file:///home/mh/Musig/Peter%20Gabriel/So%5C05%20-%20Mercy% 20Street.mp3 2005-07-11 09:58:16.6702 New track for file:///home/mh/Musig/Peter%20Gabriel/So%5C05%20-% 20Mercy%20Street.mp3 2005-07-11 09:58:16.6711 readTag was 1 for file:///home/mh/Musig/Peter%20Gabriel/So%5C05%20-% 20Mercy%20Street.mp3 2005-07-11 09:58:16.6730 Converting file:///home/mh/Musig/Peter%20Gabriel/So%5C05%20-% 20Mercy%20Street.mp3 to /home/mh/Musig/Peter Gabriel/So\05 - Mercy Street.mp3 2005-07-11 09:58:16.6743 mp3 file type for file:///home/mh/Musig/Peter%20Gabriel/So%5C05%20-% 20Mercy%20Street.mp3 2005-07-11 09:58:16.6751 reading tags for: file:///home/mh/Musig/Peter%20Gabriel/So%5C05%20-% 20Mercy%20Street.mp3 2005-07-11 09:58:16.6773 Info: no tags found for /home/mh/Musig/Peter Gabriel/So\05 - Mercy Street.mp3 2005-07-11 09:58:16.6782 Info: no title found, using plain title for file:///home/mh/Musig/Peter%20Gabriel/ So%5C05%20-%20Mercy%20Street.mp3 2005-07-11 09:58:16.6787 Guessing tags for: file:///home/mh/Musig/Peter%20Gabriel/So%5C05%20-% 20Mercy%20Street.mp3 ...and then it's (wrongly) _guessing_ the information from the information found in the playlist. 2005-07-11 09:58:16.6840 ARTIST => Peter Gabriel 2005-07-11 09:58:16.6845 ALBUM => So 2005-07-11 09:58:16.6850 TRACKNUM => 05 2005-07-11 09:58:16.6855 TITLE => Mercy Street This is ok so far. But then: 2005-07-11 09:58:16.6873 Adding file:///home/mh/Musig/Peter%20Gabriel/So%5C05%20-%20Mercy% 20Street.mp3 : TITLE to Peter Gabriel - Mercy Street Why is is the title set to "Peter Gabriel - Mercy Street" here? It was correctly recognized only 0.0018 seconds before :-) And shouldn't it, while scanning playlists, simply ignore local files that don't exist?
the title is being set from the EXTINF info, instead of guessTags. guessTags is called for any files that appear to have no tags. The EXTINF information, then appears to override info for the tracks. is mercy street a file that doesn't actually exist on the drive?
The file does exist. But the server is running on linux, and the playlist contains backspaces. "so\05 - mercy street.mp3" therefore isn't recoginzed as "so/05 - mercy street.mp3", but interpreted as a filename with escaped "\05" or something. Which does not exist.
The disappearing year has been fixed by change 3669. Still seeing empty albums, though.
I think what this is is the guesstags somehow kicking in when no tags are found. you can try turning off all the guesstags to confirm. What is likely needed is: 1) better fixpath to avoid the mixed up paths. 2) a -e test is some logical location prior to updateOrCreate. Annoyingly, this would likely require a pathFromFileUrl call since the fileURL is the object present at the best place for this test.
Created attachment 639 [details] check -e for m3u entries this is an example, parsing cue and reading m3u. it would have to be spread out to the rest of the read/parsing to be more complete. However, I wanted to know how well this worked against the reproducable case. let me know, michael :)
I can only speak for m3u files (don't have *.cue). But your patch is fine for me! Thanks.
Created attachment 641 [details] -e checks for all playlists may not make sense for cue's (since they can be indexed), but here is the patch to cover m3u, pls, wpl, asx. waiting for approval before comitting.
-r seems like a better check.
agreed. will merge tonight.
committed -r check at subversion change 3719. Is there anything else we should be doing to finish this one off?
The issues I encountered are all fixed (m3u only). Thanks!
cool. one more down then. can reopen of create new reports if anything related comes up.
This bug was marked resolved in Slimserver 6.1, which is several versions ago. If you're still seeing this bug, please re-open it. Thanks!