GNOME Bugzilla – Bug 624841
Audio preview of CD tracks fails if totem is not installed
Last modified: 2010-10-13 16:15:44 UTC
Originally reported in Ubuntu (https://bugs.launchpad.net/ubuntu/+source/nautilus/+bug/607793): To perform audio preview, Nautilus uses totem-audio-preview. If not present (i.e. if totem is not installed), it falls back on gst-launch-0.10 with a playbin pipeline. Unfortunately playbin doesn't understand the cdda:// URIs when trying to preview tracks on an audio CD. Steps to reproduce: 1) Uninstall totem 2) Install gstreamer0.10-tools 3) Make sure audio previews are active: gconftool-2 --type string --set /apps/nautilus/preferences/preview_sound "always" 4) Insert an audio CD, when prompted what to do choose "Open Folder" 5) Hover the mouse cursor over one of the tracks in nautilus (typically "Track n.wav") Expected result: after a delay (about 1 second), the track starts playing. Current result: nothing happens. Note that when totem is installed, the preview works.
Created attachment 166218 [details] [review] patch for play_file in fm-icon-view.c The issue is with the way nautilus builds the URI to pass to playbin. The attached patch mimics totem's behaviour to build URIs that playbin is happy with.
Review of attachment 166218 [details] [review]: ::: nautilus-2.30.1.orig/src/file-manager/fm-icon-view.c @@ +1945,3 @@ file = icon_view->details->audio_preview_file; uri = nautilus_file_get_uri (file); + gfile = g_file_new_for_uri (uri); You could just use nautilus_file_get_location () instead of getting the URI and creating a GFile later. Also, you seem to do URI->GFile->path->URI again. Why is that needed? How are the first and the last URI different?
Created attachment 167105 [details] [review] Updated patch, against git master Right, nautilus_file_get_location works too. The transformation URI->GFile->path->URI, although not straightforward, is needed in order to pass playbin a real gvfs URI. Compare the URIs for a track on an audio CD, before and after the transformation: before: cdda://sr0/Track%201.wav after: file:///home/foobar/.gvfs/cdda%20mount%20on%20sr0/Track%201.wav Although not documented, this is exactly what totem does, so I suppose it’s reasonable. I’m attaching an updated patch against git master.
Review of attachment 167105 [details] [review]: ::: src/file-manager/fm-icon-view.c @@ +2031,2 @@ file = icon_view->details->audio_preview_file; uri = nautilus_file_get_uri (file); I see the reason why you need this conversion but in this case, the following should be more elegant and readable: file = icon_view->details->audio_preview_file; gfile = nautilus_file_get_location (file); path = g_file_get_path (gfile); if (path) { uri = g_filename_to_uri (path, NULL, NULL); } else { uri = nautilus_file_get_uri (file); } g_object_unref (gfile); g_free (path); Can you try something similar and see if it works?
@Cosimo: thanks for the review. The changes you suggest are cosmetic, and the code is functionally equivalent, so I don’t see a reason why it wouldn’t work. I’m testing to be really sure, and will attach an updated patch.
Created attachment 168057 [details] [review] Updated patch, against git master This updated patch takes into account Cosimo’s style suggestions. It was successfully tested to ensure it is functionally equivalent to its previous iteration.
Can the status of this bug be set accordingly? @Cosimo: did you have a chance to test my revised patch?
Olivier, thanks for the updated patch. I pushed it to nautilus master, with a slightly better comment and commit message. Closing as FIXED.