- Revision
- 217786
- Author
- [email protected]
- Date
- 2017-06-05 09:27:19 -0700 (Mon, 05 Jun 2017)
Log Message
[GStreamer] Deadlock in MediaPlayerPrivateGStreamer::changePipelineState, web process often locks up on seeking in a youtube video that has already fully buffered
https://bugs.webkit.org/show_bug.cgi?id=170003
Reviewed by Michael Catanzaro.
When video sink is requested to render a frame, the GstBaseSink preroll mutex is taken. Then WebKit media player
schedules a repaint in the main thread, taking the draw mutex and waiting on draw condition. It can happen that
before the repaint is done in the main thread, a pause is requested in the main thread, causing a change state
from PLAYING to PAUSE. When the change state reaches the video sink gst_base_sink_change_state() tries to get
the preroll mutex. This causes a deadlock because the main thread is waiting to get the preroll mutex, but the
other thread is waiting for the main thread to do the repaint. GStreamer handles this case by calling unlock()
on the video sink before trying to get the preroll mutex, but the media player doesn't cancel the pending
repaint when using coordinated graphics. This patch adds a new signal to WebKitVideoSink "repaint-cancelled" to
notify the media player to cancel the pending prepaint.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::cancelRepaint): Release the draw mutex and notify the condition.
(WebCore::MediaPlayerPrivateGStreamerBase::repaintCancelledCallback): Call cancelRepaint().
(WebCore::MediaPlayerPrivateGStreamerBase::createVideoSink): Connect to WebKitVideoSink::repaint-cancelled.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
* platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
(webkitVideoSinkRepaintCancelled): Emit WebKitVideoSink::repaint-cancelled.
(webkitVideoSinkUnlock): Call webkitVideoSinkRepaintCancelled().
(webkitVideoSinkStop): Ditto.
(webkit_video_sink_class_init): Add WebKitVideoSink::repaint-cancelled signal.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (217785 => 217786)
--- trunk/Source/WebCore/ChangeLog 2017-06-05 16:25:55 UTC (rev 217785)
+++ trunk/Source/WebCore/ChangeLog 2017-06-05 16:27:19 UTC (rev 217786)
@@ -1,5 +1,33 @@
2017-06-05 Carlos Garcia Campos <[email protected]>
+ [GStreamer] Deadlock in MediaPlayerPrivateGStreamer::changePipelineState, web process often locks up on seeking in a youtube video that has already fully buffered
+ https://bugs.webkit.org/show_bug.cgi?id=170003
+
+ Reviewed by Michael Catanzaro.
+
+ When video sink is requested to render a frame, the GstBaseSink preroll mutex is taken. Then WebKit media player
+ schedules a repaint in the main thread, taking the draw mutex and waiting on draw condition. It can happen that
+ before the repaint is done in the main thread, a pause is requested in the main thread, causing a change state
+ from PLAYING to PAUSE. When the change state reaches the video sink gst_base_sink_change_state() tries to get
+ the preroll mutex. This causes a deadlock because the main thread is waiting to get the preroll mutex, but the
+ other thread is waiting for the main thread to do the repaint. GStreamer handles this case by calling unlock()
+ on the video sink before trying to get the preroll mutex, but the media player doesn't cancel the pending
+ repaint when using coordinated graphics. This patch adds a new signal to WebKitVideoSink "repaint-cancelled" to
+ notify the media player to cancel the pending prepaint.
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+ (WebCore::MediaPlayerPrivateGStreamerBase::cancelRepaint): Release the draw mutex and notify the condition.
+ (WebCore::MediaPlayerPrivateGStreamerBase::repaintCancelledCallback): Call cancelRepaint().
+ (WebCore::MediaPlayerPrivateGStreamerBase::createVideoSink): Connect to WebKitVideoSink::repaint-cancelled.
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
+ * platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
+ (webkitVideoSinkRepaintCancelled): Emit WebKitVideoSink::repaint-cancelled.
+ (webkitVideoSinkUnlock): Call webkitVideoSinkRepaintCancelled().
+ (webkitVideoSinkStop): Ditto.
+ (webkit_video_sink_class_init): Add WebKitVideoSink::repaint-cancelled signal.
+
+2017-06-05 Carlos Garcia Campos <[email protected]>
+
[GStreamer] Cleanup ifdefs in MediaPlayerPrivateGStreamerBase
https://bugs.webkit.org/show_bug.cgi?id=172918
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (217785 => 217786)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2017-06-05 16:25:55 UTC (rev 217785)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2017-06-05 16:27:19 UTC (rev 217786)
@@ -235,18 +235,7 @@
#endif
m_notifier.cancelPendingNotifications();
-#if USE(TEXTURE_MAPPER_GL)
-#if USE(GSTREAMER_GL)
- bool shouldCancelRepaint = !m_renderingCanBeAccelerated;
-#else
- bool shouldCancelRepaint = true;
-#endif
- if (shouldCancelRepaint) {
- m_drawTimer.stop();
- LockHolder locker(m_drawMutex);
- m_drawCondition.notifyOne();
- }
-#endif // USE(TEXTURE_MAPPER_GL)
+ cancelRepaint();
if (m_videoSink) {
g_signal_handlers_disconnect_matched(m_videoSink.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
@@ -795,7 +784,28 @@
player->triggerRepaint(sample);
}
+void MediaPlayerPrivateGStreamerBase::cancelRepaint()
+{
+#if USE(TEXTURE_MAPPER_GL)
#if USE(GSTREAMER_GL)
+ bool shouldCancelRepaint = !m_renderingCanBeAccelerated;
+#else
+ bool shouldCancelRepaint = true;
+#endif
+ if (shouldCancelRepaint) {
+ m_drawTimer.stop();
+ LockHolder locker(m_drawMutex);
+ m_drawCondition.notifyOne();
+ }
+#endif // USE(TEXTURE_MAPPER_GL)
+}
+
+void MediaPlayerPrivateGStreamerBase::repaintCancelledCallback(MediaPlayerPrivateGStreamerBase* player)
+{
+ player->cancelRepaint();
+}
+
+#if USE(GSTREAMER_GL)
GstFlowReturn MediaPlayerPrivateGStreamerBase::newSampleCallback(GstElement* sink, MediaPlayerPrivateGStreamerBase* player)
{
GRefPtr<GstSample> sample = adoptGRef(gst_app_sink_pull_sample(GST_APP_SINK(sink)));
@@ -1025,6 +1035,7 @@
m_usingFallbackVideoSink = true;
m_videoSink = webkitVideoSinkNew();
g_signal_connect_swapped(m_videoSink.get(), "repaint-requested", G_CALLBACK(repaintCallback), this);
+ g_signal_connect_swapped(m_videoSink.get(), "repaint-cancelled", G_CALLBACK(repaintCancelledCallback), this);
}
GstElement* videoSink = nullptr;
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h (217785 => 217786)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h 2017-06-05 16:25:55 UTC (rev 217785)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h 2017-06-05 16:27:19 UTC (rev 217786)
@@ -169,8 +169,10 @@
void triggerRepaint(GstSample*);
void repaint();
+ void cancelRepaint();
static void repaintCallback(MediaPlayerPrivateGStreamerBase*, GstSample*);
+ static void repaintCancelledCallback(MediaPlayerPrivateGStreamerBase*);
void notifyPlayerOfVolumeChange();
void notifyPlayerOfMute();
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp (217785 => 217786)
--- trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp 2017-06-05 16:25:55 UTC (rev 217785)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp 2017-06-05 16:27:19 UTC (rev 217786)
@@ -58,6 +58,7 @@
enum {
REPAINT_REQUESTED,
+ REPAINT_CANCELLED,
LAST_SIGNAL
};
@@ -188,6 +189,11 @@
g_signal_emit(sink, webkitVideoSinkSignals[REPAINT_REQUESTED], 0, sample);
}
+static void webkitVideoSinkRepaintCancelled(WebKitVideoSink* sink)
+{
+ g_signal_emit(sink, webkitVideoSinkSignals[REPAINT_CANCELLED], 0);
+}
+
static GRefPtr<GstSample> webkitVideoSinkRequestRender(WebKitVideoSink* sink, GstBuffer* buffer)
{
WebKitVideoSinkPrivate* priv = sink->priv;
@@ -278,6 +284,7 @@
WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(baseSink)->priv;
priv->scheduler.stop();
+ webkitVideoSinkRepaintCancelled(WEBKIT_VIDEO_SINK(baseSink));
return GST_CALL_PARENT_WITH_DEFAULT(GST_BASE_SINK_CLASS, unlock, (baseSink), TRUE);
}
@@ -296,6 +303,7 @@
WebKitVideoSinkPrivate* priv = WEBKIT_VIDEO_SINK(baseSink)->priv;
priv->scheduler.stop();
+ webkitVideoSinkRepaintCancelled(WEBKIT_VIDEO_SINK(baseSink));
if (priv->currentCaps) {
gst_caps_unref(priv->currentCaps);
priv->currentCaps = nullptr;
@@ -397,6 +405,16 @@
G_TYPE_NONE, // Return type
1, // Only one parameter
GST_TYPE_SAMPLE);
+ webkitVideoSinkSignals[REPAINT_CANCELLED] = g_signal_new("repaint-cancelled",
+ G_TYPE_FROM_CLASS(klass),
+ G_SIGNAL_RUN_LAST,
+ 0, // Class offset
+ nullptr, // Accumulator
+ nullptr, // Accumulator data
+ g_cclosure_marshal_generic,
+ G_TYPE_NONE, // Return type
+ 0, // No parameters
+ G_TYPE_NONE);
}
