GNOME Bugzilla – Bug 337429
Let source add their own custom data to a RhythmDBEntry
Last modified: 2006-05-25 23:11:53 UTC
For the iPod source, I'd like to be able to associate an Itdb_Track * (which is used by libgpod to describe a file present on the iPod) to a RhythmDBEntry. That would make things easier to handle delete for example: I could do something like: track = (Itdb_Track *)rhythmdb_entry_get_pointer (entry, RHYTHMDB_PROP_CUSTOM_DATA); (this assumes the entry is of type RHYTHMDB_ENTRY_TYPE_IPOD). It's quite easy to add, but when naively adding it, I realised that some source will probably need to free memory associated with this custom data pointer when the entry is destroyed. For that, I started to change RhythmDBEntryType from being an int to being a pointer to a structure. For now, this structure only contains a function pointer to free the custom_data member of RhythmDBEntry having this type. I'm not 100% sure that RhythmDBEntryType is the best place to have that memory freeing function (I'm not sure there's roughly a 1<->1 mapping between source and entry types, what I want is per-source custom data, and I assumed per-data-type was roughly the same). If that's wrong, then you can reject that patch right away :) One issue I found out when implementing that code is the smart playlist saving. It saves the entry-type as an int, but the entry-type is dynamically registered, so this int totally depends on the registration order (which is probably mostly hard-coded currently, but depending on that is fragile imo). For now, I have added entry_type_to_string and entry_type_from_uint functions to handle that, but it's ugly (especially in the case G_POINTER clauses of write/read_encoded_gvalue), I haven't thought much about the problem, so I don't have a clean solution to propose. Most of this patch is about changing RhythmDBEntryType from a gulong to a gpointer, there are the basic bits to add a CUSTOM_DATA RhythmDBEntry property as well, but for example the free-memory function can't be registered atm, and this new property isn't used anywhere as well, I'm posting this patch nonetheless since to my eyes it's mostly ready to be included, and other patches can go on top of that.
Created attachment 62822 [details] [review] Do what is described above ;)
(ignore the first hunk of the patch, it has nothing to do there ;)
Looks like there are some other preexisting bugs relating to custom tags/data: bug #330259 and bug #324540. Perhaps we should consolidate the discussion under one bug?
At first, I thought to something similar to what bug #330259 describes, but I decided I was too lazy to implement something complicated like that ;) On second thought, I might want to attach iPod specific data to a RhythmDBEntry of type RHYTHMDB_ENTRY_TYPE_SONG, so maybe we should go for something more generic right now ;)
Created attachment 62964 [details] [review] add python stuff Updated to have a G_TYPE_POINTER-derived GType for RhythmDBEntry, and adds the Python-related stuff. This looks fairly sane to me, and is a start on what we had been talking about in the "RhythmDB cleanup and extension" bug. It would be good to move the podcast stuff over to using this, but that would require some additions to allow saving/loading of custom data.
Created attachment 63671 [details] [review] another patch This a modified version of the patch, which only has the stuff to make RhythmDBEntryType a structure, and doesn't include the custom-data stuff. It adds post-entry-create and pre-entry-destroy callbacks. Depending on the situation, I think it would be better to either use a entry->data hash table (as Christophe is doing for the ipod stuff) or solving bug 330259 to allow arbitrary pieces of extra data.
I've committed the second patch to cvs, which will let us implement some of the virtual functions Jonathan mentioned on bug 330226. I'm not sure how we want to support per-source extra data, so I'm leaving the bug open. Bug 330259 could do it, but that might not be a good way.
I've implemented this with the latest patch I committed from bug 330226. I knew this bug existed, but couldn't find it for some reason. Entry type private data can be cleaned up in the pre-destroy vfunc. I've changed the entry type<->string conversion to use a name registered with the entry type, so now playlists.xml contains '<equals prop="type">song</equals>'. The old format is handled correctly.