[GStreamer] Adaptive streaming issues
https://bugs.webkit.org/show_bug.cgi?id=144040
Reviewed by Philippe Normand.
In the case of adaptive streaming, the GST URI downloader object is creating the source object, in our case
WebKitWebSrc, without taking its ownership. This is breaking the lifetime of the WebKitWebSrc element. We are
using GRefPtr in WebKitWebSrc to ref/unref the object when sending notifications to the main thread, ensuring
that the object is not destroyed before the main thread dispatches the message. But our smart pointers are so
smart that in case of receiving a floating reference, it's converted to a full reference, so that the first time
we try to take a ref of a WebKitWebSrc having a floating reference we are actually taking the ownership
instead. When we try to release the reference, we are actuallty destroying the object, something that the actual
owner is not expecting and causing runtime critical warnings and very often web process crashes.
(WebKitWebProcess:6863): GStreamer-CRITICAL **:
Trying to dispose element appsrc1, but it is in READY instead of the NULL state.
You need to explicitly set elements to the NULL state before
dropping the final reference, to allow them to clean up.
This problem may also be caused by a refcounting bug in the
application or some element.
(WebKitWebProcess:6863): GStreamer-CRITICAL **: gst_uri_handler_get_uri: assertion 'GST_IS_URI_HANDLER(handler)' failed
(WebKitWebProcess:6863): GStreamer-CRITICAL **: gst_uri_get_protocol: assertion 'uri != NULL' failed
This should be fixed in GST, but we can workaround it in WebKit while it's fixed in GST or to prevent this from
happening if other users make the same mistake. The idea is to add a ensureGRef() only available for GRefPtr
when using WebKitWebSrc objects that consumes the floating reference if needed before taking the actual reference.
* platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
(WTF::ensureGRef): Consume the floating ref if needed.
* platform/graphics/gstreamer/GRefPtrGStreamer.h:
* platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(webKitWebSrcChangeState): Use ensureGRef().
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@200455 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index d3ec2e3..01d5884 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,40 @@
+2016-05-05 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ [GStreamer] Adaptive streaming issues
+ https://bugs.webkit.org/show_bug.cgi?id=144040
+
+ Reviewed by Philippe Normand.
+
+ In the case of adaptive streaming, the GST URI downloader object is creating the source object, in our case
+ WebKitWebSrc, without taking its ownership. This is breaking the lifetime of the WebKitWebSrc element. We are
+ using GRefPtr in WebKitWebSrc to ref/unref the object when sending notifications to the main thread, ensuring
+ that the object is not destroyed before the main thread dispatches the message. But our smart pointers are so
+ smart that in case of receiving a floating reference, it's converted to a full reference, so that the first time
+ we try to take a ref of a WebKitWebSrc having a floating reference we are actually taking the ownership
+ instead. When we try to release the reference, we are actuallty destroying the object, something that the actual
+ owner is not expecting and causing runtime critical warnings and very often web process crashes.
+
+ (WebKitWebProcess:6863): GStreamer-CRITICAL **:
+ Trying to dispose element appsrc1, but it is in READY instead of the NULL state.
+ You need to explicitly set elements to the NULL state before
+ dropping the final reference, to allow them to clean up.
+ This problem may also be caused by a refcounting bug in the
+ application or some element.
+
+ (WebKitWebProcess:6863): GStreamer-CRITICAL **: gst_uri_handler_get_uri: assertion 'GST_IS_URI_HANDLER(handler)' failed
+
+ (WebKitWebProcess:6863): GStreamer-CRITICAL **: gst_uri_get_protocol: assertion 'uri != NULL' failed
+
+ This should be fixed in GST, but we can workaround it in WebKit while it's fixed in GST or to prevent this from
+ happening if other users make the same mistake. The idea is to add a ensureGRef() only available for GRefPtr
+ when using WebKitWebSrc objects that consumes the floating reference if needed before taking the actual reference.
+
+ * platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
+ (WTF::ensureGRef): Consume the floating ref if needed.
+ * platform/graphics/gstreamer/GRefPtrGStreamer.h:
+ * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
+ (webKitWebSrcChangeState): Use ensureGRef().
+
2016-05-05 Csaba Osztrogonác <ossy@webkit.org>
[Mac][cmake] Unreviewed speculative buildfix after r200433, just for fun.
diff --git a/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp
index 47113f6..2372243 100644
--- a/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp
+++ b/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp
@@ -342,6 +342,16 @@
return GRefPtr<WebKitWebSrc>(ptr, GRefPtrAdopt);
}
+// This method is only available for WebKitWebSrc and should not be used for any other type.
+// This is only to work around a bug in GST where the URI downloader is not taking the ownership of WebKitWebSrc.
+// See https://bugs.webkit.org/show_bug.cgi?id=144040.
+GRefPtr<WebKitWebSrc> ensureGRef(WebKitWebSrc* ptr)
+{
+ if (ptr && g_object_is_floating(ptr))
+ gst_object_ref_sink(GST_OBJECT(ptr));
+ return GRefPtr<WebKitWebSrc>(ptr);
+}
+
template <> WebKitWebSrc* refGPtr<WebKitWebSrc>(WebKitWebSrc* ptr)
{
if (ptr)
diff --git a/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h
index 81e1e0d..1a336cc 100644
--- a/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h
+++ b/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h
@@ -108,6 +108,7 @@
template<> void derefGPtr<WebKitVideoSink>(WebKitVideoSink* ptr);
template<> GRefPtr<WebKitWebSrc> adoptGRef(WebKitWebSrc* ptr);
+GRefPtr<WebKitWebSrc> ensureGRef(WebKitWebSrc* ptr);
template<> WebKitWebSrc* refGPtr<WebKitWebSrc>(WebKitWebSrc* ptr);
template<> void derefGPtr<WebKitWebSrc>(WebKitWebSrc* ptr);
diff --git a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
index 47a3f25..ffdb072 100644
--- a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
+++ b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
@@ -188,7 +188,7 @@
return;
}
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::NeedData, [protector] { webKitWebSrcNeedData(protector.get()); });
},
// enough_data
@@ -202,7 +202,7 @@
return;
}
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::EnoughData, [protector] { webKitWebSrcEnoughData(protector.get()); });
},
// seek_data
@@ -222,7 +222,7 @@
priv->requestedOffset = offset;
}
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::Seek, [protector] { webKitWebSrcSeek(protector.get()); });
return TRUE;
},
@@ -640,7 +640,7 @@
case GST_STATE_CHANGE_READY_TO_PAUSED:
{
GST_DEBUG_OBJECT(src, "READY->PAUSED");
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::Start, [protector] { webKitWebSrcStart(protector.get()); });
break;
}
@@ -648,7 +648,7 @@
{
GST_DEBUG_OBJECT(src, "PAUSED->READY");
priv->notifier.cancelPendingNotifications();
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::Stop, [protector] { webKitWebSrcStop(protector.get()); });
break;
}
