Bug 291438 - phonon-vlc reports every file (including non-media ones) as loadable.
Summary: phonon-vlc reports every file (including non-media ones) as loadable.
Status: RESOLVED NOT A BUG
Alias: None
Product: phonon-backend-vlc
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: 0.4.1
Platform: Mandriva RPMs Linux
: NOR normal
Target Milestone: 0.4.2
Assignee: Harald Sitter
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-13 12:41 UTC by Shlomi Fish
Modified: 2012-01-29 21:57 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Shlomi Fish 2012-01-13 12:41:57 UTC
Version:           0.4.1 (using Devel) 
OS:                Linux

Per the suggestions on https://bugs.kde.org/show_bug.cgi?id=267319#c35 (an Amarok bug) I've written the following patch:

[PATCH]
diff --git a/src/EngineController.cpp b/src/EngineController.cpp
index 81f39b7..39e67a7 100644
--- a/src/EngineController.cpp
+++ b/src/EngineController.cpp
@@ -53,6 +53,7 @@
 
 #include <QTextDocument>
 #include <QtCore/qmath.h>
+#include <QCoreApplication>
 
 namespace The {
     EngineController* engineController() { return EngineController::instance(); }
@@ -237,6 +238,17 @@ EngineController::canDecode( const KUrl &url ) //static
     if( !item.isLocalFile() )
         return true;
 
+#if 1
+    Phonon::MediaObject tryLoadingMedia;
+    tryLoadingMedia.setCurrentSource(url.toLocalFile());
+    debug() << "[before]try.state() == " << tryLoadingMedia.state();
+    while (tryLoadingMedia.state() == Phonon::LoadingState)
+    {
+        QCoreApplication::instance()->processEvents( QEventLoop::AllEvents );
+    }
+    debug() << "[after]try.state() == " << tryLoadingMedia.state();
+    return (tryLoadingMedia.state() == Phonon::StoppedState);
+#else
     // Phonon::BackendCapabilities::isMimeTypeAvailable is too simplistic for our purposes
     // FIXME: this variable should be updated when
     // Phonon::BackendCapabilities::notifier()'s capabilitiesChanged signal is emitted
@@ -255,6 +267,7 @@ EngineController::canDecode( const KUrl &url ) //static
     }
 
     return valid;
+#endif
 }
 
 QStringList
[/PATCH]

This patch works as expected using phonon-gstreamer as the default phonon backend, but when the backend is set to phonon-vlc, it accepts every file as valid, including many text files such as Perl modules (*.pm) or Ruby programs (*.rb). The problem is that the .state() always ends up as Phonon::StoppedState using phonon-vlc.

This problem also affects dragon player which will accept every file.

Hopefully, I'll create a small command line testcase soon. 

Reproducible: Always

Steps to Reproduce:
1. Select phonon-vlc in the KDE systemsettings.

2. Restart the session.

3. Start Dragon Player.

4. Drag and drop a text file from Dolphin to Dragon Player.

Actual Results:  
Dragon player tries to play the file.

Expected Results:  
Dragon player should reject the file.

I'm on x86-64 Magiea Linux 2/Cauldron.
Comment 1 Harald Sitter 2012-01-13 16:06:28 UTC
what is the problem with that exactly?
Comment 2 Shlomi Fish 2012-01-13 16:13:35 UTC
(In reply to comment #1)
> what is the problem with that exactly?

Well, in https://bugs.kde.org/show_bug.cgi?id=267319 , we were told that, instead of relying on mime types, Amarok should load the enqueued files into a media object and see if they can be played to determine if they should be added to the playlist (or alteratively if they are non-media files). However, with phonon-vlc, all files are getting reported as valid media files this way, which may end up cluttering Amarok's playlist with files phonon cannot play. In phonon-gstreamer, it works fine.

I think this should be consistent - if the file cannot be played, then doing ->setCurrentSource() should result in ::ErrorState in both cases.
Comment 3 Jean-Baptiste Kempf 2012-01-13 16:17:41 UTC
How do you know if you can playback a file without playing it?
Comment 4 Harald Sitter 2012-01-15 12:29:48 UTC
> I think this should be consistent - if the file cannot be played, then doing
> ->setCurrentSource() should result in ::ErrorState in both 


2 things.

1. setCurrentSource has no impact on playability, see the phonon state machine [1]. After setCurrentSource phonon may hop any number of times from stopped to loading and back, it *may* get an error there, but it does not have to. Particular error cases at this point are for example file-does-not-exist. Only once Phonon switches to the playing submachine (indicated by the box in the state machine diagram) you can assume errors are caused by playback itself.
Of course errors may appear at any point, like if you unplug an external HDD you may get file errors from the playback submachine. Therefore the example from earlier was just illustration, no hard rule to go by!

2. phonon vlc is behaving according to expectations here...
Try to play a PDF and observe. VLC has a vast amount of decoders, among them is one for text (think subtitle files). Naturally VLC can therefore 'play' text.

That being said, you will never get absolutely accurate results as both VLC and GStreamer can decode much more than audio and video (think images). With GStreamer this is an even bigger problem as IIRC it also has decoders for e.g. Microsoft Office formats. This of course depends on the amount of installed gst plugins (hence the unavoidable inconsistency :P).

If you want close to perfect results you'll first need to filter the set of all possible files through a mime whitelist (everything audio/* and video/* for example), then run the resulting subset through Phonon.

If you want even better results for Amarok you'd run the subset through Phonon while listening on an AudioDataOutput instance for output, then run this through some algorithm to decide whether there is sensible noise coming out of it. I suppose only files that actually produce sound are of interest, so of the subset  that is actually playable you'd still want to filter those that generate no audio (think video files without audio stream).

So, I still do not see the problem with the present behavior. You ask Phonon whether it can play text, and it obviously can play text (as indicated by the lack of error). If you try to play text in VLC directly you'll also get no error FWIW.

[1] http://quickgit.kde.org/?p=phonon.git&a=blob&h=6dcfdc6a2e83cac2335cd1a25173d0d0583fc342&hb=443b851d390e2717413641f671715ca4b5d1acf4&f=doc/state-machine.svg
Comment 5 Shlomi Fish 2012-01-29 21:19:06 UTC
Sorry for the late response,

(In reply to comment #4)
> > I think this should be consistent - if the file cannot be played, then doing
> > ->setCurrentSource() should result in ::ErrorState in both 
> 
> 
> 2 things.
> 

Well, given all that, I don't see how I can *cleanly* implement the phonon people's suggestion to detect if a file is playable or not by trying to play it using Amarok - all that without resorting to making sure we rely on mimetypes.

I'm OK with this bug getting closed, but it seems that there should be a way to implement a clean and straightforward interface to detect if a given file has an audio component.

Regards,

-- Shlomi Fish  

> 1. setCurrentSource has no impact on playability, see the phonon state machine
> [1]. After setCurrentSource phonon may hop any number of times from stopped to
> loading and back, it *may* get an error there, but it does not have to.
> Particular error cases at this point are for example file-does-not-exist. Only
> once Phonon switches to the playing submachine (indicated by the box in the
> state machine diagram) you can assume errors are caused by playback itself.
> Of course errors may appear at any point, like if you unplug an external HDD
> you may get file errors from the playback submachine. Therefore the example
> from earlier was just illustration, no hard rule to go by!
> 
> 2. phonon vlc is behaving according to expectations here...
> Try to play a PDF and observe. VLC has a vast amount of decoders, among them is
> one for text (think subtitle files). Naturally VLC can therefore 'play' text.
> 
> That being said, you will never get absolutely accurate results as both VLC and
> GStreamer can decode much more than audio and video (think images). With
> GStreamer this is an even bigger problem as IIRC it also has decoders for e.g.
> Microsoft Office formats. This of course depends on the amount of installed gst
> plugins (hence the unavoidable inconsistency :P).
> 
> If you want close to perfect results you'll first need to filter the set of all
> possible files through a mime whitelist (everything audio/* and video/* for
> example), then run the resulting subset through Phonon.
> 
> If you want even better results for Amarok you'd run the subset through Phonon
> while listening on an AudioDataOutput instance for output, then run this
> through some algorithm to decide whether there is sensible noise coming out of
> it. I suppose only files that actually produce sound are of interest, so of the
> subset  that is actually playable you'd still want to filter those that
> generate no audio (think video files without audio stream).
> 
> So, I still do not see the problem with the present behavior. You ask Phonon
> whether it can play text, and it obviously can play text (as indicated by the
> lack of error). If you try to play text in VLC directly you'll also get no
> error FWIW.
> 
> [1]
> http://quickgit.kde.org/?p=phonon.git&a=blob&h=6dcfdc6a2e83cac2335cd1a25173d0d0583fc342&hb=443b851d390e2717413641f671715ca4b5d1acf4&f=doc/state-machine.svg
Comment 6 Harald Sitter 2012-01-29 21:57:20 UTC
Since you apparently did not get the bottom line of point 2:

There is no way to get 100% accurate support checks without hogging the CPU and making indexing about 3000 times slower. There is however a way to get a pretty good approximation which is a first line of defense with a raw sanity filter (i.e. ditch all files that are not of audio/* or video/* mimetype), and then a second line which checks if Phonon can actually play the file.
This will perform sensibly well as only mimetypes that are desired to begin with are checked, hence the overhead from trial-and-error playback should not be "too bad".

Using this approach also will filter out text right away, even if Phonon could technically "play" it. Or pictures for that matter.

What certainly could be done is introduction of a Phonon::SupportProbe class, which would be thread-safe and allow the backends to construct a much thinner pipeline (e.g. in terms of phonon gstreamer I am reasonable certain you can check support of a file without having to attach audio outputs for example). Which then would allow concurrent probing of multiple files with less overhead... But that is hardly within the scope of pvlc :P

Anyhow. Phonon VLC is behaving as expected here.