diff --git a/p2p/base/sessiondescription.h b/p2p/base/sessiondescription.h index 565722c519..e7e6d89346 100644 --- a/p2p/base/sessiondescription.h +++ b/p2p/base/sessiondescription.h @@ -182,12 +182,8 @@ class SessionDescription { }; // Indicates whether a ContentDescription was an offer or an answer, as -// described in http://www.ietf.org/rfc/rfc3264.txt. CA_UPDATE -// indicates a jingle update message which contains a subset of a full -// session description -enum ContentAction { - CA_OFFER, CA_PRANSWER, CA_ANSWER, CA_UPDATE -}; +// described in http://www.ietf.org/rfc/rfc3264.txt. +enum ContentAction { CA_OFFER, CA_PRANSWER, CA_ANSWER }; // Indicates whether a ContentDescription was sent by the local client // or received from the remote client. diff --git a/pc/channel.cc b/pc/channel.cc index 00aab7ff0b..ab7cde3e5e 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -117,8 +117,8 @@ void RtpParametersFromMediaDescription( const RtpHeaderExtensions& extensions, RtpParameters* params) { // TODO(pthatcher): Remove this once we're sure no one will give us - // a description without codecs (currently a CA_UPDATE with just - // streams can). + // a description without codecs. Currently the ORTC implementation is relying + // on this. if (desc->has_codecs()) { params->codecs = desc->codecs(); } @@ -950,11 +950,6 @@ bool BaseChannel::SetRtpTransportParameters( ContentSource src, const RtpHeaderExtensions& extensions, std::string* error_desc) { - if (action == CA_UPDATE) { - // These parameters never get changed by a CA_UDPATE. - return true; - } - std::vector encrypted_extension_ids; for (const webrtc::RtpExtension& extension : extensions) { if (extension.encrypt) { @@ -1031,10 +1026,6 @@ bool BaseChannel::SetSrtp_n(const std::vector& cryptos, const std::vector& encrypted_extension_ids, std::string* error_desc) { TRACE_EVENT0("webrtc", "BaseChannel::SetSrtp_w"); - if (action == CA_UPDATE) { - // no crypto params. - return true; - } bool ret = false; bool dtls = false; ret = CheckSrtpConfig_n(cryptos, &dtls, error_desc); @@ -1162,10 +1153,6 @@ bool BaseChannel::SetRtcpMux_n(bool enable, UpdateWritableState_n(); } break; - case CA_UPDATE: - // No RTCP mux info. - ret = true; - break; default: break; } @@ -1200,43 +1187,9 @@ bool BaseChannel::RemoveRecvStream_w(uint32_t ssrc) { bool BaseChannel::UpdateLocalStreams_w(const std::vector& streams, ContentAction action, std::string* error_desc) { - if (!(action == CA_OFFER || action == CA_ANSWER || - action == CA_PRANSWER || action == CA_UPDATE)) + if (!(action == CA_OFFER || action == CA_ANSWER || action == CA_PRANSWER)) return false; - // If this is an update, streams only contain streams that have changed. - if (action == CA_UPDATE) { - for (StreamParamsVec::const_iterator it = streams.begin(); - it != streams.end(); ++it) { - const StreamParams* existing_stream = - GetStreamByIds(local_streams_, it->groupid, it->id); - if (!existing_stream && it->has_ssrcs()) { - if (media_channel()->AddSendStream(*it)) { - local_streams_.push_back(*it); - RTC_LOG(LS_INFO) << "Add send stream ssrc: " << it->first_ssrc(); - } else { - std::ostringstream desc; - desc << "Failed to add send stream ssrc: " << it->first_ssrc(); - SafeSetError(desc.str(), error_desc); - return false; - } - } else if (existing_stream && !it->has_ssrcs()) { - if (!media_channel()->RemoveSendStream(existing_stream->first_ssrc())) { - std::ostringstream desc; - desc << "Failed to remove send stream with ssrc " - << it->first_ssrc() << "."; - SafeSetError(desc.str(), error_desc); - return false; - } - RemoveStreamBySsrc(&local_streams_, existing_stream->first_ssrc()); - } else { - RTC_LOG(LS_WARNING) << "Ignore unsupported stream update"; - } - } - return true; - } - // Else streams are all the streams we want to send. - // Check for streams that have been removed. bool ret = true; for (StreamParamsVec::const_iterator it = local_streams_.begin(); @@ -1273,46 +1226,9 @@ bool BaseChannel::UpdateRemoteStreams_w( const std::vector& streams, ContentAction action, std::string* error_desc) { - if (!(action == CA_OFFER || action == CA_ANSWER || - action == CA_PRANSWER || action == CA_UPDATE)) + if (!(action == CA_OFFER || action == CA_ANSWER || action == CA_PRANSWER)) return false; - // If this is an update, streams only contain streams that have changed. - if (action == CA_UPDATE) { - for (StreamParamsVec::const_iterator it = streams.begin(); - it != streams.end(); ++it) { - const StreamParams* existing_stream = - GetStreamByIds(remote_streams_, it->groupid, it->id); - if (!existing_stream && it->has_ssrcs()) { - if (AddRecvStream_w(*it)) { - remote_streams_.push_back(*it); - RTC_LOG(LS_INFO) << "Add remote stream ssrc: " << it->first_ssrc(); - } else { - std::ostringstream desc; - desc << "Failed to add remote stream ssrc: " << it->first_ssrc(); - SafeSetError(desc.str(), error_desc); - return false; - } - } else if (existing_stream && !it->has_ssrcs()) { - if (!RemoveRecvStream_w(existing_stream->first_ssrc())) { - std::ostringstream desc; - desc << "Failed to remove remote stream with ssrc " - << it->first_ssrc() << "."; - SafeSetError(desc.str(), error_desc); - return false; - } - RemoveStreamBySsrc(&remote_streams_, existing_stream->first_ssrc()); - } else { - RTC_LOG(LS_WARNING) - << "Ignore unsupported stream update." - << " Stream exists? " << (existing_stream != nullptr) - << " new stream = " << it->ToString(); - } - } - return true; - } - // Else streams are all the streams we want to receive. - // Check for streams that have been removed. bool ret = true; for (StreamParamsVec::const_iterator it = remote_streams_.begin(); @@ -2225,9 +2141,8 @@ bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content, return false; } - // If the remote data doesn't have codecs and isn't an update, it - // must be empty, so ignore it. - if (!data->has_codecs() && action != CA_UPDATE) { + // If the remote data doesn't have codecs, it must be empty, so ignore it. + if (!data->has_codecs()) { return true; } diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 36d7b0026e..52cb419495 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -29,7 +29,6 @@ using cricket::CA_OFFER; using cricket::CA_PRANSWER; using cricket::CA_ANSWER; -using cricket::CA_UPDATE; using cricket::DtlsTransportInternal; using cricket::FakeVoiceMediaChannel; using cricket::StreamParams; @@ -43,7 +42,6 @@ const cricket::VideoCodec kH264SvcCodec(99, "H264-SVC"); const cricket::DataCodec kGoogleDataCodec(101, "google-data"); const uint32_t kSsrc1 = 0x1111; const uint32_t kSsrc2 = 0x2222; -const uint32_t kSsrc3 = 0x3333; const int kAudioPts[] = {0, 8}; const int kVideoPts[] = {97, 99}; enum class NetworkIsWorker { Yes, No }; @@ -651,35 +649,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_EQ(0, rtcp_mux_activated_callbacks2_); } - // Test that SetRemoteContent properly deals with a content update. - void TestSetRemoteContentUpdate() { - CreateChannels(0, 0); - typename T::Content content; - CreateContent(RTCP_MUX | SECURE, kPcmuCodec, kH264Codec, &content); - EXPECT_EQ(0U, media_channel1_->codecs().size()); - EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); - ASSERT_EQ(1U, media_channel1_->codecs().size()); - EXPECT_TRUE(CodecMatches(content.codecs()[0], - media_channel1_->codecs()[0])); - // Now update with other codecs. - typename T::Content update_content; - update_content.set_partial(true); - CreateContent(0, kIsacCodec, kH264SvcCodec, - &update_content); - EXPECT_TRUE(channel1_->SetRemoteContent(&update_content, CA_UPDATE, NULL)); - ASSERT_EQ(1U, media_channel1_->codecs().size()); - EXPECT_TRUE(CodecMatches(update_content.codecs()[0], - media_channel1_->codecs()[0])); - // Now update without any codecs. This is ignored. - typename T::Content empty_content; - empty_content.set_partial(true); - EXPECT_TRUE(channel1_->SetRemoteContent(&empty_content, CA_UPDATE, NULL)); - ASSERT_EQ(1U, media_channel1_->codecs().size()); - EXPECT_TRUE(CodecMatches(update_content.codecs()[0], - media_channel1_->codecs()[0])); - } - // Test that Add/RemoveStream properly forward to the media channel. void TestStreams() { CreateChannels(0, 0); @@ -692,141 +661,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_EQ(0U, media_channel1_->recv_streams().size()); } - // Test that SetLocalContent properly handles adding and removing StreamParams - // to the local content description. - // This test uses the CA_UPDATE action that don't require a full - // MediaContentDescription to do an update. - void TestUpdateStreamsInLocalContent() { - cricket::StreamParams stream1; - stream1.groupid = "group1"; - stream1.id = "stream1"; - stream1.ssrcs.push_back(kSsrc1); - stream1.cname = "stream1_cname"; - - cricket::StreamParams stream2; - stream2.groupid = "group2"; - stream2.id = "stream2"; - stream2.ssrcs.push_back(kSsrc2); - stream2.cname = "stream2_cname"; - - cricket::StreamParams stream3; - stream3.groupid = "group3"; - stream3.id = "stream3"; - stream3.ssrcs.push_back(kSsrc3); - stream3.cname = "stream3_cname"; - - CreateChannels(0, 0); - typename T::Content content1; - CreateContent(0, kPcmuCodec, kH264Codec, &content1); - content1.AddStream(stream1); - EXPECT_EQ(0u, media_channel1_->send_streams().size()); - EXPECT_TRUE(channel1_->SetLocalContent(&content1, CA_OFFER, NULL)); - - ASSERT_EQ(1u, media_channel1_->send_streams().size()); - EXPECT_EQ(stream1, media_channel1_->send_streams()[0]); - - // Update the local streams by adding another sending stream. - // Use a partial updated session description. - typename T::Content content2; - content2.AddStream(stream2); - content2.AddStream(stream3); - content2.set_partial(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content2, CA_UPDATE, NULL)); - ASSERT_EQ(3u, media_channel1_->send_streams().size()); - EXPECT_EQ(stream1, media_channel1_->send_streams()[0]); - EXPECT_EQ(stream2, media_channel1_->send_streams()[1]); - EXPECT_EQ(stream3, media_channel1_->send_streams()[2]); - - // Update the local streams by removing the first sending stream. - // This is done by removing all SSRCS for this particular stream. - typename T::Content content3; - stream1.ssrcs.clear(); - content3.AddStream(stream1); - content3.set_partial(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content3, CA_UPDATE, NULL)); - ASSERT_EQ(2u, media_channel1_->send_streams().size()); - EXPECT_EQ(stream2, media_channel1_->send_streams()[0]); - EXPECT_EQ(stream3, media_channel1_->send_streams()[1]); - - // Update the local streams with a stream that does not change. - // THe update is ignored. - typename T::Content content4; - content4.AddStream(stream2); - content4.set_partial(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content4, CA_UPDATE, NULL)); - ASSERT_EQ(2u, media_channel1_->send_streams().size()); - EXPECT_EQ(stream2, media_channel1_->send_streams()[0]); - EXPECT_EQ(stream3, media_channel1_->send_streams()[1]); - } - - // Test that SetRemoteContent properly handles adding and removing - // StreamParams to the remote content description. - // This test uses the CA_UPDATE action that don't require a full - // MediaContentDescription to do an update. - void TestUpdateStreamsInRemoteContent() { - cricket::StreamParams stream1; - stream1.id = "Stream1"; - stream1.groupid = "1"; - stream1.ssrcs.push_back(kSsrc1); - stream1.cname = "stream1_cname"; - - cricket::StreamParams stream2; - stream2.id = "Stream2"; - stream2.groupid = "2"; - stream2.ssrcs.push_back(kSsrc2); - stream2.cname = "stream2_cname"; - - cricket::StreamParams stream3; - stream3.id = "Stream3"; - stream3.groupid = "3"; - stream3.ssrcs.push_back(kSsrc3); - stream3.cname = "stream3_cname"; - - CreateChannels(0, 0); - typename T::Content content1; - CreateContent(0, kPcmuCodec, kH264Codec, &content1); - content1.AddStream(stream1); - EXPECT_EQ(0u, media_channel1_->recv_streams().size()); - EXPECT_TRUE(channel1_->SetRemoteContent(&content1, CA_OFFER, NULL)); - - ASSERT_EQ(1u, media_channel1_->codecs().size()); - ASSERT_EQ(1u, media_channel1_->recv_streams().size()); - EXPECT_EQ(stream1, media_channel1_->recv_streams()[0]); - - // Update the remote streams by adding another sending stream. - // Use a partial updated session description. - typename T::Content content2; - content2.AddStream(stream2); - content2.AddStream(stream3); - content2.set_partial(true); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_UPDATE, NULL)); - ASSERT_EQ(3u, media_channel1_->recv_streams().size()); - EXPECT_EQ(stream1, media_channel1_->recv_streams()[0]); - EXPECT_EQ(stream2, media_channel1_->recv_streams()[1]); - EXPECT_EQ(stream3, media_channel1_->recv_streams()[2]); - - // Update the remote streams by removing the first stream. - // This is done by removing all SSRCS for this particular stream. - typename T::Content content3; - stream1.ssrcs.clear(); - content3.AddStream(stream1); - content3.set_partial(true); - EXPECT_TRUE(channel1_->SetRemoteContent(&content3, CA_UPDATE, NULL)); - ASSERT_EQ(2u, media_channel1_->recv_streams().size()); - EXPECT_EQ(stream2, media_channel1_->recv_streams()[0]); - EXPECT_EQ(stream3, media_channel1_->recv_streams()[1]); - - // Update the remote streams with a stream that does not change. - // The update is ignored. - typename T::Content content4; - content4.AddStream(stream2); - content4.set_partial(true); - EXPECT_TRUE(channel1_->SetRemoteContent(&content4, CA_UPDATE, NULL)); - ASSERT_EQ(2u, media_channel1_->recv_streams().size()); - EXPECT_EQ(stream2, media_channel1_->recv_streams()[0]); - EXPECT_EQ(stream3, media_channel1_->recv_streams()[1]); - } - // Test that SetLocalContent and SetRemoteContent properly // handles adding and removing StreamParams when the action is a full // CA_OFFER / CA_ANSWER. @@ -2317,22 +2151,10 @@ TEST_F(VoiceChannelSingleThreadTest, TestSetContentsRtcpMuxWithPrAnswer) { Base::TestSetContentsRtcpMux(); } -TEST_F(VoiceChannelSingleThreadTest, TestSetRemoteContentUpdate) { - Base::TestSetRemoteContentUpdate(); -} - TEST_F(VoiceChannelSingleThreadTest, TestStreams) { Base::TestStreams(); } -TEST_F(VoiceChannelSingleThreadTest, TestUpdateStreamsInLocalContent) { - Base::TestUpdateStreamsInLocalContent(); -} - -TEST_F(VoiceChannelSingleThreadTest, TestUpdateRemoteStreamsInContent) { - Base::TestUpdateStreamsInRemoteContent(); -} - TEST_F(VoiceChannelSingleThreadTest, TestChangeStreamParamsInContent) { Base::TestChangeStreamParamsInContent(); } @@ -2680,22 +2502,10 @@ TEST_F(VoiceChannelDoubleThreadTest, TestSetContentsRtcpMuxWithPrAnswer) { Base::TestSetContentsRtcpMux(); } -TEST_F(VoiceChannelDoubleThreadTest, TestSetRemoteContentUpdate) { - Base::TestSetRemoteContentUpdate(); -} - TEST_F(VoiceChannelDoubleThreadTest, TestStreams) { Base::TestStreams(); } -TEST_F(VoiceChannelDoubleThreadTest, TestUpdateStreamsInLocalContent) { - Base::TestUpdateStreamsInLocalContent(); -} - -TEST_F(VoiceChannelDoubleThreadTest, TestUpdateRemoteStreamsInContent) { - Base::TestUpdateStreamsInRemoteContent(); -} - TEST_F(VoiceChannelDoubleThreadTest, TestChangeStreamParamsInContent) { Base::TestChangeStreamParamsInContent(); } @@ -3041,22 +2851,10 @@ TEST_F(VideoChannelSingleThreadTest, TestSetContentsRtcpMuxWithPrAnswer) { Base::TestSetContentsRtcpMux(); } -TEST_F(VideoChannelSingleThreadTest, TestSetRemoteContentUpdate) { - Base::TestSetRemoteContentUpdate(); -} - TEST_F(VideoChannelSingleThreadTest, TestStreams) { Base::TestStreams(); } -TEST_F(VideoChannelSingleThreadTest, TestUpdateStreamsInLocalContent) { - Base::TestUpdateStreamsInLocalContent(); -} - -TEST_F(VideoChannelSingleThreadTest, TestUpdateRemoteStreamsInContent) { - Base::TestUpdateStreamsInRemoteContent(); -} - TEST_F(VideoChannelSingleThreadTest, TestChangeStreamParamsInContent) { Base::TestChangeStreamParamsInContent(); } @@ -3276,22 +3074,10 @@ TEST_F(VideoChannelDoubleThreadTest, TestSetContentsRtcpMuxWithPrAnswer) { Base::TestSetContentsRtcpMux(); } -TEST_F(VideoChannelDoubleThreadTest, TestSetRemoteContentUpdate) { - Base::TestSetRemoteContentUpdate(); -} - TEST_F(VideoChannelDoubleThreadTest, TestStreams) { Base::TestStreams(); } -TEST_F(VideoChannelDoubleThreadTest, TestUpdateStreamsInLocalContent) { - Base::TestUpdateStreamsInLocalContent(); -} - -TEST_F(VideoChannelDoubleThreadTest, TestUpdateRemoteStreamsInContent) { - Base::TestUpdateStreamsInRemoteContent(); -} - TEST_F(VideoChannelDoubleThreadTest, TestChangeStreamParamsInContent) { Base::TestChangeStreamParamsInContent(); } @@ -3584,22 +3370,10 @@ TEST_F(RtpDataChannelSingleThreadTest, TestSetContentsRtcpMux) { Base::TestSetContentsRtcpMux(); } -TEST_F(RtpDataChannelSingleThreadTest, TestSetRemoteContentUpdate) { - Base::TestSetRemoteContentUpdate(); -} - TEST_F(RtpDataChannelSingleThreadTest, TestStreams) { Base::TestStreams(); } -TEST_F(RtpDataChannelSingleThreadTest, TestUpdateStreamsInLocalContent) { - Base::TestUpdateStreamsInLocalContent(); -} - -TEST_F(RtpDataChannelSingleThreadTest, TestUpdateRemoteStreamsInContent) { - Base::TestUpdateStreamsInRemoteContent(); -} - TEST_F(RtpDataChannelSingleThreadTest, TestChangeStreamParamsInContent) { Base::TestChangeStreamParamsInContent(); } @@ -3720,22 +3494,10 @@ TEST_F(RtpDataChannelDoubleThreadTest, TestSetContentsRtcpMux) { Base::TestSetContentsRtcpMux(); } -TEST_F(RtpDataChannelDoubleThreadTest, TestSetRemoteContentUpdate) { - Base::TestSetRemoteContentUpdate(); -} - TEST_F(RtpDataChannelDoubleThreadTest, TestStreams) { Base::TestStreams(); } -TEST_F(RtpDataChannelDoubleThreadTest, TestUpdateStreamsInLocalContent) { - Base::TestUpdateStreamsInLocalContent(); -} - -TEST_F(RtpDataChannelDoubleThreadTest, TestUpdateRemoteStreamsInContent) { - Base::TestUpdateStreamsInRemoteContent(); -} - TEST_F(RtpDataChannelDoubleThreadTest, TestChangeStreamParamsInContent) { Base::TestChangeStreamParamsInContent(); }