- Revision
- 213224
- Author
- [email protected]
- Date
- 2017-03-01 09:04:43 -0800 (Wed, 01 Mar 2017)
Log Message
[GTK] fast/canvas/canvas-createPattern-video-loading.html makes its subsequent test timeout
https://bugs.webkit.org/show_bug.cgi?id=169019
Reviewed by Xabier Rodriguez-Calvar.
Source/WebCore:
The timeout happens normally when the media player is deleted and the pipeline state is set to NULL. The call to
gst_element_set_state() never returns because of different deadlocks with the video sink. Sometimes the deadlock
happens with the sample mutex used by VideoRenderRequestScheduler. VideoRenderRequestScheduler::requestRender()
calls webkitVideoSinkRepaintRequested() with the lock held, that ends up calling
MediaPlayerPrivateGStreamerBase::triggerRepaint(). When rendering can't be accelerated the draw timer is
scheduled and triggerRepaint blocks until the timer is fired. If the media player is destroyed before the timer
is fired, when setting the pipeline state to NULL, other VideoRenderRequestScheduler methods can be called, like
stop() that tries to get the sample mutex that is still held by requestRender(). So, first we need to make
sure that requestRender() releases the lock before calling webkitVideoSinkRepaintRequested(). But that's not
enough, we also need to ensure that the pipeline is set to NULL state after everyting has been properly
stopped. This is currently done in ~MediaPlayerPrivateGStreamer that happens before
~MediaPlayerPrivateGStreamerBase, so gst_element_set_state() is hanging before allowing the
MediaPlayerPrivateGStreamerBase to be cleaned up. We should move the call to the end of
~MediaPlayerPrivateGStreamerBase and ensure the draw timer and mutex are properly cleaned up before.
Fixes: fast/canvas/canvas-createPattern-video-loading.html
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer): Do not reset pipeline here.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase): Stop the draw mutex and notify the
lock to ensure we unblock. Do the pipeline reset at the end.
* platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
(VideoRenderRequestScheduler::requestRender): Release the mutex lock before calling webkitVideoSinkRepaintRequested().
LayoutTests:
Unskip tests previously skipped because of this timeout.
* platform/gtk/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (213223 => 213224)
--- trunk/LayoutTests/ChangeLog 2017-03-01 15:43:54 UTC (rev 213223)
+++ trunk/LayoutTests/ChangeLog 2017-03-01 17:04:43 UTC (rev 213224)
@@ -1,3 +1,14 @@
+2017-03-01 Carlos Garcia Campos <[email protected]>
+
+ [GTK] fast/canvas/canvas-createPattern-video-loading.html makes its subsequent test timeout
+ https://bugs.webkit.org/show_bug.cgi?id=169019
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ Unskip tests previously skipped because of this timeout.
+
+ * platform/gtk/TestExpectations:
+
2017-03-01 Fujii Hironori <[email protected]>
[GTK] fast/canvas/canvas-createPattern-video-loading.html makes a following test timeout
Modified: trunk/LayoutTests/platform/gtk/TestExpectations (213223 => 213224)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2017-03-01 15:43:54 UTC (rev 213223)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2017-03-01 17:04:43 UTC (rev 213224)
@@ -1791,8 +1791,7 @@
webkit.org/b/163781 media/muted-video-is-playing-audio.html [ Timeout ]
-webkit.org/b/163850 imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html [ Skip ]
-webkit.org/b/163850 imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-within-document.html [ Skip ]
+webkit.org/b/163850 imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-within-document.html [ Timeout ]
webkit.org/b/168373 http/tests/media/track-in-band-hls-metadata-crash.html [ Timeout ]
webkit.org/b/168373 media/media-fullscreen-loop-inline.html [ Timeout ]
@@ -1806,8 +1805,6 @@
webkit.org/b/168569 http/tests/appcache/main-resource-fallback-for-network-error-crash.html [ Timeout ]
-webkit.org/b/169019 fast/canvas/canvas-createPattern-video-loading.html [ Skip ]
-
#////////////////////////////////////////////////////////////////////////////////////////
# End of Tests timing out
#////////////////////////////////////////////////////////////////////////////////////////
Modified: trunk/Source/WebCore/ChangeLog (213223 => 213224)
--- trunk/Source/WebCore/ChangeLog 2017-03-01 15:43:54 UTC (rev 213223)
+++ trunk/Source/WebCore/ChangeLog 2017-03-01 17:04:43 UTC (rev 213224)
@@ -1,3 +1,35 @@
+2017-03-01 Carlos Garcia Campos <[email protected]>
+
+ [GTK] fast/canvas/canvas-createPattern-video-loading.html makes its subsequent test timeout
+ https://bugs.webkit.org/show_bug.cgi?id=169019
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ The timeout happens normally when the media player is deleted and the pipeline state is set to NULL. The call to
+ gst_element_set_state() never returns because of different deadlocks with the video sink. Sometimes the deadlock
+ happens with the sample mutex used by VideoRenderRequestScheduler. VideoRenderRequestScheduler::requestRender()
+ calls webkitVideoSinkRepaintRequested() with the lock held, that ends up calling
+ MediaPlayerPrivateGStreamerBase::triggerRepaint(). When rendering can't be accelerated the draw timer is
+ scheduled and triggerRepaint blocks until the timer is fired. If the media player is destroyed before the timer
+ is fired, when setting the pipeline state to NULL, other VideoRenderRequestScheduler methods can be called, like
+ stop() that tries to get the sample mutex that is still held by requestRender(). So, first we need to make
+ sure that requestRender() releases the lock before calling webkitVideoSinkRepaintRequested(). But that's not
+ enough, we also need to ensure that the pipeline is set to NULL state after everyting has been properly
+ stopped. This is currently done in ~MediaPlayerPrivateGStreamer that happens before
+ ~MediaPlayerPrivateGStreamerBase, so gst_element_set_state() is hanging before allowing the
+ MediaPlayerPrivateGStreamerBase to be cleaned up. We should move the call to the end of
+ ~MediaPlayerPrivateGStreamerBase and ensure the draw timer and mutex are properly cleaned up before.
+
+ Fixes: fast/canvas/canvas-createPattern-video-loading.html
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+ (WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer): Do not reset pipeline here.
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+ (WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase): Stop the draw mutex and notify the
+ lock to ensure we unblock. Do the pipeline reset at the end.
+ * platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
+ (VideoRenderRequestScheduler::requestRender): Release the mutex lock before calling webkitVideoSinkRepaintRequested().
+
2017-03-01 Tomas Popela <[email protected]>
Unreviewed compiler warning fix after r213218
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (213223 => 213224)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2017-03-01 15:43:54 UTC (rev 213223)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2017-03-01 17:04:43 UTC (rev 213224)
@@ -222,7 +222,6 @@
gst_bus_remove_signal_watch(bus.get());
gst_bus_set_sync_handler(bus.get(), nullptr, nullptr, nullptr);
g_signal_handlers_disconnect_matched(m_pipeline.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
- gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
}
}
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (213223 => 213224)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2017-03-01 15:43:54 UTC (rev 213223)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2017-03-01 17:04:43 UTC (rev 213224)
@@ -225,6 +225,14 @@
#endif
m_notifier.cancelPendingNotifications();
+#if USE(GSTREAMER_GL) || USE(COORDINATED_GRAPHICS_THREADED)
+ m_drawTimer.stop();
+ {
+ LockHolder locker(m_drawMutex);
+ m_drawCondition.notifyOne();
+ }
+#endif
+
if (m_videoSink) {
g_signal_handlers_disconnect_matched(m_videoSink.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
#if USE(GSTREAMER_GL)
@@ -250,6 +258,9 @@
#if ENABLE(LEGACY_ENCRYPTED_MEDIA)
m_cdmSession = nullptr;
#endif
+
+ if (m_pipeline)
+ gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
}
void MediaPlayerPrivateGStreamerBase::setPipeline(GstElement* pipeline)
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp (213223 => 213224)
--- trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp 2017-03-01 15:43:54 UTC (rev 213223)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp 2017-03-01 17:04:43 UTC (rev 213224)
@@ -113,9 +113,10 @@
return false;
#if USE(COORDINATED_GRAPHICS_THREADED)
- if (LIKELY(GST_IS_SAMPLE(m_sample.get())))
- webkitVideoSinkRepaintRequested(sink, m_sample.get());
- m_sample = nullptr;
+ auto sample = WTFMove(m_sample);
+ locker.unlockEarly();
+ if (LIKELY(GST_IS_SAMPLE(sample.get())))
+ webkitVideoSinkRepaintRequested(sink, sample.get());
#else
m_sink = sink;
m_timer.startOneShot(0);
