GNOME Bugzilla – Bug 743359
allow read/write methods based on device capabilities
Last modified: 2016-01-21 12:56:01 UTC
Created attachment 295195 [details] [review] allow methods based on device capabilities Read and write support is allowed currently only if android.com extension is detected. However necessary functions can be provided also by different extensions. Therefor check capabilities instead of the extensions. Unfortunately the patch doesn't work, because of the following libmtp bug: http://sourceforge.net/p/libmtp/bugs/1188/ The patch is untested yet, so I hope my ideas are correct... There is downstream bug report: https://bugzilla.redhat.com/show_bug.cgi?id=1184728
So, the only area where there's a meaningful benefit is with GetPartialObject, where libmtp will fall back to the 'standard' 32bit version if it's available. The direct-write methods are all Android proprietary, and so trying to distinguish them on a more fine-grained basis is mostly pointless and send_partial_object makes no sense without the stateful methods (create/append/replace/close). Additionally, you need to remember that GetPartialObject is limited to 32bit offsets so you need a graceful failure path when confronted with a large file. This is the main reason why I didn't want to deal with it originally.
Created attachment 295259 [details] [review] allow methods based on device capabilities I suspected it would not be so easy :-) I don't think 32bit offset is such limitation. There is a proposed patch for reading only, but have to be tested properly once the libmtp upstream bug will be fixed...
Review of attachment 295259 [details] [review]: You need to check that 'offset' is < 2^32 - bytes_requested doesn't matter. Unfortunately, Linus is barely around to do anything for libmtp. I should have asked for commit privileges when I had the chance :-/ It means it'll be a while before that change goes in.
Linus came back long enough to give some people admin privs, including me, so I can look at getting your patch in upstream.
Cool, thanks!
Created attachment 319438 [details] [review] mtp: Allow reading if GetPartialObject is supported The libmtp has been fixed, so we might finally fix this bug. There is updated patch... It would be nice to test this with some mtp device which doesn't support android extensions, but I do not have any...
I also don't have anything to test with, but the change looks logical. ship it. I used to have an old Galaxy Note with Samsung's MTP stack that had this feature combination but I sold it a while ago. I think Samsung use the google stack now.
Review of attachment 319438 [details] [review]: Ship it
Comment on attachment 319438 [details] [review] mtp: Allow reading if GetPartialObject is supported commit e37131e9e8d1a543fa1ded6bde629e09230fbff3
Thanks for the review...