<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[213224] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/213224">213224</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2017-03-01 09:04:43 -0800 (Wed, 01 Mar 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsplatformgtkTestExpectations">trunk/LayoutTests/platform/gtk/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsgstreamerMediaPlayerPrivateGStreamercpp">trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsgstreamerMediaPlayerPrivateGStreamerBasecpp">trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsgstreamerVideoSinkGStreamercpp">trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (213223 => 213224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-03-01 15:43:54 UTC (rev 213223)
+++ trunk/LayoutTests/ChangeLog 2017-03-01 17:04:43 UTC (rev 213224)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+2017-03-01 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ [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:
+
</ins><span class="cx"> 2017-03-01 Fujii Hironori <Hironori.Fujii@sony.com>
</span><span class="cx">
</span><span class="cx"> [GTK] fast/canvas/canvas-createPattern-video-loading.html makes a following test timeout
</span></span></pre></div>
<a id="trunkLayoutTestsplatformgtkTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/gtk/TestExpectations (213223 => 213224)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1791,8 +1791,7 @@
</span><span class="cx">
</span><span class="cx"> webkit.org/b/163781 media/muted-video-is-playing-audio.html [ Timeout ]
</span><span class="cx">
</span><del>-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 ]
</del><ins>+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 ]
</ins><span class="cx">
</span><span class="cx"> webkit.org/b/168373 http/tests/media/track-in-band-hls-metadata-crash.html [ Timeout ]
</span><span class="cx"> webkit.org/b/168373 media/media-fullscreen-loop-inline.html [ Timeout ]
</span><span class="lines">@@ -1806,8 +1805,6 @@
</span><span class="cx">
</span><span class="cx"> webkit.org/b/168569 http/tests/appcache/main-resource-fallback-for-network-error-crash.html [ Timeout ]
</span><span class="cx">
</span><del>-webkit.org/b/169019 fast/canvas/canvas-createPattern-video-loading.html [ Skip ]
-
</del><span class="cx"> #////////////////////////////////////////////////////////////////////////////////////////
</span><span class="cx"> # End of Tests timing out
</span><span class="cx"> #////////////////////////////////////////////////////////////////////////////////////////
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (213223 => 213224)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2017-03-01 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ [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().
+
</ins><span class="cx"> 2017-03-01 Tomas Popela <tpopela@redhat.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed compiler warning fix after r213218
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsgstreamerMediaPlayerPrivateGStreamercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (213223 => 213224)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -222,7 +222,6 @@
</span><span class="cx"> gst_bus_remove_signal_watch(bus.get());
</span><span class="cx"> gst_bus_set_sync_handler(bus.get(), nullptr, nullptr, nullptr);
</span><span class="cx"> g_signal_handlers_disconnect_matched(m_pipeline.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
</span><del>- gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
</del><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsgstreamerMediaPlayerPrivateGStreamerBasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (213223 => 213224)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -225,6 +225,14 @@
</span><span class="cx"> #endif
</span><span class="cx"> m_notifier.cancelPendingNotifications();
</span><span class="cx">
</span><ins>+#if USE(GSTREAMER_GL) || USE(COORDINATED_GRAPHICS_THREADED)
+ m_drawTimer.stop();
+ {
+ LockHolder locker(m_drawMutex);
+ m_drawCondition.notifyOne();
+ }
+#endif
+
</ins><span class="cx"> if (m_videoSink) {
</span><span class="cx"> g_signal_handlers_disconnect_matched(m_videoSink.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
</span><span class="cx"> #if USE(GSTREAMER_GL)
</span><span class="lines">@@ -250,6 +258,9 @@
</span><span class="cx"> #if ENABLE(LEGACY_ENCRYPTED_MEDIA)
</span><span class="cx"> m_cdmSession = nullptr;
</span><span class="cx"> #endif
</span><ins>+
+ if (m_pipeline)
+ gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void MediaPlayerPrivateGStreamerBase::setPipeline(GstElement* pipeline)
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsgstreamerVideoSinkGStreamercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp (213223 => 213224)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -113,9 +113,10 @@
</span><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> #if USE(COORDINATED_GRAPHICS_THREADED)
</span><del>- if (LIKELY(GST_IS_SAMPLE(m_sample.get())))
- webkitVideoSinkRepaintRequested(sink, m_sample.get());
- m_sample = nullptr;
</del><ins>+ auto sample = WTFMove(m_sample);
+ locker.unlockEarly();
+ if (LIKELY(GST_IS_SAMPLE(sample.get())))
+ webkitVideoSinkRepaintRequested(sink, sample.get());
</ins><span class="cx"> #else
</span><span class="cx"> m_sink = sink;
</span><span class="cx"> m_timer.startOneShot(0);
</span></span></pre>
</div>
</div>
</body>
</html>