Changeset 235110 in webkit

Timestamp:

Aug 21, 2018 2:07:33 AM (6 years ago)

Author:

Philippe Normand

Message:

[GStreamer][MSE] Generic main thread notification support
​https://bugs.webkit.org/show_bug.cgi?id=188647

Patch by Philippe Normand <​philn@igalia.com> on 2018-08-21
Reviewed by Xabier Rodriguez-Calvar.

Using GstBus for main thread notifications has the side effect of "leaking" the
application messages to the media player, leading to CPU cycles wasting.

No new tests, existing MSE tests cover this change.

• platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:

(webkit_media_src_init):
(webKitMediaSrcFinalize):
(webKitMediaSrcSetMediaPlayerPrivate):

• platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp (modified) (7 diffs)
• platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-08-21  Philippe Normand  <philn@igalia.com>
+
+        [GStreamer][MSE] Generic main thread notification support
+        https://bugs.webkit.org/show_bug.cgi?id=188647
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Using GstBus for main thread notifications has the side effect of "leaking" the
+        application messages to the media player, leading to CPU cycles wasting.
+
+        No new tests, existing MSE tests cover this change.
+
+        * platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:
+        (webkit_media_src_init):
+        (webKitMediaSrcFinalize):
+        (webKitMediaSrcSetMediaPlayerPrivate):
+        * platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h:
+
 2018-08-21  Philippe Normand  <philn@igalia.com>
 

trunk/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp

@@ -41 +41 @@
 #include "WebKitMediaSourceGStreamerPrivate.h"
 
-#include <gst/app/app.h>
-#include <gst/app/gstappsrc.h>
-#include <gst/gst.h>
-#include <gst/pbutils/missing-plugins.h>
 #include <gst/pbutils/pbutils.h>
 #include <gst/video/video.h>
 #include <wtf/Condition.h>
 #include <wtf/MainThread.h>
+#include <wtf/RefPtr.h>
 #include <wtf/text/CString.h>
 
@@ -86 +83 @@
 
 static Stream* getStreamByAppsrc(WebKitMediaSrc*, GstElement*);
+static void seekNeedsDataMainThread(WebKitMediaSrc*);
+static void notifyReadyForMoreSamplesMainThread(WebKitMediaSrc*, Stream*);
 
 static void enabledAppsrcNeedData(GstAppSrc* appsrc, guint, gpointer userData)
@@ -120 +119 @@
 
         switch (appsrcSeekDataNextAction) {
-        case MediaSourceSeekToTime: {
-            GstStructure* structure = gst_structure_new_empty("seek-needs-data");
-            GstMessage* message = gst_message_new_application(GST_OBJECT(appsrc), structure);
-            gst_bus_post(webKitMediaSrc->priv->bus.get(), message);
-            GST_TRACE("seek-needs-data message posted to the bus");
-            break;
-        }
+        case MediaSourceSeekToTime:
+            webKitMediaSrc->priv->notifier->notify(WebKitMediaSrcMainThreadNotification::SeekNeedsData, [webKitMediaSrc] {
+                seekNeedsDataMainThread(webKitMediaSrc);
+            });
+            break;
         case Nothing:
             break;
@@ -138 +135 @@
         appsrcStream = getStreamByAppsrc(webKitMediaSrc, GST_ELEMENT(appsrc));
 
-        if (appsrcStream && appsrcStream->type != WebCore::Invalid) {
-            GstStructure* structure = gst_structure_new("ready-for-more-samples", "appsrc-stream", G_TYPE_POINTER, appsrcStream, nullptr);
-            GstMessage* message = gst_message_new_application(GST_OBJECT(appsrc), structure);
-            gst_bus_post(webKitMediaSrc->priv->bus.get(), message);
-            GST_TRACE("ready-for-more-samples message posted to the bus");
-        }
+        if (appsrcStream && appsrcStream->type != WebCore::Invalid)
+            webKitMediaSrc->priv->notifier->notify(WebKitMediaSrcMainThreadNotification::ReadyForMoreSamples, [webKitMediaSrc, appsrcStream] {
+                notifyReadyForMoreSamplesMainThread(webKitMediaSrc, appsrcStream);
+            });
 
         GST_OBJECT_UNLOCK(webKitMediaSrc);
@@ -265 +260 @@
     source->priv->appsrcSeekDataNextAction = Nothing;
     source->priv->flowCombiner = GUniquePtr<GstFlowCombiner>(gst_flow_combiner_new());
+    source->priv->notifier = WebCore::MainThreadNotifier<WebKitMediaSrcMainThreadNotification>::create();
 
     // No need to reset Stream.appsrcNeedDataFlag because there are no Streams at this point yet.
@@ -283 +279 @@
 
     priv->seekTime = MediaTime::invalidTime();
+
+    source->priv->notifier->invalidate();
 
     if (priv->mediaPlayerPrivate)
@@ -695 +693 @@
 }
 
-static void applicationMessageCallback(GstBus*, GstMessage* message, WebKitMediaSrc* source)
-{
-    ASSERT(WTF::isMainThread());
-    ASSERT(GST_MESSAGE_TYPE(message) == GST_MESSAGE_APPLICATION);
-
-    const GstStructure* structure = gst_message_get_structure(message);
-
-    if (gst_structure_has_name(structure, "seek-needs-data")) {
-        seekNeedsDataMainThread(source);
-        return;
-    }
-
-    if (gst_structure_has_name(structure, "ready-for-more-samples")) {
-        Stream* appsrcStream = nullptr;
-        gst_structure_get(structure, "appsrc-stream", G_TYPE_POINTER, &appsrcStream, nullptr);
-        ASSERT(appsrcStream);
-
-        notifyReadyForMoreSamplesMainThread(source, appsrcStream);
-        return;
-    }
-
-    ASSERT_NOT_REACHED();
-}
-
 void webKitMediaSrcSetMediaPlayerPrivate(WebKitMediaSrc* source, WebCore::MediaPlayerPrivateGStreamerMSE* mediaPlayerPrivate)
 {
     GST_OBJECT_LOCK(source);
-    if (source->priv->mediaPlayerPrivate && source->priv->mediaPlayerPrivate != mediaPlayerPrivate && source->priv->bus)
-        g_signal_handlers_disconnect_by_func(source->priv->bus.get(), gpointer(applicationMessageCallback), source);
 
     // Set to nullptr on MediaPlayerPrivateGStreamer destruction, never a dangling pointer.
     source->priv->mediaPlayerPrivate = mediaPlayerPrivate;
-    source->priv->bus = mediaPlayerPrivate ? adoptGRef(gst_pipeline_get_bus(GST_PIPELINE(mediaPlayerPrivate->pipeline()))) : nullptr;
-    if (source->priv->bus) {
-        // MediaPlayerPrivateGStreamer has called gst_bus_add_signal_watch() at this point, so we can subscribe.
-        g_signal_connect(source->priv->bus.get(), "message::application", G_CALLBACK(applicationMessageCallback), source);
-    }
     GST_OBJECT_UNLOCK(source);
 }

trunk/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h

@@ -25 +25 @@
 #include "AudioTrackPrivateGStreamer.h"
 #include "GUniquePtrGStreamer.h"
+#include "MainThreadNotifier.h"
 #include "SourceBufferPrivateGStreamer.h"
 #include "VideoTrackPrivateGStreamer.h"
@@ -31 +32 @@
 #include <gst/app/gstappsrc.h>
 #include <gst/gst.h>
-#include <wtf/Condition.h>
-#include <wtf/RefPtr.h>
+#include <wtf/Forward.h>
 #include <wtf/glib/GRefPtr.h>
 
@@ -96 +96 @@
 };
 
+enum WebKitMediaSrcMainThreadNotification {
+    ReadyForMoreSamples = 1 << 0,
+    SeekNeedsData = 1 << 1
+};
+
 struct _WebKitMediaSrcPrivate {
     // Used to coordinate the release of Stream track info.
@@ -119 +124 @@
     int appsrcNeedDataCount;
 
-    GRefPtr<GstBus> bus;
     WebCore::MediaPlayerPrivateGStreamerMSE* mediaPlayerPrivate;
 
+    RefPtr<WebCore::MainThreadNotifier<WebKitMediaSrcMainThreadNotification>> notifier;
     GUniquePtr<GstFlowCombiner> flowCombiner;
 };