Update bundle behavior to match BundlePolicy spec in http://rtcweb-wg.github.io/jsep/.

BUG=1574
R=juberti@webrtc.org, pthatcher@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/36659004

Cr-Commit-Position: refs/heads/master@{#8851}
This commit is contained in:
Donald Curtis 2015-03-24 09:29:54 -07:00
parent e61c64dbb1
commit 0e209b03bf
5 changed files with 188 additions and 100 deletions

View File

@ -339,8 +339,7 @@ bool PeerConnection::Initialize(
// To handle both internal and externally created port allocator, we will
// enable BUNDLE here.
int portallocator_flags = port_allocator_->flags();
portallocator_flags |= cricket::PORTALLOCATOR_ENABLE_BUNDLE |
cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
portallocator_flags |= cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
cricket::PORTALLOCATOR_ENABLE_IPV6;
bool value;

View File

@ -1571,13 +1571,6 @@ void WebRtcSession::RemoveUnusedChannelsAndTransports(
// TODO(mallinath) - Add a correct error code if the channels are not creatued
// due to BUNDLE is enabled but rtcp-mux is disabled.
bool WebRtcSession::CreateChannels(const SessionDescription* desc) {
// Disabling the BUNDLE flag in PortAllocator if offer disabled it.
bool bundle_enabled = desc->HasGroup(cricket::GROUP_TYPE_BUNDLE);
if (state() == STATE_INIT && !bundle_enabled) {
port_allocator()->set_flags(port_allocator()->flags() &
~cricket::PORTALLOCATOR_ENABLE_BUNDLE);
}
// Creating the media channels and transport proxies.
const cricket::ContentInfo* voice = cricket::GetFirstAudioContent(desc);
if (voice && !voice->rejected && !voice_channel_) {
@ -1604,6 +1597,21 @@ bool WebRtcSession::CreateChannels(const SessionDescription* desc) {
}
}
// Enable bundle before when kMaxBundle policy is in effect.
if (bundle_policy_ == PeerConnectionInterface::kBundlePolicyMaxBundle) {
const cricket::ContentGroup* local_bundle_group =
BaseSession::local_description()->GetGroupByName(
cricket::GROUP_TYPE_BUNDLE);
if (!local_bundle_group) {
LOG(LS_WARNING) << "max-bundle specified without BUNDLE specified";
return false;
}
if (!BaseSession::BundleContentGroup(local_bundle_group)) {
LOG(LS_WARNING) << "max-bundle failed to enable bundling.";
return false;
}
}
return true;
}

View File

@ -371,7 +371,6 @@ class WebRtcSessionTest : public testing::Test {
SocketAddress(), SocketAddress(), SocketAddress()));
allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP |
cricket::PORTALLOCATOR_DISABLE_RELAY |
cricket::PORTALLOCATOR_ENABLE_BUNDLE |
cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG);
EXPECT_TRUE(channel_manager_->Init());
desc_factory_->set_add_legacy_streams(false);
@ -1248,7 +1247,6 @@ class WebRtcSessionTest : public testing::Test {
allocator_->AddRelay(relay_server);
allocator_->set_step_delay(cricket::kMinimumStepDelay);
allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP |
cricket::PORTALLOCATOR_ENABLE_BUNDLE |
cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG);
}
@ -2597,39 +2595,97 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionInvalidIceCredentials) {
EXPECT_FALSE(session_->SetRemoteDescription(modified_offer, &error));
}
TEST_F(WebRtcSessionTest, VerifyBundleFlagInPA) {
// This test verifies BUNDLE flag in PortAllocator, if BUNDLE information in
// local description is removed by the application, BUNDLE flag should be
// disabled in PortAllocator. By default BUNDLE is enabled in the WebRtc.
Init();
EXPECT_TRUE((cricket::PORTALLOCATOR_ENABLE_BUNDLE &
allocator_->flags()) == cricket::PORTALLOCATOR_ENABLE_BUNDLE);
rtc::scoped_ptr<SessionDescriptionInterface> offer(CreateOffer());
cricket::SessionDescription* offer_copy =
offer->description()->Copy();
offer_copy->RemoveGroupByName(cricket::GROUP_TYPE_BUNDLE);
JsepSessionDescription* modified_offer =
new JsepSessionDescription(JsepSessionDescription::kOffer);
modified_offer->Initialize(offer_copy, "1", "1");
SetLocalDescriptionWithoutError(modified_offer);
EXPECT_FALSE(allocator_->flags() & cricket::PORTALLOCATOR_ENABLE_BUNDLE);
}
TEST_F(WebRtcSessionTest, TestDisabledBundleInAnswer) {
Init();
// kBundlePolicyBalanced bundle policy with and answer contains BUNDLE.
TEST_F(WebRtcSessionTest, TestBalancedBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced);
mediastream_signaling_.SendAudioVideoStream1();
EXPECT_TRUE((cricket::PORTALLOCATOR_ENABLE_BUNDLE &
allocator_->flags()) == cricket::PORTALLOCATOR_ENABLE_BUNDLE);
PeerConnectionInterface::RTCOfferAnswerOptions options;
options.use_rtp_mux = true;
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
CreateRemoteAnswer(session_->local_description());
SetRemoteDescriptionWithoutError(answer);
EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
}
// kBundlePolicyBalanced bundle policy but no BUNDLE in the answer.
TEST_F(WebRtcSessionTest, TestBalancedNoBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced);
mediastream_signaling_.SendAudioVideoStream1();
PeerConnectionInterface::RTCOfferAnswerOptions options;
options.use_rtp_mux = true;
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
mediastream_signaling_.SendAudioVideoStream2();
// Remove BUNDLE from the answer.
rtc::scoped_ptr<SessionDescriptionInterface> answer(
CreateRemoteAnswer(session_->local_description()));
cricket::SessionDescription* answer_copy = answer->description()->Copy();
answer_copy->RemoveGroupByName(cricket::GROUP_TYPE_BUNDLE);
JsepSessionDescription* modified_answer =
new JsepSessionDescription(JsepSessionDescription::kAnswer);
modified_answer->Initialize(answer_copy, "1", "1");
SetRemoteDescriptionWithoutError(modified_answer); //
EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
}
// kBundlePolicyMaxBundle policy with BUNDLE in the answer.
TEST_F(WebRtcSessionTest, TestMaxBundleBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyMaxBundle);
mediastream_signaling_.SendAudioVideoStream1();
PeerConnectionInterface::RTCOfferAnswerOptions options;
options.use_rtp_mux = true;
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
CreateRemoteAnswer(session_->local_description());
SetRemoteDescriptionWithoutError(answer);
EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
}
// kBundlePolicyMaxBundle policy but no BUNDLE in the answer.
TEST_F(WebRtcSessionTest, TestMaxBundleNoBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyMaxBundle);
mediastream_signaling_.SendAudioVideoStream1();
PeerConnectionInterface::RTCOfferAnswerOptions options;
options.use_rtp_mux = true;
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
mediastream_signaling_.SendAudioVideoStream2();
// Remove BUNDLE from the answer.
rtc::scoped_ptr<SessionDescriptionInterface> answer(
CreateRemoteAnswer(session_->local_description()));
cricket::SessionDescription* answer_copy = answer->description()->Copy();
@ -2638,22 +2694,63 @@ TEST_F(WebRtcSessionTest, TestDisabledBundleInAnswer) {
new JsepSessionDescription(JsepSessionDescription::kAnswer);
modified_answer->Initialize(answer_copy, "1", "1");
SetRemoteDescriptionWithoutError(modified_answer);
EXPECT_TRUE((cricket::PORTALLOCATOR_ENABLE_BUNDLE &
allocator_->flags()) == cricket::PORTALLOCATOR_ENABLE_BUNDLE);
video_channel_ = media_engine_->GetVideoChannel(0);
voice_channel_ = media_engine_->GetVoiceChannel(0);
EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
}
ASSERT_EQ(1u, video_channel_->recv_streams().size());
EXPECT_TRUE(kVideoTrack2 == video_channel_->recv_streams()[0].id);
// kBundlePolicyMaxCompat bundle policy with and answer contains BUNDLE.
TEST_F(WebRtcSessionTest, TestMaxCompatBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyMaxCompat);
mediastream_signaling_.SendAudioVideoStream1();
ASSERT_EQ(1u, voice_channel_->recv_streams().size());
EXPECT_TRUE(kAudioTrack2 == voice_channel_->recv_streams()[0].id);
PeerConnectionInterface::RTCOfferAnswerOptions options;
options.use_rtp_mux = true;
ASSERT_EQ(1u, video_channel_->send_streams().size());
EXPECT_TRUE(kVideoTrack1 == video_channel_->send_streams()[0].id);
ASSERT_EQ(1u, voice_channel_->send_streams().size());
EXPECT_TRUE(kAudioTrack1 == voice_channel_->send_streams()[0].id);
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
CreateRemoteAnswer(session_->local_description());
SetRemoteDescriptionWithoutError(answer);
// This should lead to an audio-only call but isn't implemented
// correctly yet.
EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
}
// kBundlePolicyMaxCompat bundle policy but no BUNDLE in the answer.
TEST_F(WebRtcSessionTest, TestMaxCompatNoBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyMaxCompat);
mediastream_signaling_.SendAudioVideoStream1();
PeerConnectionInterface::RTCOfferAnswerOptions options;
options.use_rtp_mux = true;
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
mediastream_signaling_.SendAudioVideoStream2();
// Remove BUNDLE from the answer.
rtc::scoped_ptr<SessionDescriptionInterface> answer(
CreateRemoteAnswer(session_->local_description()));
cricket::SessionDescription* answer_copy = answer->description()->Copy();
answer_copy->RemoveGroupByName(cricket::GROUP_TYPE_BUNDLE);
JsepSessionDescription* modified_answer =
new JsepSessionDescription(JsepSessionDescription::kAnswer);
modified_answer->Initialize(answer_copy, "1", "1");
SetRemoteDescriptionWithoutError(modified_answer); //
EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
}
// This test verifies that SetLocalDescription and SetRemoteDescription fails
@ -2661,8 +2758,6 @@ TEST_F(WebRtcSessionTest, TestDisabledBundleInAnswer) {
TEST_F(WebRtcSessionTest, TestDisabledRtcpMuxWithBundleEnabled) {
Init();
mediastream_signaling_.SendAudioVideoStream1();
EXPECT_TRUE((cricket::PORTALLOCATOR_ENABLE_BUNDLE &
allocator_->flags()) == cricket::PORTALLOCATOR_ENABLE_BUNDLE);
PeerConnectionInterface::RTCOfferAnswerOptions options;
options.use_rtp_mux = true;
@ -3153,7 +3248,6 @@ TEST_F(WebRtcSessionTest, TestIceStatesBasicIPv6) {
// Runs the loopback call test with BUNDLE and STUN enabled.
TEST_F(WebRtcSessionTest, TestIceStatesBundle) {
allocator_->set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
cricket::PORTALLOCATOR_ENABLE_BUNDLE |
cricket::PORTALLOCATOR_DISABLE_TCP |
cricket::PORTALLOCATOR_DISABLE_RELAY);
TestLoopbackCall();

View File

@ -537,23 +537,6 @@ TransportProxy* BaseSession::GetTransportProxy(
return (iter != transports_.end()) ? iter->second : NULL;
}
TransportProxy* BaseSession::GetTransportProxy(const Transport* transport) {
for (TransportMap::iterator iter = transports_.begin();
iter != transports_.end(); ++iter) {
TransportProxy* transproxy = iter->second;
if (transproxy->impl() == transport) {
return transproxy;
}
}
return NULL;
}
TransportProxy* BaseSession::GetFirstTransportProxy() {
if (transports_.empty())
return NULL;
return transports_.begin()->second;
}
void BaseSession::DestroyTransportProxy(
const std::string& content_name) {
TransportMap::iterator iter = transports_.find(content_name);
@ -643,8 +626,9 @@ bool BaseSession::MaybeEnableMuxingSupport() {
for (TransportMap::iterator iter = transports_.begin();
iter != transports_.end(); ++iter) {
ASSERT(iter->second->negotiated());
if (!iter->second->negotiated())
if (!iter->second->negotiated()) {
return false;
}
}
// If both sides agree to BUNDLE, mux all the specified contents onto the
@ -654,56 +638,63 @@ bool BaseSession::MaybeEnableMuxingSupport() {
// BUNDLE the same way?
bool candidates_allocated = IsCandidateAllocationDone();
const ContentGroup* local_bundle_group =
local_description()->GetGroupByName(GROUP_TYPE_BUNDLE);
local_description_->GetGroupByName(GROUP_TYPE_BUNDLE);
const ContentGroup* remote_bundle_group =
remote_description()->GetGroupByName(GROUP_TYPE_BUNDLE);
if (local_bundle_group && remote_bundle_group &&
local_bundle_group->FirstContentName()) {
const std::string* content_name = local_bundle_group->FirstContentName();
const ContentInfo* content =
local_description_->GetContentByName(*content_name);
if (!content) {
LOG(LS_WARNING) << "Content \"" << *content_name
<< "\" referenced in BUNDLE group is not present";
return false;
}
if (!SetSelectedProxy(content->name, local_bundle_group)) {
remote_description_->GetGroupByName(GROUP_TYPE_BUNDLE);
if (local_bundle_group && remote_bundle_group) {
if (!BundleContentGroup(local_bundle_group)) {
LOG(LS_WARNING) << "Failed to set up BUNDLE";
return false;
}
// If we weren't done gathering before, we might be done now, as a result
// of enabling mux.
LOG(LS_INFO) << "Enabling BUNDLE, bundling onto transport: "
<< *content_name;
if (!candidates_allocated) {
MaybeCandidateAllocationDone();
}
} else {
LOG(LS_INFO) << "No BUNDLE information, not bundling.";
LOG(LS_INFO) << "BUNDLE group missing from remote or local description.";
}
return true;
}
bool BaseSession::SetSelectedProxy(const std::string& content_name,
const ContentGroup* muxed_group) {
TransportProxy* selected_proxy = GetTransportProxy(content_name);
if (!selected_proxy) {
bool BaseSession::BundleContentGroup(const ContentGroup* bundle_group) {
const std::string* content_name = bundle_group->FirstContentName();
if (!content_name) {
LOG(LS_INFO) << "No content names specified in BUNDLE group.";
return true;
}
const ContentInfo* content =
local_description_->GetContentByName(*content_name);
if (!content) {
LOG(LS_WARNING) << "Content \"" << *content_name
<< "\" referenced in BUNDLE group"
<< " not present in local description";
return false;
}
TransportProxy* selected_proxy = GetTransportProxy(*content_name);
if (!selected_proxy) {
LOG(LS_WARNING) << "No transport found for content \""
<< *content_name << "\".";
return false;
}
ASSERT(selected_proxy->negotiated());
for (TransportMap::iterator iter = transports_.begin();
iter != transports_.end(); ++iter) {
// If content is part of the mux group, then repoint its proxy at the
// transport object that we have chosen to mux onto. If the proxy
// is already pointing at the right object, it will be a no-op.
if (muxed_group->HasContentName(iter->first) &&
if (bundle_group->HasContentName(iter->first) &&
!iter->second->SetupMux(selected_proxy)) {
LOG(LS_WARNING) << "Failed to bundle " << iter->first << " to "
<< *content_name;
return false;
}
LOG(LS_INFO) << "Bundling " << iter->first << " to " << *content_name;
}
return true;
}

View File

@ -331,8 +331,6 @@ class BaseSession : public sigslot::has_slots<>,
const TransportMap& transport_proxies() const { return transports_; }
// Get a TransportProxy by content_name or transport. NULL if not found.
TransportProxy* GetTransportProxy(const std::string& content_name);
TransportProxy* GetTransportProxy(const Transport* transport);
TransportProxy* GetFirstTransportProxy();
void DestroyTransportProxy(const std::string& content_name);
// TransportProxy is owned by session. Return proxy just for convenience.
TransportProxy* GetOrCreateTransportProxy(const std::string& content_name);
@ -411,6 +409,10 @@ class BaseSession : public sigslot::has_slots<>,
// Fires the new description signal according to the current state.
virtual void SignalNewDescription();
// This method will delete the Transport and TransportChannelImpls
// and replace those with the Transport object of the first
// MediaContent in bundle_group.
bool BundleContentGroup(const ContentGroup* bundle_group);
private:
// Helper methods to push local and remote transport descriptions.
@ -423,12 +425,6 @@ class BaseSession : public sigslot::has_slots<>,
void MaybeCandidateAllocationDone();
// This method will delete the Transport and TransportChannelImpls and
// replace those with the selected Transport objects. Selection is done
// based on the content_name and in this case first MediaContent information
// is used for mux.
bool SetSelectedProxy(const std::string& content_name,
const ContentGroup* muxed_group);
// Log session state.
void LogState(State old_state, State new_state);