Move SDP m= line matching from BaseChannel to WebRtcSession
This is part of the work towards implementing Unified Plan.
The logic for correlating m= lines to channels is changing in
Unified Plan. Moving this logic to WebRtcSession means that we do
not need to add a flag to BaseChannel to indicate which logic it
should use (i.e., Plan B vs. Unified Plan) and can keep those
details in WebRtcSession.
Bug: webrtc:8183
Change-Id: I729da73ece01fd20f45e82f8956a02c4cad2469e
Reviewed-on: https://chromium-review.googlesource.com/653490
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#19786}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 18ee1d55b4cdd7faef75464e39ec06568790adbcdiff --git a/pc/channel.cc b/pc/channel.cc
index d99a105..8e82925 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -111,13 +111,6 @@
return direction == MD_SENDRECV || direction == MD_SENDONLY;
}
-static const MediaContentDescription* GetContentDescription(
- const ContentInfo* cinfo) {
- if (cinfo == NULL)
- return NULL;
- return static_cast<const MediaContentDescription*>(cinfo->description);
-}
-
template <class Codec>
void RtpParametersFromMediaDescription(
const MediaContentDescriptionImpl<Codec>* desc,
@@ -746,34 +739,6 @@
}
}
-bool BaseChannel::PushdownLocalDescription(
- const SessionDescription* local_desc, ContentAction action,
- std::string* error_desc) {
- const ContentInfo* content_info = GetFirstContent(local_desc);
- const MediaContentDescription* content_desc =
- GetContentDescription(content_info);
- if (content_desc && content_info && !content_info->rejected &&
- !SetLocalContent(content_desc, action, error_desc)) {
- LOG(LS_ERROR) << "Failure in SetLocalContent with action " << action;
- return false;
- }
- return true;
-}
-
-bool BaseChannel::PushdownRemoteDescription(
- const SessionDescription* remote_desc, ContentAction action,
- std::string* error_desc) {
- const ContentInfo* content_info = GetFirstContent(remote_desc);
- const MediaContentDescription* content_desc =
- GetContentDescription(content_info);
- if (content_desc && content_info && !content_info->rejected &&
- !SetRemoteContent(content_desc, action, error_desc)) {
- LOG(LS_ERROR) << "Failure in SetRemoteContent with action " << action;
- return false;
- }
- return true;
-}
-
void BaseChannel::EnableMedia_w() {
RTC_DCHECK(worker_thread_ == rtc::Thread::Current());
if (enabled_)
@@ -1728,11 +1693,6 @@
LOG(LS_INFO) << "Changing voice state, recv=" << recv << " send=" << send;
}
-const ContentInfo* VoiceChannel::GetFirstContent(
- const SessionDescription* sdesc) {
- return GetFirstAudioContent(sdesc);
-}
-
bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
ContentAction action,
std::string* error_desc) {
@@ -2015,11 +1975,6 @@
}
}
-const ContentInfo* VideoChannel::GetFirstContent(
- const SessionDescription* sdesc) {
- return GetFirstVideoContent(sdesc);
-}
-
bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
ContentAction action,
std::string* error_desc) {
@@ -2201,11 +2156,6 @@
payload, result));
}
-const ContentInfo* RtpDataChannel::GetFirstContent(
- const SessionDescription* sdesc) {
- return GetFirstDataContent(sdesc);
-}
-
bool RtpDataChannel::CheckDataChannelTypeFromContent(
const DataContentDescription* content,
std::string* error_desc) {
diff --git a/pc/channel.h b/pc/channel.h
index b95bd52..4ff21fe 100644
--- a/pc/channel.h
+++ b/pc/channel.h
@@ -121,12 +121,6 @@
DtlsTransportInternal* rtcp_dtls_transport);
void SetTransports(rtc::PacketTransportInternal* rtp_packet_transport,
rtc::PacketTransportInternal* rtcp_packet_transport);
- bool PushdownLocalDescription(const SessionDescription* local_desc,
- ContentAction action,
- std::string* error_desc);
- bool PushdownRemoteDescription(const SessionDescription* remote_desc,
- ContentAction action,
- std::string* error_desc);
// Channel control
bool SetLocalContent(const MediaContentDescription* content,
ContentAction action,
@@ -302,9 +296,6 @@
void UpdateMediaSendRecvState();
virtual void UpdateMediaSendRecvState_w() = 0;
- // Gets the content info appropriate to the channel (audio or video).
- virtual const ContentInfo* GetFirstContent(
- const SessionDescription* sdesc) = 0;
bool UpdateLocalStreams_w(const std::vector<StreamParams>& streams,
ContentAction action,
std::string* error_desc);
@@ -509,7 +500,6 @@
rtc::CopyOnWriteBuffer* packet,
const rtc::PacketTime& packet_time) override;
void UpdateMediaSendRecvState_w() override;
- const ContentInfo* GetFirstContent(const SessionDescription* sdesc) override;
bool SetLocalContent_w(const MediaContentDescription* content,
ContentAction action,
std::string* error_desc) override;
@@ -589,7 +579,6 @@
private:
// overrides from BaseChannel
void UpdateMediaSendRecvState_w() override;
- const ContentInfo* GetFirstContent(const SessionDescription* sdesc) override;
bool SetLocalContent_w(const MediaContentDescription* content,
ContentAction action,
std::string* error_desc) override;
@@ -699,7 +688,6 @@
typedef rtc::TypedMessageData<bool> DataChannelReadyToSendMessageData;
// overrides from BaseChannel
- const ContentInfo* GetFirstContent(const SessionDescription* sdesc) override;
// Checks that data channel type is RTP.
bool CheckDataChannelTypeFromContent(const DataContentDescription* content,
std::string* error_desc);
diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc
index 5279683..b16e60d 100644
--- a/pc/channel_unittest.cc
+++ b/pc/channel_unittest.cc
@@ -511,17 +511,13 @@
// overridden in specialized classes
}
- // Creates a cricket::SessionDescription with one MediaContent and one stream.
+ // Creates a MediaContent with one stream.
// kPcmuCodec is used as audio codec and kH264Codec is used as video codec.
- cricket::SessionDescription* CreateSessionDescriptionWithStream(
- uint32_t ssrc) {
- typename T::Content content;
- cricket::SessionDescription* sdesc = new cricket::SessionDescription();
- CreateContent(SECURE, kPcmuCodec, kH264Codec, &content);
- AddLegacyStreamInContent(ssrc, 0, &content);
- sdesc->AddContent("DUMMY_CONTENT_NAME",
- cricket::NS_JINGLE_RTP, content.Copy());
- return sdesc;
+ typename T::Content* CreateMediaContentWithStream(uint32_t ssrc) {
+ typename T::Content* content = new typename T::Content();
+ CreateContent(SECURE, kPcmuCodec, kH264Codec, content);
+ AddLegacyStreamInContent(ssrc, 0, content);
+ return content;
}
// Will manage the lifetime of a CallThread, making sure it's
@@ -1829,41 +1825,39 @@
void TestSetContentFailure() {
CreateChannels(0, 0);
- auto sdesc = cricket::SessionDescription();
- sdesc.AddContent(cricket::CN_AUDIO, cricket::NS_JINGLE_RTP,
- new cricket::AudioContentDescription());
- sdesc.AddContent(cricket::CN_VIDEO, cricket::NS_JINGLE_RTP,
- new cricket::VideoContentDescription());
-
std::string err;
+ std::unique_ptr<typename T::Content> content(
+ CreateMediaContentWithStream(1));
+
media_channel1_->set_fail_set_recv_codecs(true);
- EXPECT_FALSE(channel1_->PushdownLocalDescription(
- &sdesc, cricket::CA_OFFER, &err));
- EXPECT_FALSE(channel1_->PushdownLocalDescription(
- &sdesc, cricket::CA_ANSWER, &err));
+ EXPECT_FALSE(
+ channel1_->SetLocalContent(content.get(), cricket::CA_OFFER, &err));
+ EXPECT_FALSE(
+ channel1_->SetLocalContent(content.get(), cricket::CA_ANSWER, &err));
media_channel1_->set_fail_set_send_codecs(true);
- EXPECT_FALSE(channel1_->PushdownRemoteDescription(
- &sdesc, cricket::CA_OFFER, &err));
+ EXPECT_FALSE(
+ channel1_->SetRemoteContent(content.get(), cricket::CA_OFFER, &err));
+
media_channel1_->set_fail_set_send_codecs(true);
- EXPECT_FALSE(channel1_->PushdownRemoteDescription(
- &sdesc, cricket::CA_ANSWER, &err));
+ EXPECT_FALSE(
+ channel1_->SetRemoteContent(content.get(), cricket::CA_ANSWER, &err));
}
void TestSendTwoOffers() {
CreateChannels(0, 0);
std::string err;
- std::unique_ptr<cricket::SessionDescription> sdesc1(
- CreateSessionDescriptionWithStream(1));
- EXPECT_TRUE(channel1_->PushdownLocalDescription(
- sdesc1.get(), cricket::CA_OFFER, &err));
+ std::unique_ptr<typename T::Content> content1(
+ CreateMediaContentWithStream(1));
+ EXPECT_TRUE(
+ channel1_->SetLocalContent(content1.get(), cricket::CA_OFFER, &err));
EXPECT_TRUE(media_channel1_->HasSendStream(1));
- std::unique_ptr<cricket::SessionDescription> sdesc2(
- CreateSessionDescriptionWithStream(2));
- EXPECT_TRUE(channel1_->PushdownLocalDescription(
- sdesc2.get(), cricket::CA_OFFER, &err));
+ std::unique_ptr<typename T::Content> content2(
+ CreateMediaContentWithStream(2));
+ EXPECT_TRUE(
+ channel1_->SetLocalContent(content2.get(), cricket::CA_OFFER, &err));
EXPECT_FALSE(media_channel1_->HasSendStream(1));
EXPECT_TRUE(media_channel1_->HasSendStream(2));
}
@@ -1872,16 +1866,16 @@
CreateChannels(0, 0);
std::string err;
- std::unique_ptr<cricket::SessionDescription> sdesc1(
- CreateSessionDescriptionWithStream(1));
- EXPECT_TRUE(channel1_->PushdownRemoteDescription(
- sdesc1.get(), cricket::CA_OFFER, &err));
+ std::unique_ptr<typename T::Content> content1(
+ CreateMediaContentWithStream(1));
+ EXPECT_TRUE(
+ channel1_->SetRemoteContent(content1.get(), cricket::CA_OFFER, &err));
EXPECT_TRUE(media_channel1_->HasRecvStream(1));
- std::unique_ptr<cricket::SessionDescription> sdesc2(
- CreateSessionDescriptionWithStream(2));
- EXPECT_TRUE(channel1_->PushdownRemoteDescription(
- sdesc2.get(), cricket::CA_OFFER, &err));
+ std::unique_ptr<typename T::Content> content2(
+ CreateMediaContentWithStream(2));
+ EXPECT_TRUE(
+ channel1_->SetRemoteContent(content2.get(), cricket::CA_OFFER, &err));
EXPECT_FALSE(media_channel1_->HasRecvStream(1));
EXPECT_TRUE(media_channel1_->HasRecvStream(2));
}
@@ -1891,25 +1885,25 @@
std::string err;
// Receive offer
- std::unique_ptr<cricket::SessionDescription> sdesc1(
- CreateSessionDescriptionWithStream(1));
- EXPECT_TRUE(channel1_->PushdownRemoteDescription(
- sdesc1.get(), cricket::CA_OFFER, &err));
+ std::unique_ptr<typename T::Content> content1(
+ CreateMediaContentWithStream(1));
+ EXPECT_TRUE(
+ channel1_->SetRemoteContent(content1.get(), cricket::CA_OFFER, &err));
EXPECT_TRUE(media_channel1_->HasRecvStream(1));
// Send PR answer
- std::unique_ptr<cricket::SessionDescription> sdesc2(
- CreateSessionDescriptionWithStream(2));
- EXPECT_TRUE(channel1_->PushdownLocalDescription(
- sdesc2.get(), cricket::CA_PRANSWER, &err));
+ std::unique_ptr<typename T::Content> content2(
+ CreateMediaContentWithStream(2));
+ EXPECT_TRUE(
+ channel1_->SetLocalContent(content2.get(), cricket::CA_PRANSWER, &err));
EXPECT_TRUE(media_channel1_->HasRecvStream(1));
EXPECT_TRUE(media_channel1_->HasSendStream(2));
// Send answer
- std::unique_ptr<cricket::SessionDescription> sdesc3(
- CreateSessionDescriptionWithStream(3));
- EXPECT_TRUE(channel1_->PushdownLocalDescription(
- sdesc3.get(), cricket::CA_ANSWER, &err));
+ std::unique_ptr<typename T::Content> content3(
+ CreateMediaContentWithStream(3));
+ EXPECT_TRUE(
+ channel1_->SetLocalContent(content3.get(), cricket::CA_ANSWER, &err));
EXPECT_TRUE(media_channel1_->HasRecvStream(1));
EXPECT_FALSE(media_channel1_->HasSendStream(2));
EXPECT_TRUE(media_channel1_->HasSendStream(3));
@@ -1920,25 +1914,25 @@
std::string err;
// Send offer
- std::unique_ptr<cricket::SessionDescription> sdesc1(
- CreateSessionDescriptionWithStream(1));
- EXPECT_TRUE(channel1_->PushdownLocalDescription(
- sdesc1.get(), cricket::CA_OFFER, &err));
+ std::unique_ptr<typename T::Content> content1(
+ CreateMediaContentWithStream(1));
+ EXPECT_TRUE(
+ channel1_->SetLocalContent(content1.get(), cricket::CA_OFFER, &err));
EXPECT_TRUE(media_channel1_->HasSendStream(1));
// Receive PR answer
- std::unique_ptr<cricket::SessionDescription> sdesc2(
- CreateSessionDescriptionWithStream(2));
- EXPECT_TRUE(channel1_->PushdownRemoteDescription(
- sdesc2.get(), cricket::CA_PRANSWER, &err));
+ std::unique_ptr<typename T::Content> content2(
+ CreateMediaContentWithStream(2));
+ EXPECT_TRUE(channel1_->SetRemoteContent(content2.get(),
+ cricket::CA_PRANSWER, &err));
EXPECT_TRUE(media_channel1_->HasSendStream(1));
EXPECT_TRUE(media_channel1_->HasRecvStream(2));
// Receive answer
- std::unique_ptr<cricket::SessionDescription> sdesc3(
- CreateSessionDescriptionWithStream(3));
- EXPECT_TRUE(channel1_->PushdownRemoteDescription(
- sdesc3.get(), cricket::CA_ANSWER, &err));
+ std::unique_ptr<typename T::Content> content3(
+ CreateMediaContentWithStream(3));
+ EXPECT_TRUE(
+ channel1_->SetRemoteContent(content3.get(), cricket::CA_ANSWER, &err));
EXPECT_TRUE(media_channel1_->HasSendStream(1));
EXPECT_FALSE(media_channel1_->HasRecvStream(2));
EXPECT_TRUE(media_channel1_->HasRecvStream(3));
diff --git a/pc/webrtcsession.cc b/pc/webrtcsession.cc
index b423663..c78dbc4 100644
--- a/pc/webrtcsession.cc
+++ b/pc/webrtcsession.cc
@@ -847,6 +847,21 @@
return true;
}
+// TODO(steveanton): Eventually it'd be nice to store the channels as a single
+// vector of BaseChannel pointers instead of separate voice and video channel
+// vectors. At that point, this will become a simple getter.
+std::vector<cricket::BaseChannel*> WebRtcSession::Channels() const {
+ std::vector<cricket::BaseChannel*> channels;
+ channels.insert(channels.end(), voice_channels_.begin(),
+ voice_channels_.end());
+ channels.insert(channels.end(), video_channels_.begin(),
+ video_channels_.end());
+ if (rtp_data_channel_) {
+ channels.push_back(rtp_data_channel_.get());
+ }
+ return channels;
+}
+
void WebRtcSession::LogState(State old_state, State new_state) {
LOG(LS_INFO) << "Session:" << id()
<< " Old state:" << GetStateString(old_state)
@@ -953,30 +968,40 @@
cricket::ContentAction action,
cricket::ContentSource source,
std::string* err) {
- auto set_content = [this, action, source, err](cricket::BaseChannel* ch) {
- if (!ch) {
- return true;
- } else if (source == cricket::CS_LOCAL) {
- return ch->PushdownLocalDescription(local_description()->description(),
- action, err);
- } else {
- return ch->PushdownRemoteDescription(remote_description()->description(),
- action, err);
+ const SessionDescription* sdesc =
+ (source == cricket::CS_LOCAL ? local_description() : remote_description())
+ ->description();
+ RTC_DCHECK(sdesc);
+ bool all_success = true;
+ for (auto* channel : Channels()) {
+ // TODO(steveanton): Add support for multiple channels of the same type.
+ const ContentInfo* content_info =
+ cricket::GetFirstMediaContent(sdesc->contents(), channel->media_type());
+ if (!content_info) {
+ continue;
}
- };
-
- bool ret = (set_content(voice_channel()) && set_content(video_channel()) &&
- set_content(rtp_data_channel()));
+ const MediaContentDescription* content_desc =
+ static_cast<const MediaContentDescription*>(content_info->description);
+ if (content_desc && !content_info->rejected) {
+ bool success = (source == cricket::CS_LOCAL)
+ ? channel->SetLocalContent(content_desc, action, err)
+ : channel->SetRemoteContent(content_desc, action, err);
+ if (!success) {
+ all_success = false;
+ break;
+ }
+ }
+ }
// Need complete offer/answer with an SCTP m= section before starting SCTP,
// according to https://tools.ietf.org/html/draft-ietf-mmusic-sctp-sdp-19
if (sctp_transport_ && local_description() && remote_description() &&
cricket::GetFirstDataContent(local_description()->description()) &&
cricket::GetFirstDataContent(remote_description()->description())) {
- ret &= network_thread_->Invoke<bool>(
+ all_success &= network_thread_->Invoke<bool>(
RTC_FROM_HERE,
rtc::Bind(&WebRtcSession::PushdownSctpParameters_n, this, source));
}
- return ret;
+ return all_success;
}
bool WebRtcSession::PushdownSctpParameters_n(cricket::ContentSource source) {
diff --git a/pc/webrtcsession.h b/pc/webrtcsession.h
index 4d52a5c..686ac1e 100644
--- a/pc/webrtcsession.h
+++ b/pc/webrtcsession.h
@@ -393,6 +393,9 @@
kAnswer,
};
+ // Return all managed, non-null channels.
+ std::vector<cricket::BaseChannel*> Channels() const;
+
// Non-const versions of local_description()/remote_description(), for use
// internally.
SessionDescriptionInterface* mutable_local_description() {
