Revert "Fix bug where we assume new m= sections will always be bundled."
This reverts commit d2b885fd91909f1b17fb11292a8c989d5d883b22.
Reason for revert: Speculative revert for Chromium importer
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}
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:12906, webrtc:12999
Change-Id: I00179d7573f322ad539ff16cad1f85320cfb2270
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227081
Reviewed-by: Björn Terelius <terelius@google.com>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34578}
diff --git a/pc/jsep_transport_collection.cc b/pc/jsep_transport_collection.cc
index 98b8cd2..ce068d9 100644
--- a/pc/jsep_transport_collection.cc
+++ b/pc/jsep_transport_collection.cc
@@ -20,53 +20,21 @@
namespace webrtc {
-void BundleManager::Update(const cricket::SessionDescription* description,
- SdpType type) {
+void BundleManager::Update(const cricket::SessionDescription* description) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
- // Rollbacks should call Rollback, not Update.
- RTC_DCHECK(type != SdpType::kRollback);
- bool bundle_groups_changed = false;
- // TODO(bugs.webrtc.org/3349): Do this for kPrAnswer as well. To make this
- // work, we also need to make sure PRANSWERs don't call
- // MaybeDestroyJsepTransport, because the final answer may need the destroyed
- // transport if it changes the BUNDLE group.
- if (bundle_policy_ == PeerConnectionInterface::kBundlePolicyMaxBundle ||
- type == SdpType::kAnswer) {
- // If our policy is "max-bundle" or this is an answer, update all bundle
- // groups.
- bundle_groups_changed = true;
- bundle_groups_.clear();
- for (const cricket::ContentGroup* new_bundle_group :
- description->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE)) {
- bundle_groups_.push_back(
- std::make_unique<cricket::ContentGroup>(*new_bundle_group));
- RTC_DLOG(LS_VERBOSE) << "Establishing bundle group "
- << new_bundle_group->ToString();
- }
- } else if (type == SdpType::kOffer) {
- // If this is an offer, update existing bundle groups.
- // We do this because as per RFC 8843, section 7.3.2, the answerer cannot
- // remove an m= section from an existing BUNDLE group without rejecting it.
- // Thus any m= sections added to a BUNDLE group in this offer can
- // preemptively start using the bundled transport, as there is no possible
- // non-bundled fallback.
- for (const cricket::ContentGroup* new_bundle_group :
- description->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE)) {
- // Attempt to find a matching existing group.
- for (const std::string& mid : new_bundle_group->content_names()) {
- auto it = established_bundle_groups_by_mid_.find(mid);
- if (it != established_bundle_groups_by_mid_.end()) {
- *it->second = *new_bundle_group;
- bundle_groups_changed = true;
- RTC_DLOG(LS_VERBOSE)
- << "Establishing bundle group " << new_bundle_group->ToString();
- break;
- }
- }
- }
+ bundle_groups_.clear();
+ for (const cricket::ContentGroup* new_bundle_group :
+ description->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE)) {
+ bundle_groups_.push_back(
+ std::make_unique<cricket::ContentGroup>(*new_bundle_group));
+ RTC_DLOG(LS_VERBOSE) << "Establishing bundle group "
+ << new_bundle_group->ToString();
}
- if (bundle_groups_changed) {
- RefreshEstablishedBundleGroupsByMid();
+ established_bundle_groups_by_mid_.clear();
+ for (const auto& bundle_group : bundle_groups_) {
+ for (const std::string& content_name : bundle_group->content_names()) {
+ established_bundle_groups_by_mid_[content_name] = bundle_group.get();
+ }
}
}
@@ -124,34 +92,6 @@
bundle_groups_.erase(bundle_group_it);
}
-void BundleManager::Rollback() {
- RTC_DCHECK_RUN_ON(&sequence_checker_);
- bundle_groups_.clear();
- for (const auto& bundle_group : stable_bundle_groups_) {
- bundle_groups_.push_back(
- std::make_unique<cricket::ContentGroup>(*bundle_group));
- }
- RefreshEstablishedBundleGroupsByMid();
-}
-
-void BundleManager::Commit() {
- RTC_DCHECK_RUN_ON(&sequence_checker_);
- stable_bundle_groups_.clear();
- for (const auto& bundle_group : bundle_groups_) {
- stable_bundle_groups_.push_back(
- std::make_unique<cricket::ContentGroup>(*bundle_group));
- }
-}
-
-void BundleManager::RefreshEstablishedBundleGroupsByMid() {
- established_bundle_groups_by_mid_.clear();
- for (const auto& bundle_group : bundle_groups_) {
- for (const std::string& content_name : bundle_group->content_names()) {
- established_bundle_groups_by_mid_[content_name] = bundle_group.get();
- }
- }
-}
-
void JsepTransportCollection::RegisterTransport(
const std::string& mid,
std::unique_ptr<cricket::JsepTransport> transport) {
@@ -217,6 +157,8 @@
if (it != mid_to_transport_.end() && it->second == jsep_transport)
return true;
+ pending_mids_.push_back(mid);
+
// The map_change_callback must be called before destroying the
// transport, because it removes references to the transport
// in the RTP demuxer.
@@ -249,33 +191,17 @@
RTC_DCHECK(IsConsistent());
}
-bool JsepTransportCollection::RollbackTransports() {
+void JsepTransportCollection::RollbackTransports() {
RTC_DCHECK_RUN_ON(&sequence_checker_);
- bool ret = true;
- // First, remove any new mid->transport mappings.
- for (const auto& kv : mid_to_transport_) {
- if (stable_mid_to_transport_.count(kv.first) == 0) {
- ret = ret && map_change_callback_(kv.first, nullptr);
- }
+ for (auto&& mid : pending_mids_) {
+ RemoveTransportForMid(mid);
}
- // Next, restore old mappings.
- for (const auto& kv : stable_mid_to_transport_) {
- auto it = mid_to_transport_.find(kv.first);
- if (it == mid_to_transport_.end() || it->second != kv.second) {
- ret = ret && map_change_callback_(kv.first, kv.second);
- }
- }
- mid_to_transport_ = stable_mid_to_transport_;
- DestroyUnusedTransports();
- RTC_DCHECK(IsConsistent());
- return ret;
+ pending_mids_.clear();
}
void JsepTransportCollection::CommitTransports() {
RTC_DCHECK_RUN_ON(&sequence_checker_);
- stable_mid_to_transport_ = mid_to_transport_;
- DestroyUnusedTransports();
- RTC_DCHECK(IsConsistent());
+ pending_mids_.clear();
}
bool JsepTransportCollection::TransportInUse(
@@ -286,11 +212,6 @@
return true;
}
}
- for (const auto& kv : stable_mid_to_transport_) {
- if (kv.second == jsep_transport) {
- return true;
- }
- }
return false;
}
@@ -298,7 +219,7 @@
cricket::JsepTransport* transport) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
// Don't destroy the JsepTransport if there are still media sections referring
- // to it, or if it will be needed in case of rollback.
+ // to it.
if (TransportInUse(transport)) {
return;
}
@@ -312,23 +233,6 @@
RTC_DCHECK(IsConsistent());
}
-void JsepTransportCollection::DestroyUnusedTransports() {
- RTC_DCHECK_RUN_ON(&sequence_checker_);
- bool need_state_change_callback = false;
- auto it = jsep_transports_by_name_.begin();
- while (it != jsep_transports_by_name_.end()) {
- if (TransportInUse(it->second.get())) {
- ++it;
- } else {
- it = jsep_transports_by_name_.erase(it);
- need_state_change_callback = true;
- }
- }
- if (need_state_change_callback) {
- state_change_callback_();
- }
-}
-
bool JsepTransportCollection::IsConsistent() {
RTC_DCHECK_RUN_ON(&sequence_checker_);
for (const auto& it : jsep_transports_by_name_) {
@@ -337,6 +241,13 @@
<< " is not in use, transport " << it.second.get();
return false;
}
+ const auto& lookup = mid_to_transport_.find(it.first);
+ if (lookup->second != it.second.get()) {
+ // Not an error, but unusual.
+ RTC_DLOG(LS_INFO) << "Note: Mid " << it.first << " was registered to "
+ << it.second.get() << " but currently maps to "
+ << lookup->second;
+ }
}
return true;
}
diff --git a/pc/jsep_transport_collection.h b/pc/jsep_transport_collection.h
index 9357037..0dd528d 100644
--- a/pc/jsep_transport_collection.h
+++ b/pc/jsep_transport_collection.h
@@ -18,7 +18,6 @@
#include <utility>
#include <vector>
-#include "api/peer_connection_interface.h"
#include "api/sequence_checker.h"
#include "pc/jsep_transport.h"
#include "pc/session_description.h"
@@ -42,8 +41,7 @@
// 6) Change the logic to do what's right.
class BundleManager {
public:
- explicit BundleManager(PeerConnectionInterface::BundlePolicy bundle_policy)
- : bundle_policy_(bundle_policy) {
+ BundleManager() {
// Allow constructor to be called on a different thread.
sequence_checker_.Detach();
}
@@ -60,27 +58,17 @@
bool IsFirstMidInGroup(const std::string& mid) const;
// Update the groups description. This completely replaces the group
// description with the one from the SessionDescription.
- void Update(const cricket::SessionDescription* description, SdpType type);
+ void Update(const cricket::SessionDescription* description);
// Delete a MID from the group that contains it.
void DeleteMid(const cricket::ContentGroup* bundle_group,
const std::string& mid);
// Delete a group.
void DeleteGroup(const cricket::ContentGroup* bundle_group);
- // Roll back to previous stable state.
- void Rollback();
- // Commit current bundle groups.
- void Commit();
private:
- // Recalculate established_bundle_groups_by_mid_ from bundle_groups_.
- void RefreshEstablishedBundleGroupsByMid() RTC_RUN_ON(sequence_checker_);
-
RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_;
- PeerConnectionInterface::BundlePolicy bundle_policy_;
std::vector<std::unique_ptr<cricket::ContentGroup>> bundle_groups_
RTC_GUARDED_BY(sequence_checker_);
- std::vector<std::unique_ptr<cricket::ContentGroup>> stable_bundle_groups_
- RTC_GUARDED_BY(sequence_checker_);
std::map<std::string, cricket::ContentGroup*>
established_bundle_groups_by_mid_;
};
@@ -120,25 +108,17 @@
// Remove a transport for a MID. This may destroy a transport if it is
// no longer in use.
void RemoveTransportForMid(const std::string& mid);
- // Roll back to previous stable mid-to-transport mappings.
- bool RollbackTransports();
- // Commit pending mid-transport mappings (rollback is no longer possible),
- // and destroy unused transports because we know now we'll never need them
- // again.
+ // Roll back pending mid-to-transport mappings.
+ void RollbackTransports();
+ // Commit pending mid-transport mappings (rollback is no longer possible).
void CommitTransports();
-
- private:
- // Returns true if any mid currently maps to this transport, in either the
- // pending or stable mapping.
+ // Returns true if any mid currently maps to this transport.
bool TransportInUse(cricket::JsepTransport* jsep_transport) const;
- // Destroy a transport if it's no longer in use. This includes whether it
- // will be needed in case of rollback.
+ private:
+ // Destroy a transport if it's no longer in use.
void MaybeDestroyJsepTransport(cricket::JsepTransport* transport);
- // Destroys all transports that are no longer in use.
- void DestroyUnusedTransports();
-
bool IsConsistent(); // For testing only: Verify internal structure.
RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_;
@@ -150,10 +130,8 @@
// (BaseChannel/SctpTransport) and the JsepTransport underneath.
std::map<std::string, cricket::JsepTransport*> mid_to_transport_
RTC_GUARDED_BY(sequence_checker_);
- // A snapshot of mid_to_transport_ at the last stable state. Used for
- // rollback.
- std::map<std::string, cricket::JsepTransport*> stable_mid_to_transport_
- RTC_GUARDED_BY(sequence_checker_);
+ // Keep track of mids that have been mapped to transports. Used for rollback.
+ std::vector<std::string> pending_mids_ RTC_GUARDED_BY(sequence_checker_);
// Callback used to inform subscribers of altered transports.
const std::function<bool(const std::string& mid,
cricket::JsepTransport* transport)>
diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index 8b9596b..47fddd4 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -55,8 +55,7 @@
UpdateAggregateStates_n();
}),
config_(config),
- active_reset_srtp_params_(config.active_reset_srtp_params),
- bundles_(config.bundle_policy) {
+ active_reset_srtp_params_(config.active_reset_srtp_params) {
// The |transport_observer| is assumed to be non-null.
RTC_DCHECK(config_.transport_observer);
RTC_DCHECK(config_.rtcp_handler);
@@ -375,18 +374,13 @@
}
}
-RTCError JsepTransportController::RollbackTransports() {
+void JsepTransportController::RollbackTransports() {
if (!network_thread_->IsCurrent()) {
- return network_thread_->Invoke<RTCError>(
- RTC_FROM_HERE, [=] { return RollbackTransports(); });
+ network_thread_->Invoke<void>(RTC_FROM_HERE, [=] { RollbackTransports(); });
+ return;
}
RTC_DCHECK_RUN_ON(network_thread_);
- bundles_.Rollback();
- if (!transports_.RollbackTransports()) {
- LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR,
- "Failed to roll back transport state.");
- }
- return RTCError::OK();
+ transports_.RollbackTransports();
}
rtc::scoped_refptr<webrtc::IceTransportInterface>
@@ -557,6 +551,17 @@
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 ||
@@ -577,8 +582,6 @@
description->transport_infos()[i];
if (content_info.rejected) {
- // This may cause groups to be removed from |bundles_.bundle_groups()|.
- HandleRejectedContent(content_info);
continue;
}
@@ -644,7 +647,6 @@
}
if (type == SdpType::kAnswer) {
transports_.CommitTransports();
- bundles_.Commit();
}
return RTCError::OK();
}
@@ -680,44 +682,7 @@
}
}
- if (type == SdpType::kOffer) {
- // For an offer, we need to verify that there is not a conflicting mapping
- // between existing and new bundle groups. For example, if the existing
- // groups are [[1,2],[3,4]] and new are [[1,3],[2,4]] or [[1,2,3,4]], or
- // vice versa. Switching things around like this requires a separate offer
- // that removes the relevant sections from their group, as per RFC 8843,
- // section 7.5.2.
- std::map<const cricket::ContentGroup*, const cricket::ContentGroup*>
- new_bundle_groups_by_existing_bundle_groups;
- std::map<const cricket::ContentGroup*, const cricket::ContentGroup*>
- existing_bundle_groups_by_new_bundle_groups;
- for (const cricket::ContentGroup* new_bundle_group : new_bundle_groups) {
- for (const std::string& mid : new_bundle_group->content_names()) {
- cricket::ContentGroup* existing_bundle_group =
- bundles_.LookupGroupByMid(mid);
- if (!existing_bundle_group) {
- continue;
- }
- auto it = new_bundle_groups_by_existing_bundle_groups.find(
- existing_bundle_group);
- if (it != new_bundle_groups_by_existing_bundle_groups.end() &&
- it->second != new_bundle_group) {
- return RTCError(RTCErrorType::INVALID_PARAMETER,
- "MID " + mid + " in the offer has changed group.");
- }
- new_bundle_groups_by_existing_bundle_groups.insert(
- std::make_pair(existing_bundle_group, new_bundle_group));
- it = existing_bundle_groups_by_new_bundle_groups.find(new_bundle_group);
- if (it != existing_bundle_groups_by_new_bundle_groups.end() &&
- it->second != existing_bundle_group) {
- return RTCError(RTCErrorType::INVALID_PARAMETER,
- "MID " + mid + " in the offer has changed group.");
- }
- existing_bundle_groups_by_new_bundle_groups.insert(
- std::make_pair(new_bundle_group, existing_bundle_group));
- }
- }
- } else if (type == SdpType::kAnswer) {
+ if (type == SdpType::kAnswer) {
std::vector<const cricket::ContentGroup*> offered_bundle_groups =
local ? remote_desc_->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE)
: local_desc_->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE);
@@ -797,7 +762,9 @@
"max-bundle is used but no bundle group found.");
}
- bundles_.Update(description, type);
+ if (ShouldUpdateBundleGroup(type, description)) {
+ bundles_.Update(description);
+ }
for (const auto& bundle_group : bundles_.bundle_groups()) {
if (!bundle_group->FirstContentName())
@@ -905,6 +872,26 @@
rtp_abs_sendtime_extn_id, transport_info.description);
}
+bool JsepTransportController::ShouldUpdateBundleGroup(
+ SdpType type,
+ const cricket::SessionDescription* description) {
+ if (config_.bundle_policy ==
+ PeerConnectionInterface::kBundlePolicyMaxBundle) {
+ return true;
+ }
+
+ if (type != SdpType::kAnswer) {
+ return false;
+ }
+
+ RTC_DCHECK(local_desc_ && remote_desc_);
+ std::vector<const cricket::ContentGroup*> local_bundles =
+ local_desc_->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE);
+ std::vector<const cricket::ContentGroup*> remote_bundles =
+ remote_desc_->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE);
+ return !local_bundles.empty() && !remote_bundles.empty();
+}
+
std::vector<int> JsepTransportController::GetEncryptedHeaderExtensionIds(
const cricket::ContentInfo& content_info) {
const cricket::MediaContentDescription* content_desc =
@@ -1000,6 +987,21 @@
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()) {
diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h
index 3b20bbb..71b01bf 100644
--- a/pc/jsep_transport_controller.h
+++ b/pc/jsep_transport_controller.h
@@ -224,7 +224,9 @@
void SetActiveResetSrtpParams(bool active_reset_srtp_params);
- RTCError RollbackTransports();
+ // For now the rollback only removes mid to transport mappings
+ // and deletes unused transports, but doesn't consider anything more complex.
+ void RollbackTransports();
// F: void(const std::string&, const std::vector<cricket::Candidate>&)
template <typename F>
@@ -340,6 +342,9 @@
const std::vector<int>& encrypted_extension_ids,
int rtp_abs_sendtime_extn_id);
+ bool ShouldUpdateBundleGroup(SdpType type,
+ const cricket::SessionDescription* description);
+
std::map<const cricket::ContentGroup*, std::vector<int>>
MergeEncryptedHeaderExtensionIdsForBundles(
const cricket::SessionDescription* description);
diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc
index bc7cfeb..a06f580 100644
--- a/pc/jsep_transport_controller_unittest.cc
+++ b/pc/jsep_transport_controller_unittest.cc
@@ -1608,356 +1608,6 @@
EXPECT_EQ(mid5_transport, mid6_transport);
}
-TEST_F(JsepTransportControllerTest,
- MultipleBundleGroupsSectionsAddedInSubsequentOffer) {
- static const char kMid1Audio[] = "1_audio";
- static const char kMid2Audio[] = "2_audio";
- static const char kMid3Audio[] = "3_audio";
- static const char kMid4Video[] = "4_video";
- static const char kMid5Video[] = "5_video";
- static const char kMid6Video[] = "6_video";
-
- CreateJsepTransportController(JsepTransportController::Config());
- // Start by grouping (kMid1Audio,kMid2Audio) and (kMid4Video,kMid4f5Video).
- cricket::ContentGroup bundle_group1(cricket::GROUP_TYPE_BUNDLE);
- bundle_group1.AddContentName(kMid1Audio);
- bundle_group1.AddContentName(kMid2Audio);
- cricket::ContentGroup bundle_group2(cricket::GROUP_TYPE_BUNDLE);
- bundle_group2.AddContentName(kMid4Video);
- bundle_group2.AddContentName(kMid5Video);
-
- auto local_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(local_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid5Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- local_offer->AddGroup(bundle_group1);
- local_offer->AddGroup(bundle_group2);
-
- auto remote_answer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(remote_answer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(remote_answer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(remote_answer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(remote_answer.get(), kMid5Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- remote_answer->AddGroup(bundle_group1);
- remote_answer->AddGroup(bundle_group2);
-
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_offer.get())
- .ok());
- EXPECT_TRUE(transport_controller_
- ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get())
- .ok());
-
- // Add kMid3Audio and kMid6Video to the respective audio/video bundle groups.
- cricket::ContentGroup new_bundle_group1(cricket::GROUP_TYPE_BUNDLE);
- bundle_group1.AddContentName(kMid3Audio);
- cricket::ContentGroup new_bundle_group2(cricket::GROUP_TYPE_BUNDLE);
- bundle_group2.AddContentName(kMid6Video);
-
- auto subsequent_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(subsequent_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(subsequent_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(subsequent_offer.get(), kMid3Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid5Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid6Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- subsequent_offer->AddGroup(bundle_group1);
- subsequent_offer->AddGroup(bundle_group2);
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, subsequent_offer.get())
- .ok());
- auto mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio);
- auto mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio);
- auto mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio);
- auto mid4_transport = transport_controller_->GetRtpTransport(kMid4Video);
- auto mid5_transport = transport_controller_->GetRtpTransport(kMid5Video);
- auto mid6_transport = transport_controller_->GetRtpTransport(kMid6Video);
- EXPECT_NE(mid1_transport, mid4_transport);
- EXPECT_EQ(mid1_transport, mid2_transport);
- EXPECT_EQ(mid2_transport, mid3_transport);
- EXPECT_EQ(mid4_transport, mid5_transport);
- EXPECT_EQ(mid5_transport, mid6_transport);
-}
-
-TEST_F(JsepTransportControllerTest,
- MultipleBundleGroupsCombinedInSubsequentOffer) {
- static const char kMid1Audio[] = "1_audio";
- static const char kMid2Audio[] = "2_audio";
- static const char kMid3Video[] = "3_video";
- static const char kMid4Video[] = "4_video";
-
- CreateJsepTransportController(JsepTransportController::Config());
- // Start by grouping (kMid1Audio,kMid2Audio) and (kMid3Video,kMid4Video).
- cricket::ContentGroup bundle_group1(cricket::GROUP_TYPE_BUNDLE);
- bundle_group1.AddContentName(kMid1Audio);
- bundle_group1.AddContentName(kMid2Audio);
- cricket::ContentGroup bundle_group2(cricket::GROUP_TYPE_BUNDLE);
- bundle_group2.AddContentName(kMid3Video);
- bundle_group2.AddContentName(kMid4Video);
-
- auto local_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(local_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- local_offer->AddGroup(bundle_group1);
- local_offer->AddGroup(bundle_group2);
-
- auto remote_answer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(remote_answer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(remote_answer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(remote_answer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(remote_answer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- remote_answer->AddGroup(bundle_group1);
- remote_answer->AddGroup(bundle_group2);
-
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_offer.get())
- .ok());
- EXPECT_TRUE(transport_controller_
- ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get())
- .ok());
-
- // Switch to grouping (kMid1Audio,kMid2Audio,kMid3Video,kMid4Video).
- // This is a illegal without first removing m= sections from their groups.
- cricket::ContentGroup new_bundle_group(cricket::GROUP_TYPE_BUNDLE);
- new_bundle_group.AddContentName(kMid1Audio);
- new_bundle_group.AddContentName(kMid2Audio);
- new_bundle_group.AddContentName(kMid3Video);
- new_bundle_group.AddContentName(kMid4Video);
-
- auto subsequent_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(subsequent_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(subsequent_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- subsequent_offer->AddGroup(new_bundle_group);
- EXPECT_FALSE(
- transport_controller_
- ->SetLocalDescription(SdpType::kOffer, subsequent_offer.get())
- .ok());
-}
-
-TEST_F(JsepTransportControllerTest,
- MultipleBundleGroupsSplitInSubsequentOffer) {
- static const char kMid1Audio[] = "1_audio";
- static const char kMid2Audio[] = "2_audio";
- static const char kMid3Video[] = "3_video";
- static const char kMid4Video[] = "4_video";
-
- CreateJsepTransportController(JsepTransportController::Config());
- // Start by grouping (kMid1Audio,kMid2Audio,kMid3Video,kMid4Video).
- cricket::ContentGroup bundle_group(cricket::GROUP_TYPE_BUNDLE);
- bundle_group.AddContentName(kMid1Audio);
- bundle_group.AddContentName(kMid2Audio);
- bundle_group.AddContentName(kMid3Video);
- bundle_group.AddContentName(kMid4Video);
-
- auto local_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(local_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- local_offer->AddGroup(bundle_group);
-
- auto remote_answer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(remote_answer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(remote_answer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(remote_answer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(remote_answer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- remote_answer->AddGroup(bundle_group);
-
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_offer.get())
- .ok());
- EXPECT_TRUE(transport_controller_
- ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get())
- .ok());
-
- // Switch to grouping (kMid1Audio,kMid2Audio) and (kMid3Video,kMid4Video).
- // This is a illegal without first removing m= sections from their groups.
- cricket::ContentGroup new_bundle_group1(cricket::GROUP_TYPE_BUNDLE);
- new_bundle_group1.AddContentName(kMid1Audio);
- new_bundle_group1.AddContentName(kMid2Audio);
- cricket::ContentGroup new_bundle_group2(cricket::GROUP_TYPE_BUNDLE);
- new_bundle_group2.AddContentName(kMid3Video);
- new_bundle_group2.AddContentName(kMid4Video);
-
- auto subsequent_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(subsequent_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(subsequent_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- subsequent_offer->AddGroup(new_bundle_group1);
- subsequent_offer->AddGroup(new_bundle_group2);
- EXPECT_FALSE(
- transport_controller_
- ->SetLocalDescription(SdpType::kOffer, subsequent_offer.get())
- .ok());
-}
-
-TEST_F(JsepTransportControllerTest,
- MultipleBundleGroupsShuffledInSubsequentOffer) {
- static const char kMid1Audio[] = "1_audio";
- static const char kMid2Audio[] = "2_audio";
- static const char kMid3Video[] = "3_video";
- static const char kMid4Video[] = "4_video";
-
- CreateJsepTransportController(JsepTransportController::Config());
- // Start by grouping (kMid1Audio,kMid2Audio) and (kMid3Video,kMid4Video).
- cricket::ContentGroup bundle_group1(cricket::GROUP_TYPE_BUNDLE);
- bundle_group1.AddContentName(kMid1Audio);
- bundle_group1.AddContentName(kMid2Audio);
- cricket::ContentGroup bundle_group2(cricket::GROUP_TYPE_BUNDLE);
- bundle_group2.AddContentName(kMid3Video);
- bundle_group2.AddContentName(kMid4Video);
-
- auto local_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(local_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- local_offer->AddGroup(bundle_group1);
- local_offer->AddGroup(bundle_group2);
-
- auto remote_answer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(remote_answer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(remote_answer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(remote_answer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(remote_answer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- remote_answer->AddGroup(bundle_group1);
- remote_answer->AddGroup(bundle_group2);
-
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_offer.get())
- .ok());
- EXPECT_TRUE(transport_controller_
- ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get())
- .ok());
-
- // Switch to grouping (kMid1Audio,kMid3Video) and (kMid2Audio,kMid3Video).
- // This is a illegal without first removing m= sections from their groups.
- cricket::ContentGroup new_bundle_group1(cricket::GROUP_TYPE_BUNDLE);
- new_bundle_group1.AddContentName(kMid1Audio);
- new_bundle_group1.AddContentName(kMid3Video);
- cricket::ContentGroup new_bundle_group2(cricket::GROUP_TYPE_BUNDLE);
- new_bundle_group2.AddContentName(kMid2Audio);
- new_bundle_group2.AddContentName(kMid4Video);
-
- auto subsequent_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(subsequent_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(subsequent_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- subsequent_offer->AddGroup(new_bundle_group1);
- subsequent_offer->AddGroup(new_bundle_group2);
- EXPECT_FALSE(
- transport_controller_
- ->SetLocalDescription(SdpType::kOffer, subsequent_offer.get())
- .ok());
-}
-
// Tests that only a subset of all the m= sections are bundled.
TEST_F(JsepTransportControllerTest, BundleSubsetOfMediaSections) {
CreateJsepTransportController(JsepTransportController::Config());
@@ -2409,220 +2059,4 @@
.ok());
}
-TEST_F(JsepTransportControllerTest, RollbackRestoresRejectedTransport) {
- static const char kMid1Audio[] = "1_audio";
-
- // Perform initial offer/answer.
- CreateJsepTransportController(JsepTransportController::Config());
- auto local_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- std::unique_ptr<cricket::SessionDescription> remote_answer(
- local_offer->Clone());
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_offer.get())
- .ok());
- EXPECT_TRUE(transport_controller_
- ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get())
- .ok());
-
- auto mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio);
-
- // Apply a reoffer which rejects the m= section, causing the transport to be
- // set to null.
- auto local_reoffer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_reoffer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- local_reoffer->contents()[0].rejected = true;
-
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_reoffer.get())
- .ok());
- auto old_mid1_transport = mid1_transport;
- mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio);
- EXPECT_EQ(nullptr, mid1_transport);
-
- // Rolling back shouldn't just create a new transport for MID 1, it should
- // restore the old transport.
- EXPECT_TRUE(transport_controller_->RollbackTransports().ok());
- mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio);
- EXPECT_EQ(old_mid1_transport, mid1_transport);
-}
-
-// If an offer with a modified BUNDLE group causes a MID->transport mapping to
-// change, rollback should restore the previous mapping.
-TEST_F(JsepTransportControllerTest, RollbackRestoresPreviousTransportMapping) {
- static const char kMid1Audio[] = "1_audio";
- static const char kMid2Audio[] = "2_audio";
- static const char kMid3Audio[] = "3_audio";
-
- // Perform an initial offer/answer to establish a (kMid1Audio,kMid2Audio)
- // group.
- CreateJsepTransportController(JsepTransportController::Config());
- cricket::ContentGroup bundle_group(cricket::GROUP_TYPE_BUNDLE);
- bundle_group.AddContentName(kMid1Audio);
- bundle_group.AddContentName(kMid2Audio);
-
- auto local_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid2Audio, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(local_offer.get(), kMid3Audio, kIceUfrag3, kIcePwd3,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- local_offer->AddGroup(bundle_group);
-
- std::unique_ptr<cricket::SessionDescription> remote_answer(
- local_offer->Clone());
-
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_offer.get())
- .ok());
- EXPECT_TRUE(transport_controller_
- ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get())
- .ok());
-
- auto mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio);
- auto mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio);
- auto mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio);
- EXPECT_EQ(mid1_transport, mid2_transport);
- EXPECT_NE(mid1_transport, mid3_transport);
-
- // Apply a reoffer adding kMid3Audio to the group; transport mapping should
- // change, even without an answer, since this is an existing group.
- bundle_group.AddContentName(kMid3Audio);
- auto local_reoffer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_reoffer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_reoffer.get(), kMid2Audio, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddAudioSection(local_reoffer.get(), kMid3Audio, kIceUfrag3, kIcePwd3,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- local_reoffer->AddGroup(bundle_group);
-
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_reoffer.get())
- .ok());
-
- // Store the old transport pointer and verify that the offer actually changed
- // transports.
- auto old_mid3_transport = mid3_transport;
- mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio);
- mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio);
- mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio);
- EXPECT_EQ(mid1_transport, mid2_transport);
- EXPECT_EQ(mid1_transport, mid3_transport);
-
- // Rolling back shouldn't just create a new transport for MID 3, it should
- // restore the old transport.
- EXPECT_TRUE(transport_controller_->RollbackTransports().ok());
- mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio);
- EXPECT_EQ(old_mid3_transport, mid3_transport);
-}
-
-// Test that if an offer adds a MID to a specific BUNDLE group and is then
-// rolled back, it can be added to a different BUNDLE group in a new offer.
-// This is effectively testing that rollback resets the BundleManager state.
-TEST_F(JsepTransportControllerTest, RollbackAndAddToDifferentBundleGroup) {
- static const char kMid1Audio[] = "1_audio";
- static const char kMid2Audio[] = "2_audio";
- static const char kMid3Audio[] = "3_audio";
-
- // Perform an initial offer/answer to establish two bundle groups, each with
- // one MID.
- CreateJsepTransportController(JsepTransportController::Config());
- cricket::ContentGroup bundle_group1(cricket::GROUP_TYPE_BUNDLE);
- bundle_group1.AddContentName(kMid1Audio);
- cricket::ContentGroup bundle_group2(cricket::GROUP_TYPE_BUNDLE);
- bundle_group2.AddContentName(kMid2Audio);
-
- auto local_offer = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(local_offer.get(), kMid2Audio, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- local_offer->AddGroup(bundle_group1);
- local_offer->AddGroup(bundle_group2);
-
- std::unique_ptr<cricket::SessionDescription> remote_answer(
- local_offer->Clone());
-
- EXPECT_TRUE(transport_controller_
- ->SetLocalDescription(SdpType::kOffer, local_offer.get())
- .ok());
- EXPECT_TRUE(transport_controller_
- ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get())
- .ok());
-
- // Apply an offer that adds kMid3Audio to the first BUNDLE group.,
- cricket::ContentGroup modified_bundle_group1(cricket::GROUP_TYPE_BUNDLE);
- modified_bundle_group1.AddContentName(kMid1Audio);
- modified_bundle_group1.AddContentName(kMid3Audio);
- auto subsequent_offer_1 = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(subsequent_offer_1.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer_1.get(), kMid2Audio, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer_1.get(), kMid3Audio, kIceUfrag3, kIcePwd3,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- subsequent_offer_1->AddGroup(modified_bundle_group1);
- subsequent_offer_1->AddGroup(bundle_group2);
-
- EXPECT_TRUE(
- transport_controller_
- ->SetLocalDescription(SdpType::kOffer, subsequent_offer_1.get())
- .ok());
-
- auto mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio);
- auto mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio);
- auto mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio);
- EXPECT_NE(mid1_transport, mid2_transport);
- EXPECT_EQ(mid1_transport, mid3_transport);
-
- // Rollback and expect the transport to be reset.
- EXPECT_TRUE(transport_controller_->RollbackTransports().ok());
- EXPECT_EQ(nullptr, transport_controller_->GetRtpTransport(kMid3Audio));
-
- // Apply an offer that adds kMid3Audio to the second BUNDLE group.,
- cricket::ContentGroup modified_bundle_group2(cricket::GROUP_TYPE_BUNDLE);
- modified_bundle_group2.AddContentName(kMid2Audio);
- modified_bundle_group2.AddContentName(kMid3Audio);
- auto subsequent_offer_2 = std::make_unique<cricket::SessionDescription>();
- AddAudioSection(subsequent_offer_2.get(), kMid1Audio, kIceUfrag1, kIcePwd1,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer_2.get(), kMid2Audio, kIceUfrag2, kIcePwd2,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- AddVideoSection(subsequent_offer_2.get(), kMid3Audio, kIceUfrag3, kIcePwd3,
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
- nullptr);
- subsequent_offer_2->AddGroup(bundle_group1);
- subsequent_offer_2->AddGroup(modified_bundle_group2);
-
- EXPECT_TRUE(
- transport_controller_
- ->SetLocalDescription(SdpType::kOffer, subsequent_offer_2.get())
- .ok());
-
- mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio);
- mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio);
- mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio);
- EXPECT_NE(mid1_transport, mid2_transport);
- EXPECT_EQ(mid2_transport, mid3_transport);
-}
-
} // namespace webrtc
diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc
index be05af9..08754c6 100644
--- a/pc/peer_connection_bundle_unittest.cc
+++ b/pc/peer_connection_bundle_unittest.cc
@@ -909,11 +909,11 @@
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 @@
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
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 929736e..533bd84 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -2737,10 +2737,7 @@
transceiver->internal()->set_mid(state.mid());
transceiver->internal()->set_mline_index(state.mline_index());
}
- RTCError e = transport_controller()->RollbackTransports();
- if (!e.ok()) {
- return e;
- }
+ transport_controller()->RollbackTransports();
transceivers()->DiscardStableStates();
pending_local_description_.reset();
pending_remote_description_.reset();
