Bug 1325991 - sections with bundle-only should have port set to 0. r=drno
Now that Chrome release is bundle-aware, let's reapply the patch to
properly emit port 0 for m-lines in sections with the bundle-only
attribute.
MozReview-Commit-ID: 8RftSXIzIpD
--- a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
@@ -819,31 +819,36 @@ protected:
}
}
void CheckDefaultRtpCandidate(bool expectDefault,
const SdpMediaSection& msection,
size_t transportLevel,
const std::string& context) const
{
+ Address expectedAddress = "0.0.0.0";
+ Port expectedPort = 9U;
+
if (expectDefault) {
// Copy so we can be terse and use []
auto defaultCandidates = mDefaultCandidates;
- ASSERT_EQ(defaultCandidates[transportLevel][RTP].first,
- msection.GetConnection().GetAddress())
- << context << " (level " << msection.GetLevel() << ")";
- ASSERT_EQ(defaultCandidates[transportLevel][RTP].second,
- msection.GetPort())
- << context << " (level " << msection.GetLevel() << ")";
- } else {
- ASSERT_EQ("0.0.0.0", msection.GetConnection().GetAddress())
- << context << " (level " << msection.GetLevel() << ")";
- ASSERT_EQ(9U, msection.GetPort())
- << context << " (level " << msection.GetLevel() << ")";
+ expectedAddress = defaultCandidates[transportLevel][RTP].first;
+ expectedPort = defaultCandidates[transportLevel][RTP].second;
}
+
+ // if bundle-only attribute is present, expect port 0
+ const SdpAttributeList& attrs = msection.GetAttributeList();
+ if (attrs.HasAttribute(SdpAttribute::kBundleOnlyAttribute)) {
+ expectedPort = 0U;
+ }
+
+ ASSERT_EQ(expectedAddress, msection.GetConnection().GetAddress())
+ << context << " (level " << msection.GetLevel() << ")";
+ ASSERT_EQ(expectedPort, msection.GetPort())
+ << context << " (level " << msection.GetLevel() << ")";
}
void CheckDefaultRtcpCandidate(bool expectDefault,
const SdpMediaSection& msection,
size_t transportLevel,
const std::string& context) const
{
if (expectDefault) {
@@ -1069,21 +1074,25 @@ private:
auto& msection = sdp->GetMediaSection(i);
if (msection.GetMediaType() == SdpMediaSection::kApplication) {
ASSERT_EQ(SdpMediaSection::kDtlsSctp, msection.GetProtocol());
} else {
ASSERT_EQ(SdpMediaSection::kUdpTlsRtpSavpf, msection.GetProtocol());
}
- if (msection.GetPort() == 0) {
+ const SdpAttributeList& attrs = msection.GetAttributeList();
+ bool bundle_only = attrs.HasAttribute(SdpAttribute::kBundleOnlyAttribute);
+
+ // port 0 only means disabled when the bundle-only attribute is missing
+ if (!bundle_only && msection.GetPort() == 0) {
ValidateDisabledMSection(&msection);
continue;
}
- const SdpAttributeList& attrs = msection.GetAttributeList();
+
ASSERT_EQ(source.mIceUfrag, attrs.GetIceUfrag());
ASSERT_EQ(source.mIcePwd, attrs.GetIcePwd());
const SdpFingerprintAttributeList& fps = attrs.GetFingerprint();
for (auto fp = fps.mFingerprints.begin(); fp != fps.mFingerprints.end();
++fp) {
std::string alg_str = "None";
if (fp->hashFunc == SdpFingerprintAttributeList::kSha1) {
@@ -4248,20 +4257,22 @@ TEST_P(JsepSessionTest, TestMaxBundle)
std::string offer = mSessionOff.GetLocalDescription();
SipccSdpParser parser;
UniquePtr<Sdp> parsedOffer = parser.Parse(offer);
ASSERT_TRUE(parsedOffer.get());
ASSERT_FALSE(
parsedOffer->GetMediaSection(0).GetAttributeList().HasAttribute(
SdpAttribute::kBundleOnlyAttribute));
+ ASSERT_NE(0U, parsedOffer->GetMediaSection(0).GetPort());
for (size_t i = 1; i < parsedOffer->GetMediaSectionCount(); ++i) {
ASSERT_TRUE(
parsedOffer->GetMediaSection(i).GetAttributeList().HasAttribute(
SdpAttribute::kBundleOnlyAttribute));
+ ASSERT_EQ(0U, parsedOffer->GetMediaSection(i).GetPort());
}
CheckPairs(mSessionOff, "Offerer pairs");
CheckPairs(mSessionAns, "Answerer pairs");
EXPECT_EQ(1U, GetActiveTransportCount(mSessionOff));
EXPECT_EQ(1U, GetActiveTransportCount(mSessionAns));
}
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -662,16 +662,18 @@ JsepSessionImpl::SetupBundle(Sdp* sdp) c
// m-section
useBundleOnly = !mids.empty();
break;
}
if (useBundleOnly) {
attrs.SetAttribute(
new SdpFlagAttribute(SdpAttribute::kBundleOnlyAttribute));
+ // Set port to 0 for sections with bundle-only attribute. (mjf)
+ sdp->GetMediaSection(i).SetPort(0);
}
mids.push_back(attrs.GetMid());
}
}
if (mids.size() > 1) {
UniquePtr<SdpGroupAttributeList> groupAttr(new SdpGroupAttributeList);
--- a/media/webrtc/signaling/src/sdp/SdpHelper.cpp
+++ b/media/webrtc/signaling/src/sdp/SdpHelper.cpp
@@ -419,24 +419,29 @@ SdpHelper::SetDefaultAddresses(const std
void
SdpHelper::SetDefaultAddresses(const std::string& defaultCandidateAddr,
uint16_t defaultCandidatePort,
const std::string& defaultRtcpCandidateAddr,
uint16_t defaultRtcpCandidatePort,
SdpMediaSection* msection)
{
msection->GetConnection().SetAddress(defaultCandidateAddr);
- msection->SetPort(defaultCandidatePort);
+ SdpAttributeList& attrList = msection->GetAttributeList();
+
+ // only set the port if there is no bundle-only attribute
+ if (!attrList.HasAttribute(SdpAttribute::kBundleOnlyAttribute)) {
+ msection->SetPort(defaultCandidatePort);
+ }
if (!defaultRtcpCandidateAddr.empty()) {
sdp::AddrType ipVersion = sdp::kIPv4;
if (defaultRtcpCandidateAddr.find(':') != std::string::npos) {
ipVersion = sdp::kIPv6;
}
- msection->GetAttributeList().SetAttribute(new SdpRtcpAttribute(
+ attrList.SetAttribute(new SdpRtcpAttribute(
defaultRtcpCandidatePort,
sdp::kInternet,
ipVersion,
defaultRtcpCandidateAddr));
}
}
nsresult