Bug 34085

Summary: [GTK] Should use GStreamer codec installation infrastructure
Product: WebKit Reporter: Sebastian Dröge (slomo) <slomo>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gustavo, joone.hur, joone, manday, otte, pnormand, spenap
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 34318    
Bug Blocks:    
Attachments:
Description Flags
Patch mrobinson: review+, mrobinson: commit-queue-

Description Sebastian Dröge (slomo) 2010-01-25 06:52:54 PST
If a codec is missing for playing some stream with GStreamer, WebKit should support calling the GStreamer codec installation infrastructure.

Relevant documentation:
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-gstpbutilsmissingplugins.html
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-gstpbutilsinstallplugins.html

In short, a message is posted on the bus with information about missing stuff. WebKit should start the installation then and restart playback after things are installed. Also all following ERROR/WARNING messages after the missing plugin message should be dropped until playback is restarted.
Comment 1 Gustavo Noronha (kov) 2010-01-29 05:14:31 PST
I guess this should follow a policy decided by the application - some applications may want to never request installation of missing codecs. Looks like a good thing to start implementing our MediaPlayerClient, and our media player API for.
Comment 2 Philippe Normand 2011-06-06 00:55:06 PDT
*** Bug 62088 has been marked as a duplicate of this bug. ***
Comment 3 ManDay 2011-06-06 01:22:17 PDT
I find this idea rather problematic. Users usually want their package manager to handle packages, not the application internally. I suggest to *not* implement this.
Comment 4 Sebastian Dröge (slomo) 2011-06-06 01:32:26 PDT
That's exactly the point of the GStreamer codec installation infrastructure. It will use a distribution specific script/application to install the codecs, which then use the distribution's package manager.
Comment 5 ManDay 2011-06-06 01:48:35 PDT
My apologies, I wasn't aware of that. However, I must insist that you shall not remove control from the user, nor do I approve of overly fancy auto-mechanisms.

But as I understood it, you also suggest that a message asks the user whether he wants to install Codec X/Y or not.

I then, to merge bug #62088 into this thread, suggest that

1) If neither the audio nor the video codec is installed, or either of them is not installed, a "Warning" icon appears in the place where the video would actually play, clicking it removes it and opens a context menu in which

a) The required codices can be viewed
b) The required codices can be installed by a click of the mouse

2) If all codecs are avaible, no warning appears but a context menu still provides information about which codices are in use.
Comment 6 ManDay 2011-06-06 01:49:52 PDT
*** Bug 62088 has been marked as a duplicate of this bug. ***
Comment 7 ManDay 2011-10-02 10:42:30 PDT
*** Bug 69225 has been marked as a duplicate of this bug. ***
Comment 8 Simon Pena 2012-06-18 03:29:35 PDT
This should / could be fixed together with bug #34318: handling missing codecs should be one specific case of error/warning/info handling.
Comment 9 Philippe Normand 2013-03-29 08:14:09 PDT
Created attachment 195748 [details]
Patch
Comment 10 Martin Robinson 2013-03-29 08:17:35 PDT
Comment on attachment 195748 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195748&action=review

Very simple and nice! Please fix the style issues when landing.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:50
> +#include <gst/pbutils/missing-plugins.h>

Should be listed alphabetically in the list.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:782
> +            GstInstallPluginsReturn result = gst_install_plugins_async(&detail, 0, mediaPlayerPrivatePluginInstallerResultFunction, reinterpret_cast<gpointer>(this));

There's no cast necessary to go from a class pointer to void* and reinterpret_cast here can hide real errors, so please remove the cast.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:35
> +#include <gst/pbutils/install-plugins.h>

Please list this alphabetically.
Comment 11 Philippe Normand 2013-03-29 08:18:19 PDT
(In reply to comment #1)
> I guess this should follow a policy decided by the application - some applications may want to never request installation of missing codecs. Looks like a good thing to start implementing our MediaPlayerClient, and our media player API for.

For now I left this aside. Actually I'm not sure an application would ever want to not install codecs. If this is the case I think that if no codec-installer program is found nothing will be done anyway.
Comment 12 Sebastian Dröge (slomo) 2013-03-30 00:34:22 PDT
Looks good to me
Comment 13 Philippe Normand 2013-03-30 06:06:55 PDT
Committed r147279: <http://trac.webkit.org/changeset/147279>