admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add test for m-line recycling

Browse Source 
which adds test coverage for the offer path.
Removes a DCHECK which is no longer required as the error
is handled in the individual handlers.

BUG=webrtc:15471

Change-Id: I982d517a313cd84574c57974e9d8390a6b78012c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/321840
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#40928}
This commit is contained in:
Philipp Hancke 2023-09-28 09:44:06 +02:00 committed by WebRTC LUCI CQ
parent 08d431ec34
commit 635061b65e
2 changed files with 43 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
pc/media_session.cc
View File

@ -1672,9 +1672,6 @@ MediaSessionDescriptionFactory::CreateOfferOrError(
msection_index < current_description->contents().size()) {
current_content = &current_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 +

42
pc/sdp_offer_answer_unittest.cc
View File

@ -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));