I guess that this patch doesn't work with libmtp 0.3.x ?

In that case, we probably need to specifically detect libmtp.so.8.3 like we do for libmtp.so.8

yes, i tested again with libmtp 0.3.7 and this patch breaks mtp. Version 1.0.0 will need to be specifically detected for compatibility.

I've just noticed that you "translated" time_t to uint.
I wonder if that shouldn't be a long, or maybe system dependent ?

Some parts of mono seems to assume it's a long :
http://anonsvn.mono-project.com/viewvc/trunk/mcs/class/Mono.Posix/Mono.Unix.Native/NativeConvert.cs?view=markup

Comment on attachment 142516 [details] [review]
fix compatibility with libmtp 1.0.0

Please make this work with the older libmtp as Bertrand mentioned.

Sorry - I had a shot at defining at detecting libmtp.so.8.3, but I've got no experience with autogen and am afraid this is out of my depth...

You could look at the git commit where I added the original detection to see how it was done.

I'll give it another shot - the problem (for me) is the regex to detect the newer version. 

Oh, and modificationdate definitely needs to be a uint, with long only one track is returned from the mtp.

You might want to try to check for the presence of the field in the struct, with something like the following :

AC_CHECK_MEMBER([struct LIBMTP_track_struct.modificationdate], [have_libmtp_track_modificationdate=yes], [have_libmtp_track_modificationdate=no], [[#include <libmtp.h>]])

I think this is more in the spirit of autoconf : check for features and artifacts, not for version numbers.

Created attachment 143943 [details] [review]
updated version of the patch

Thanks Bertrand, that helped a lot. Here's an updated version of the patch which should work fine with both libmtp>=1.0.0 and earlier versions.

Thanks for the patch, I committed it with the following changes :
- Moved the call to AC_CHECK_MEMBER, so that it's only done if libmtp is present
- In Makefile.am, use "BUILD_DEFINES +=..." so that the previous value is not overwritten
- Some formatting fixes.

Re-opening the bug, because apparently the fix doesn't work on 64 bit systems : time_t is a long on those machines.

Simple C test case (courtesy of Alan McGovern) :

#include <stdio.h>
#include <time.h>

int main ()
{
	printf ("%d", sizeof (time_t));
}


4 -> time_t is uint
8 -> time_t is long

Created attachment 150283 [details] [review]
patch to resolve some issues with the binding

Banshee MTP support is currently broken on most 64 bit systems with libmtp >= 1.0.0 (as of time of writing).

Combining the attached patch with autoconf foo to detect whether time_t is 32 or 64 bit will fix the issue. Unfortunately we can't just assume that on 64bit systems time_t is 64bit and on 32bit systems it's 32bit.

Created attachment 150285 [details] [review]
Patch to add autoconf foo and fix the libmtp binding for 'modification date'

Here's an updated patch which adds the autoconf foo to detect when time_t is 8 bytes instead of 4. There's probably a better way of doing it, but I'm no expert. I couldn't test to see if this fixes the issues on 64bit systems as I left my player at home. I can test later today/tomorrow/sometime though.

In the meantime anyone on a 64bit system who's been suffering from this bug should try it and let me know if it works.

*** Bug 601622 has been marked as a duplicate of this bug. ***

Review of attachment 150285 [details] [review]:

Thanks for the patch !
Looks fine to me, but I'd like to wait for confirmation by someone who experiences the bug.

::: build/m4/banshee/dap-mtp.m4
@@ +28,3 @@
 	AM_CONDITIONAL(LIBMTP_TRACK_STRUCT_HAS_MODDATE, [test "$LIBMTP_HAS_MODDATE" = "yes"])
+	AC_CHECK_SIZEOF(time_t)
+	AM_CONDITIONAL(LIBMTP_SIZEOF_TIME_T_64, [test "x$SIZEOF_TIME_T" = "x8"])

How about calling that LIBMTP_SIZEOF_TIME_T_IS_LONG ?
I think the 64 is not really clear. </nitpick>

Created attachment 150351 [details] [review]
fixes MTP on 64bit systems with libmtp >= 1.0.0

Ok, so I just tested and the initial patch doesn't work. Supposedly AM_CHECK_SIZEOF should create a variable called SIZEOF_TIME_T, but (at least on my system) this doesn't happen. The above check always returns false because SIZEOF_TIME_T is undefined/empty.

The updated patch (attached) uses the cached variable instead of the SIZEOF_TIME_T variable. It probably isn't the right way of doing it - but it works. Anyone with better autofoo knowledge should fix whatever stupid mistake I made that makes the original version fail ;)

As it stands - the attached patch is a verified[0] fix for the issue and so should be committed whenever it's reviewed and deemed to be ok.

[0] Tested on opensuse 11.2 with libmtp 1.0.1 and a creative Zen 16gb.

Review of attachment 150351 [details] [review]:

Thanks, Alan!