After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 374078 - Track transfer does not work for certain file names
Track transfer does not work for certain file names
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: iPod
0.9.6
Other All
: Normal normal
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
: 347744 489170 506668 511824 (view as bug list)
Depends on:
Blocks: 338564 415641
 
 
Reported: 2006-11-11 23:45 UTC by Michel Alexandre Salim
Modified: 2008-09-20 07:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch against SVN doing what doctau suggests (520 bytes, patch)
2007-01-08 09:45 UTC, Christophe Fergeau
none Details | Review
Patch to fix filenames cleanly (4.06 KB, patch)
2008-06-14 21:06 UTC, Colin Leroy
none Details | Review
new version of previous patch (4.09 KB, patch)
2008-06-14 21:26 UTC, Colin Leroy
committed Details | Review

Description Michel Alexandre Salim 2006-11-11 23:45:04 UTC
Please describe the problem:
When transferring files, Rhythmbox cannot copy the following files:

02 Humoresque: City Montage.flac
05 Girl Crazy: Embraceable You (arr. Russell Warner).flac
07 Tale of Tsar Saltan: The Flight of the Bumblebee (arr. Franz Waxman).flac

The bizarre thing is that the following track gets transferred despite also containing a colon in the name:
06 Violin Sonata No. 1 in G minor, BWV 1001: IV. Presto.flac
08 Symphonie espagnole in D minor, Op. 21: I. Allegro non troppo.flac



Steps to reproduce:
1. Create files with the above names
2. Drag-and-drop them to iPod


Actual results:
The first three files cannot be transferred

Expected results:
All files transferred

Does this happen every time?
Always

Other information:
Comment 1 Christophe Fergeau 2006-11-11 23:52:57 UTC
What do you mean exactly by "cannot be transferred"? Does rhythmbox display an error message? Or does it seem to be able to copy them, and then the iPod refuses to play them?
Comment 2 Michel Alexandre Salim 2006-11-12 01:11:34 UTC
It displays this error:

Error transferring track

Could not open vfs file "file:///media/MICHEL's%20IP/iPod_Control/Music/f29/02%20Humoresque:%20City%20Montag.flac"for writing: Invalid parameters.

It created the f29 directory but did not copy anything.
Comment 3 James "Doc" Livingston 2006-11-12 07:46:18 UTC
It's probably a FAT32 partition, which doesn't allow colons in file names.

We could write a helper function which used gnome_vfs_volume_get_filesystem_type() to get the filesystem type, and remove colons and backslashes if it's "fat", "vfat".
Comment 4 Michel Alexandre Salim 2006-11-12 18:55:22 UTC
Yes, it is a Windows-formatted iPod. Out of curiousity, is there any digital audio player out there that does not use FAT, apart from a Mac-formatted iPod? (I'm not sure you can use colons there either, it might do funny things if the drive is then read by OS X, at least)
Comment 5 Christophe Fergeau 2006-11-13 13:14:33 UTC
Ok, I can confirm that it's caused by the ':', since with the below patch, I can successfully transfer tracks that were causing issues (the track was named "7:25"). The filenames on the iPod have a limited length, the files containing ':' that you successfully transferred probably had their name truncated before the ':'.

Index: sources/rb-ipod-source.c
===================================================================
RCS file: /cvs/gnome/rhythmbox/sources/rb-ipod-source.c,v
retrieving revision 1.68
diff -u -r1.68 rb-ipod-source.c
--- sources/rb-ipod-source.c    10 Nov 2006 07:25:51 -0000      1.68
+++ sources/rb-ipod-source.c    13 Nov 2006 13:13:49 -0000
@@ -1158,7 +1158,7 @@
         */
        pc_filename = utf8_to_ascii (tmp);
        g_free (tmp);
-
+        g_strdelimit (pc_filename, ":", ' ');
        g_assert (g_utf8_validate (pc_filename, -1, NULL));
        /* Now we have a valid UTF-8 filename, try to find out where to put
         * it on the iPod

I guess there are more characters than the ":" to filter out, but I don't have time to figure out which ones now...
Comment 6 James "Doc" Livingston 2006-11-14 11:11:17 UTC
I've looked at what characters Windows things are illegal on FAT partitions, so the following might be what we need.

g_strdelimit (pc_filename, "\"", '\'');
g_strdelimit (pc_filename, ":/|<>*?\\", '_');
Comment 7 Christophe Fergeau 2007-01-08 09:45:52 UTC
Created attachment 79726 [details] [review]
Patch against SVN doing what doctau suggests

Here is a patch against svn doing the appropriate substitution. However, this should be done in the generic track transfer code when the destination is a FAT partition, thus this patch is not meant to be applied ;)
Comment 8 Christophe Fergeau 2007-06-12 13:36:07 UTC
Is that related to bug #347744 ? Or is the other bug the exact opposite of that one (ie this one is about library=>ipod, and the other one anything=>library ?)
Comment 9 Thomas M. Hinkle 2007-06-18 02:15:43 UTC
Just adding that I'm observing the same behavior here with colons and question marks in file names. It would be great if we could get this bug accepted and the patch applied!
Comment 10 Jonathan Matthew 2007-10-22 22:07:00 UTC
*** Bug 489170 has been marked as a duplicate of this bug. ***
Comment 11 Ross Burton 2007-12-29 11:10:30 UTC
I'm also hitting this, and teuf's patch looks great to me (it's basically the same logic as in Sound Juicer).  Please review and commit!

I notice the bug is marked as needs-work, why is this?
Comment 12 Christophe Fergeau 2007-12-29 11:25:27 UTC
My patch does substitution in the iPod plugin code while it should be done in some generic code 
Comment 13 Ross Burton 2007-12-29 11:59:12 UTC
Can't it be committed and then the generic code written?  It's just a fix-up, but its a known to work fix up. :)

Keeping track of what tracks failed when copying across 80 albums and renaming them isn't exactly how I planned on spending my Saturday.
Comment 14 Jonathan Matthew 2008-01-01 12:59:14 UTC
*** Bug 506668 has been marked as a duplicate of this bug. ***
Comment 15 Jonathan Matthew 2008-01-24 22:03:02 UTC
*** Bug 511824 has been marked as a duplicate of this bug. ***
Comment 16 Paul Drain 2008-01-25 00:17:06 UTC
teuf's patch works here too, I have to agree with Ross -- telling a user how easy it is to copy their 80G iPod to their library, flatten it and move all the files back using Rhythmbox is great ...

... but it looks silly when there's a bunch of cascaded windows telling them nearly ten percent of their library wasn't copied correctly.
Comment 17 Michel Alexandre Salim 2008-02-14 13:54:42 UTC
Any update? Perhaps a TODO file could be maintained tracking all the changes that would be nice to generalize, so that the relevant patches can in the meantime be applied.
Comment 18 Colin Leroy 2008-06-14 21:06:19 UTC
Created attachment 112752 [details] [review]
Patch to fix filenames cleanly

This patch fixes the destination URIs cleanly according to the removable source's filesystem, and is in general code. I hope it's good enough to get in svn :)
Comment 19 Colin Leroy 2008-06-14 21:26:12 UTC
Created attachment 112753 [details] [review]
new version of previous patch

This patch moves rb_sanitize_uri_for_filesystem() to rb-file-helpers.c as suggested by Christophe.
Comment 20 Jonathan Matthew 2008-06-14 22:52:19 UTC
I'm not sure if it can possibly cause any problems, but one slight concern is that it will only work with file:// URIs, and we could be transferring to sftp, smb, dav, ftp, etc.  I'm not sure you can get information on the underlying filesystem type in those cases, so we probably wouldn't be able to do anything anyway.

I'm hoping to finish up the gio port soon, which will mean that the gnome-vfs code here will need to be rewritten, but rb_uri_get_filesystem_type at least will be much simpler.

Otherwise, this looks good.
Comment 21 Colin Leroy 2008-06-15 09:05:26 UTC
(In reply to comment #20)
> I'm not sure if it can possibly cause any problems, but one slight concern is
> that it will only work with file:// URIs, and we could be transferring to sftp,
> smb, dav, ftp, etc.  I'm not sure you can get information on the underlying
> filesystem type in those cases, so we probably wouldn't be able to do anything
> anyway.
 
Yes, in my opinion if doing network transfers, it'd be up to the receiving host to fix the names if necessary. We couldn't even know the remote FS. 

> I'm hoping to finish up the gio port soon, which will mean that the gnome-vfs
> code here will need to be rewritten, but rb_uri_get_filesystem_type at least
> will be much simpler.

Ah, nice; In the meantime though, I'd love to see this bug officially fixed so that I'll be able to stop patching my rhythmbox package each time I upgrade my distro ;)

> Otherwise, this looks good.

Thanks!
Comment 22 Christophe Fergeau 2008-06-15 12:32:16 UTC
I reworked rb_uri_get_filesystem_type so that it doesn't leak memory in the error cases and committed that patch to svn in r5751
Comment 23 Jonathan Matthew 2008-09-05 06:07:50 UTC
*** Bug 550117 has been marked as a duplicate of this bug. ***
Comment 24 Jonathan Matthew 2008-09-20 07:21:06 UTC
*** Bug 347744 has been marked as a duplicate of this bug. ***