Changeset 279285 in webkit

Timestamp:

Jun 25, 2021, 8:39:12 AM (4 years ago)

Author:

Philippe Normand

Message:

[GStreamer] TextCombiner has unlinked internal encoders
​https://bugs.webkit.org/show_bug.cgi?id=227362

Reviewed by Xabier Rodriguez-Calvar.

Each combiner pad can receive multiple caps events for the same stream, so we can't really
rely on those to modify the internal topology of the combiner. Instead we can check the
sticky events from the pad chain function, this is done at most once per pad. In case a caps
event was sticked to the pad, the combiner now reacts properly.

This issue was detected while running
media/track/in-band/track-in-band-kate-ogg-cues-added-once.html with playbin3 enabled. 6
different encoders where added to the combiner while only 2 are needed actually.

• platform/graphics/gstreamer/TextCombinerGStreamer.cpp:

(webKitTextCombinerHandleCaps):

• platform/graphics/gstreamer/TextCombinerGStreamer.h:
• platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp:

(webkitTextCombinerPadChain):
(webkitTextCombinerPadConstructed):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/gstreamer/TextCombinerGStreamer.cpp (modified) (1 diff)
• platform/graphics/gstreamer/TextCombinerGStreamer.h (modified) (1 diff)
• platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-06-25  Philippe Normand  <pnormand@igalia.com>
+
+        [GStreamer] TextCombiner has unlinked internal encoders
+        https://bugs.webkit.org/show_bug.cgi?id=227362
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Each combiner pad can receive multiple caps events for the same stream, so we can't really
+        rely on those to modify the internal topology of the combiner. Instead we can check the
+        sticky events from the pad chain function, this is done at most once per pad. In case a caps
+        event was sticked to the pad, the combiner now reacts properly.
+
+        This issue was detected while running
+        media/track/in-band/track-in-band-kate-ogg-cues-added-once.html with playbin3 enabled. 6
+        different encoders where added to the combiner while only 2 are needed actually.
+
+        * platform/graphics/gstreamer/TextCombinerGStreamer.cpp:
+        (webKitTextCombinerHandleCaps):
+        * platform/graphics/gstreamer/TextCombinerGStreamer.h:
+        * platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp:
+        (webkitTextCombinerPadChain):
+        (webkitTextCombinerPadConstructed):
+
 2021-06-17  Sergio Villar Senin  <svillar@igalia.com>
 

trunk/Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp

@@ -51 +51 @@
 using namespace WebCore;
 
-void webKitTextCombinerHandleCapsEvent(WebKitTextCombiner* combiner, GstPad* pad, GstEvent* event)
-{
-    GstCaps* caps;
-    gst_event_parse_caps(event, &caps);
+void webKitTextCombinerHandleCaps(WebKitTextCombiner* combiner, GstPad* pad, const GstCaps* caps)
+{
     ASSERT(caps);
     GST_DEBUG_OBJECT(combiner, "Handling caps %" GST_PTR_FORMAT, caps);

trunk/Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.h

@@ -57 +57 @@
 GstElement* webkitTextCombinerNew();
 
-void webKitTextCombinerHandleCapsEvent(WebKitTextCombiner*, GstPad*, GstEvent*);
+void webKitTextCombinerHandleCaps(WebKitTextCombiner*, GstPad*, const GstCaps*);
 
 #endif // ENABLE(VIDEO) && USE(GSTREAMER)

trunk/Source/WebCore/platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp

@@ -37 +37 @@
     GRefPtr<GstTagList> tags;
     GRefPtr<GstPad> innerCombinerPad;
+    bool shouldProcessStickyEvents { true };
 };
 
@@ -54 +55 @@
 {
     switch (GST_EVENT_TYPE(event)) {
-    case GST_EVENT_CAPS:
-        webKitTextCombinerHandleCapsEvent(WEBKIT_TEXT_COMBINER(parent), pad, event);
-        break;
     case GST_EVENT_TAG: {
         auto* combinerPad = WEBKIT_TEXT_COMBINER_PAD(pad);
@@ -78 +76 @@
     }
     return gst_pad_event_default(pad, parent, event);
+}
+
+static GstFlowReturn webkitTextCombinerPadChain(GstPad* pad, GstObject* parent, GstBuffer* buffer)
+{
+    auto* combinerPad = WEBKIT_TEXT_COMBINER_PAD(pad);
+
+    if (combinerPad->priv->shouldProcessStickyEvents) {
+        gst_pad_sticky_events_foreach(pad, [](GstPad* pad, GstEvent** event, gpointer) -> gboolean {
+            if (GST_EVENT_TYPE(*event) != GST_EVENT_CAPS)
+                return TRUE;
+
+            auto* combinerPad = WEBKIT_TEXT_COMBINER_PAD(pad);
+            auto parent = adoptGRef(gst_pad_get_parent(pad));
+            GstCaps* caps;
+            gst_event_parse_caps(*event, &caps);
+            combinerPad->priv->shouldProcessStickyEvents = false;
+            webKitTextCombinerHandleCaps(WEBKIT_TEXT_COMBINER(parent.get()), pad, caps);
+            return FALSE;
+        }, nullptr);
+    }
+
+    return gst_proxy_pad_chain_default(pad, parent, buffer);
 }
 
@@ -121 +141 @@
     gst_ghost_pad_construct(GST_GHOST_PAD(object));
     gst_pad_set_event_function(GST_PAD_CAST(object), webkitTextCombinerPadEvent);
+    gst_pad_set_chain_function(GST_PAD_CAST(object), webkitTextCombinerPadChain);
 }