Title: [271517] trunk
Revision
271517
Author
[email protected]
Date
2021-01-15 05:59:34 -0800 (Fri, 15 Jan 2021)

Log Message

[GStreamer] Clean-up the TextSink
https://bugs.webkit.org/show_bug.cgi?id=220651

Reviewed by Xabier Rodriguez-Calvar.

Source/WebCore:

The GStreamer sink used to collect WebVTT cues is now more self-contained and uses the
player only to hand-off samples to the corresponding TextTrack implementation for further
processing. This is only a refactoring, existing tests in media/track cover this change.

* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::handleTextSample):
(WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin):
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
* platform/graphics/gstreamer/TextSinkGStreamer.cpp:
(webkitTextSinkHandleSample):
(webkitTextSinkConstructed):
(webkitTextSinkQuery):
(webkit_text_sink_class_init):
(webkitTextSinkNew):
* platform/graphics/gstreamer/TextSinkGStreamer.h:

Tools:

* Scripts/webkitpy/style/checker.py: Add GStreamer TextSink implementation to GObject
classes allow-list.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (271516 => 271517)


--- trunk/Source/WebCore/ChangeLog	2021-01-15 12:39:41 UTC (rev 271516)
+++ trunk/Source/WebCore/ChangeLog	2021-01-15 13:59:34 UTC (rev 271517)
@@ -1,3 +1,26 @@
+2021-01-15  Philippe Normand  <[email protected]>
+
+        [GStreamer] Clean-up the TextSink
+        https://bugs.webkit.org/show_bug.cgi?id=220651
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        The GStreamer sink used to collect WebVTT cues is now more self-contained and uses the
+        player only to hand-off samples to the corresponding TextTrack implementation for further
+        processing. This is only a refactoring, existing tests in media/track cover this change.
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::handleTextSample):
+        (WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin):
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
+        * platform/graphics/gstreamer/TextSinkGStreamer.cpp:
+        (webkitTextSinkHandleSample):
+        (webkitTextSinkConstructed):
+        (webkitTextSinkQuery):
+        (webkit_text_sink_class_init):
+        (webkitTextSinkNew):
+        * platform/graphics/gstreamer/TextSinkGStreamer.h:
+
 2021-01-15  Zalan Bujtas  <[email protected]>
 
         [LFC][Integration] REGRESSION (r270123) facebook.com birthday dropdown do not work when creating new account

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (271516 => 271517)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2021-01-15 12:39:41 UTC (rev 271516)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2021-01-15 13:59:34 UTC (rev 271517)
@@ -1217,39 +1217,16 @@
     purgeInvalidTextTracks(validTextStreams);
 }
 
-GstFlowReturn MediaPlayerPrivateGStreamer::newTextSampleCallback(MediaPlayerPrivateGStreamer* player)
+void MediaPlayerPrivateGStreamer::handleTextSample(GstSample* sample, const char* streamId)
 {
-    player->newTextSample();
-    return GST_FLOW_OK;
-}
+    for (auto& track : m_textTracks.values()) {
+        if (!strcmp(track->streamId().utf8().data(), streamId)) {
+            track->handleSample(sample);
+            return;
+        }
+    }
 
-void MediaPlayerPrivateGStreamer::newTextSample()
-{
-    if (!m_textAppSink)
-        return;
-
-    GRefPtr<GstEvent> streamStartEvent = adoptGRef(
-        gst_pad_get_sticky_event(m_textAppSinkPad.get(), GST_EVENT_STREAM_START, 0));
-
-    GRefPtr<GstSample> sample;
-    g_signal_emit_by_name(m_textAppSink.get(), "pull-sample", &sample.outPtr(), nullptr);
-    ASSERT(sample);
-
-    if (streamStartEvent) {
-        bool found = FALSE;
-        const gchar* id;
-        gst_event_parse_stream_start(streamStartEvent.get(), &id);
-        for (auto& track : m_textTracks.values()) {
-            if (!strcmp(track->streamId().utf8().data(), id)) {
-                track->handleSample(sample);
-                found = true;
-                break;
-            }
-        }
-        if (!found)
-            GST_WARNING("Got sample with unknown stream ID %s.", id);
-    } else
-        GST_WARNING("Unable to handle sample with no stream start event.");
+    GST_WARNING_OBJECT(m_pipeline.get(), "Got sample with unknown stream ID %s.", streamId);
 }
 
 MediaTime MediaPlayerPrivateGStreamer::platformDuration() const
@@ -2788,18 +2765,11 @@
     ASSERT(textCombiner);
     g_object_set(m_pipeline.get(), "text-stream-combiner", textCombiner, nullptr);
 
-    m_textAppSink = webkitTextSinkNew();
-    ASSERT(m_textAppSink);
+    m_textSink = webkitTextSinkNew(makeWeakPtr(*this));
+    ASSERT(m_textSink);
 
-    m_textAppSinkPad = adoptGRef(gst_element_get_static_pad(m_textAppSink.get(), "sink"));
-    ASSERT(m_textAppSinkPad);
+    g_object_set(m_pipeline.get(), "text-sink", m_textSink.get(), nullptr);
 
-    auto textCaps = adoptGRef(gst_caps_new_empty_simple("application/x-subtitle-vtt"));
-    g_object_set(m_textAppSink.get(), "emit-signals", TRUE, "enable-last-sample", FALSE, "caps", textCaps.get(), nullptr);
-    g_signal_connect_swapped(m_textAppSink.get(), "new-sample", G_CALLBACK(newTextSampleCallback), this);
-
-    g_object_set(m_pipeline.get(), "text-sink", m_textAppSink.get(), nullptr);
-
     if (!m_audioSink)
         m_audioSink = createAudioSink();
 

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h (271516 => 271517)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2021-01-15 12:39:41 UTC (rev 271516)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2021-01-15 13:59:34 UTC (rev 271517)
@@ -223,6 +223,8 @@
     void flushCurrentBuffer();
 #endif
 
+    void handleTextSample(GstSample*, const char* streamId);
+
 #if !RELEASE_LOG_DISABLED
     const Logger& logger() const final { return m_logger; }
     const char* logClassName() const override { return "MediaPlayerPrivateGStreamer"; }
@@ -296,7 +298,6 @@
     void notifyPlayerOfVideo();
     void notifyPlayerOfAudio();
     void notifyPlayerOfText();
-    void newTextSample();
 
     void ensureAudioSourceProvider();
     void setAudioStreamProperties(GObject*);
@@ -307,7 +308,6 @@
     static void videoChangedCallback(MediaPlayerPrivateGStreamer*);
     static void audioChangedCallback(MediaPlayerPrivateGStreamer*);
     static void textChangedCallback(MediaPlayerPrivateGStreamer*);
-    static GstFlowReturn newTextSampleCallback(MediaPlayerPrivateGStreamer*);
 
     void timeChanged();
     void loadingFailed(MediaPlayer::NetworkState, MediaPlayer::ReadyState = MediaPlayer::ReadyState::HaveNothing, bool forceNotifications = false);
@@ -456,8 +456,7 @@
 #endif
 
     Atomic<bool> m_isPlayerShuttingDown;
-    GRefPtr<GstElement> m_textAppSink;
-    GRefPtr<GstPad> m_textAppSinkPad;
+    GRefPtr<GstElement> m_textSink;
     GstStructure* m_mediaLocations { nullptr };
     int m_mediaLocationCurrentIndex { 0 };
     bool m_isPlaybackRatePaused { false };

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.cpp (271516 => 271517)


--- trunk/Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.cpp	2021-01-15 12:39:41 UTC (rev 271516)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.cpp	2021-01-15 13:59:34 UTC (rev 271517)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2013 Cable Television Laboratories, Inc.
+ * Copyright (C) 2021 Igalia S.L
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -24,78 +25,115 @@
  */
 
 #include "config.h"
+#include "TextSinkGStreamer.h"
+
 #if ENABLE(VIDEO) && USE(GSTREAMER)
 
-#include "TextSinkGStreamer.h"
+#include "GStreamerCommon.h"
+#include "MediaPlayerPrivateGStreamer.h"
+#include <gst/app/gstappsink.h>
+#include <wtf/glib/WTFGType.h>
 
 GST_DEBUG_CATEGORY_STATIC(webkitTextSinkDebug);
 #define GST_CAT_DEFAULT webkitTextSinkDebug
 
-#define webkit_text_sink_parent_class parent_class
-G_DEFINE_TYPE_WITH_CODE(WebKitTextSink, webkit_text_sink, GST_TYPE_APP_SINK,
-    GST_DEBUG_CATEGORY_INIT(webkitTextSinkDebug, "webkittextsink", 0,
-        "webkit text sink"));
+using namespace WebCore;
 
-enum {
-    Prop0,
-    PropSync,
-    PropLast
+struct _WebKitTextSinkPrivate {
+    GRefPtr<GstElement> appSink;
+    WeakPtr<MediaPlayerPrivateGStreamer> mediaPlayerPrivate;
+    const char* streamId { nullptr };
 };
 
-static void webkit_text_sink_init(WebKitTextSink* sink)
+#define webkit_text_sink_parent_class parent_class
+WEBKIT_DEFINE_TYPE_WITH_CODE(WebKitTextSink, webkit_text_sink, GST_TYPE_BIN,
+    GST_DEBUG_CATEGORY_INIT(webkitTextSinkDebug, "webkittextsink", 0, "webkit text sink"))
+
+static void webkitTextSinkHandleSample(WebKitTextSink* self, GRefPtr<GstSample>&& sample)
 {
-    /* We want to get cues as quickly as possible so WebKit has time to handle them,
-     * and we don't want cues to block when they come in the wrong order. */
-    gst_base_sink_set_sync(GST_BASE_SINK(sink), false);
+    auto* priv = self->priv;
+    if (!priv->streamId) {
+        auto pad = adoptGRef(gst_element_get_static_pad(priv->appSink.get(), "sink"));
+        auto streamStartEvent = adoptGRef(gst_pad_get_sticky_event(pad.get(), GST_EVENT_STREAM_START, 0));
+
+        if (streamStartEvent)
+            gst_event_parse_stream_start(streamStartEvent.get(), &priv->streamId);
+    }
+
+    if (priv->streamId) {
+        // As the mediaPlayerPrivate WeakPtr is constructed from the main thread, we have to use it
+        // from the main thread as well.
+        callOnMainThreadAndWait([priv, sample = WTFMove(sample)] {
+            priv->mediaPlayerPrivate->handleTextSample(sample.get(), priv->streamId);
+        });
+        return;
+    }
+    GST_WARNING_OBJECT(self, "Unable to handle sample with no stream start event.");
 }
 
-static void webkitTextSinkGetProperty(GObject*, guint /* propertyId */,
-    GValue*, GParamSpec*)
+static void webkitTextSinkConstructed(GObject* object)
 {
-    /* Do nothing with PropSync */
-}
+    GST_CALL_PARENT(G_OBJECT_CLASS, constructed, (object));
 
-static void webkitTextSinkSetProperty(GObject*, guint /* propertyId */,
-    const GValue*, GParamSpec*)
-{
-    /* Do nothing with PropSync */
+    auto* sink = WEBKIT_TEXT_SINK(object);
+    auto* priv = sink->priv;
+
+    priv->appSink = gst_element_factory_make("appsink", nullptr);
+    gst_bin_add(GST_BIN_CAST(sink), priv->appSink.get());
+
+    auto pad = adoptGRef(gst_element_get_static_pad(priv->appSink.get(), "sink"));
+    gst_element_add_pad(GST_ELEMENT_CAST(sink), gst_ghost_pad_new("sink", pad.get()));
+
+    auto textCaps = adoptGRef(gst_caps_new_empty_simple("application/x-subtitle-vtt"));
+    g_object_set(priv->appSink.get(), "emit-signals", TRUE, "enable-last-sample", FALSE, "caps", textCaps.get(), nullptr);
+
+    g_signal_connect(priv->appSink.get(), "new-sample", G_CALLBACK(+[](GstElement* appSink, WebKitTextSink* sink) -> GstFlowReturn {
+        webkitTextSinkHandleSample(sink, adoptGRef(gst_app_sink_pull_sample(GST_APP_SINK(appSink))));
+        return GST_FLOW_OK;
+    }), sink);
+
+    g_signal_connect(priv->appSink.get(), "new-preroll", G_CALLBACK(+[](GstElement* appSink, WebKitTextSink* sink) -> GstFlowReturn {
+        webkitTextSinkHandleSample(sink, adoptGRef(gst_app_sink_pull_preroll(GST_APP_SINK(appSink))));
+        return GST_FLOW_OK;
+    }), sink);
+
+    // We want to get cues as quickly as possible so WebKit has time to handle them,
+    // and we don't want cues to block when they come in the wrong order.
+    gst_base_sink_set_sync(GST_BASE_SINK_CAST(sink->priv->appSink.get()), false);
 }
 
-static gboolean webkitTextSinkQuery(GstElement *element, GstQuery *query)
+static gboolean webkitTextSinkQuery(GstElement* element, GstQuery* query)
 {
     switch (GST_QUERY_TYPE(query)) {
     case GST_QUERY_DURATION:
     case GST_QUERY_POSITION:
-        /* Ignore duration and position because we don't want the seek bar to be
-         * based on where the cues are. */
+        // Ignore duration and position because we don't want the seek bar to be based on where the cues are.
         return false;
     default:
-        WebKitTextSink* sink = WEBKIT_TEXT_SINK(element);
-        GstElement* parent = GST_ELEMENT(&sink->parent);
-        return GST_ELEMENT_CLASS(parent_class)->query(parent, query);
+        return GST_CALL_PARENT_WITH_DEFAULT(GST_ELEMENT_CLASS, query, (element, query), FALSE);
     }
 }
 
 static void webkit_text_sink_class_init(WebKitTextSinkClass* klass)
 {
-    GObjectClass* gobjectClass = G_OBJECT_CLASS(klass);
-    GstElementClass* elementClass = GST_ELEMENT_CLASS(klass);
+    auto* gobjectClass = G_OBJECT_CLASS(klass);
+    auto* elementClass = GST_ELEMENT_CLASS(klass);
 
-    gst_element_class_set_metadata(elementClass, "WebKit text sink", "Generic",
-        "An appsink that ignores the sync property and position and duration queries",
+    gst_element_class_set_metadata(elementClass, "WebKit text sink", GST_ELEMENT_FACTORY_KLASS_SINK,
+        "WebKit's text sink collecting cues encoded in WebVTT by the WebKit text-combiner",
         "Brendan Long <[email protected]>");
 
-    gobjectClass->get_property = GST_DEBUG_FUNCPTR(webkitTextSinkGetProperty);
-    gobjectClass->set_property = GST_DEBUG_FUNCPTR(webkitTextSinkSetProperty);
+    gobjectClass->constructed = GST_DEBUG_FUNCPTR(webkitTextSinkConstructed);
     elementClass->query = GST_DEBUG_FUNCPTR(webkitTextSinkQuery);
-
-    /* Override "sync" so playsink doesn't mess with our appsink */
-    g_object_class_override_property(gobjectClass, PropSync, "sync");
 }
 
-GstElement* webkitTextSinkNew()
+GstElement* webkitTextSinkNew(WeakPtr<MediaPlayerPrivateGStreamer>&& player)
 {
-    return GST_ELEMENT(g_object_new(WEBKIT_TYPE_TEXT_SINK, nullptr));
+    auto* element = GST_ELEMENT_CAST(g_object_new(WEBKIT_TYPE_TEXT_SINK, nullptr));
+    auto* sink = WEBKIT_TEXT_SINK(element);
+    ASSERT(isMainThread());
+    sink->priv->mediaPlayerPrivate = WTFMove(player);
+    return element;
 }
 
 #endif // ENABLE(VIDEO) && USE(GSTREAMER)

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.h (271516 => 271517)


--- trunk/Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.h	2021-01-15 12:39:41 UTC (rev 271516)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.h	2021-01-15 13:59:34 UTC (rev 271517)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2013 Cable Television Laboratories, Inc.
+ * Copyright (C) 2021 Igalia S.L
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -27,11 +28,17 @@
 
 #if ENABLE(VIDEO) && USE(GSTREAMER)
 
-#include <glib-object.h>
-#include <gst/app/gstappsink.h>
 #include <gst/gst.h>
+#include <wtf/Forward.h>
 
+namespace WebCore {
+class MediaPlayerPrivateGStreamer;
+}
+
+G_BEGIN_DECLS
+
 #define WEBKIT_TYPE_TEXT_SINK webkit_text_sink_get_type()
+GType webkit_text_sink_get_type(void);
 
 #define WEBKIT_TEXT_SINK(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), WEBKIT_TYPE_TEXT_SINK, WebKitTextSink))
 #define WEBKIT_TEXT_SINK_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass), WEBKIT_TYPE_TEXT_SINK, WebKitTextSinkClass))
@@ -41,23 +48,19 @@
 
 typedef struct _WebKitTextSink WebKitTextSink;
 typedef struct _WebKitTextSinkClass WebKitTextSinkClass;
+typedef struct _WebKitTextSinkPrivate WebKitTextSinkPrivate;
 
 struct _WebKitTextSink {
-    GstAppSink parent;
+    GstBin parent;
+    WebKitTextSinkPrivate* priv;
 };
 
 struct _WebKitTextSinkClass {
-    GstAppSinkClass parentClass;
-
-    // Future padding
-    void (* _webkit_reserved1)(void);
-    void (* _webkit_reserved2)(void);
-    void (* _webkit_reserved3)(void);
-    void (* _webkit_reserved4)(void);
-    void (* _webkit_reserved5)(void);
-    void (* _webkit_reserved6)(void);
+    GstBinClass parentClass;
 };
 
-GstElement* webkitTextSinkNew();
+GstElement* webkitTextSinkNew(WTF::WeakPtr<WebCore::MediaPlayerPrivateGStreamer>&&);
 
+G_END_DECLS
+
 #endif // ENABLE(VIDEO) && USE(GSTREAMER)

Modified: trunk/Tools/ChangeLog (271516 => 271517)


--- trunk/Tools/ChangeLog	2021-01-15 12:39:41 UTC (rev 271516)
+++ trunk/Tools/ChangeLog	2021-01-15 13:59:34 UTC (rev 271517)
@@ -1,3 +1,13 @@
+2021-01-15  Philippe Normand  <[email protected]>
+
+        [GStreamer] Clean-up the TextSink
+        https://bugs.webkit.org/show_bug.cgi?id=220651
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        * Scripts/webkitpy/style/checker.py: Add GStreamer TextSink implementation to GObject
+        classes allow-list.
+
 2021-01-15  Rob Buis  <[email protected]>
 
         Use event loop to set title

Modified: trunk/Tools/Scripts/webkitpy/style/checker.py (271516 => 271517)


--- trunk/Tools/Scripts/webkitpy/style/checker.py	2021-01-15 12:39:41 UTC (rev 271516)
+++ trunk/Tools/Scripts/webkitpy/style/checker.py	2021-01-15 13:59:34 UTC (rev 271517)
@@ -223,6 +223,8 @@
       os.path.join('Source', 'WebCore', 'platform', 'graphics', 'gstreamer', 'TextCombinerGStreamer.h'),
       os.path.join('Source', 'WebCore', 'platform', 'graphics', 'gstreamer', 'TextCombinerPadGStreamer.cpp'),
       os.path.join('Source', 'WebCore', 'platform', 'graphics', 'gstreamer', 'TextCombinerPadGStreamer.h'),
+      os.path.join('Source', 'WebCore', 'platform', 'graphics', 'gstreamer', 'TextSinkGStreamer.cpp'),
+      os.path.join('Source', 'WebCore', 'platform', 'graphics', 'gstreamer', 'TextSinkGStreamer.h'),
       os.path.join('Source', 'WebCore', 'platform', 'graphics', 'gstreamer', 'VideoSinkGStreamer.cpp'),
       os.path.join('Source', 'WebCore', 'platform', 'graphics', 'gstreamer', 'WebKitWebSourceGStreamer.cpp'),
       os.path.join('Source', 'WebCore', 'platform', 'graphics', 'gstreamer', 'WebKitAudioSinkGStreamer.cpp'),
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to