Bug 66936 - [GStreamer] fullscreen video pause/play fails
Summary: [GStreamer] fullscreen video pause/play fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-25 07:07 PDT by Philippe Normand
Modified: 2011-09-29 01:03 PDT (History)
3 users (show)

See Also:


Attachments
proposed patch (8.43 KB, patch)
2011-08-25 07:13 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
proposed patch (11.04 KB, patch)
2011-08-26 03:27 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
proposed patch (11.35 KB, patch)
2011-08-26 03:29 PDT, Philippe Normand
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2011-08-25 07:07:58 PDT
In fullscreen the webkitsink goes to PAUSE asynchronously whereas autovideosink goes to PAUSE just fine. Then if we want to resume playback webkitsink tries to go to PLAYING again and is starved because buffers are blocked by the identity element upstream :(

I think we can just avoid the use of identity and in the webkitsink skip rendering of the buffers if we're fullscreen already.
Comment 1 Philippe Normand 2011-08-25 07:13:07 PDT
Created attachment 105177 [details]
proposed patch
Comment 2 Philippe Normand 2011-08-25 09:55:48 PDT
Comment on attachment 105177 [details]
proposed patch

Clearing r?, I'll provide a LayoutTest for this patch
Comment 3 Philippe Normand 2011-08-26 02:45:30 PDT
Hum I failed to come up with a test showing the issue, in fact it's pretty internal to the MediaPlayerPrivate, the video appears freezed but the MediaPlayer continues to issue timeupdate events.
Comment 4 Philippe Normand 2011-08-26 03:27:23 PDT
Created attachment 105341 [details]
proposed patch
Comment 5 Philippe Normand 2011-08-26 03:29:39 PDT
Created attachment 105342 [details]
proposed patch
Comment 6 Alexis Menard (darktears) 2011-08-26 07:59:22 PDT
Comment on attachment 105342 [details]
proposed patch

How this relate to https://bugs.webkit.org/show_bug.cgi?id=58548 ?

I can give a try.

Otherwise the change looks good to me.
Comment 7 Philippe Normand 2011-08-26 08:07:54 PDT
(In reply to comment #6)
> (From update of attachment 105342 [details])
> How this relate to https://bugs.webkit.org/show_bug.cgi?id=58548 ?
> 

That's a different issue, bug 58548 is related to the segment handling and seeking. I need to get back to that one :)

> I can give a try.
> 
> Otherwise the change looks good to me.

Yes, please try on Qt!
Comment 8 Martin Robinson 2011-09-28 09:22:39 PDT
Comment on attachment 105342 [details]
proposed patch

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

This looks sane to me.

> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:377
>  GstElement*
> -webkit_video_sink_new(void)
> +webkit_video_sink_new(WebCore::GStreamerGWorld* gstGWorld)

This should be one line.

> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:379
> +    GstElement* element = (GstElement*)g_object_new(WEBKIT_TYPE_VIDEO_SINK, 0);

Please use a C++-style cast or if GST_ELMENT is available, use that.

> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:381
> +    WebKitVideoSink* sink = WEBKIT_VIDEO_SINK(element);
> +    sink->priv->gstGWorld = gstGWorld;

This should be one line.
Comment 9 Philippe Normand 2011-09-29 01:03:07 PDT
Committed r96310: <http://trac.webkit.org/changeset/96310>