Bug 34318
| Summary: | [GStreamer][WK2] Should show an error to the user if something goes wrong | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Sebastian Dröge (slomo) <slomo> |
| Component: | Media | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED WORKSFORME | ||
| Severity: | Normal | CC: | bugs-noreply, calvaris, danilo.eu, eric.carlson, gustavo, plaes, pnormand, tomi.kyostila, xan.lopez |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | OS X 10.5 | ||
| Bug Depends on: | |||
| Bug Blocks: | 34085 | ||
Sebastian Dröge (slomo)
Hi,
the GStreamer media stuff in WebKit currently silently stops playback if an error happens. Instead it should show an error to the user. An error can be produced by using the patch from #34317 and trying to seek in a video on vimeo.com (it doesn't allow range requests).
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Philippe Normand
*** Bug 34760 has been marked as a duplicate of this bug. ***
Philippe Normand
It also silently fails if no audio sink could be created by playbin2
Sebastian Dröge (slomo)
kov said, that there should be a per-frame or per-view (I'd vote for per-view) object, that has signals for errors and which can be used to control the volume and stuff from the application.
Sounds like the way to go IMHO
Sebastian Dröge (slomo)
Proposed API, for the errors at least:
WebKitMediaPlayerController:
Signals:
void error (GObject *o, GError *error, const gchar *debug)
void warning (GObject *o, GError *error, const gchar *debug)
void info (GObject *o, GError *error, const gchar *debug)
WebKitWebView:
Methods:
web_kit_web_view_get_media_player_controller (WebKitWebView *view)
Later this could be extended to also have methods like:
void web_kit_media_player_controller_set_volume(WebKitMediaPlayerController *controller, gdouble vol)
gdouble web_kit_media_player_controller_get_volume(WebKitMediaPlayerController *controller)
[...]
Not sure if setting volume this way is a good idea though, it would mean that all media players in the current view have the same volume. There must be a better API for volume ;)
Philippe Normand
I see other ports implementing a MediaPlayerProxy.. Maybe we could take inspiration from that?
Philippe Normand
(In reply to comment #4)
> Proposed API, for the errors at least:
>
...
>
> Not sure if setting volume this way is a good idea though, it would mean that
> all media players in the current view have the same volume. There must be a
> better API for volume ;)
What about naming it "global volume" or something like that? So one could set the view volume on a global basis and then tweak it using the <video> controls UI if needed.
Philippe Normand
After discussing with Gustavo during the WebKitGTK+ hackfest, here is a proposed plan (I hope I got it right :D).
At WebKit level we'd like to be notified of the MediaPlayer various state changes, errors and so on. The idea is to add a new class in WebCore called MediaPlayerPlatformClient, attached to the MediaPlayer. In the case of the GTK+ port there would be a WebMediaPlayerClientGtk in WebCoreSupport which would create a GObject that can be delivered by a WebView signal each time a new MediaPlayer is created.
Here's a dump of my notes about the proposed changes:
WebCore
=======
MediaPlayer:
------------
when created get the ChromeClient and call ::mediaPlayerCreated()
public:
MediaPlayerPlatformClient* platformClient() const;
private:
MediaPlayerPlatformClient* m_platformClient;
ChromeClient:
-------------
virtual mediaPlayerCreated(Frame*, MediaPlayer*);
MediaPlayerPlatformClient:
--------------------------
#if ENABLE(GLIB_SUPPORT)
handleError(GError*) { m_client->handleError(err) }
handleInfo(...)
handleWarning(...)
#endif
volume notifications
state notifications
... like MediaPlayer API
created(Frame*);
private:
MediaPlayer* m_player;
MediaPlayerPrivateGStreamer:
----------------------------
example use-case:
m_player->platformClient()->handleError(err)
WebKit/gtk/WebCoreSupport
=========================
ChromeClientGtk:
----------------
mediaPlayerCreated(Frame*, MediaPlayer*) {
playerClient = new WebKitMediaPlayerClient();
mediaPlayer->platformClient()->setWebKitMediaPlayerPlatformClient(playerClient);
g_signal_emit(webView, "media-player-created", playerClient);
}
WebMediaPlayerClientGtk:
------------------------
public:
setWebKitMediaPlayerPlatformClient(WebKitMediaPlayerClient*);
handleError(GError*) { g_signal_emit(m_webkitMediaPlayerClient, "error", ...); }
private:
WebKitMediaPlayerClient* m_webkitMediaPlayerClient;
WebKit/gtk/webkit
=================
WebKitMediaPlayerClient:
Signals:
void error (GObject *o, GError *error, const gchar *debug)
void warning (GObject *o, GError *error, const gchar *debug)
void info (GObject *o, GError *error, const gchar *debug)
WebKitWebView:
Signal:
media-player-created(WebKitMediaPlayerClient*)
Philippe Normand
What do you think about this proposal Eric?
Priit Laes (IRC: plaes)
Does this proposal also cover reporting about missing plugins?
Sebastian Dröge (slomo)
Not really, that requires different handling of the error messages.
Xabier Rodríguez Calvar
I will have a look at this, but I cannot promise to complete the job because of lack of time.
Philippe Normand
*** Bug 96170 has been marked as a duplicate of this bug. ***
Danilo de Paula
Is there anyone still working on this?
Philippe Normand
We should do this for WK2.
Philippe Normand
(In reply to comment #13)
> Is there anyone still working on this?
Nope!
Xabier Rodríguez Calvar
AFAIK errors are reported to JS as expected.