GNOME Bugzilla – Bug 343785
Mass storage updates
Last modified: 2006-06-13 18:22:16 UTC
The attached patch addresses a number of issues with and adds features to mass storage DAP support. Below is a summary of the parts of this that I have been involved with, I'll leave Gabriel to comment on the parts he wrote: Bugs fixed: * Starting Banshee with the DAP already plugged in would cause a crash. This was caused by starting the scanning of tracks on the device before the GUI was loaded. As scanning uses ImportManager which in turn uses the GUI this caused the crash. Fixed by delaying the scanning until after the GUI is initialized. * Filenames of tracks copied to the DAP are not escaped properly. Fixed by escaping the filename components using the existing FilenamePattern.Escape method. * The DAP volume's name is always used as the name in the interface, even if it's empty. Fixed by checking for an empty volume name and reverting to the device name if necessary. * When finding the VFS volume for the DAP the block device was incorrectly matched against the non-existing block_device attribute, which caused the incorrect VFS volume to be associate with the DAP or for loading to fail entirely. Fixed by correcting the attribute to block.device. * When finding the VFS volume for the DAP it was assumed that VFS would already know about the device, which is not always the case. Fixed by allowing the VFS volume to be found when VFS subsequently figures out it exists. New features: * Support for locating mass storage DAPs by the presence of a .is_audio_player file in the root of the device as well as by hal attributes. Creating this file is much easier than trying to create fdi files for use with unsupported DAPs. Rhythmbox also supports this method of device detection. In order to support this the device scanning on startup had to be modified such that all volumes on the system are passed to the DAP plugins for checking, I can't foresee any situations in which this would be problematic.
Created attachment 66712 [details] [review] Mass storage fixes/updates
*** Bug 337800 has been marked as a duplicate of this bug. ***
*** Bug 337829 has been marked as a duplicate of this bug. ***
Just remembered, the patch also adds meta-info based duplicate detection so you can't copy the same track to the device multiple times .
Just commenting to remind me to take a good look at this later, I'm out of time for today. Thanks James, Gabriel!
Created attachment 66745 [details] [review] Patch updated for CVS
*** Bug 340781 has been marked as a duplicate of this bug. ***
Created attachment 66863 [details] [review] Updated patch This is a slightly updated version of the patch.
Gabriel, The additional logic you've added around checking the mounted status seems unnecessary, specifically: private bool mounted = false ... volume = monitor.GetVolumeForPath(MountPoint); if (volume == null) { // Gnome VFS doesn't know volume is mounted yet monitor.VolumeMounted += OnVolumeMounted; is_read_only = true; } else { mounted = true; is_read_only = volume.IsReadOnly; } If volume is null it doesn't mean that the volume isn't mounted yet (we can't ever get to this situation due to the previous check on the volume.is_mounted property) rather it just means that VFS hasn't noticed that it's mounted yet. This means we can always carry on regardless of what VFS says as we've already established that it's mounted. Also, you seem to have reverted my change to base duplicate detection on meta info rather than filename. I think meta info detection should be more reliable?
James, Regarding the extra logic, I was noticing that my iAudio would appear in the source list but it's tracks would not be loaded when I plugged it in after starting Banshee -- so I added that logic and it worked. Perhaps the problem was somewhere else - if it's consistently working for you with out it, ok. Regarding duplicate detection, I didn't mean to revert anything - I was trying to merge changes to the patch that I had made and must have missed something. Can you elaborate or attach an updated version of the patch that has your changes? Thanks
I committed the patch I attached above. I looked through your most recently attached patch, James, and couldn't tell what you meant by using meta data to detect duplicates. Since there were so many other changes in this patch, I figured it was better to get them in and we'll tweak/fix problems one by one from now on.
Ok, no worries. I've opened bugs #344797 and #344795 for the only problems I've seen so far.