GNOME Bugzilla – Bug 451265
cannot create playlist on ipod
Last modified: 2010-08-03 23:11:04 UTC
The bug has been opened on https://bugs.launchpad.net/ubuntu/+source/rhythmbox/+bug/117925 "Binary package hint: rhythmbox rhythmbox 0.11.0-0ubuntu1, ubuntu gutsy I cannot create playlists on my ipod, I have to use gtkpod. Is rhythmbox supposed to create playlists? It has very low support for ipods.." It should also allow to delete a playlist when right clicking on it
Created attachment 91732 [details] [review] patch This lets you add new (static) playlists, and rename or delete existing ones. It also forces a DB save when quitting, so that you don't lose changes if you quit before the async save is run.
Looks good to me, would be nicer to consolidate the code of RBiPodDB::dispose() and RBIPodDb::flush() Why was this code removed ? - /* The playlist was added to the iPod database, - * 'action' is no longer responsible for its memory - * handling - */ - action->playlist = NULL; I assume this is because it was useless anyway?
any news from this feature?
Created attachment 108858 [details] [review] Update to latest svn Patch updated to latest svn, there were just 2 really minor conflicts. I'll apply that patch soon if noone complains loudly
This version of the patch is missing the new files (rb-ipod-static-playlist-source.{c,h}). Assuming there were no changes in those files since the original version of the patch, it looks OK to me.
Created attachment 108984 [details] [review] Same patch as earlier with the missing files Sorry for the missing files, here is an updated patch. I didn't touch the new files anyway, so they should be the same as in the original patch.
I finally committed it in r5747
Created attachment 112984 [details] [review] Patch to flush DB I find that the playlists aren't saved when exiting Rhythmbox or ejecting the ipod. Re-adding this patch fixes it, but may not be the correct fix.
I think this patch works around an actual bug : 16:35 < teuf> when the ipod is ejected 16:35 < teuf> the ipod source is supposed to be destroyed 16:35 < teuf> and the rb-ipod-db object should be disposed at the same time 16:35 < teuf> and RbIpodDb::dispose has some code to sync the database 16:36 < teuf> I *think* that dispose method isn't called 16:36 < teuf> which probably means a ref on RbIpodSource or RbIpodDB is leaked somewhere 16:36 < teuf> and I *think* without the playlist patch, things are working as expected
Created attachment 112990 [details] [review] Fix extra ref Fixes the real bug, an extra reference. Thanks to Christophe :-)
Committed, thanks a lot for tracking it down.
This patch wasn't correct, there's a ref missing on every RbIpodStaticPlaylistSource and there are g_warnings on the console because of that: when RbIpodDB is disposed, its Itdb_iTunesDB is freed as well, which triggers a g_object_unref on every RbIpodStaticPlaylistSource, but those source were already disposed so g_object_unref complains. The heart of the problem is that every RbIpodStaticPlaylistSource holds a ref on RbIpodDb, and at the same time, the RbIpodDb holds a ref on every RbIpodStaticPlaylistSource (through the user_data member of the Itdb_Playlist contained in the Itdb_iTunesDB structure wrapped by RbIpodDB). This creates a ref cycle between those objects :-/ We probably can get away with it using a weak pointer to the RbIpodDb in RbIpodStaticPlaylistSource.
Created attachment 113033 [details] [review] Better patch to fix the saving This patch reverts the incorrect previous one, and moves the unreffing from _dispose to _delete_thyself in RbIpodStaticPlaylistSource.
Ok, should be properly fixed now, thanks for working on that nasty referencing bug :)
This bug still exists in 0.12.8 See https://bugs.launchpad.net/ubuntu/+source/rhythmbox/+bug/496681 and https://bugs.launchpad.net/rhythmbox/+bug/117925
(In reply to comment #15) > This bug still exists in 0.12.8 That's not helpful. Test with the latest version, or ask your distribution to update it.
Tested with 0.13, still not fixed. Will try from git once I figure it out.