diff --git a/pc/media_session.cc b/pc/media_session.cc index 2dee09608c..d58689633b 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -1672,9 +1672,6 @@ MediaSessionDescriptionFactory::CreateOfferOrError( msection_index < current_description->contents().size()) { current_content = ¤t_description->contents()[msection_index]; // Media type must match unless this media section is being recycled. - RTC_DCHECK(current_content->name != media_description_options.mid || - IsMediaContentOfType(current_content, - media_description_options.type)); } RTCError error; switch (media_description_options.type) { @@ -2284,8 +2281,7 @@ RTCError MediaSessionDescriptionFactory::AddAudioContentForOffer( if (current_content && !current_content->rejected && current_content->name == media_description_options.mid) { if (!IsMediaContentOfType(current_content, MEDIA_TYPE_AUDIO)) { - // TODO(bugs.webrtc.org/15471): add a unit test for this since - // it is not clear how this can happen for offers. + // Can happen if the remote side re-uses a MID while recycling. LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Media type for content with mid='" + current_content->name + @@ -2384,8 +2380,7 @@ RTCError MediaSessionDescriptionFactory::AddVideoContentForOffer( if (current_content && !current_content->rejected && current_content->name == media_description_options.mid) { if (!IsMediaContentOfType(current_content, MEDIA_TYPE_VIDEO)) { - // TODO(bugs.webrtc.org/15471): add a unit test for this since - // it is not clear how this can happen for offers. + // Can happen if the remote side re-uses a MID while recycling. LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Media type for content with mid='" + current_content->name + diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc index 448f85a3e2..94ceff10ac 100644 --- a/pc/sdp_offer_answer_unittest.cc +++ b/pc/sdp_offer_answer_unittest.cc @@ -1015,7 +1015,7 @@ class SdpOfferAnswerShuffleMediaTypes }; TEST_P(SdpOfferAnswerShuffleMediaTypes, - RecyclingWithDifferentKindAndSameMidFails) { + RecyclingWithDifferentKindAndSameMidFailsAnswer) { bool audio_first = GetParam(); auto pc1 = CreatePeerConnection(); auto pc2 = CreatePeerConnection(); @@ -1053,6 +1053,46 @@ TEST_P(SdpOfferAnswerShuffleMediaTypes, EXPECT_FALSE(pc1->SetLocalDescription(std::move(answer))); } +// Similar to the previous test but with implicit rollback and creating +// an offer, triggering a different codepath. +TEST_P(SdpOfferAnswerShuffleMediaTypes, + RecyclingWithDifferentKindAndSameMidFailsOffer) { + bool audio_first = GetParam(); + auto pc1 = CreatePeerConnection(); + auto pc2 = CreatePeerConnection(); + if (audio_first) { + pc1->AddAudioTrack("audio_track", {}); + pc2->AddVideoTrack("video_track", {}); + } else { + pc2->AddAudioTrack("audio_track", {}); + pc1->AddVideoTrack("video_track", {}); + } + + auto initial_offer = pc1->CreateOfferAndSetAsLocal(); + ASSERT_EQ(initial_offer->description()->contents().size(), 1u); + auto mid1 = initial_offer->description()->contents()[0].mid(); + std::string rejected_answer_sdp = + "v=0\r\n" + "o=- 8621259572628890423 2 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "m=" + + std::string(audio_first ? "audio" : "video") + + " 0 UDP/TLS/RTP/SAVPF 111\r\n" + "c=IN IP4 0.0.0.0\r\n"; + auto rejected_answer = + CreateSessionDescription(SdpType::kAnswer, rejected_answer_sdp); + EXPECT_TRUE(pc1->SetRemoteDescription(std::move(rejected_answer))); + + auto offer = + pc2->CreateOfferAndSetAsLocal(); // This will generate a mid=0 too + ASSERT_EQ(offer->description()->contents().size(), 1u); + auto mid2 = offer->description()->contents()[0].mid(); + EXPECT_EQ(mid1, mid2); // Check that the mids collided. + EXPECT_TRUE(pc1->SetRemoteDescription(std::move(offer))); + EXPECT_FALSE(pc1->CreateOffer()); +} + INSTANTIATE_TEST_SUITE_P(SdpOfferAnswerShuffleMediaTypes, SdpOfferAnswerShuffleMediaTypes, ::testing::Values(true, false));