Attachment 347258 Details for Bug 188647 – Patch

[patch] Patch

bug-188647-20180816152910.patch (text/plain), 8.00 KB, created by Philippe Normand on 2018-08-16 06:29:11 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Philippe Normand

Created: 2018-08-16 06:29:11 PDT

Size: 8.00 KB

patch

obsolete

>Subversion Revision: 234920
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index bed7d527e4e8841320390c0691d6a0422030411f..9fa1e07cf74c8aef898fb45c1a97b3b8ed002a46 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-08-16  Philippe Normand  <philn@igalia.com>
>+
>+        [GStreamer][MSE] Generic main thread notification support
>+        https://bugs.webkit.org/show_bug.cgi?id=188647
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-16  Antti Koivisto  <antti@apple.com>
> 
>         Use OptionSet for ActivityState::Flags
>diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp
>index b7ad89f363127ead7546b035cae8ccbe6665fdb1..4734d4c3c4295f85d86a2d35203d120a17a416b8 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp
>@@ -40,14 +40,11 @@
> #include "VideoTrackPrivateGStreamer.h"
> #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>
> 
> GST_DEBUG_CATEGORY_STATIC(webkit_media_src_debug);
>@@ -85,6 +82,8 @@ GstAppSrcCallbacks disabledAppsrcCallbacks = {
> };
> 
> static Stream* getStreamByAppsrc(WebKitMediaSrc*, GstElement*);
>+static void seekNeedsDataMainThread(WebKitMediaSrc*);
>+static void notifyReadyForMoreSamplesMainThread(WebKitMediaSrc*, Stream*);
> 
> static void enabledAppsrcNeedData(GstAppSrc* appsrc, guint, gpointer userData)
> {
>@@ -120,10 +119,9 @@ static void enabledAppsrcNeedData(GstAppSrc* appsrc, guint, gpointer userData)
> 
>         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");
>+            webKitMediaSrc->priv->notifier->notify(MSEMainThreadNotification::SeekNeedsData, [webKitMediaSrc] {
>+                seekNeedsDataMainThread(webKitMediaSrc);
>+            });
>             break;
>         }
>         case Nothing:
>@@ -138,10 +136,10 @@ static void enabledAppsrcNeedData(GstAppSrc* appsrc, guint, gpointer userData)
>         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");
>+
>+            webKitMediaSrc->priv->notifier->notify(MSEMainThreadNotification::ReadyForMoreSamples, [webKitMediaSrc, appsrcStream] {
>+                notifyReadyForMoreSamplesMainThread(webKitMediaSrc, appsrcStream);
>+            });
>         }
> 
>         GST_OBJECT_UNLOCK(webKitMediaSrc);
>@@ -264,6 +262,7 @@ static void webkit_media_src_init(WebKitMediaSrc* source)
>     source->priv->appsrcNeedDataCount = 0;
>     source->priv->appsrcSeekDataNextAction = Nothing;
>     source->priv->flowCombiner = GUniquePtr<GstFlowCombiner>(gst_flow_combiner_new());
>+    source->priv->notifier = WebCore::MainThreadNotifier<MSEMainThreadNotification>::create();
> 
>     // No need to reset Stream.appsrcNeedDataFlag because there are no Streams at this point yet.
> }
>@@ -283,6 +282,8 @@ void webKitMediaSrcFinalize(GObject* object)
> 
>     priv->seekTime = MediaTime::invalidTime();
> 
>+    source->priv->notifier->invalidate();
>+
>     if (priv->mediaPlayerPrivate)
>         webKitMediaSrcSetMediaPlayerPrivate(source, nullptr);
> 
>@@ -694,43 +695,12 @@ static void notifyReadyForMoreSamplesMainThread(WebKitMediaSrc* source, Stream*
>     GST_OBJECT_UNLOCK(source);
> }
> 
>-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);
> }
> 
>diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h b/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h
>index 9d7eab9f42de7db50da86b8aceb5fe51cf97ecdf..5ef248354669f76fc8b96bf0df2b9c363a863bdf 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h
>+++ b/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamerPrivate.h
>@@ -24,14 +24,14 @@
> 
> #include "AudioTrackPrivateGStreamer.h"
> #include "GUniquePtrGStreamer.h"
>+#include "MainThreadNotifier.h"
> #include "SourceBufferPrivateGStreamer.h"
> #include "VideoTrackPrivateGStreamer.h"
> #include "WebKitMediaSourceGStreamer.h"
> 
> #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>
> 
> namespace WebCore {
>@@ -97,6 +97,11 @@ enum OnSeekDataAction {
>     MediaSourceSeekToTime
> };
> 
>+enum MSEMainThreadNotification {
>+    SeekNeedsData = 1 << 0,
>+    ReadyForMoreSamples = 1 << 1
>+};
>+
> struct _WebKitMediaSrcPrivate {
>     // Used to coordinate the release of Stream track info.
>     Lock streamLock;
>@@ -120,9 +125,9 @@ struct _WebKitMediaSrcPrivate {
>     int appsrcSeekDataCount;
>     int appsrcNeedDataCount;
> 
>-    GRefPtr<GstBus> bus;
>     WebCore::MediaPlayerPrivateGStreamerMSE* mediaPlayerPrivate;
> 
>+    RefPtr<WebCore::MainThreadNotifier<MSEMainThreadNotification>> notifier;
>     GUniquePtr<GstFlowCombiner> flowCombiner;
> };
>

Flags:

calvaris: review+
calvaris: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 188647: 347258