The problem is that:
- the list of supported mime-types is hard-coded.
- there's no way to change the handler easily (see bug 41258)

Created attachment 97248 [details] [review]
nautilus-use-better-audio-preview.patch

- Remove the old unused audio preview code
- Use the totem-audio-preview binary (I haven't committed it, using Totem's code allows for better error checking, and free xine-lib support)
- totem_mime_types.h is copied from a totem build

We could also use a simple gstreamer application, really up to the maintainers. The totem code has a some code to avoid eating memory (commits suicide if it uses too much memory, or resources).

In the meanwhile, feel free to use:
---8<---
#!/bin/sh

gst-launch-0.10 playbin uri=fd://0
---8<---

The old audio preview code is already dead on gio-branch.

I dunno if we really want to depend on totem for such a simple thing. Maybe we can look for totem-audio-play and otherwise fall back to the script.

Also, it would be nice if it handled unhandled-type errors in such a way that we could remove that type from the supported list and not try it again.

Created attachment 97279 [details] [review]
nautilus-use-better-audio-preview-2.patch

Tested against nautilus from the gnome-2-20 branch, and works surprisingly well.

Created attachment 97286 [details] [review]
nautilus-use-better-audio-preview-3.patch

- Better warning about the origin in the .h file
- Make the array static
- Change the name of the header not to mention totem
- Makefile.am changes

This is a duplicate of bug 111511

One of the nice "features" of the gst-launch-0.10 approach will be that it will play videos in a popup if they're recognised as audio files.

We need to change the gst-launch line to:
gst-launch-0.10 uridecodebin uri=fd://0 ! audioconvert ! audioresample ! autoaudiosink

(In reply to comment #8)
> One of the nice "features" of the gst-launch-0.10 approach will be that it will
> play videos in a popup if they're recognised as audio files.

It might be cool to do this intentionally for all videos. See http://www.apple.com/macosx/features/quicklook.html.

Can we please fold this into bug 111511, which is about exactly the same bug, has been open for four and a half years, has a long cc list, and has a fairly similar patch attached?

It's great that we're finally getting some movement on this, but opening an entirely new bug (and ignoring comment 7 above) is not going to help community cohesion.

I'm very keen to see the audio previews move over to gstreamer, so I recently tested out the better-audio-preview-patch3.  I'm using Gentoo and gnome-2.20.0.  The patch applied fine, but no audio previews ever existed.  This seems to be because the Gentoo team apply a patch to remove the requirement for ESD.  Since this preview system should work without ESD, I assumed that it wouldn't matter.

Unforunately, it appears the patch attached here makes use of the can_play_sound function check.  This check either requires ESD to be present (so that the esd_open_sound call succeeds), or it checks if the audio_preview_pid value is greater than 0 (which only gets set after a sound has started playing).  Since neither of these checks will ever succeed, I was getting no audio previews.

This fix is presumably to change can_play_audio, so that if USE_OLD_AUDIO_PREVIEW is set, it will always return true, since we've already passed the check that looks for gst-launch.

I've tested this on my build, and the patch now seems to work excellently, I hope to see it in nautilus soon...  5:)

Also, I forgot to add, that the /desktop/gnome/sound/enable_esd key is still checked, which probably shouldn't be if we don't actually need ESD to play the previews.  It gets checked in preview_sound and simply examines gnome_esd_enabled_auto_value to see if it's true or not.  

Mike, would you mind attaching your fixed patch?

Created attachment 97584 [details] [review]
nautilus-use-better-audio-preview-4.patch

Patch removing checks for esd, as suggested by Mike Auty above.

Ed, that seems pretty reasonable, although I'm not certain I'd remove the esd gconf check entirely.  If people still want to build using the "new" ESD audio preview I imagine it might cause some slight upset (although given all the other checks, it doesn't seem like it does a great deal).

I can attach my patch, but it's currently designed to be applied after Gentoo's noesd patch, and yours seems just as, if not even more, clean than my hacked up version.  5:)

Mainly it's just a fertile source of confusion - I ticked "audio preview", now I have to go and tick another box somewhere else entirely?

Also, using strcmp() on mime types is wrong - need to consider aliases, parent
relationships, etc.  Do we have xdgmime in nautilus?

Created attachment 97585 [details] [review]
nautilus-use-better-audio-preview-5.patch

No, but we have gnome_vfs_mime_type_get_equivalence(), which is what we want.

(In reply to comment #16)
<snip>
> Also, using strcmp() on mime types is wrong - need to consider aliases, parent
> relationships, etc.  Do we have xdgmime in nautilus?

The list of mime-types is direct from totem, so all the mime-types and their aliases are listed, so there's no need for the expensive gnome_vfs_mime_type_get_equivalence(). Maybe building a hashtable for quick lookup would be useful as well.

(In reply to comment #18)
> The list of mime-types is direct from totem, so all the mime-types and their
> aliases are listed, so there's no need for the expensive
> gnome_vfs_mime_type_get_equivalence(). Maybe building a hashtable for quick
> lookup would be useful as well.

In that case it's missing various mime types of the form audio/x-*+ogg, in particular audio/x-vorbis+ogg (the mime type that recent shared-mime-info (0.22?) detects for Vorbis-in-Ogg audio).  audio/x-vorbis+ogg is-a-subtype-of audio/ogg.

If that's the same list as is used in Totem, then other usages (the .desktop file in particular) go through xdgmime and so use the xdgmime aliases and subclass tables.  Using xdgmime would at least ensure consistency (i.e. if Totem appears in the Open With list, then it can preview it).

Is calling into xdgmime really that painful?  It seems reasonably efficient internally - btree searches and all that.  Maybe a cache from mime type to is-audio-previewable?

You're leaking mime_type in sound_preview_type_supported().

Created attachment 97671 [details] [review]
nautilus-use-better-audio-preview-6.patch

Fix leak.

Tested the latest version of the patch (-6) with a one-line change to make it apply to Gentoo patched Nautilus 2.20. So far it mostly works, but I had some problems, who could or could not depend on my local setup:

1) ogg files are initially detected as "application/ogg"; this is not among the listed mimetypes so the preview doesn't work unless you select the files to make nautilus recognize them as "audio/x-vorbis+ogg"; this is inconsistent with the behavior of the other audio files which are played when simply hovering the mouse on the icon.

2) adding "application/ogg" to the supported mimetypes list fixes this glitch, but it introduces the problem described in comment 8; ogg video files are initially recognized as "application/ogg" as well, and nautilus tries to preview them.

3) changing the gst-launch pipeline to "uridecodebin uri=fd://0 ! audioconvert ! audioresample ! autoaudiosink" breaks preview of mp3 files. The command line output of the command is:

Pipeline is PREROLLING ...
ERROR: from element /pipeline0/uridecodebin0/source: Internal data flow error.
Additional debug info:
gstbasesrc.c(1816): gst_base_src_loop (): /pipeline0/uridecodebin0/source:
streaming task paused, reason error (-5)
ERROR: pipeline doesn't want to preroll.
Setting pipeline to NULL ...

However this could be a local problem (I'm using the latest release of all gstreamer components and I suppose that mp3 are played with gst-plugins-mad). Does it work for you? (it works here with playbin.)

4) Preview in nautilus of my musepack files doesn't work with playbin nor with uridecodebin, even though it works from the command line.

Concluding, preview is fine for mp3 and flac, works with some glitches with ogg, doesn't work at all with musepack. I didn't test other file types.

According to [1], "*.ogg" should now only be used for Ogg Vorbis; use .ogv for Ogg Theora etc.  This means that only .ogx should be recognised as application/ogg before magic checking.  I'll file a bug to get this thrashed out on f.d.o.

Hm - it might be useful, for these sorts of container formats, for the audio preview cb to trigger mime magic detection on mouseover.

1. http://wiki.xiph.org/index.php/MIME_Types_and_File_Extensions

f.d.o bug for *.ogg: https://bugs.freedesktop.org/show_bug.cgi?id=12890

(In reply to comment #22)
> 3) changing the gst-launch pipeline to "uridecodebin uri=fd://0 ! audioconvert
> ! audioresample ! autoaudiosink" breaks preview of mp3 files. The command line
> output of the command is:
> 
> Pipeline is PREROLLING ...
> ERROR: from element /pipeline0/uridecodebin0/source: Internal data flow error.
> Additional debug info:
> gstbasesrc.c(1816): gst_base_src_loop (): /pipeline0/uridecodebin0/source:
> streaming task paused, reason error (-5)
> ERROR: pipeline doesn't want to preroll.
> Setting pipeline to NULL ...
> 
> However this could be a local problem (I'm using the latest release of all
> gstreamer components and I suppose that mp3 are played with gst-plugins-mad).
> Does it work for you? (it works here with playbin.)

It works for me using:
gst-launch-0.10 uridecodebin uri=fd://0 ! audioconvert ! audioresample ! autoaudiosink incorporated into Bastian's patch

gstreamer-0.10.14-1
gstreamer-plugins-base-0.10.14-1
gstreamer-plugins-good-0.10.6-3
gstreamer-plugins-bad-0.10.5-2
gstreamer-plugins-ugly-0.10.6-1

% gst-inspect | grep mp3
ffmpeg:  ffdemux_mp3: FFMPEG MPEG audio demuxer
ffmpeg:  ffdec_mp3on4: FFMPEG MP3ON4 decoder
ffmpeg:  ffdec_mp3adu: FFMPEG ADU-formatted MPEG-1 layer 3 audio decoder
ffmpeg:  ffdec_mp3: FFMPEG MPEG-1 layer 3 audio decoder
mad:  mad: mad mp3 decoder
typefindfunctions: audio/mpeg: mp3, mp2, mp1, mpga
typefindfunctions: application/x-id3v1: mp3, mp2, mp1, mpga, ogg, flac, tta
typefindfunctions: application/x-id3v2: mp3, mp2, mp1, mpga, ogg, flac, tta
mpegaudioparse:  mp3parse: MPEG1 Audio Parser
lame:  lame: L.A.M.E. mp3 encoder

so it's something with your setup.

> It works for me using:
> gst-launch-0.10 uridecodebin uri=fd://0 ! audioconvert ! audioresample !
> autoaudiosink incorporated into Bastian's patch
> 
> 
> so it's something with your setup.
> 

Actually it was a problem with the vanilla release of gst-plugins-0.10.6. If anybody has the same problem, this patch (http://webcvs.freedesktop.org/gstreamer/gst-plugins-good/gst/id3demux/gstid3demux.c?r1=1.29&r2=1.30) should fix it.

Musepack files still don't work, however.

I commited a patch loosely based on this to the gio-branch. It also removes the gnome-vfs code, etc.

I tried to keep the loading of the file in nautilus and piping it to the play app, but I failed to handle SIGPIPE on child dying in a good way, so in the end i instead pass the uri to the helper.

(In reply to comment #27)
> I commited a patch loosely based on this to the gio-branch. It also removes the
> gnome-vfs code, etc.
> 
> I tried to keep the loading of the file in nautilus and piping it to the play
> app, but I failed to handle SIGPIPE on child dying in a good way, so in the end
> i instead pass the uri to the helper.

Passing a URI to the helper means that this won't work properly for remote locations that require authentication, when it used to.

Hadess:
a) We currently only preview local files
b) With gio the authentication is shared (the mount is session global)

Looks great.  Looking forward to the gio branch already.  Anyone want to close bug 111511?

*** Bug 111511 has been marked as a duplicate of this bug. ***

*** Bug 167946 has been marked as a duplicate of this bug. ***