Bug 1362581 - Don't offer to receive extensions that are send only on reoffer; r?bwc
Currently, when we switch roles after a ICE restart, we offer to receive any
extensions that the other side offered. This can lead to us offering to receive
an extension which is send only for Firefox. This change filters these extensions
out so that they do not appear in the offer.
MozReview-Commit-ID: LeaAuieoYih
--- a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
@@ -1425,16 +1425,52 @@ TEST_P(JsepSessionTest, DISABLED_Renegot
}
ASSERT_EQ(answererPairs.size(), newOffererPairs.size());
for (size_t i = 0; i < answererPairs.size(); ++i) {
ASSERT_TRUE(Equals(answererPairs[i], newOffererPairs[i]));
}
}
+// We don't want to offer to receive extensions that are send only.
+// This tests that we filter them out properly when we reoffer.
+// See Bug 1362581.
+TEST_F(JsepSessionTest, RenegotiationSwappedRolesCheckAudioLevelExt)
+{
+ AddTracks(*mSessionOff, "audio,video");
+ mSessionOff->AddAudioRtpExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level",
+ SdpDirectionAttribute::Direction::kSendonly);
+ std::string offer = CreateOffer();
+
+ SetLocalOffer(offer, CHECK_SUCCESS);
+ SetRemoteOffer(offer, CHECK_SUCCESS);
+
+ UniquePtr<Sdp> parsedOffer(Parse(offer));
+ auto& offerMediaAttrs = parsedOffer->GetMediaSection(0).GetAttributeList();
+ ASSERT_TRUE(offerMediaAttrs.HasAttribute(SdpAttribute::kExtmapAttribute));
+ auto& offerExtmap = offerMediaAttrs.GetExtmap().mExtmaps;
+ ASSERT_EQ(1U, offerExtmap.size());
+ ASSERT_EQ("urn:ietf:params:rtp-hdrext:ssrc-audio-level",
+ offerExtmap[0].extensionname);
+
+ std::string answer = CreateAnswer();
+ SetLocalAnswer(answer, CHECK_SUCCESS);
+ SetRemoteAnswer(answer, CHECK_SUCCESS);
+
+ UniquePtr<Sdp> parsedAnswer(Parse(answer));
+ auto& answerMediaAttrs = parsedAnswer->GetMediaSection(0).GetAttributeList();
+ ASSERT_FALSE(answerMediaAttrs.HasAttribute(SdpAttribute::kExtmapAttribute));
+
+ SwapOfferAnswerRoles();
+
+ std::string reoffer = CreateOffer();
+ UniquePtr<Sdp> parsedReoffer(Parse(reoffer));
+ auto& reofferMediaAttrs = parsedReoffer->GetMediaSection(0).GetAttributeList();
+ ASSERT_FALSE(reofferMediaAttrs.HasAttribute(SdpAttribute::kExtmapAttribute));
+}
TEST_P(JsepSessionTest, RenegotiationOffererAddsTrack)
{
AddTracks(*mSessionOff);
AddTracks(*mSessionAns);
OfferAnswer();
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -526,16 +526,30 @@ JsepSessionImpl::BindRemoteTracks(SdpMed
if (!track.mAssignedMLine.isSome()) {
MOZ_ASSERT(false);
continue;
}
auto& msection = sdp->GetMediaSection(*track.mAssignedMLine);
+ // Filter out extensions that are send only so we don't offer to
+ // receive them.
+ auto& attrs = msection.GetAttributeList();
+ if (attrs.HasAttribute(SdpAttribute::kExtmapAttribute)) {
+ UniquePtr<SdpExtmapAttributeList> new_list(new SdpExtmapAttributeList);
+ for (auto& ext : attrs.GetExtmap().mExtmaps) {
+ if (ext.direction != SdpDirectionAttribute::kSendonly) {
+ new_list->mExtmaps.push_back(ext);
+ }
+ }
+ attrs.RemoveAttribute(SdpAttribute::kExtmapAttribute);
+ attrs.SetAttribute(new_list.release());
+ }
+
if (mSdpHelper.MsectionIsDisabled(msection)) {
// TODO(bug 1095226) Content probably disabled this? Should we allow
// content to do this?
continue;
}
track.mTrack->AddToOffer(&msection);