Chromium Code Reviews

Unified Diff: webrtc/pc/webrtcsdp_unittest.cc

Issue 2995463002: Ignore "b=AS:-1" instead of treating as a hard error. (Closed)
Patch Set: Created 3 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
« webrtc/pc/webrtcsdp.cc ('K') | « webrtc/pc/webrtcsdp.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/pc/webrtcsdp_unittest.cc
diff --git a/webrtc/pc/webrtcsdp_unittest.cc b/webrtc/pc/webrtcsdp_unittest.cc
index 24b196f50e00c3ec7ba97a775ce3a389cad0b6a9..4a99279b1183f721cde6593ec94e5010c746f4b6 100644
--- a/webrtc/pc/webrtcsdp_unittest.cc
+++ b/webrtc/pc/webrtcsdp_unittest.cc
@@ -3343,19 +3343,26 @@ TEST_F(WebRtcSdpTest, DeserializeLargeBandwidthLimit) {
ExpectParseFailure(std::string(kSdpWithLargeBandwidth), "foo=fail");
}
-// Similar to the above, except that negative values are illegal, not just
-// error-prone as large values are.
-// https://bugs.chromium.org/p/chromium/issues/detail?id=675361
-TEST_F(WebRtcSdpTest, DeserializingNegativeBandwidthLimitFails) {
+// Negative values for b=AS should 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, NegativeBandwidthLimitIgnored) {
static const char kSdpWithNegativeBandwidth[] =
"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:-1000\r\n";
+ "b=AS:-1234\r\n";
- ExpectParseFailure(std::string(kSdpWithNegativeBandwidth), "b=AS:-1000");
+ JsepSessionDescription jdesc_output(kDummyString);
+ EXPECT_TRUE(SdpDeserialize(kSdpWithNegativeBandwidth, &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
« webrtc/pc/webrtcsdp.cc ('K') | « webrtc/pc/webrtcsdp.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine