GNOME Bugzilla – Bug 749639
afc: Show iDevices through MTP
Last modified: 2017-03-02 19:11:51 UTC
.
Created attachment 303668 [details] [review] afc: Remove attempts at thumbnailing photos iDevices had a list of thumbnails available through afc:/// for photos stored in the DCIM/ sub-directory, but the code to handle this was always pretty fragile, relying on the iOS version to know which of a number of different ways the thumbnails could be called or where they would be stored. It's easier to create the thumbnails ourselves from the JPG files, or even access photos through PTP so that thumbnails are readily accessible and in a way that doesn't change with each iOS release.
Created attachment 303669 [details] [review] gphoto2: Show iDevices through MTP There's no interesting data for users in afc:/// so it's best to show iDevices through MTP instead. Stop hiding iDevices through the MTP.
Created attachment 303670 [details] [review] afc: Correct debug string g_vfs_afc_monitor_create_volume() can actually create volumeS. So correct the debug output not to be misleading.
Created attachment 303671 [details] [review] afc: Add debug info when house arrest fails to start As can happen with some iOS and libplist combinations.
Review of attachment 303668 [details] [review]: Seems sensible.
Review of attachment 303669 [details] [review]: This is PTP rather than MTP isn't it? Otherwise seems OK.
Review of attachment 303670 [details] [review]: Looks good.
Review of attachment 303671 [details] [review]: ::: monitor/afc/afcvolume.c @@ +160,3 @@ if (lerr != LOCKDOWN_E_SUCCESS) { + g_print ("Couldn't start com.apple.mobile.house_arrest for UUID %s: %d\n", self->uuid, lerr); Probably better as g_warn()?
Created attachment 323851 [details] [review] afc: Remove attempts at thumbnailing photos iDevices had a list of thumbnails available through afc:/// for photos stored in the DCIM/ sub-directory, but the code to handle this was always pretty fragile, relying on the iOS version to know which of a number of different ways the thumbnails could be called or where they would be stored. It's easier to create the thumbnails ourselves from the JPG files, or even access photos through PTP so that thumbnails are readily accessible and in a way that doesn't change with each iOS release.
Created attachment 323852 [details] [review] gphoto2: Show iDevices through MTP There's no interesting data for users in afc:/// so it's best to show iDevices through MTP instead. Stop hiding iDevices through the MTP.
Created attachment 323853 [details] [review] afc: Don't mount the default AFC service There's no interesting data for users in afc:/// so it's best to show iDevices through MTP instead. The only reason we'd want to use this mount is if we had support for accessing the iTunes database. Unfortunately, this database needs to be encrypted, and we don't know how to encrypt it. Even then, we would probably want the default AFC service to be hidden so only applications access it, and not users.
Created attachment 323854 [details] [review] afc: Add debug info when house arrest fails to start As can happen with some iOS and libplist combinations.
Review of attachment 323854 [details] [review]: It looks good!
Review of attachment 323851 [details] [review]: Makes sense!
Review of attachment 323852 [details] [review]: It makes sense to me to remove the condition if AFC backend is not used for showing files, however I am not sure that the patch is really doing what is in the description. The patch removes the code from gPhoto2 volume monitor to ignore iDevices, however still there is a code to ignore MTP devices. So it seems to me that there isn't any difference, because it returns just several lines later. Isn't necessary any modification also in MTP volume monitor allowing mounting of iDevices? Otherwise, it seems to me that iDevices are beeing mounted over MTP already.
Review of attachment 323853 [details] [review]: What was the content of afc:///? It was something different then what we see over MTP? So is the AFC backend useless now? What is the content of the house arrest mount?
Attachment 323851 [details] pushed as fe83e09 - afc: Remove attempts at thumbnailing photos Attachment 323852 [details] pushed as 7ca0618 - gphoto2: Show iDevices through MTP Attachment 323853 [details] pushed as 0b68656 - afc: Don't mount the default AFC service Attachment 323854 [details] pushed as 87ee9ea - afc: Add debug info when house arrest fails to start
Review of attachment 323852 [details] [review]: It's actually not MTP, but PTP as handled by the gphoto2 plugin. So this patch is correct (it's not handled by MTP).
Review of attachment 323853 [details] [review]: Content was implementation details, such as a raw list of photos (with a database that I guess Apple apps on OSX edit when removing photos for example), and an encrypted iTunes database that we don't know how to edit either. The net result of this is usually: - appears as an iTunes database in Rhythmbox but creates an unusable database when edited - photos can be removed from the "disk" but still appear in the list of photos but are unreadable So it doesn't make much sense to show it. The "Documents on iPhone" mount shows applications, and allows adding files when the application supports it. Say, drag'n'dropping comic books in the CBR/CBZ reading app, or movies into the movie playback app. Much more useful, and works.
Created attachment 327140 [details] [review] afc: Do not use G_FILESYSTEM_PREVIEW_TYPE_IF_LOCAL I think we should change G_FILESYSTEM_PREVIEW_TYPE_IF_LOCAL to G_FILESYSTEM_PREVIEW_TYPE_IF_ALWAY, when the thumbnailing code is removed...
Comment on attachment 327140 [details] [review] afc: Do not use G_FILESYSTEM_PREVIEW_TYPE_IF_LOCAL Pushed to master in a modified version as commit a71469c...
*** Bug 779481 has been marked as a duplicate of this bug. ***