From b7446ed257b199ddc5fe20d96ba2edb6259ba22d Mon Sep 17 00:00:00 2001 From: Amit Hilbuch Date: Mon, 28 Jan 2019 12:25:25 -0800 Subject: [PATCH] Removing receive RIDs and Simulcast Layers. In the January 22nd 2019 WebRTC meeting it was agreed that an offer for sending (or receiving) simulcast should only contain the RIDs of the layers that are sent by the client. This change removes the complexity that was added to support sending and receiving the single layer (and RID) that are sent from the server. Bug: webrtc:10076 Change-Id: I8bae1336d5cb8ba2f91c5b62332dc69e67ddfd47 Reviewed-on: https://webrtc-review.googlesource.com/c/120242 Reviewed-by: Steve Anton Reviewed-by: Seth Hampson Commit-Queue: Amit Hilbuch Cr-Commit-Position: refs/heads/master@{#26432} --- pc/media_session.cc | 11 +---------- pc/media_session.h | 9 --------- pc/media_session_unittest.cc | 34 ++++------------------------------ pc/peer_connection.cc | 34 ++++++++++------------------------ pc/session_description.h | 15 --------------- pc/webrtc_sdp.cc | 25 +------------------------ pc/webrtc_sdp_unittest.cc | 33 --------------------------------- 7 files changed, 16 insertions(+), 145 deletions(-) diff --git a/pc/media_session.cc b/pc/media_session.cc index 2e2f0cdc36..e840b33fc1 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -472,19 +472,10 @@ static void AddSimulcastToMediaDescription( return; } - RTC_DCHECK(!media_description_options.receive_rids.empty()); - RTC_DCHECK(ValidateSimulcastLayers( - media_description_options.receive_rids, - media_description_options.receive_simulcast_layers)); - StreamParams receive_stream; - receive_stream.set_rids(media_description_options.receive_rids); - description->set_receive_stream(receive_stream); - + // Only negotiate the send layers. SimulcastDescription simulcast; simulcast.send_layers() = media_description_options.sender_options[0].simulcast_layers; - simulcast.receive_layers() = - media_description_options.receive_simulcast_layers; description->set_simulcast_description(simulcast); } diff --git a/pc/media_session.h b/pc/media_session.h index 538e2195bb..54fa7daec9 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -77,15 +77,6 @@ struct MediaDescriptionOptions { // Note: There's no equivalent "RtpReceiverOptions" because only send // stream information goes in the local descriptions. std::vector sender_options; - // |receive_rids| and |receive_simulcast_layers| are used with spec-compliant - // simulcast. When Simulcast is used, they should both not be empty. - // All RIDs in |receive_simulcast_layers| must appear in receive_rids as well. - // |receive_rids| could also be used outside of simulcast. It is possible to - // add restrictions on the incoming stream during negotiation outside the - // simulcast scenario. This is currently not fully supported, as meaningful - // restrictions are not handled by this library. - std::vector receive_rids; - SimulcastLayerList receive_simulcast_layers; private: // Doesn't DCHECK on |type|. diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index 8525d61525..d1a151f9c2 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -2032,9 +2032,7 @@ static void CheckSimulcastInSessionDescription( const SessionDescription* description, const std::string& content_name, const std::vector& send_rids, - const SimulcastLayerList& send_layers, - const RidDescription& receive_rid, - const SimulcastLayer& receive_layer) { + const SimulcastLayerList& send_layers) { ASSERT_NE(description, nullptr); const ContentInfo* content = description->GetContentByName(content_name); ASSERT_NE(content, nullptr); @@ -2049,22 +2047,12 @@ static void CheckSimulcastInSessionDescription( EXPECT_THAT(rids, Pointwise(RidDescriptionEquals(), send_rids)); - ASSERT_TRUE(cd->has_receive_stream()); - const StreamParams& receive_stream = cd->receive_stream(); - EXPECT_THAT(receive_stream.ssrcs, IsEmpty()); - EXPECT_TRUE(receive_stream.has_rids()); - ASSERT_THAT(receive_stream.rids(), SizeIs(1)); - - EXPECT_EQ(receive_rid.rid, receive_stream.rids()[0].rid); - EXPECT_EQ(receive_rid.direction, receive_stream.rids()[0].direction); - EXPECT_TRUE(cd->HasSimulcast()); const SimulcastDescription& simulcast = cd->simulcast_description(); EXPECT_THAT(simulcast.send_layers(), SizeIs(send_layers.size())); EXPECT_THAT(simulcast.send_layers(), Pointwise(Eq(), send_layers)); - ASSERT_THAT(simulcast.receive_layers().GetAllLayers(), SizeIs(1)); - EXPECT_EQ(receive_layer, simulcast.receive_layers().GetAllLayers()[0]); + ASSERT_THAT(simulcast.receive_layers().GetAllLayers(), SizeIs(0)); } // Create an offer with spec-compliant simulcast video stream. @@ -2073,11 +2061,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateCompliantSimulcastOffer) { AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video", RtpTransceiverDirection::kSendRecv, kActive, &opts); - RidDescription receive_rid("1", RidDirection::kReceive); - SimulcastLayer receive_layer(receive_rid.rid, false); - opts.media_description_options[0].receive_rids = {receive_rid}; - opts.media_description_options[0].receive_simulcast_layers.AddLayer( - receive_layer); std::vector send_rids; send_rids.push_back(RidDescription("f", RidDirection::kSend)); send_rids.push_back(RidDescription("h", RidDirection::kSend)); @@ -2092,8 +2075,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateCompliantSimulcastOffer) { std::unique_ptr offer = f1_.CreateOffer(opts, nullptr); CheckSimulcastInSessionDescription(offer.get(), "video", send_rids, - simulcast_layers, receive_rid, - receive_layer); + simulcast_layers); } // Create an offer that signals RIDs (not SSRCs) without Simulcast. @@ -2114,7 +2096,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestOfferWithRidsNoSimulcast) { ASSERT_NE(content, nullptr); const MediaContentDescription* cd = content->media_description(); ASSERT_NE(cd, nullptr); - EXPECT_FALSE(cd->has_receive_stream()); const StreamParamsVec& streams = cd->streams(); ASSERT_THAT(streams, SizeIs(1)); const StreamParams& stream = streams[0]; @@ -2140,11 +2121,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateCompliantSimulcastAnswer) { RtpTransceiverDirection::kSendRecv, kActive, &answer_opts); - RidDescription receive_rid("1", RidDirection::kReceive); - SimulcastLayer receive_layer(receive_rid.rid, false); - answer_opts.media_description_options[0].receive_rids = {receive_rid}; - answer_opts.media_description_options[0].receive_simulcast_layers.AddLayer( - receive_layer); std::vector rid_descriptions{ RidDescription("f", RidDirection::kSend), RidDescription("h", RidDirection::kSend), @@ -2161,8 +2137,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateCompliantSimulcastAnswer) { f2_.CreateAnswer(offer.get(), answer_opts, nullptr); CheckSimulcastInSessionDescription(answer.get(), "video", rid_descriptions, - simulcast_layers, receive_rid, - receive_layer); + simulcast_layers); } // Create an answer that signals RIDs (not SSRCs) without Simulcast. @@ -2197,7 +2172,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestAnswerWithRidsNoSimulcast) { ASSERT_NE(content, nullptr); const MediaContentDescription* cd = content->media_description(); ASSERT_NE(cd, nullptr); - EXPECT_FALSE(cd->has_receive_stream()); const StreamParamsVec& streams = cd->streams(); ASSERT_THAT(streams, SizeIs(1)); const StreamParams& stream = streams[0]; diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 33f4ce166a..393526d11b 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -4140,20 +4140,6 @@ void PeerConnection::GetOptionsForPlanBOffer( offer_answer_options.num_simulcast_layers); } -static void GetSimulcastInformationFromRtpParameters( - const RtpParameters& parameters, - RidDirection direction, - std::vector* rids, - SimulcastLayerList* layers) { - for (const RtpEncodingParameters& encoding : parameters.encodings) { - if (encoding.rid.empty()) { - continue; - } - rids->push_back(RidDescription(encoding.rid, direction)); - layers->AddLayer(SimulcastLayer(encoding.rid, !encoding.active)); - } -} - static cricket::MediaDescriptionOptions GetMediaDescriptionOptionsForTransceiver( rtc::scoped_refptr> @@ -4180,27 +4166,27 @@ GetMediaDescriptionOptionsForTransceiver( // The following sets up RIDs and Simulcast. // RIDs are included if Simulcast is requested or if any RID was specified. RtpParameters send_parameters = transceiver->sender()->GetParameters(); - RtpParameters receive_parameters = transceiver->receiver()->GetParameters(); bool has_rids = std::any_of(send_parameters.encodings.begin(), send_parameters.encodings.end(), [](const RtpEncodingParameters& encoding) { return !encoding.rid.empty(); }); - std::vector send_rids, receive_rids; - SimulcastLayerList send_layers, receive_layers; - GetSimulcastInformationFromRtpParameters(send_parameters, RidDirection::kSend, - &send_rids, &send_layers); - GetSimulcastInformationFromRtpParameters(receive_parameters, - RidDirection::kReceive, - &receive_rids, &receive_layers); + std::vector send_rids; + SimulcastLayerList send_layers; + for (const RtpEncodingParameters& encoding : send_parameters.encodings) { + if (encoding.rid.empty()) { + continue; + } + send_rids.push_back(RidDescription(encoding.rid, RidDirection::kSend)); + send_layers.AddLayer(SimulcastLayer(encoding.rid, !encoding.active)); + } + if (has_rids) { sender_options.rids = send_rids; } sender_options.simulcast_layers = send_layers; - media_description_options.receive_simulcast_layers = receive_layers; - media_description_options.receive_rids = receive_rids; // When RIDs are configured, we must set num_sim_layers to 0 to. // Otherwise, num_sim_layers must be 1 because either there is no // simulcast, or simulcast is acheived by munging the SDP. diff --git a/pc/session_description.h b/pc/session_description.h index 13da227a57..430cbdb04c 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -157,20 +157,6 @@ class MediaContentDescription { send_streams_.push_back(sp); } - // In Unified Plan (ex. Simulcast scenario) the receive stream might need - // to be specified in the media section description to allow specifying - // restrictions and identifying it within the session (see also RID). - const StreamParams& receive_stream() const { - RTC_DCHECK(has_receive_stream()); - return receive_stream_.value(); - } - - bool has_receive_stream() const { return receive_stream_.has_value(); } - - void set_receive_stream(const StreamParams& receive_stream) { - receive_stream_ = receive_stream; - } - // Sets the CNAME of all StreamParams if it have not been set. void SetCnameIfEmpty(const std::string& cname) { for (cricket::StreamParamsVec::iterator it = send_streams_.begin(); @@ -241,7 +227,6 @@ class MediaContentDescription { std::vector rtp_header_extensions_; bool rtp_header_extensions_set_ = false; StreamParamsVec send_streams_; - absl::optional receive_stream_; bool conference_mode_ = false; webrtc::RtpTransceiverDirection direction_ = webrtc::RtpTransceiverDirection::kSendRecv; diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 1b2ff158b4..97f2455d7a 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -1677,16 +1677,6 @@ void BuildRtpContentAttributes(const MediaContentDescription* media_desc, } } - if (media_desc->has_receive_stream()) { - const auto& rids = media_desc->receive_stream().rids(); - for (const RidDescription& rid_description : rids) { - InitAttrLine(kAttributeRid, &os); - os << kSdpDelimiterColon - << serializer.SerializeRidDescription(rid_description); - AddLine(os.str(), message); - } - } - // Simulcast (a=simulcast) // https://tools.ietf.org/html/draft-ietf-mmusic-sdp-simulcast-13#section-5.1 if (media_desc->HasSimulcast()) { @@ -3180,7 +3170,7 @@ bool ParseContent(const std::string& message, // If simulcast is specifed, split the rids into send and receive. // Rids that do not appear in simulcast attribute will be removed. // If it is not specified, we assume that all rids are for send layers. - std::vector send_rids, receive_rids; + std::vector send_rids; if (!simulcast.empty()) { // Verify that the rids in simulcast match rids in sdp. RemoveInvalidRidsFromSimulcast(rids, &simulcast); @@ -3197,12 +3187,6 @@ bool ParseContent(const std::string& message, send_rids.push_back(iter->second); } - for (const auto& layer : simulcast.receive_layers().GetAllLayers()) { - auto iter = rid_map.find(layer.rid); - RTC_DCHECK(iter != rid_map.end()); - receive_rids.push_back(iter->second); - } - media_desc->set_simulcast_description(simulcast); } else { send_rids = rids; @@ -3224,13 +3208,6 @@ bool ParseContent(const std::string& message, CreateTrackWithNoSsrcs(stream_ids, track_id, send_rids, &tracks); } - // Create receive track when we have incoming receive rids. - if (!receive_rids.empty()) { - StreamParams receive_track; - receive_track.set_rids(receive_rids); - media_desc->set_receive_stream(receive_track); - } - // Add the ssrc group to the track. for (const SsrcGroup& ssrc_group : ssrc_groups) { if (ssrc_group.ssrcs.empty()) { diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 7b1c50a6cd..654246e243 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -1410,10 +1410,6 @@ class WebRtcSdpTest : public testing::Test { // streams EXPECT_EQ(cd1->streams(), cd2->streams()); - EXPECT_EQ(cd1->has_receive_stream(), cd2->has_receive_stream()); - if (cd1->has_receive_stream() && cd2->has_receive_stream()) { - EXPECT_EQ(cd1->receive_stream(), cd2->receive_stream()); - } // extmap-allow-mixed EXPECT_EQ(cd1->extmap_allow_mixed_enum(), cd2->extmap_allow_mixed_enum()); @@ -4086,8 +4082,6 @@ TEST_F(WebRtcSdpTest, TestDeserializeSimulcastAttribute) { EXPECT_FALSE(media->streams().empty()); const std::vector& rids = media->streams()[0].rids(); CompareRidDescriptionIds(rids, {"1", "2", "3"}); - ASSERT_TRUE(media->has_receive_stream()); - CompareRidDescriptionIds(media->receive_stream().rids(), {"4", "5", "6"}); } // Validates that deserialization removes rids that do not appear in SDP @@ -4124,8 +4118,6 @@ TEST_F(WebRtcSdpTest, TestDeserializeSimulcastAttributeRemovesUnknownRids) { EXPECT_FALSE(media->streams().empty()); const std::vector& rids = media->streams()[0].rids(); CompareRidDescriptionIds(rids, {"1", "3"}); - ASSERT_TRUE(media->has_receive_stream()); - CompareRidDescriptionIds(media->receive_stream().rids(), {"4"}); } // Validates that Simulcast removes rids that appear in both send and receive. @@ -4153,8 +4145,6 @@ TEST_F(WebRtcSdpTest, EXPECT_FALSE(media->streams().empty()); const std::vector& rids = media->streams()[0].rids(); CompareRidDescriptionIds(rids, {"1", "3"}); - ASSERT_TRUE(media->has_receive_stream()); - CompareRidDescriptionIds(media->receive_stream().rids(), {"4"}); } // Ignores empty rid line. @@ -4180,7 +4170,6 @@ TEST_F(WebRtcSdpTest, TestDeserializeIgnoresEmptyRidLines) { EXPECT_FALSE(media->streams().empty()); const std::vector& rids = media->streams()[0].rids(); CompareRidDescriptionIds(rids, {"1", "2"}); - ASSERT_FALSE(media->has_receive_stream()); } // Ignores malformed rid lines. @@ -4207,7 +4196,6 @@ TEST_F(WebRtcSdpTest, TestDeserializeIgnoresMalformedRidLines) { EXPECT_FALSE(media->streams().empty()); const std::vector& rids = media->streams()[0].rids(); CompareRidDescriptionIds(rids, {"5"}); - ASSERT_FALSE(media->has_receive_stream()); } // Removes RIDs that specify a different format than the m= section. @@ -4234,8 +4222,6 @@ TEST_F(WebRtcSdpTest, TestDeserializeRemovesRidsWithInvalidCodec) { EXPECT_EQ("1", rids[0].rid); EXPECT_EQ(1ul, rids[0].payload_types.size()); EXPECT_EQ(120, rids[0].payload_types[0]); - - ASSERT_FALSE(media->has_receive_stream()); } // Ignores duplicate rid lines @@ -4263,8 +4249,6 @@ TEST_F(WebRtcSdpTest, TestDeserializeIgnoresDuplicateRidLines) { EXPECT_FALSE(media->streams().empty()); const std::vector& rids = media->streams()[0].rids(); CompareRidDescriptionIds(rids, {"1", "3"}); - ASSERT_TRUE(media->has_receive_stream()); - CompareRidDescriptionIds(media->receive_stream().rids(), {"4"}); } // Simulcast serialization integration test. @@ -4283,16 +4267,6 @@ TEST_F(WebRtcSdpTest, SerializeSimulcast_ComplexSerialization) { send_rids.push_back(RidDescription("3", RidDirection::kSend)); send_rids.push_back(RidDescription("4", RidDirection::kSend)); send_stream.set_rids(send_rids); - StreamParams recv_stream; - std::vector recv_rids; - recv_rids.push_back(RidDescription("6", RidDirection::kReceive)); - recv_rids.push_back(RidDescription("7", RidDirection::kReceive)); - recv_rids.push_back(RidDescription("8", RidDirection::kReceive)); - recv_rids.push_back(RidDescription("9", RidDirection::kReceive)); - recv_rids.push_back(RidDescription("10", RidDirection::kReceive)); - recv_rids.push_back(RidDescription("11", RidDirection::kReceive)); - recv_stream.set_rids(recv_rids); - media->set_receive_stream(recv_stream); SimulcastDescription& simulcast = media->simulcast_description(); simulcast.send_layers().AddLayerWithAlternatives( @@ -4300,13 +4274,6 @@ TEST_F(WebRtcSdpTest, SerializeSimulcast_ComplexSerialization) { simulcast.send_layers().AddLayerWithAlternatives( {SimulcastLayer("4", false), SimulcastLayer("3", false)}); - simulcast.receive_layers().AddLayerWithAlternatives( - {SimulcastLayer("6", false), SimulcastLayer("7", false)}); - simulcast.receive_layers().AddLayer(SimulcastLayer("8", true)); - simulcast.receive_layers().AddLayerWithAlternatives( - {SimulcastLayer("9", false), SimulcastLayer("10", true), - SimulcastLayer("11", false)}); - TestSerialize(jdesc_); }