Make RTCRtpSender.setParameters to activate specific encodings
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Dec 2018 17:40:26 +0000 (17:40 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Dec 2018 17:40:26 +0000 (17:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192732

Reviewed by Eric Carlson.

Source/ThirdParty/libwebrtc:

* Configurations/libwebrtc.iOS.exp:
* Configurations/libwebrtc.iOSsim.exp:
* Configurations/libwebrtc.mac.exp:

Source/WebCore:

The conversion between libwebrtc and WebCore is lossy for send parameters.
Libwebrtc checking the differences of values, call to setParameters will often fail.

Given some parameters cannot be exposed, the sender backend keeps the
current set of parameters when gathered and reuses them when parameters are set.

For encodings, we only change activate/maxBitRate/maxFrameRate as
these are the most important parameters to be able to modify.

Covered by added tests in webrtc/video.html.

* Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp:
(WebCore::LibWebRTCRtpSenderBackend::getParameters const):
(WebCore::LibWebRTCRtpSenderBackend::setParameters):
* Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h:
* Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp:
(WebCore::fromRTCRtpSendParameters):
(WebCore::fromRTCEncodingParameters): Deleted.
* Modules/mediastream/libwebrtc/LibWebRTCUtils.h:

LayoutTests:

* webrtc/video-expected.txt:
* webrtc/video.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239252 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/webrtc/video-expected.txt
LayoutTests/webrtc/video.html
Source/ThirdParty/libwebrtc/ChangeLog
Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOS.exp
Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOSsim.exp
Source/ThirdParty/libwebrtc/Configurations/libwebrtc.mac.exp
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.h

index cc8b9a8..28d445a 100644 (file)
@@ -1,3 +1,13 @@
+2018-12-15  Youenn Fablet  <youenn@apple.com>
+
+        Make RTCRtpSender.setParameters to activate specific encodings
+        https://bugs.webkit.org/show_bug.cgi?id=192732
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/video-expected.txt:
+        * webrtc/video.html:
+
 2018-12-15  Nikita Vasilyev  <nvasilyev@apple.com>
 
         Web Inspector: Styles: toggling selected properties may cause data corruption
index ce31e19..1c5554d 100644 (file)
@@ -1,4 +1,6 @@
 
 
 PASS Basic video exchange 
+PASS Call setParameters to disable sending a given encoding 
+PASS Call setParameters to reenable sending a given encoding 
 
index 4a7ca11..f75a460 100644 (file)
@@ -39,37 +39,103 @@ function testImage()
     assert_true(data[index + 2] < 100);
 }
 
-promise_test((test) => {
+var pc1, pc2;
+promise_test(async (test) => {
     if (window.testRunner)
         testRunner.setUserMediaPermission(true);
 
-    return navigator.mediaDevices.getUserMedia({video: {advanced: [{width:{min:640}}, {height:{min:480} } ]}}).then((stream) => {
-        if (window.internals)
-            assert_true(internals.pageMediaState().includes('HasActiveVideoCaptureDevice'), "Unexpected HasActiveVideoCaptureDevice");
-        return new Promise((resolve, reject) => {
-            createConnections((firstConnection) => {
-                var track = stream.getVideoTracks()[0];
-                firstConnection.addTrack(stream.getVideoTracks()[0], stream);
-            }, (secondConnection) => {
-                secondConnection.ontrack = (trackEvent) => {
-                    assert_true(trackEvent.track instanceof MediaStreamTrack);
-                    assert_true(trackEvent.receiver instanceof RTCRtpReceiver);
-                    assert_true(Array.isArray(trackEvent.streams), "Array.isArray() should return true");
-                    assert_true(Object.isFrozen(trackEvent.streams), "Object.isFrozen() should return true");
-                    assert_equals(trackEvent.track.id, stream.getVideoTracks()[0].id);
-                    assert_equals(trackEvent.track, trackEvent.streams[0].getVideoTracks()[0]);
-                    resolve(trackEvent.streams[0]);
-                };
-            });
-            setTimeout(() => reject("Test timed out"), 5000);
+    const localStream = await navigator.mediaDevices.getUserMedia({video: {advanced: [{width:{min:640}}, {height:{min:480} } ]}});
+    if (window.internals)
+        assert_true(internals.pageMediaState().includes('HasActiveVideoCaptureDevice'), "Unexpected HasActiveVideoCaptureDevice");
+    const stream = await new Promise((resolve, reject) => {
+        createConnections((firstConnection) => {
+            pc1 = firstConnection;
+            firstConnection.addTrack(localStream.getVideoTracks()[0], localStream);
+        }, (secondConnection) => {
+            pc2 = secondConnection;
+            secondConnection.ontrack = (trackEvent) => {
+                assert_true(trackEvent.track instanceof MediaStreamTrack);
+                assert_true(trackEvent.receiver instanceof RTCRtpReceiver);
+                assert_true(Array.isArray(trackEvent.streams), "Array.isArray() should return true");
+                assert_true(Object.isFrozen(trackEvent.streams), "Object.isFrozen() should return true");
+                assert_equals(trackEvent.track.id, localStream.getVideoTracks()[0].id);
+                assert_equals(trackEvent.track, trackEvent.streams[0].getVideoTracks()[0]);
+                resolve(trackEvent.streams[0]);
+            };
         });
-    }).then((stream) => {
-        video.srcObject = stream;
-        return video.play();
-    }).then(test.step_func(() => {
-        testImage();
-    }));
+        setTimeout(() => reject("Test timed out"), 5000);
+    });
+
+    video.srcObject = stream;
+    await video.play();
+
+    testImage();
 }, "Basic video exchange");
+
+async function getInboundRTPStatsNumberOfDecodedFrames(connection)
+{
+    var report = await connection.getStats();
+    var framesDecoded;
+    report.forEach((statItem) => {
+        if (statItem.type === "inbound-rtp")
+            framesDecoded = statItem.framesDecoded;
+    });
+    return framesDecoded;
+}
+
+async function testFrameDecodedIncreased(connection, count, previousFramesNumber)
+{
+    if (previousFramesNumber === undefined) {
+        let number = await getInboundRTPStatsNumberOfDecodedFrames(connection);
+        await waitFor(1000);
+        return testFrameDecodedIncreased(connection, 0, number);
+    }
+
+    var number = await getInboundRTPStatsNumberOfDecodedFrames(connection);
+    if (previousFramesNumber && number > previousFramesNumber)
+        return;
+
+    if (count >= 20)
+        return Promise.reject("test increasing frame encoded timed out");
+
+    await waitFor(1000);
+    return testFrameDecodedIncreased(connection, ++count, previousFramesNumber);
+}
+
+async function testFrameDecodedDidNotIncreased(connection, count, previousFramesNumber)
+{
+    if (previousFramesNumber === undefined) {
+        let number = await getInboundRTPStatsNumberOfDecodedFrames(connection);
+        await waitFor(100);
+        return testFrameDecodedDidNotIncreased(connection, 0, number);
+    }
+
+    var number = await getInboundRTPStatsNumberOfDecodedFrames(connection);
+    if (previousFramesNumber && number == previousFramesNumber)
+        return;
+
+    if (count >= 20)
+        return Promise.reject("test increasing frame encoded timed out");
+
+    await waitFor(100);
+    return testFrameDecodedIncreased(connection, ++count, number);
+}
+
+promise_test(async (test) => {
+   let p = pc1.getSenders()[0].getParameters();
+   p.encodings[0].active = false;
+   await pc1.getSenders()[0].setParameters(p);
+
+   await testFrameDecodedDidNotIncreased(pc2);
+}, "Call setParameters to disable sending a given encoding");
+
+promise_test(async (test) => {
+   let p = pc1.getSenders()[0].getParameters();
+   p.encodings[0].active = true;
+   await pc1.getSenders()[0].setParameters(p);
+
+   await testFrameDecodedIncreased(pc2);
+}, "Call setParameters to reenable sending a given encoding");
         </script>
     </body>
 </html>
index 7ac3e1a..6e71086 100644 (file)
@@ -1,3 +1,14 @@
+2018-12-15  Youenn Fablet  <youenn@apple.com>
+
+        Make RTCRtpSender.setParameters to activate specific encodings
+        https://bugs.webkit.org/show_bug.cgi?id=192732
+
+        Reviewed by Eric Carlson.
+
+        * Configurations/libwebrtc.iOS.exp:
+        * Configurations/libwebrtc.iOSsim.exp:
+        * Configurations/libwebrtc.mac.exp:
+
 2018-12-14  Youenn Fablet  <youenn@apple.com>
 
         kVTVideoEncoderSpecification_Usage should not be set if VCP is not enabled
index 62286ed..a3be564 100644 (file)
@@ -238,3 +238,11 @@ __ZNK6webrtc20AudioSourceInterface7optionsEv
 __ZTVN6webrtc20AudioSourceInterfaceE
 __ZN6webrtc23PeerConnectionInterface21peer_connection_stateEv
 __ZNK3rtc14NetworkManager16GetMdnsResponderEv
+__ZN6webrtc18RtpCodecParametersC1ERKS0_
+__ZN6webrtc18RtpCodecParametersC1Ev
+__ZN6webrtc18RtpCodecParametersD1Ev
+__ZN6webrtc13RtpParametersC1ERKS0_
+__ZN6webrtc16RtpFecParametersC1ERKS0_
+__ZN6webrtc16RtpFecParametersD1Ev
+__ZN6webrtc16RtpRtxParametersC1ERKS0_
+__ZN6webrtc16RtpRtxParametersD1Ev
index e3126bf..666262a 100644 (file)
@@ -239,3 +239,11 @@ __ZNK6webrtc20AudioSourceInterface7optionsEv
 __ZTVN6webrtc20AudioSourceInterfaceE
 __ZN6webrtc23PeerConnectionInterface21peer_connection_stateEv
 __ZNK3rtc14NetworkManager16GetMdnsResponderEv
+__ZN6webrtc18RtpCodecParametersC1ERKS0_
+__ZN6webrtc18RtpCodecParametersC1Ev
+__ZN6webrtc18RtpCodecParametersD1Ev
+__ZN6webrtc13RtpParametersC1ERKS0_
+__ZN6webrtc16RtpFecParametersC1ERKS0_
+__ZN6webrtc16RtpFecParametersD1Ev
+__ZN6webrtc16RtpRtxParametersC1ERKS0_
+__ZN6webrtc16RtpRtxParametersD1Ev
index e3126bf..666262a 100644 (file)
@@ -239,3 +239,11 @@ __ZNK6webrtc20AudioSourceInterface7optionsEv
 __ZTVN6webrtc20AudioSourceInterfaceE
 __ZN6webrtc23PeerConnectionInterface21peer_connection_stateEv
 __ZNK3rtc14NetworkManager16GetMdnsResponderEv
+__ZN6webrtc18RtpCodecParametersC1ERKS0_
+__ZN6webrtc18RtpCodecParametersC1Ev
+__ZN6webrtc18RtpCodecParametersD1Ev
+__ZN6webrtc13RtpParametersC1ERKS0_
+__ZN6webrtc16RtpFecParametersC1ERKS0_
+__ZN6webrtc16RtpFecParametersD1Ev
+__ZN6webrtc16RtpRtxParametersC1ERKS0_
+__ZN6webrtc16RtpRtxParametersD1Ev
index ed2d073..f83f35a 100644 (file)
@@ -1,3 +1,30 @@
+2018-12-15  Youenn Fablet  <youenn@apple.com>
+
+        Make RTCRtpSender.setParameters to activate specific encodings
+        https://bugs.webkit.org/show_bug.cgi?id=192732
+
+        Reviewed by Eric Carlson.
+
+        The conversion between libwebrtc and WebCore is lossy for send parameters.
+        Libwebrtc checking the differences of values, call to setParameters will often fail.
+
+        Given some parameters cannot be exposed, the sender backend keeps the
+        current set of parameters when gathered and reuses them when parameters are set.
+
+        For encodings, we only change activate/maxBitRate/maxFrameRate as
+        these are the most important parameters to be able to modify.
+
+        Covered by added tests in webrtc/video.html.
+
+        * Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp:
+        (WebCore::LibWebRTCRtpSenderBackend::getParameters const):
+        (WebCore::LibWebRTCRtpSenderBackend::setParameters):
+        * Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h:
+        * Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp:
+        (WebCore::fromRTCRtpSendParameters):
+        (WebCore::fromRTCEncodingParameters): Deleted.
+        * Modules/mediastream/libwebrtc/LibWebRTCUtils.h:
+
 2018-12-14  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Avoid creating and evaluating in the InspectorOverlay page on iOS as it is unused
index 0f7d2b5..2c1c015 100644 (file)
@@ -117,7 +117,8 @@ RTCRtpSendParameters LibWebRTCRtpSenderBackend::getParameters() const
     if (!m_rtcSender)
         return { };
 
-    return toRTCRtpSendParameters(m_rtcSender->GetParameters());
+    m_currentParameters = m_rtcSender->GetParameters();
+    return toRTCRtpSendParameters(*m_currentParameters);
 }
 
 void LibWebRTCRtpSenderBackend::setParameters(const RTCRtpSendParameters& parameters, DOMPromiseDeferred<void>&& promise)
@@ -126,7 +127,17 @@ void LibWebRTCRtpSenderBackend::setParameters(const RTCRtpSendParameters& parame
         promise.reject(NotSupportedError);
         return;
     }
-    auto error = m_rtcSender->SetParameters(fromRTCRtpSendParameters(parameters));
+
+    if (!m_currentParameters) {
+        promise.reject(Exception { InvalidStateError, "getParameters must be called before setParameters"_s });
+        return;
+    }
+
+    auto rtcParameters = WTFMove(*m_currentParameters);
+    updateRTCRtpSendParameters(parameters, rtcParameters);
+    m_currentParameters = std::nullopt;
+
+    auto error = m_rtcSender->SetParameters(rtcParameters);
     if (!error.ok()) {
         promise.reject(Exception { InvalidStateError, error.message() });
         return;
index ad568d0..ee615a4 100644 (file)
@@ -114,6 +114,7 @@ private:
     WeakPtr<LibWebRTCPeerConnectionBackend> m_peerConnectionBackend;
     rtc::scoped_refptr<webrtc::RtpSenderInterface> m_rtcSender;
     Source m_source;
+    mutable std::optional<webrtc::RtpParameters> m_currentParameters;
 };
 
 } // namespace WebCore
index e812777..ddbb5f0 100644 (file)
@@ -72,31 +72,6 @@ static inline RTCRtpEncodingParameters toRTCEncodingParameters(const webrtc::Rtp
     return parameters;
 }
 
-static inline webrtc::RtpEncodingParameters fromRTCEncodingParameters(const RTCRtpEncodingParameters& parameters)
-{
-    webrtc::RtpEncodingParameters rtcParameters;
-
-    if (parameters.dtx) {
-        switch (*parameters.dtx) {
-        case RTCDtxStatus::Disabled:
-            rtcParameters.dtx = webrtc::DtxStatus::DISABLED;
-            break;
-        case RTCDtxStatus::Enabled:
-            rtcParameters.dtx = webrtc::DtxStatus::ENABLED;
-        }
-    }
-    rtcParameters.active = parameters.active;
-    if (parameters.maxBitrate)
-        rtcParameters.max_bitrate_bps = parameters.maxBitrate;
-    if (parameters.maxFramerate)
-        rtcParameters.max_framerate = parameters.maxFramerate;
-    rtcParameters.rid = parameters.rid.utf8().data();
-    if (parameters.scaleResolutionDownBy != 1)
-        rtcParameters.scale_resolution_down_by = parameters.scaleResolutionDownBy;
-
-    return rtcParameters;
-}
-
 static inline RTCRtpHeaderExtensionParameters toRTCHeaderExtensionParameters(const webrtc::RtpHeaderExtensionParameters& rtcParameters)
 {
     RTCRtpHeaderExtensionParameters parameters;
@@ -185,13 +160,28 @@ RTCRtpSendParameters toRTCRtpSendParameters(const webrtc::RtpParameters& rtcPara
     return parameters;
 }
 
-webrtc::RtpParameters fromRTCRtpSendParameters(const RTCRtpSendParameters& parameters)
+void updateRTCRtpSendParameters(const RTCRtpSendParameters& parameters, webrtc::RtpParameters& currentParameters)
 {
-    webrtc::RtpParameters rtcParameters;
+    webrtc::RtpParameters rtcParameters = currentParameters;
+
     rtcParameters.transaction_id = parameters.transactionId.utf8().data();
 
-    for (auto& encoding : parameters.encodings)
-        rtcParameters.encodings.push_back(fromRTCEncodingParameters(encoding));
+    if (parameters.encodings.size() != rtcParameters.encodings.size()) {
+        // If encodings size is different, setting parameters will fail. Let's make it so.
+        rtcParameters.encodings.clear();
+        return;
+    }
+
+    // We copy all current encodings parameters and only update parameters that can actually be usefully updated.
+    for (size_t i = 0; i < parameters.encodings.size(); ++i) {
+        rtcParameters.encodings[i].active = parameters.encodings[i].active;
+        if (parameters.encodings[i].maxBitrate)
+            rtcParameters.encodings[i].max_bitrate_bps = parameters.encodings[i].maxBitrate;
+        if (parameters.encodings[i].maxFramerate)
+            rtcParameters.encodings[i].max_framerate = parameters.encodings[i].maxFramerate;
+    }
+
+    rtcParameters.header_extensions.clear();
     for (auto& extension : parameters.headerExtensions)
         rtcParameters.header_extensions.push_back(fromRTCHeaderExtensionParameters(extension));
     // Codecs parameters are readonly
@@ -207,7 +197,6 @@ webrtc::RtpParameters fromRTCRtpSendParameters(const RTCRtpSendParameters& param
         rtcParameters.degradation_preference = webrtc::DegradationPreference::BALANCED;
         break;
     }
-    return rtcParameters;
 }
 
 RTCRtpTransceiverDirection toRTCRtpTransceiverDirection(webrtc::RtpTransceiverDirection rtcDirection)
index 8436747..b449aeb 100644 (file)
@@ -44,9 +44,9 @@ struct RTCRtpTransceiverInit;
 enum class RTCRtpTransceiverDirection;
 
 RTCRtpParameters toRTCRtpParameters(const webrtc::RtpParameters&);
-webrtc::RtpParameters fromRTCRtpParameters(const RTCRtpParameters&);
+void updateRTCRtpSendParameters(const RTCRtpSendParameters&, webrtc::RtpParameters&);
 RTCRtpSendParameters toRTCRtpSendParameters(const webrtc::RtpParameters&);
-webrtc::RtpParameters fromRTCRtpSendParameters(const RTCRtpSendParameters&);
+webrtc::RtpParameters fromRTCRtpSendParameters(const RTCRtpSendParameters&, const webrtc::RtpParameters& currentParameters);
 
 RTCRtpTransceiverDirection toRTCRtpTransceiverDirection(webrtc::RtpTransceiverDirection);
 webrtc::RtpTransceiverDirection fromRTCRtpTransceiverDirection(RTCRtpTransceiverDirection);