Revert "Modify Bundle logic to not add & destroy extra transport at add-track"

This reverts commit I41cae74605fecf454900a958776b95607ccf3745

Reason for revert: Needed in order to cherry pick this revert into M93,
in order to fix crbug.com/1236202.

Original change description:

> If a bundle is established, then per JSEP, the offerer is required to
> include the new track in the bundle, and per BUNDLE, the answerer has
> to either accept the track as part of the bundle or reject the track;
> it cannot move it out of the group. So we will never need the transport.
>
> Bug: webrtc:12837
> Change-Id: I41cae74605fecf454900a958776b95607ccf3745
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221601
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#34290}

TBR=hta@webrtc.org

Bug: webrtc:12837, chromium:1236202
Change-Id: Ie59e2ad5168e6829eefa67b1031b8f400ed66507
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227822
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34669}
This commit is contained in:
Taylor Brandstetter
2021-08-06 12:58:10 -07:00
committed by WebRTC LUCI CQ
parent 99fb5945b9
commit a03d494224
5 changed files with 5 additions and 55 deletions

View File

@ -551,17 +551,6 @@ RTCError JsepTransportController::ApplyDescription_n(
MergeEncryptedHeaderExtensionIdsForBundles(description);
}
// Because the creation of transports depends on whether
// certain mids are present, we have to process rejection
// before we try to create transports.
for (size_t i = 0; i < description->contents().size(); ++i) {
const cricket::ContentInfo& content_info = description->contents()[i];
if (content_info.rejected) {
// This may cause groups to be removed from `bundles_.bundle_groups()`.
HandleRejectedContent(content_info);
}
}
for (const cricket::ContentInfo& content_info : description->contents()) {
// Don't create transports for rejected m-lines and bundled m-lines.
if (content_info.rejected ||
@ -580,8 +569,9 @@ RTCError JsepTransportController::ApplyDescription_n(
const cricket::ContentInfo& content_info = description->contents()[i];
const cricket::TransportInfo& transport_info =
description->transport_infos()[i];
if (content_info.rejected) {
// This may cause groups to be removed from `bundles_.bundle_groups()`.
HandleRejectedContent(content_info);
continue;
}
@ -987,21 +977,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
if (transport) {
return RTCError::OK();
}
// If we have agreed to a bundle, the new mid will be added to the bundle
// according to JSEP, and the responder can't move it out of the group
// according to BUNDLE. So don't create a transport.
// The MID will be added to the bundle elsewhere in the code.
if (bundles_.bundle_groups().size() > 0) {
const auto& default_bundle_group = bundles_.bundle_groups()[0];
if (default_bundle_group->content_names().size() > 0) {
auto bundle_transport =
GetJsepTransportByName(default_bundle_group->content_names()[0]);
if (bundle_transport) {
transports_.SetTransportForMid(content_info.name, bundle_transport);
return RTCError::OK();
}
}
}
const cricket::MediaContentDescription* content_desc =
content_info.media_description();
if (certificate_ && !content_desc->cryptos().empty()) {

View File

@ -2051,6 +2051,7 @@ TEST_F(JsepTransportControllerTest, ChangeTaggedMediaSectionMaxBundle) {
EXPECT_TRUE(transport_controller_
->SetLocalDescription(SdpType::kOffer, local_reoffer.get())
.ok());
std::unique_ptr<cricket::SessionDescription> remote_reanswer(
local_reoffer->Clone());
EXPECT_TRUE(

View File

@ -3639,20 +3639,6 @@ TEST_P(PeerConnectionIntegrationInteropTest,
ASSERT_TRUE(ExpectNewFrames(media_expectations));
}
TEST_P(PeerConnectionIntegrationTest, NewTracksDoNotCauseNewCandidates) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
caller()->AddAudioVideoTracks();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
ASSERT_TRUE_WAIT(DtlsConnected(), kDefaultTimeout);
caller()->ExpectCandidates(0);
callee()->ExpectCandidates(0);
caller()->AddAudioTrack();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
}
INSTANTIATE_TEST_SUITE_P(
PeerConnectionIntegrationTest,
PeerConnectionIntegrationInteropTest,

View File

@ -91,8 +91,7 @@ bool RtcpMuxFilter::SetAnswer(bool answer_enable, ContentSource src) {
}
if (!ExpectAnswer(src)) {
RTC_LOG(LS_ERROR) << "Invalid state for RTCP mux answer, state is "
<< state_ << ", source is " << src;
RTC_LOG(LS_ERROR) << "Invalid state for RTCP mux answer";
return false;
}

View File

@ -17,7 +17,6 @@
#include <algorithm>
#include <functional>
#include <limits>
#include <list>
#include <map>
#include <memory>
@ -705,11 +704,6 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
audio_concealed_stat_ = *track_stats->concealed_samples;
}
// Sets number of candidates expected
void ExpectCandidates(int candidate_count) {
candidates_expected_ = candidate_count;
}
private:
explicit PeerConnectionIntegrationWrapper(const std::string& debug_name)
: debug_name_(debug_name) {}
@ -1095,9 +1089,6 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
}
}
// Check if we expected to have a candidate.
EXPECT_GT(candidates_expected_, 1);
candidates_expected_--;
std::string ice_sdp;
EXPECT_TRUE(candidate->ToString(&ice_sdp));
if (signaling_message_receiver_ == nullptr || !signal_ice_candidates_) {
@ -1181,9 +1172,6 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
peer_connection_signaling_state_history_;
webrtc::FakeRtcEventLogFactory* event_log_factory_;
// Number of ICE candidates expected. The default is no limit.
int candidates_expected_ = std::numeric_limits<int>::max();
// Variables for tracking delay stats on an audio track
int audio_packets_stat_ = 0;
double audio_delay_stat_ = 0.0;