Ignore "b=AS:-1" instead of treating as a hard error.
Follow up to https://codereview.webrtc.org/2989243002/.
It turns out that "b=AS:-1" was being used to mean "no bandwidth limit",
even though just omitting "b=AS" completely will do that. So we should
treat this as a soft error for now, and give applications time to
transition to doing the standard thing.
BUG=chromium:675361
Review-Url: https://codereview.webrtc.org/2995463002
Cr-Commit-Position: refs/heads/master@{#19244}
diff --git a/webrtc/pc/webrtcsdp.cc b/webrtc/pc/webrtcsdp.cc
index bc2ec89..e514fe7 100644
--- a/webrtc/pc/webrtcsdp.cc
+++ b/webrtc/pc/webrtcsdp.cc
@@ -2719,6 +2719,16 @@
if (!GetValueFromString(line, bandwidth, &b, error)) {
return false;
}
+ // TODO(deadbeef): Historically, applications may be setting a value
+ // of -1 to mean "unset any previously set bandwidth limit", even
+ // though ommitting the "b=AS" entirely will do just that. Once we've
+ // transitioned applications to doing the right thing, it would be
+ // better to treat this as a hard error instead of just ignoring it.
+ if (b == -1) {
+ LOG(LS_WARNING) << "Ignoring \"b=AS:-1\"; will be treated as \"no "
+ "bandwidth limit\".";
+ continue;
+ }
if (b < 0) {
return ParseFailed(line, "b=AS value can't be negative.", error);
}
diff --git a/webrtc/pc/webrtcsdp_unittest.cc b/webrtc/pc/webrtcsdp_unittest.cc
index 24b196f..56066cf 100644
--- a/webrtc/pc/webrtcsdp_unittest.cc
+++ b/webrtc/pc/webrtcsdp_unittest.cc
@@ -3358,6 +3358,29 @@
ExpectParseFailure(std::string(kSdpWithNegativeBandwidth), "b=AS:-1000");
}
+// An exception to the above rule: a value of -1 for b=AS should just be
+// ignored, resulting in "kAutoBandwidth" in the deserialized object.
+// Applications historically may be using "b=AS:-1" to mean "no bandwidth
+// limit", but this is now what ommitting the attribute entirely will do, so
+// ignoring it will have the intended effect.
+TEST_F(WebRtcSdpTest, BandwidthLimitOfNegativeOneIgnored) {
+ static const char kSdpWithBandwidthOfNegativeOne[] =
+ "v=0\r\n"
+ "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "m=video 3457 RTP/SAVPF 120\r\n"
+ "b=AS:-1\r\n";
+
+ JsepSessionDescription jdesc_output(kDummyString);
+ EXPECT_TRUE(SdpDeserialize(kSdpWithBandwidthOfNegativeOne, &jdesc_output));
+ const ContentInfo* vc = GetFirstVideoContent(jdesc_output.description());
+ ASSERT_NE(nullptr, vc);
+ const VideoContentDescription* vcd =
+ static_cast<const VideoContentDescription*>(vc->description);
+ EXPECT_EQ(cricket::kAutoBandwidth, vcd->bandwidth());
+}
+
// Test that "ufrag"/"pwd" in the candidate line itself are ignored, and only
// the "a=ice-ufrag"/"a=ice-pwd" attributes are used.
// Regression test for:
