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'),