Attachment 357363 Details for Bug 192732 – Patch

[patch] Patch

bug-192732-20181214175810.patch (text/plain), 18.03 KB, created by youenn fablet on 2018-12-14 17:58:02 PST

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-12-14 17:58:02 PST

Size: 18.03 KB

patch

obsolete

>Subversion Revision: 239205
>diff --git a/Source/ThirdParty/libwebrtc/ChangeLog b/Source/ThirdParty/libwebrtc/ChangeLog
>index 75accfafdab88a038d109cc70c710fc71a5ccff1..e0a60a591ed598af17bdee5614b0e76e3309add7 100644
>--- a/Source/ThirdParty/libwebrtc/ChangeLog
>+++ b/Source/ThirdParty/libwebrtc/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-12-14  Youenn Fablet  <youenn@apple.com>
>+
>+        Make RTCRtpSender.setParameters to activate specific encodings
>+        https://bugs.webkit.org/show_bug.cgi?id=192732
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * Configurations/libwebrtc.iOS.exp:
>+        * Configurations/libwebrtc.iOSsim.exp:
>+        * Configurations/libwebrtc.mac.exp:
>+
> 2018-12-14  Youenn Fablet  <youenn@apple.com>
> 
>         Set kVTVideoEncoderSpecification_Usage both when creating the compression session and once created
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f63519b5014d2695af8e0878c6f6d780b4d0308d..3234015a4681eace33a2a19ed1b5d3a954fa0738 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-12-14  Youenn Fablet  <youenn@apple.com>
>+
>+        Make RTCRtpSender.setParameters to activate specific encodings
>+        https://bugs.webkit.org/show_bug.cgi?id=192732
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The conversion from libwebrtc to 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 and reuses it 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  Youenn Fablet  <youenn@apple.com>
> 
>         getSenders/getReceivers() should not return closed transceiver senders/receivers
>diff --git a/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOS.exp b/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOS.exp
>index 62286edc552daadd29d3143738507175564deeb8..a3be5647403f4e6e2f25b0ea639a662047ea91b4 100644
>--- a/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOS.exp
>+++ b/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOS.exp
>@@ -238,3 +238,11 @@ __ZNK6webrtc20AudioSourceInterface7optionsEv
> __ZTVN6webrtc20AudioSourceInterfaceE
> __ZN6webrtc23PeerConnectionInterface21peer_connection_stateEv
> __ZNK3rtc14NetworkManager16GetMdnsResponderEv
>+__ZN6webrtc18RtpCodecParametersC1ERKS0_
>+__ZN6webrtc18RtpCodecParametersC1Ev
>+__ZN6webrtc18RtpCodecParametersD1Ev
>+__ZN6webrtc13RtpParametersC1ERKS0_
>+__ZN6webrtc16RtpFecParametersC1ERKS0_
>+__ZN6webrtc16RtpFecParametersD1Ev
>+__ZN6webrtc16RtpRtxParametersC1ERKS0_
>+__ZN6webrtc16RtpRtxParametersD1Ev
>diff --git a/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOSsim.exp b/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOSsim.exp
>index e3126bf06fd9870aa9f3f99c4753ce1c80138c85..666262a5090189bc9f90558eed92b8bfed1d6859 100644
>--- a/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOSsim.exp
>+++ b/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOSsim.exp
>@@ -239,3 +239,11 @@ __ZNK6webrtc20AudioSourceInterface7optionsEv
> __ZTVN6webrtc20AudioSourceInterfaceE
> __ZN6webrtc23PeerConnectionInterface21peer_connection_stateEv
> __ZNK3rtc14NetworkManager16GetMdnsResponderEv
>+__ZN6webrtc18RtpCodecParametersC1ERKS0_
>+__ZN6webrtc18RtpCodecParametersC1Ev
>+__ZN6webrtc18RtpCodecParametersD1Ev
>+__ZN6webrtc13RtpParametersC1ERKS0_
>+__ZN6webrtc16RtpFecParametersC1ERKS0_
>+__ZN6webrtc16RtpFecParametersD1Ev
>+__ZN6webrtc16RtpRtxParametersC1ERKS0_
>+__ZN6webrtc16RtpRtxParametersD1Ev
>diff --git a/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.mac.exp b/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.mac.exp
>index e3126bf06fd9870aa9f3f99c4753ce1c80138c85..666262a5090189bc9f90558eed92b8bfed1d6859 100644
>--- a/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.mac.exp
>+++ b/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.mac.exp
>@@ -239,3 +239,11 @@ __ZNK6webrtc20AudioSourceInterface7optionsEv
> __ZTVN6webrtc20AudioSourceInterfaceE
> __ZN6webrtc23PeerConnectionInterface21peer_connection_stateEv
> __ZNK3rtc14NetworkManager16GetMdnsResponderEv
>+__ZN6webrtc18RtpCodecParametersC1ERKS0_
>+__ZN6webrtc18RtpCodecParametersC1Ev
>+__ZN6webrtc18RtpCodecParametersD1Ev
>+__ZN6webrtc13RtpParametersC1ERKS0_
>+__ZN6webrtc16RtpFecParametersC1ERKS0_
>+__ZN6webrtc16RtpFecParametersD1Ev
>+__ZN6webrtc16RtpRtxParametersC1ERKS0_
>+__ZN6webrtc16RtpRtxParametersD1Ev
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp
>index 0f7d2b5963777a40b0366b676cabc57036b3a230..f7b78510e54341be966ece668acca9495877389f 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.cpp
>@@ -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,13 @@ 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 error = m_rtcSender->SetParameters(fromRTCRtpSendParameters(parameters, *m_currentParameters));
>     if (!error.ok()) {
>         promise.reject(Exception { InvalidStateError, error.message() });
>         return;
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h
>index ad568d0d4b1ded6d5bf5a3d33b3302474567e6da..ee615a40c4fb398c6b04310d8c1ce61e6818a166 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCRtpSenderBackend.h
>@@ -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
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp
>index e812777f12752068eea223bfb82d45efb2077bf5..4033e4c1a941fd46569a43781f17de7ae15b119c 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.cpp
>@@ -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,26 @@ RTCRtpSendParameters toRTCRtpSendParameters(const webrtc::RtpParameters& rtcPara
>     return parameters;
> }
> 
>-webrtc::RtpParameters fromRTCRtpSendParameters(const RTCRtpSendParameters& parameters)
>+webrtc::RtpParameters fromRTCRtpSendParameters(const RTCRtpSendParameters& parameters, const 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 encodings size is different, setting parameters will fail. Let's make it so.
>+    if (parameters.encodings.size() != rtcParameters.encodings.size())
>+        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
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.h
>index 8436747de30cc1ae674bc4a19b464a64417dfaa1..0cbff3004c2ee44c6248ffc3bf4f9d293378d1dc 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.h
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCUtils.h
>@@ -46,7 +46,7 @@ enum class RTCRtpTransceiverDirection;
> RTCRtpParameters toRTCRtpParameters(const webrtc::RtpParameters&);
> webrtc::RtpParameters fromRTCRtpParameters(const RTCRtpParameters&);
> 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);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index dc4e0419abbed4699a6b81a2774f6f0a2f77a9e5..455a78184d3dc81321f80ff20df9a6d260f9f5ea 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-12-14  Youenn Fablet  <youenn@apple.com>
>+
>+        Make RTCRtpSender.setParameters to activate specific encodings
>+        https://bugs.webkit.org/show_bug.cgi?id=192732
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * webrtc/video-expected.txt:
>+        * webrtc/video.html:
>+
> 2018-12-13  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         Make HTMLConverter work across shadow boundaries
>diff --git a/LayoutTests/webrtc/video-expected.txt b/LayoutTests/webrtc/video-expected.txt
>index ce31e195190de2fc5c88483bfdbf05fbffd4d9ca..1c5554d4ec16ed1f4d41900928916dc0565494c4 100644
>--- a/LayoutTests/webrtc/video-expected.txt
>+++ b/LayoutTests/webrtc/video-expected.txt
>@@ -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 
> 
>diff --git a/LayoutTests/webrtc/video.html b/LayoutTests/webrtc/video.html
>index 4a7ca11b7957ba6c444e5ea8a2fedaac3bc317e1..f75a46028a19381605646cdb0277fd279367a400 100644
>--- a/LayoutTests/webrtc/video.html
>+++ b/LayoutTests/webrtc/video.html
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 192732: 357363 | 357369