GNOME Bugzilla – Bug 546962
Patch to upgrade rhythmbox to libmtp 0.3.0
Last modified: 2008-08-09 22:24:36 UTC
I have written a patch to upgrade Rhythmbox to use libmtp 0.3.0. We need to apply this to the Fedora package to be able to move the entire distribution to libmtp 0.3.0. It is possible to use autoconf magic to define some preprocessor #define to make it compile with older libmtp as well, I haven't looked into it. One option is to check the library for the LIBMTP_Detect_Raw_Devices() function for example.
Created attachment 116160 [details] [review] Patch to migrate IF to libmtp 0.3.0 and add a few features too
*** Bug 543645 has been marked as a duplicate of this bug. ***
libmtp 0.3.0 is really new, a configure check would be really nice especially since there would be only 1 line to #ifdef. Ratings on mtp devices have to be divided/multiplied by 20 ? I'm checking because it's exactly the same for ipods, so I want to make sure this is not a copy and paste error ;)
Ratings on MTP devices are 0..100 and Rhythmbox awards 0..5 stars which means 20 x [0 .. 5] = [0 .. 100] and I guess the iPod ratings are also 0..100 probably the MTP rating was plagiarized from the iPod range.
Created attachment 116164 [details] [review] Patch to make autoconf detect 0.3.0+ and #define HAVE_LIBMTP_030 if found I don't have a working GNOME build system on this machine, but can you test if this patch to configure.ac will properly #define HAVE_LIBMTP_030 so that the code can be altered accordingly?
Created attachment 116169 [details] [review] tested configure.ac changes I tweaked your patch a little, this one seems to work though I only tested it without libmtp 0.3.0
Created attachment 116184 [details] [review] Patch to migrate IF to libmtp 0.3.0 and add a few features too A newer version that checks for the #define from the other patch.
If these two work out together on your setup, I suggest they be applied!
Created attachment 116186 [details] [review] Cumulative patch with ChangeLog This should be good to commit though it would be nice if someone could confirm things still work when you don't have libmtp/when you have libmtp 0.3.0
I seem to have problems when compiling from source with this, version (or I just got deps or something wrong) so put it on hold for the time being. (Investigating.)
Created attachment 116195 [details] [review] Diet patch that only add support for libmtp 0.3.0 Sorry have to revert out the playcount and rating additions, this just doesn't work for the time being. This also fixes a minor bug that made configure never detect libmtp 0.3.0 and not compile for it. Tested with source compile and libmtp 0.3.0.
FYI my code failed with: (rhythmbox:5683): Rhythmbox-WARNING **: cmd_next: Unhandled error: Problem occurred without error being set. This is a bug in Rhythmbox or GStreamer. ** ** RhythmDB:ERROR:(rhythmdb.c:4977):rhythmdb_entry_get_ulong: code should not be reached Looks like I'm looking up something I shouldn't :-/
rhythmdb doesn't do type conversion, so you have to access the rating using rhythmdb_entry_get_double rather than rhythmdb_entry_get_ulong.
I've committed a slightly modified version of this - PKG_CHECK_EXISTS is more appropriate for the second check, and avoids some problems I was having that looked like pkg-config results being cached. I've checked this with libmtp 0.2.6 and 0.3.0, and without libmtp installed.
Created attachment 116216 [details] [review] Handle playcount and rating on MTP devices This patch will probably work to update playcount and rating but I don't have a device for testing
Thanks Jonathan, the commit works like a charm! Christophe, your latest patch also work swell, can it be committed? libmtp 0.3.0 and 0.2.x has the same API here so works fine on both.