Terrible video quality when using TransformStream with Simulcast

https://bugs.webkit.org/show_bug.cgi?id=257803 rdar://110395571 Reviewed by Jean-Yves Avenard. The libwebrtc backend provides one callback per ssrc generated by a sender. In non simulcast cases, there is only one callback. In simulcast case, there is one callback per simulcast encoding. Before the patch, we would use one callback for all video encoded frames, thus sending all simulcast streams in the same ssrc. This would confuse receivers and would lead to decoding failures, leading to sending PLI/FIR, thus leading to bad video quality. We are now storing these callbacks in a map, and use the callback associated to the ssrc of the frame. This allows to correctly send each simulcast stream with its associated ssrc. Covered by added test case. * LayoutTests/http/wpt/webrtc/video-script-transform-simulcast-expected.txt: Added. * LayoutTests/http/wpt/webrtc/video-script-transform-simulcast.html: Added. * LayoutTests/platform/glib/TestExpectations: * Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformBackend.cpp: (WebCore::LibWebRTCRtpTransformBackend::addOutputCallback): (WebCore::LibWebRTCRtpTransformBackend::removeOutputCallback): (WebCore::LibWebRTCRtpTransformBackend::sendFrameToOutput): (WebCore::LibWebRTCRtpTransformBackend::processTransformedFrame): (WebCore::LibWebRTCRtpTransformBackend::Transform): (WebCore::LibWebRTCRtpTransformBackend::RegisterTransformedFrameCallback): (WebCore::LibWebRTCRtpTransformBackend::RegisterTransformedFrameSinkCallback): (WebCore::LibWebRTCRtpTransformBackend::UnregisterTransformedFrameCallback): (WebCore::LibWebRTCRtpTransformBackend::UnregisterTransformedFrameSinkCallback): (WebCore::LibWebRTCRtpTransformBackend::setOutputCallback): Deleted. * Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformBackend.h: Canonical link: https://commits.webkit.org/268912@main
WebKit · Oct 5, 2023 · 1fc26a7 · 1fc26a7
1 parent 455093e
commit 1fc26a7
Show file tree

Hide file tree

Showing 5 changed files with 169 additions and 21 deletions.
diff --git a/LayoutTests/http/wpt/webrtc/video-script-transform-simulcast-expected.txt b/LayoutTests/http/wpt/webrtc/video-script-transform-simulcast-expected.txt
@@ -0,0 +1,5 @@
+
+
+PASS setup
+PASS videos are playing with the correct resolutions
+
diff --git a/LayoutTests/http/wpt/webrtc/video-script-transform-simulcast.html b/LayoutTests/http/wpt/webrtc/video-script-transform-simulcast.html
@@ -0,0 +1,127 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <script src="/resources/testharness.js"></script>
+        <script src="/resources/testharnessreport.js"></script>
+        <script src="/webrtc/third_party/sdp/sdp.js"></script>
+        <script src="/webrtc/simulcast/simulcast.js"></script>
+    </head>
+    <body>
+        <video id="videoL" controls autoplay playsInline></video>
+        <video id="videoM" controls autoplay playsInline></video>
+        <script>
+function waitForMessage(port, data)
+{
+    let gotMessage;
+    const promise = new Promise((resolve, reject) => {
+        gotMessage = resolve;
+        setTimeout(() => { reject("did not get " + data) }, 5000);
+    });
+    port.onmessage = event => {
+       if (event.data === data)
+           gotMessage();
+    };
+    return promise;
+}
+
+let worker;
+promise_test(async (test) => {
+    worker = new Worker('context-transform.js');
+    const data = await new Promise(resolve => worker.onmessage = (event) => resolve(event.data));
+    assert_equals(data, "registered");
+
+    const localStream = await navigator.mediaDevices.getUserMedia({video: true});
+
+    const senderChannel = new MessageChannel;
+    const receiverChannelL = new MessageChannel;
+    const receiverChannelM = new MessageChannel;
+    let sender, receiverM, receiverL;
+    const senderTransform = new RTCRtpScriptTransform(worker, {name:'MockRTCRtpTransform', mediaType:'video', side:'sender', port:senderChannel.port2}, [senderChannel.port2]);
+    const receiverTransformL = new RTCRtpScriptTransform(worker, {name:'MockRTCRtpTransform', mediaType:'video', side:'receiver', port:receiverChannelL.port2}, [receiverChannelL.port2]);
+    const receiverTransformM = new RTCRtpScriptTransform(worker, {name:'MockRTCRtpTransform', mediaType:'video', side:'receiver', port:receiverChannelM.port2}, [receiverChannelM.port2]);
+    senderTransform.port = senderChannel.port1;
+    receiverTransformL.port = receiverChannelL.port1;
+    receiverTransformM.port = receiverChannelM.port1;
+
+    const promise1 = waitForMessage(senderTransform.port, "started video sender");
+    const promise2 = waitForMessage(receiverTransformL.port, "started video receiver");
+    const promise3 = waitForMessage(receiverTransformM.port, "started video receiver");
+
+    await Promise.all([promise1, promise2, promise3]);
+
+    const pc1 = new RTCPeerConnection();
+    const pc2 = new RTCPeerConnection();
+
+    pc1.onicecandidate = e => {
+        const candidate = e.candidate;
+        if (!candidate)
+            return;
+        const newCandidate = { candidate: candidate.candidate, sdpMid: "l", sdpMLineIndex:candidate.sdpMLineIndex };
+        pc2.addIceCandidate(newCandidate);
+    }
+    pc2.onicecandidate = e => {
+        const candidate = e.candidate;
+        if (!candidate)
+            return;
+        const newCandidate = { candidate: candidate.candidate, sdpMid: "0", sdpMLineIndex:candidate.sdpMLineIndex };
+        pc1.addIceCandidate(newCandidate);
+    }
+
+    sender = pc1.addTransceiver("video", {sendEncodings: [{rid: "l", scaleResolutionDownBy: 2}, {rid: "m", scaleResolutionDownBy: 1}]}).sender;
+    sender.replaceTrack(localStream.getVideoTracks()[0]);
+    sender.setStreams(localStream);
+    sender.transform = senderTransform;
+
+    const streamPromise = new Promise((resolve, reject) => {
+        pc2.ontrack = (trackEvent) => {
+            if (!receiverL) {
+                receiverL = trackEvent.receiver;
+                receiverL.transform = receiverTransformL;
+                videoL.srcObject = trackEvent.streams[0];
+                return;
+            }
+
+            receiverM = trackEvent.receiver;
+            receiverM.transform = receiverTransformM;
+            videoM.srcObject = trackEvent.streams[0];
+            resolve();
+        };
+        setTimeout(() => reject("Getting stream timed out"), 5000);
+    });
+
+    const offer = await pc1.createOffer();
+    await pc1.setLocalDescription(offer);
+    await pc2.setRemoteDescription({
+        type: 'offer',
+        sdp: swapRidAndMidExtensionsInSimulcastOffer(offer, ["l", "m"]),
+    });
+    const answer = await pc2.createAnswer();
+    await pc2.setLocalDescription(answer);
+    await pc1.setRemoteDescription({
+        type: 'answer',
+        sdp: swapRidAndMidExtensionsInSimulcastAnswer(answer, pc1.localDescription, ["l", "m"]),
+    });
+
+    await streamPromise;
+}, "setup");
+
+async function waitForVideoSize(video, width, height)
+{
+    const max = 200
+    let counter = 0;
+    while (++counter < max && video.videoWidth != width && video.videoHeight != height)
+        await waitFor(50);
+
+    if (counter === max)
+        return Promise.reject("Video size not expected : " + video.videoWidth + " " + video.videoHeight);
+}
+
+promise_test(async () => {
+    await Promise.all([videoL.play(), videoM.play()]);
+    await waitForVideoSize(videoL, 320, 240);
+    await waitForVideoSize(videoM, 640, 480);
+}, "videos are playing with the correct resolutions");
+        </script>
+    </body>
+</html>
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -1872,6 +1872,7 @@ webrtc/sframe-transform-buffer-source.html [ Skip ]
 webrtc/video-sframe.html [ Skip ]
 webkit.org/b/219825 webrtc/sframe-keys.html [ Skip ]
 webkit.org/b/229055 http/wpt/webrtc/sframe-transform-error.html [ Skip ]
+http/wpt/webrtc/video-script-transform-simulcast.html [ Skip ]
 
 imported/w3c/web-platform-tests/webrtc-extensions/RTCRtpTransceiver-headerExtensionControl.html [ Pass Failure ]
 

diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformBackend.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformBackend.cpp
@@ -42,20 +42,34 @@ void LibWebRTCRtpTransformBackend::clearTransformableFrameCallback()
     setInputCallback({ });
 }
 
-void LibWebRTCRtpTransformBackend::setOutputCallback(rtc::scoped_refptr<webrtc::TransformedFrameCallback>&& callback)
+void LibWebRTCRtpTransformBackend::addOutputCallback(rtc::scoped_refptr<webrtc::TransformedFrameCallback>&& callback, uint32_t ssrc)
 {
-    Locker locker { m_outputCallbackLock };
-    m_outputCallback = WTFMove(callback);
+    Locker locker { m_outputCallbacksLock };
+    m_outputCallbacks.insert_or_assign(ssrc, WTFMove(callback));
 }
 
-void LibWebRTCRtpTransformBackend::processTransformedFrame(RTCRtpTransformableFrame& frame)
+void LibWebRTCRtpTransformBackend::removeOutputCallback(uint32_t ssrc)
 {
-    auto rtcFrame = static_cast<LibWebRTCRtpTransformableFrame&>(frame).takeRTCFrame();
-    if (!rtcFrame)
+    Locker locker { m_outputCallbacksLock };
+    m_outputCallbacks.erase(ssrc);
+}
+
+void LibWebRTCRtpTransformBackend::sendFrameToOutput(std::unique_ptr<webrtc::TransformableFrameInterface>&& rtcFrame)
+{
+    Locker locker { m_outputCallbacksLock };
+    if (m_outputCallbacks.size() == 1) {
+        m_outputCallbacks.begin()->second->OnTransformedFrame(WTFMove(rtcFrame));
         return;
-    Locker locker { m_outputCallbackLock };
-    if (m_outputCallback)
-        m_outputCallback->OnTransformedFrame(WTFMove(rtcFrame));
+    }
+    auto iterator = m_outputCallbacks.find(rtcFrame->GetSsrc());
+    if (iterator != m_outputCallbacks.end())
+        iterator->second->OnTransformedFrame(WTFMove(rtcFrame));
+}
+
+void LibWebRTCRtpTransformBackend::processTransformedFrame(RTCRtpTransformableFrame& frame)
+{
+    if (auto rtcFrame = static_cast<LibWebRTCRtpTransformableFrame&>(frame).takeRTCFrame())
+        sendFrameToOutput(WTFMove(rtcFrame));
 }
 
 void LibWebRTCRtpTransformBackend::Transform(std::unique_ptr<webrtc::TransformableFrameInterface> rtcFrame)
@@ -68,29 +82,27 @@ void LibWebRTCRtpTransformBackend::Transform(std::unique_ptr<webrtc::Transformab
         }
     }
     // In case of no input callback, make the transform a no-op.
-    Locker locker { m_outputCallbackLock };
-    if (m_outputCallback)
-        m_outputCallback->OnTransformedFrame(WTFMove(rtcFrame));
+    sendFrameToOutput(WTFMove(rtcFrame));
 }
 
 void LibWebRTCRtpTransformBackend::RegisterTransformedFrameCallback(rtc::scoped_refptr<webrtc::TransformedFrameCallback> callback)
 {
-    setOutputCallback(WTFMove(callback));
+    addOutputCallback(WTFMove(callback), 0);
 }
 
-void LibWebRTCRtpTransformBackend::RegisterTransformedFrameSinkCallback(rtc::scoped_refptr<webrtc::TransformedFrameCallback> callback, uint32_t /* ssrc */)
+void LibWebRTCRtpTransformBackend::RegisterTransformedFrameSinkCallback(rtc::scoped_refptr<webrtc::TransformedFrameCallback> callback, uint32_t ssrc)
 {
-    RegisterTransformedFrameCallback(WTFMove(callback));
+    addOutputCallback(WTFMove(callback), ssrc);
 }
 
 void LibWebRTCRtpTransformBackend::UnregisterTransformedFrameCallback()
 {
-    setOutputCallback(nullptr);
+    removeOutputCallback(0);
 }
 
-void LibWebRTCRtpTransformBackend::UnregisterTransformedFrameSinkCallback(uint32_t /* ssrc */)
+void LibWebRTCRtpTransformBackend::UnregisterTransformedFrameSinkCallback(uint32_t ssrc)
 {
-    UnregisterTransformedFrameCallback();
+    removeOutputCallback(ssrc);
 }
 
 } // namespace WebCore

diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformBackend.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpTransformBackend.h
@@ -30,6 +30,7 @@
 #include "RTCRtpTransformBackend.h"
 #include <webrtc/api/scoped_refptr.h>
 #include <wtf/Lock.h>
+#include <wtf/StdUnorderedMap.h>
 
 ALLOW_UNUSED_PARAMETERS_BEGIN
 ALLOW_DEPRECATED_DECLARATIONS_BEGIN
@@ -52,7 +53,9 @@ class LibWebRTCRtpTransformBackend : public RTCRtpTransformBackend, public webrt
     MediaType mediaType() const final { return m_mediaType; }
 
 private:
-    void setOutputCallback(rtc::scoped_refptr<webrtc::TransformedFrameCallback>&&);
+    void sendFrameToOutput(std::unique_ptr<webrtc::TransformableFrameInterface>&&);
+    void addOutputCallback(rtc::scoped_refptr<webrtc::TransformedFrameCallback>&&, uint32_t ssrc);
+    void removeOutputCallback(uint32_t ssrc);
 
     // RTCRtpTransformBackend
     void processTransformedFrame(RTCRtpTransformableFrame&) final;
@@ -74,8 +77,8 @@ class LibWebRTCRtpTransformBackend : public RTCRtpTransformBackend, public webrt
     Lock m_inputCallbackLock;
     Callback m_inputCallback WTF_GUARDED_BY_LOCK(m_inputCallbackLock);
 
-    Lock m_outputCallbackLock;
-    rtc::scoped_refptr<webrtc::TransformedFrameCallback> m_outputCallback WTF_GUARDED_BY_LOCK(m_outputCallbackLock);
+    Lock m_outputCallbacksLock;
+    StdUnorderedMap<uint32_t, rtc::scoped_refptr<webrtc::TransformedFrameCallback>> m_outputCallbacks WTF_GUARDED_BY_LOCK(m_outputCallbacksLock);
 };
 
 inline LibWebRTCRtpTransformBackend::LibWebRTCRtpTransformBackend(MediaType mediaType, Side side)