zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Reland "Fix bug where we assume new m= sections will always be bundled.""

Browse Source 
This reverts commit 704a834f685eb96c9fcf891ca345557bef4d138a.

Reason for revert: Reverting this in order to revert
https://webrtc-review.googlesource.com/c/src/+/221601, so we can
merge that revert to M93.

Original change's description:
> Reland "Fix bug where we assume new m= sections will always be bundled."
>
> This is a reland of d2b885fd91909f1b17fb11292a8c989d5d883b22, after
> making sure transports that are just being kept alive in case of
> rollback don't contribute to connection state, which broke a WPT.
>
> Original change's description:
> > Fix bug where we assume new m= sections will always be bundled.
> >
> > A recent change [1] assumes that all new m= sections will share the
> > first BUNDLE group (if one already exists), which avoids generating
> > ICE candidates that are ultimately unnecessary. This is fine for JSEP
> > endpoints, but it breaks the following scenarios for non-JSEP endpoints:
> >
> > * Remote offer adding a new m= section that's not part of any BUNDLE
> >   group.
> > * Remote offer adding an m= section to the second BUNDLE group.
> >
> > The latter is specifically problematic for any application that wants
> > to bundle all audio streams in one group and all video streams in
> > another group when using Unified Plan SDP, to replicate the behavior of
> > using Plan B without bundling. It may try to add a video stream only
> > for WebRTC to bundle it with audio.
> >
> > This is fixed by doing some minor re-factoring, having BundleManager
> > update the bundle groups at offer time.
> >
> > Also:
> > * Added some additional validation for multiple bundle groups in a
> >   subsequent offer, since that now becomes relevant.
> > * Improved rollback support, because now rolling back an offer may need
> >   to not only remove mid->transport mappings but alter them.
> >
> > [1]: https://webrtc-review.googlesource.com/c/src/+/221601
> >
> > Bug: webrtc:12906, webrtc:12999
> > Change-Id: I4c6e7020c0be33a782d3608dee88e4e2fceb1be1
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225642
> > Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#34544}
>
> Bug: webrtc:12906, webrtc:12999
> Change-Id: I68bf988b1918dd2d51de76e53e4fd696fea5a09b
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227120
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#34596}

TBR=hta@webrtc.org

Bug: webrtc:12906, webrtc:12999
Change-Id: I129d9eb3b9831317fa24b0263db191027246cb99
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227821
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34666}
This commit is contained in:
Taylor Brandstetter
2021-08-05 14:02:43 -07:00
committed by WebRTC LUCI CQ
parent 7b78a3142d
commit b92f9856b5
8 changed files with 107 additions and 952 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
pc/peer_connection_bundle_unittest.cc
View File

@ -909,11 +909,11 @@ TEST_F(PeerConnectionBundleTestUnifiedPlan, MultipleBundleGroups) {
EXPECT_TRUE(
caller->SetLocalDescription(CloneSessionDescription(offer.get())));
EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer)));
callee->SetRemoteDescription(std::move(offer));
auto answer = callee->CreateAnswer();
EXPECT_TRUE(
callee->SetLocalDescription(CloneSessionDescription(answer.get())));
EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer)));
caller->SetRemoteDescription(std::move(answer));
// Verify bundling on sender side.
auto senders = caller->pc()->GetSenders();
@ -938,59 +938,4 @@ TEST_F(PeerConnectionBundleTestUnifiedPlan, MultipleBundleGroups) {
EXPECT_NE(receiver0_transport, receiver2_transport);
}
// Test that, with the "max-compat" bundle policy, it's possible to add an m=
// section that's not part of an existing bundle group.
TEST_F(PeerConnectionBundleTestUnifiedPlan, AddNonBundledSection) {
RTCConfiguration config;
config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxCompat;
auto caller = CreatePeerConnection(config);
caller->AddAudioTrack("0_audio");
caller->AddAudioTrack("1_audio");
auto callee = CreatePeerConnection(config);
// Establish an existing BUNDLE group.
auto offer = caller->CreateOffer(RTCOfferAnswerOptions());
EXPECT_TRUE(
caller->SetLocalDescription(CloneSessionDescription(offer.get())));
EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer)));
auto answer = callee->CreateAnswer();
EXPECT_TRUE(
callee->SetLocalDescription(CloneSessionDescription(answer.get())));
EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer)));
// Add a track but munge SDP so it's not part of the bundle group.
caller->AddAudioTrack("3_audio");
offer = caller->CreateOffer(RTCOfferAnswerOptions());
offer->description()->RemoveGroupByName(cricket::GROUP_TYPE_BUNDLE);
cricket::ContentGroup bundle_group(cricket::GROUP_TYPE_BUNDLE);
bundle_group.AddContentName("0");
bundle_group.AddContentName("1");
offer->description()->AddGroup(bundle_group);
EXPECT_TRUE(
caller->SetLocalDescription(CloneSessionDescription(offer.get())));
EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer)));
answer = callee->CreateAnswer();
EXPECT_TRUE(
callee->SetLocalDescription(CloneSessionDescription(answer.get())));
EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer)));
// Verify bundling on the sender side.
auto senders = caller->pc()->GetSenders();
ASSERT_EQ(senders.size(), 3u);
auto sender0_transport = senders[0]->dtls_transport();
auto sender1_transport = senders[1]->dtls_transport();
auto sender2_transport = senders[2]->dtls_transport();
EXPECT_EQ(sender0_transport, sender1_transport);
EXPECT_NE(sender0_transport, sender2_transport);
// Verify bundling on receiver side.
auto receivers = callee->pc()->GetReceivers();
ASSERT_EQ(receivers.size(), 3u);
auto receiver0_transport = receivers[0]->dtls_transport();
auto receiver1_transport = receivers[1]->dtls_transport();
auto receiver2_transport = receivers[2]->dtls_transport();
EXPECT_EQ(receiver0_transport, receiver1_transport);
EXPECT_NE(receiver0_transport, receiver2_transport);
}
} // namespace webrtc