Title: [231414] releases/WebKitGTK/webkit-2.20/Source/WebCore
- Revision
- 231414
- Author
- [email protected]
- Date
- 2018-05-07 01:18:44 -0700 (Mon, 07 May 2018)
Log Message
Merge r230627 - [GTK] [gstreamer] video won't unpause when built with -DUSE_GSTREAMER_GL=OFF
https://bugs.webkit.org/show_bug.cgi?id=183362
Reviewed by Carlos Garcia Campos.
Remove the drawCancelled flag and use a new one to indicate that the player is being destroyed.
That new flag is only enabled on destruction and it's not modified by cancelRepaint(), which
can be used to handle the pause event without avoiding future renderings. Also cancelRepaint()
has only effect when not in AC mode.
Covered by existent tests.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
(WebCore::MediaPlayerPrivateGStreamerBase::triggerRepaint):
(WebCore::MediaPlayerPrivateGStreamerBase::cancelRepaint):
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (231413 => 231414)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog 2018-05-07 08:18:39 UTC (rev 231413)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog 2018-05-07 08:18:44 UTC (rev 231414)
@@ -1,3 +1,23 @@
+2018-04-13 Miguel Gomez <[email protected]>
+
+ [GTK] [gstreamer] video won't unpause when built with -DUSE_GSTREAMER_GL=OFF
+ https://bugs.webkit.org/show_bug.cgi?id=183362
+
+ Reviewed by Carlos Garcia Campos.
+
+ Remove the drawCancelled flag and use a new one to indicate that the player is being destroyed.
+ That new flag is only enabled on destruction and it's not modified by cancelRepaint(), which
+ can be used to handle the pause event without avoiding future renderings. Also cancelRepaint()
+ has only effect when not in AC mode.
+
+ Covered by existent tests.
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+ (WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
+ (WebCore::MediaPlayerPrivateGStreamerBase::triggerRepaint):
+ (WebCore::MediaPlayerPrivateGStreamerBase::cancelRepaint):
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
+
2018-04-08 Yusuke Suzuki <[email protected]>
Use alignas instead of compiler-specific attributes
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (231413 => 231414)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2018-05-07 08:18:39 UTC (rev 231413)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2018-05-07 08:18:44 UTC (rev 231414)
@@ -241,6 +241,9 @@
MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase()
{
+ // Flag the player as being destroyed, so triggerRepaint will ignore incoming samples.
+ m_destroying = true;
+
#if ENABLE(ENCRYPTED_MEDIA)
m_protectionCondition.notifyAll();
#endif
@@ -259,7 +262,7 @@
if (m_volumeElement)
g_signal_handlers_disconnect_matched(m_volumeElement.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
- // This will release the GStreamer thread from m_drawCondition if AC is disabled.
+ // This will release the GStreamer thread from m_drawCondition in non AC mode in case there's an ongoing triggerRepaint call.
cancelRepaint();
// The change to GST_STATE_NULL state is always synchronous. So after this gets executed we don't need to worry
@@ -789,6 +792,10 @@
void MediaPlayerPrivateGStreamerBase::triggerRepaint(GstSample* sample)
{
+ // Do not try to process new frames if the player is being destroyed by the main thread.
+ if (m_destroying)
+ return;
+
bool triggerResize;
{
WTF::GMutexLocker<GMutex> lock(m_sampleMutex);
@@ -803,8 +810,6 @@
if (!m_renderingCanBeAccelerated) {
LockHolder locker(m_drawMutex);
- if (m_drawCancelled)
- return;
m_drawTimer.startOneShot(0_s);
m_drawCondition.wait(m_drawMutex);
return;
@@ -816,8 +821,6 @@
#else
{
LockHolder lock(m_drawMutex);
- if (m_drawCancelled)
- return;
if (!m_platformLayerProxy->scheduleUpdateOnCompositorThread([this] { this->pushTextureToCompositor(); }))
return;
m_drawCondition.wait(m_drawMutex);
@@ -833,14 +836,16 @@
void MediaPlayerPrivateGStreamerBase::cancelRepaint()
{
- LockHolder locker(m_drawMutex);
-
+ // The goal of this function is to release the GStreamer thread from m_drawCondition in triggerRepaint() in non-AC case,
+ // to avoid a deadlock if the player gets paused while waiting for drawing (see https://bugs.webkit.org/show_bug.cgi?id=170003):
+ // the main thread is waiting for the GStreamer thread to pause, but the GStreamer thread is locked waiting for the
+ // main thread to draw. This deadlock doesn't happen when using AC because the sample is processed (not painted) in the compositor
+ // thread, so the main thread can request the pause and wait if the GStreamer thread is waiting for the compositor thread.
if (!m_renderingCanBeAccelerated) {
m_drawTimer.stop();
+ LockHolder locker(m_drawMutex);
+ m_drawCondition.notifyOne();
}
-
- m_drawCancelled = true;
- m_drawCondition.notifyOne();
}
void MediaPlayerPrivateGStreamerBase::repaintCancelledCallback(MediaPlayerPrivateGStreamerBase* player)
Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h (231413 => 231414)
--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h 2018-05-07 08:18:39 UTC (rev 231413)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h 2018-05-07 08:18:44 UTC (rev 231414)
@@ -232,7 +232,7 @@
Condition m_drawCondition;
Lock m_drawMutex;
- bool m_drawCancelled { false };
+ bool m_destroying { false };
RunLoop::Timer<MediaPlayerPrivateGStreamerBase> m_drawTimer;
#if USE(TEXTURE_MAPPER_GL)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
