Remove special handling of -1 in bandwidth modifiers This is nonstandard, and appears unused. Bug: webrtc:502249086 Change-Id: I5b35b43e8ca1d55802e377511d6afa151459bbe4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/485820 Commit-Queue: Harald Alvestrand <hta@webrtc.org> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48095}
diff --git a/api/webrtc_sdp.cc b/api/webrtc_sdp.cc index 42e0650..895a5cb 100644 --- a/api/webrtc_sdp.cc +++ b/api/webrtc_sdp.cc
@@ -2700,9 +2700,7 @@ ReportSdpBandwidth(kSdpBandwidthParseFailure); return false; } - if (b == -1) { - ReportSdpBandwidth(kSdpBandwidthNegativeOne); - } else if (b == 0) { + if (b == 0) { ReportSdpBandwidth(kSdpBandwidthZero); } else if (b > 0 && b <= INT_MAX / 1000) { ReportSdpBandwidth(kSdpBandwidthSmall); @@ -2711,16 +2709,6 @@ } else { ReportSdpBandwidth(kSdpBandwidthNegative); } - // 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 (bandwidth_type == kApplicationSpecificBandwidth && b == -1) { - RTC_LOG(LS_WARNING) << "Ignoring \"b=AS:-1\"; will be treated as \"no " - "bandwidth limit\"."; - continue; - } if (b < 0) { return ParseFailed( *line, "b=" + bandwidth_type + " value can't be negative.", error);
diff --git a/api/webrtc_sdp_unittest.cc b/api/webrtc_sdp_unittest.cc index eb3a1d1..f1cf117 100644 --- a/api/webrtc_sdp_unittest.cc +++ b/api/webrtc_sdp_unittest.cc
@@ -4021,29 +4021,6 @@ 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"; - - std::unique_ptr<SessionDescriptionInterface> jdesc_output = - SdpDeserialize(kSdpWithBandwidthOfNegativeOne); - ASSERT_THAT(jdesc_output, NotNull()); - const VideoContentDescription* vcd = - GetFirstVideoContentDescription(jdesc_output->description()); - ASSERT_THAT(vcd, NotNull()); - EXPECT_EQ(kAutoBandwidth, vcd->bandwidth()); -} - TEST_F(WebRtcSdpTest, SdpBandwidthMetrics) { metrics::Reset(); auto get_sdp = [](absl::string_view value) { @@ -4057,10 +4034,11 @@ return sb.Release(); }; - // kSdpBandwidthNegativeOne + // This used to be kSdpBandwidthNegativeOne, but special + // treatment is removed. SdpDeserialize(get_sdp("-1")); EXPECT_METRIC_EQ(1, metrics::NumEvents("WebRTC.PeerConnection.SdpBandwidth", - kSdpBandwidthNegativeOne)); + kSdpBandwidthNegative)); // kSdpBandwidthZero SdpDeserialize(get_sdp("0")); @@ -4079,7 +4057,7 @@ // kSdpBandwidthNegative SdpDeserialize(get_sdp("-1000")); - EXPECT_METRIC_EQ(1, metrics::NumEvents("WebRTC.PeerConnection.SdpBandwidth", + EXPECT_METRIC_EQ(2, metrics::NumEvents("WebRTC.PeerConnection.SdpBandwidth", kSdpBandwidthNegative)); // kSdpBandwidthParseFailure