Missing media in SDP if setConfiguration() is called after createDataChannel() or addTransceiver() https://bugs.webkit.org/show_bug.cgi?id=273318

cadubentzen · cadubentzen · commit 631510a69f88 · 2024-08-28T17:15:08.000+02:00
Reviewed by Xabier Rodriguez-Calvar. GStreamerMediaEndpoint::setConfiguration() was tearing down the pipeline if one already existed and creating a new one, which is an issue if any data channels or transceivers are created before RTCPeerConnection.setConfiguration(). The issue is fixed by creating the pipeline earlier in GStreamerMediaEndpoint's contructor, so that data channels or transceivers aren't discarded if created/added before setConfiguration(). Credit to Philippe Normand <philn@igalia.com> for finding the issue and fixing it. I wrote the layout test, which fails without his fix. * LayoutTests/webrtc/setConfiguration-after-createDataChannel-or-addTransceiver-expected.txt: Added. * LayoutTests/webrtc/setConfiguration-after-createDataChannel-or-addTransceiver.html: Added. * Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp: (WebCore::GStreamerMediaEndpoint::GStreamerMediaEndpoint): (WebCore::GStreamerMediaEndpoint::setConfiguration): Canonical link: https://commits.webkit.org/278099@main
diff --git a/LayoutTests/webrtc/setConfiguration-after-createDataChannel-or-addTransceiver-expected.txt b/LayoutTests/webrtc/setConfiguration-after-createDataChannel-or-addTransceiver-expected.txt
@@ -0,0 +1,5 @@
+
+PASS setConfiguration after data channel is created
+PASS setConfiguration after video transceiver is added
+PASS setConfiguration after audio transceiver is added
+
diff --git a/LayoutTests/webrtc/setConfiguration-after-createDataChannel-or-addTransceiver.html b/LayoutTests/webrtc/setConfiguration-after-createDataChannel-or-addTransceiver.html
@@ -0,0 +1,39 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Testing media fields in SDP when setConfiguration comes after createDataChannel/addTransceiver</title>
+        <script src="../resources/testharness.js"></script>
+        <script src="../resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <script src ="routines.js"></script>
+        <script>
+function testMediaInSDP(addTransceiverOrDataChannel, regex) {
+    return async (test) => {
+        const pc = new RTCPeerConnection();
+        addTransceiverOrDataChannel(pc);
+        pc.setConfiguration({});
+        await pc.setLocalDescription();
+        const sdp = pc.localDescription.sdp;
+        assert_true(regex.test(sdp));
+    }
+}
+
+promise_test(testMediaInSDP(
+        pc => pc.createDataChannel("data-channel"),
+        /\r\nm=application.*webrtc-datachannel\r\n/),
+    'setConfiguration after data channel is created');
+
+promise_test(testMediaInSDP(
+        pc => pc.addTransceiver("video"),
+        /\r\nm=video.*\r\n/),
+    'setConfiguration after video transceiver is added');
+
+promise_test(testMediaInSDP(
+        pc => pc.addTransceiver("audio"),
+        /\r\nm=audio.*\r\n/),
+    'setConfiguration after audio transceiver is added');
+        </script>
+    </body>
+</html>
diff --git a/Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp b/Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp
@@ -80,6 +80,8 @@ GStreamerMediaEndpoint::GStreamerMediaEndpoint(GStreamerPeerConnectionBackend& p
     std::call_once(debugRegisteredFlag, [] {
         GST_DEBUG_CATEGORY_INIT(webkit_webrtc_endpoint_debug, "webkitwebrtcendpoint", 0, "WebKit WebRTC end-point");
     });
+
+    initializePipeline();
 }
 
 GStreamerMediaEndpoint::~GStreamerMediaEndpoint()
@@ -244,12 +246,6 @@ void GStreamerMediaEndpoint::disposeElementChain(GstElement* element)
 
 bool GStreamerMediaEndpoint::setConfiguration(MediaEndpointConfiguration& configuration)
 {
-    if (m_pipeline)
-        teardownPipeline();
-
-    if (!initializePipeline())
-        return false;
-
     auto bundlePolicy = bundlePolicyFromConfiguration(configuration);
     auto iceTransportPolicy = iceTransportPolicyFromConfiguration(configuration);
     g_object_set(m_webrtcBin.get(), "bundle-policy", bundlePolicy, "ice-transport-policy", iceTransportPolicy, nullptr);
