diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 0596869eba..c361468c68 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -2150,9 +2150,8 @@ RTCError PeerConnection::ApplyRemoteDescription( transceiver->Stop(); } if (!content->rejected && !media_desc->streams().empty()) { - const auto& stream = content->media_description()->streams()[0]; transceiver->internal()->receiver_internal()->SetupMediaChannel( - stream.first_ssrc()); + media_desc->streams()[0].first_ssrc()); } } for (auto event : track_events) { @@ -2410,8 +2409,13 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, if (!transceiver) { auto sender = CreateSender(media_desc->type(), nullptr, {rtc::CreateRandomUuid()}); - auto receiver = - CreateReceiver(media_desc->type(), media_desc->streams()[0].id); + std::string receiver_id; + if (!media_desc->streams().empty()) { + receiver_id = media_desc->streams()[0].id; + } else { + receiver_id = rtc::CreateRandomUuid(); + } + auto receiver = CreateReceiver(media_desc->type(), receiver_id); transceiver = CreateAndAddTransceiver(sender, receiver); transceiver->internal()->set_direction( RtpTransceiverDirection::kRecvOnly); @@ -3304,13 +3308,22 @@ GetMediaDescriptionOptionsForTransceiver( cricket::MediaDescriptionOptions media_description_options( transceiver->internal()->media_type(), mid, transceiver->direction(), transceiver->stopped()); - cricket::SenderOptions sender_options; - sender_options.track_id = transceiver->sender()->id(); - sender_options.stream_ids = transceiver->sender()->stream_ids(); - // TODO(bugs.webrtc.org/7600): Set num_sim_layers to the number of encodings - // set in the RTP parameters when the transceiver was added. - sender_options.num_sim_layers = 1; - media_description_options.sender_options.push_back(sender_options); + // This behavior is specified in JSEP. The gist is that: + // 1. The MSID is included if the RtpTransceiver's direction is sendonly or + // sendrecv. + // 2. If the MSID is included, then it must be included in any subsequent + // offer/answer exactly the same until the RtpTransceiver is stopped. + if (!transceiver->stopped() && + (RtpTransceiverDirectionHasSend(transceiver->direction()) || + transceiver->internal()->has_ever_been_used_to_send())) { + cricket::SenderOptions sender_options; + sender_options.track_id = transceiver->sender()->id(); + sender_options.stream_ids = transceiver->sender()->stream_ids(); + // TODO(bugs.webrtc.org/7600): Set num_sim_layers to the number of encodings + // set in the RTP parameters when the transceiver was added. + sender_options.num_sim_layers = 1; + media_description_options.sender_options.push_back(sender_options); + } return media_description_options; } diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc index 092b3c74b9..737b0c96f7 100644 --- a/pc/peerconnection_jsep_unittest.cc +++ b/pc/peerconnection_jsep_unittest.cc @@ -913,6 +913,104 @@ TEST_F(PeerConnectionJsepTest, EXPECT_NE(kTrackId, streams[0].id); } +// Test that if the transceiver is recvonly or inactive, then no MSID +// information is included in the offer. +TEST_F(PeerConnectionJsepTest, NoMsidInOfferIfTransceiverDirectionHasNoSend) { + auto caller = CreatePeerConnection(); + + RtpTransceiverInit init_recvonly; + init_recvonly.direction = RtpTransceiverDirection::kRecvOnly; + ASSERT_TRUE(caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init_recvonly)); + + RtpTransceiverInit init_inactive; + init_inactive.direction = RtpTransceiverDirection::kInactive; + ASSERT_TRUE(caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init_inactive)); + + auto offer = caller->CreateOffer(); + auto contents = offer->description()->contents(); + ASSERT_EQ(2u, contents.size()); + // MSID is specified in the first stream, so no streams means no MSID. + EXPECT_EQ(0u, contents[0].media_description()->streams().size()); + EXPECT_EQ(0u, contents[1].media_description()->streams().size()); +} + +// Test that if an answer negotiates transceiver directions of recvonly or +// inactive, then no MSID information is included in the answer. +TEST_F(PeerConnectionJsepTest, NoMsidInAnswerIfNoRespondingTracks) { + auto caller = CreatePeerConnection(); + auto callee = CreatePeerConnection(); + + // recvonly transceiver will get negotiated to inactive since the callee has + // no tracks to send in response. + RtpTransceiverInit init_recvonly; + init_recvonly.direction = RtpTransceiverDirection::kRecvOnly; + caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init_recvonly); + + // sendrecv transceiver will get negotiated to recvonly since the callee has + // no tracks to send in response. + RtpTransceiverInit init_sendrecv; + init_sendrecv.direction = RtpTransceiverDirection::kSendRecv; + caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init_sendrecv); + + ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + + auto answer = callee->CreateAnswer(); + auto contents = answer->description()->contents(); + ASSERT_EQ(2u, contents.size()); + // MSID is specified in the first stream, so no streams means no MSID. + EXPECT_EQ(0u, contents[0].media_description()->streams().size()); + EXPECT_EQ(0u, contents[1].media_description()->streams().size()); +} + +// Test that the MSID is included even if the transceiver direction has changed +// to inactive if the transceiver had previously sent media. +TEST_F(PeerConnectionJsepTest, IncludeMsidEvenIfDirectionHasChanged) { + auto caller = CreatePeerConnection(); + caller->AddAudioTrack("audio"); + auto callee = CreatePeerConnection(); + callee->AddAudioTrack("audio"); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + caller->pc()->GetTransceivers()[0]->SetDirection( + RtpTransceiverDirection::kInactive); + + // The transceiver direction on both sides will turn to inactive. + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + auto* offer = callee->pc()->remote_description(); + auto offer_contents = offer->description()->contents(); + ASSERT_EQ(1u, offer_contents.size()); + // MSID is specified in the first stream. If it is present, assume that MSID + // is there. + EXPECT_EQ(1u, offer_contents[0].media_description()->streams().size()); + + auto* answer = caller->pc()->remote_description(); + auto answer_contents = answer->description()->contents(); + ASSERT_EQ(1u, answer_contents.size()); + EXPECT_EQ(1u, answer_contents[0].media_description()->streams().size()); +} + +// Test that stopping a RtpTransceiver will cause future offers to not include +// any MSID information for that section. +TEST_F(PeerConnectionJsepTest, RemoveMsidIfTransceiverStopped) { + auto caller = CreatePeerConnection(); + auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + auto callee = CreatePeerConnection(); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + transceiver->Stop(); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + auto* offer = callee->pc()->remote_description(); + auto offer_contents = offer->description()->contents(); + ASSERT_EQ(1u, offer_contents.size()); + // MSID is specified in the first stream, so no streams means no MSID. + EXPECT_EQ(0u, offer_contents[0].media_description()->streams().size()); +} + // Test that the callee RtpReceiver created by a call to SetRemoteDescription // has its ID set to the signaled track ID. TEST_F(PeerConnectionJsepTest,