GNOME Bugzilla – Bug 560711
USB DAP playlist format - relative vs. absoute
Last modified: 2012-05-05 18:22:14 UTC
First off I'm greatly excited that each release seems to be getting me closer and closer to successfully using my BlackBerry Curve 8330 as a full-featured media player with Banshee. Banshee recognizes my device without the .is_audio_player file, however it doesn't recognize that it supports playlists. By using an .is_audio_player file I'm able to force Banshee into letting me try and get playlists to work. Banshee is able to locate and open the m3u playlists created by the BlackBerry, though it doesn't seem to successfully parse them and shows them as empty. Likewise the BlackBerry is able to recognize and read the playlists created in Banshee, but can't locate the files on the device. Both problems lie in absolute vs. relative paths. The BlackBerry creates its m3u files with device-specific absolute paths. (e.g.: file:///SDCard/BlackBerry/music/<artist>/<album>/<song>.mp3) Though it creates the m3u's with absolute paths the BlackBerry can also read m3u's with paths relative to where the playlist file is saved, which is the /Blackberry/music folder of the media card by default. (e.g.:<artist>/<album>/<song>.mp3) The Banshee-related problem here is that it splits the difference between a true absolute path and a relative one. (e.g.: BlackBerry/music/<artist>/<album>/<song>.mp3) While Banshee knows better than to include the full mount point from linux (e.g.:/media/MICROSD/BlackBerry/music ...) it doesn't seem like a wise idea to create a supposedly absolute path since it's impossible to know what the device may prefix onto the folder structure when it mounts it. (e.g. the BlackBerry adds SDcard/ to the absolute path) Obviously Banshee knows where it is saving the playlist file on the DAP, and it knows where the music files are relative to where the playlist is saved, so it would seem to me that creating m3u files with relative paths would be more universally appropriate than the current pseudo-absolute iteration. Another alternative might be to have some sort of option to specify whether to use absolute or relative paths. Hopefully this is detailed enough for you to understand what I'm up against here. Here's a copy of my .is_audio_player file, just for reference: audio_folders=BlackBerry/music,BlackBerry/videos folder_depth=2 output_formats=audio/mpeg,audio/x-ms-wma,audio/x-wav,audio/mp4, playlist_path=%File playlist_format=audio/x-mpegurl
Bulk changing the assignee to banshee-maint@gnome.bugs to make it easier for people to get updated on all banshee bugs by following that address. It's usually quite apparent who is working on a given bug by the comments and/or patches attached.
this is a problem for me too. so +1 for somebody to fix it.thx.
Created attachment 158336 [details] [review] Fix relative paths in playlists This patch fixed the relative paths in playlists on MassStorageDevices. By example if your music is in Music/ (on the device) and your playlists on Music/Playlists/ then the resulting entries in the playlist will be like: ../Miles Davis/Kind Of Blue/So What.ogg I tested this patch with my Nokia N900 and a generic usb stick with an .is_audio_player file. I have no other media players (ipod etc) so i could not test the impact on these devices.
Created attachment 158648 [details] [review] Fix relative paths in playlists - Rebased into current master in which Paths was moved into Hyena
Review of attachment 158648 [details] [review]: Looks good, just a few nitpicks: ::: src/Libraries/Hyena/Hyena/Paths.cs @@ +116,3 @@ + // one path is absolute, one path is relative, impossible to compare + { + if (Path.IsPathRooted (path) ^ Path.IsPathRooted (to)) What should happen when both of them are rooted? Could you add tests for all three cases (only one of each is rooted, and both are rooted)? @@ +152,3 @@ + return BuildRelativePath (path, to); + } + { Replace `/` with Path.DirectorySeparatorChar @@ +159,3 @@ + required [j - i] = pathParts [j]; + } + relativePath.Append (string.Join (Path.DirectorySeparatorChar.ToString (), required)); It's not in HACKING but it should be `String.Join` because the string type is used as a class. `new string[]` above is fine.
Alexander, Maybe i don't correctly understand your comment about the test with two rooted paths, but the first 4 asserts in FileNamePatternTests.MakePathsRelative () test for this situation. If both paths are rooted, it is checked if the one path is a child of the other, and a relative path is calculated. In any other case null is returned and the PlayListFormatBase will use the full path
Created attachment 159103 [details] [review] updated with Path.DirectorySeparatorChar and String with capital
Review of attachment 159103 [details] [review]: Indeed, I didn't notice those test cases. What I wanted to say is that when both paths are rooted, the result is inconsistent: Paths.MakePathRelative ("/foo", "/bar") == null Paths.MakePathRelative ("/foo1", "/bar") == "../foo1" This is because of the check on line 130: `path.Length < to.Length`
From IRC: (22:53:45) jeroen: the BuildRelativePath() method in Paths, always returns null, when it can't safely build a relative path (22:54:31) jeroen: only in the case when both the paths are exactly the same, an empty string is returned (which is in fact in that case the 'relative' url) (22:54:45) jeroen: i based this on the already existing code at that place The existing code is not correct for the test case I mentioned: Paths.MakePathRelative ("/foo", "/bar") should be "../foo", not null. Could you include this test case to your patch and make sure it passes? Something along these lines: - if (path.Length < to.Length) { + if (path.Length < to.Length && !Path.IsPathRooted (path)) {
Created attachment 159456 [details] [review] Updated version I have completely removed the check for the length of to and path because it was not necessary. I also added some more tests specific for the last comment: Assert.AreEqual ("../foo", Paths.MakePathRelative ("/foo", "/bar")); + Assert.AreEqual ("../foo/song.ogg", Paths.MakePathRelative ("/foo/song.ogg", "/bar")); + Assert.AreEqual ("../../foo/song.ogg", Paths.MakePathRelative ("/foo/song.ogg", "/bar/verylongdirectoryname"));
Committed, thanks Jeroen! commit 4fa8b4f3f5ab4341b92a7e37233d4cbbf335bd44 Author: Jeroen Budts <jeroen@lightyear.be> Date: Sat Apr 24 12:03:13 2010 +1000 [MassStorage] Fix relative paths in playlists (bgo#560711) Signed-off-by: Alexander Kojevnikov <alexander@kojevnikov.com>
*** Bug 609488 has been marked as a duplicate of this bug. ***