GNOME Bugzilla – Bug 548366
Sometimes when importing DAAP duplicates each song entry
Last modified: 2012-02-19 16:15:41 UTC
For some reason on some occasions DAAP will add the entire database of songs twice. The duplicate songs don't play. The song count in the side bar is 17044 instead of 8528, although the yellow bar that says its importing gives the 8528 song count On some occasions I have seen 2 copies of all the songs. The urls listed for a sample song: http://127.0.0.1:8089/1218654208/21908 <- Broken song http://127.0.0.1:8089/1841651200/21908 <- Plays fine The DAAP share isn't on 127.0.0.1 I assume there is a proxy in use here. The server is mt-daap.
I can confirm this bug on Banshee 1.2.1 using Ubuntu 8.04 from the banshee teams ppa repository. The share is a mt-daapd one.
Same thing here using 1.5.0, it would seem that it is caching the list sometimes as the last imported date will match the previous time that I loaded the list. The share in my case is mt-daap 0.2.4.2 This does not happen every time, and if I close the app and then reload it, it will load properly. If you need any log or what not please let me know what to capture.
Created attachment 147841 [details] [review] proposed patch I hit this if a DAAP source is loaded and Banshee crashes / is killed, then the same source is loaded again after restarting banshee. The proposed patch resolves by making use of the IsTemporary field of the CorePrimarySources table. Also, the DAAP playlists are marked as temporary. I'm not sure how I feel about the implementation. It may be better implemented with a static PrimarySource.ClearTemporary method called maybe in the initialization of the SourceManager. Somewhat like the PlaylistSource.ClearTemporary method and how it's called in LoadAll. Or, maybe this shouldn't even be implemented at the PrimarySource level? Anyway, here's a patch for one solution.
Created attachment 147847 [details] [review] slightly revised Just realized that another call to PurgeTracks () is needed in case there are old DaapSource records in the CorePrimarySources table that get reused.
Review of attachment 147847 [details] [review]: ::: src/Core/Banshee.Services/Banshee.Sources/DatabaseSource.cs @@ +297,3 @@ + private bool is_temporary = false; + public bool IsTemporary { change change this to an auto-property, eg public bool IsTemporary { get; set; } ::: src/Core/Banshee.Services/Banshee.Sources/PrimarySource.cs @@ +122,3 @@ "); + protected HyenaSqliteCommand purge_self_command = new HyenaSqliteCommand (@" Instead of creating one of these for every PrimarySource and keeping it around for as long as it exists, just inline the query in the Purge call. ::: src/Extensions/Banshee.Daap/Banshee.Daap/DaapSource.cs @@ +202,3 @@ { Disconnect (true); + PurgeSelfIfTemporary (); Should do this in PrimarySource.Dispose, no?
Created attachment 152287 [details] [review] next iteration Sorry this took so long, Gabriel. Applied your comments. I don't know why I didn't put the purge call in PrimarySource. Anyway, I've put it there now. Not sure about this part, however: public virtual void Dispose () { + PurgeSelfIfTemporary (); + if (Application.ShuttingDown) return; Figured I should put the purge call before the if statement to ensure it gets called. Not sure if that is appropriate, however.
*** Bug 595558 has been marked as a duplicate of this bug. ***
BTW, is it at all possible to cache the share between Banshee sessions? I have a ~20k share that takes a minute or so to load, it would be nice if Banshee would store the list between instances and only reconnect to the share as fast as possible and refresh the share in the background.
We could cache it, but then what if it's out of date? Then we have to do consistency checks, which will be even slower. The slowness of loading DAAP tracks (and the bug where it should be decided whether to cache) is bug #527192, though.
Review of attachment 152287 [details] [review]: The patch looks good, but I don't have a DAAP server to test it. It'd be great if someone could test the patch and report the result here. ::: src/Core/Banshee.Services/Banshee.Sources/PrimarySource.cs @@ +464,3 @@ + if (!IsTemporary) { + { + protected virtual void PurgeSelfIfTemporary () Should be indented with spaces here
Is there a temporary fix that I can use while I am waiting for my distro to include this patch? Thank you, for everyone's hard work. I enjoy Banshee.
(In reply to comment #2) > Same thing here using 1.5.0, it would seem that it is caching the list > sometimes as the last imported date will match the previous time that I loaded > the list. > > The share in my case is mt-daap 0.2.4.2 > > This does not happen every time, and if I close the app and then reload it, it > will load properly. > > If you need any log or what not please let me know what to capture. FYI, I got up to 5 copies of each song and the stop-restart thing is not clearing them out.
Comment on attachment 152287 [details] [review] next iteration I've committed this with a few minor changes and updates: http://git.gnome.org/browse/banshee/commit/?id=70303ded98c
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.