diff --git a/media/base/media_constants.cc b/media/base/media_constants.cc index a918078dd8..bb3403f892 100644 --- a/media/base/media_constants.cc +++ b/media/base/media_constants.cc @@ -119,4 +119,8 @@ const int kDefaultVideoMaxFramerate = 60; const size_t kConferenceMaxNumSpatialLayers = 3; const size_t kConferenceMaxNumTemporalLayers = 3; const size_t kConferenceDefaultNumTemporalLayers = 3; + +// RFC 3556 and RFC 3890 +const char kApplicationSpecificBandwidth[] = "AS"; +const char kTransportSpecificBandwidth[] = "TIAS"; } // namespace cricket diff --git a/media/base/media_constants.h b/media/base/media_constants.h index 5579b6e00c..4528167de1 100644 --- a/media/base/media_constants.h +++ b/media/base/media_constants.h @@ -145,6 +145,9 @@ extern const int kDefaultVideoMaxFramerate; extern const size_t kConferenceMaxNumSpatialLayers; extern const size_t kConferenceMaxNumTemporalLayers; extern const size_t kConferenceDefaultNumTemporalLayers; + +extern const char kApplicationSpecificBandwidth[]; +extern const char kTransportSpecificBandwidth[]; } // namespace cricket #endif // MEDIA_BASE_MEDIA_CONSTANTS_H_ diff --git a/pc/session_description.h b/pc/session_description.h index 3405accbf3..53c981a346 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -26,6 +26,7 @@ #include "api/rtp_parameters.h" #include "api/rtp_transceiver_interface.h" #include "media/base/media_channel.h" +#include "media/base/media_constants.h" #include "media/base/stream_params.h" #include "p2p/base/transport_description.h" #include "p2p/base/transport_info.h" @@ -126,6 +127,10 @@ class MediaContentDescription { virtual int bandwidth() const { return bandwidth_; } virtual void set_bandwidth(int bandwidth) { bandwidth_ = bandwidth; } + virtual std::string bandwidth_type() const { return bandwidth_type_; } + virtual void set_bandwidth_type(std::string bandwidth_type) { + bandwidth_type_ = bandwidth_type; + } virtual const std::vector& cryptos() const { return cryptos_; } virtual void AddCrypto(const CryptoParams& params) { @@ -251,6 +256,7 @@ class MediaContentDescription { bool rtcp_reduced_size_ = false; bool remote_estimate_ = false; int bandwidth_ = kAutoBandwidth; + std::string bandwidth_type_ = kApplicationSpecificBandwidth; std::string protocol_; std::vector cryptos_; std::vector rtp_header_extensions_; diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index af584791be..aa98e480ef 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -55,9 +55,11 @@ using cricket::ContentInfo; using cricket::CryptoParams; using cricket::ICE_CANDIDATE_COMPONENT_RTCP; using cricket::ICE_CANDIDATE_COMPONENT_RTP; +using cricket::kApplicationSpecificBandwidth; using cricket::kCodecParamMaxPTime; using cricket::kCodecParamMinPTime; using cricket::kCodecParamPTime; +using cricket::kTransportSpecificBandwidth; using cricket::MediaContentDescription; using cricket::MediaProtocolType; using cricket::MediaType; @@ -224,8 +226,6 @@ static const char kMediaPortRejected[] = "0"; // Use IPV4 per default. static const char kDummyAddress[] = "0.0.0.0"; static const char kDummyPort[] = "9"; -// RFC 3556 -static const char kApplicationSpecificMaximum[] = "AS"; static const char kDefaultSctpmapProtocol[] = "webrtc-datachannel"; @@ -1436,10 +1436,18 @@ void BuildMediaDescription(const ContentInfo* content_info, AddLine(os.str(), message); // RFC 4566 - // b=AS: - if (media_desc->bandwidth() >= 1000) { - InitLine(kLineTypeSessionBandwidth, kApplicationSpecificMaximum, &os); - os << kSdpDelimiterColon << (media_desc->bandwidth() / 1000); + // b=AS: or + // b=TIAS: + int bandwidth = media_desc->bandwidth(); + std::string bandwidth_type = media_desc->bandwidth_type(); + if (bandwidth_type == kApplicationSpecificBandwidth && bandwidth >= 1000) { + InitLine(kLineTypeSessionBandwidth, bandwidth_type, &os); + bandwidth /= 1000; + os << kSdpDelimiterColon << bandwidth; + AddLine(os.str(), message); + } else if (bandwidth_type == kTransportSpecificBandwidth && bandwidth > 0) { + InitLine(kLineTypeSessionBandwidth, bandwidth_type, &os); + os << kSdpDelimiterColon << bandwidth; AddLine(os.str(), message); } @@ -2983,46 +2991,61 @@ bool ParseContent(const std::string& message, // b=* (zero or more bandwidth information lines) if (IsLineType(line, kLineTypeSessionBandwidth)) { std::string bandwidth; - if (HasAttribute(line, kApplicationSpecificMaximum)) { - if (!GetValue(line, kApplicationSpecificMaximum, &bandwidth, error)) { + std::string bandwidth_type; + if (HasAttribute(line, kApplicationSpecificBandwidth)) { + if (!GetValue(line, kApplicationSpecificBandwidth, &bandwidth, error)) { return false; - } else { - int b = 0; - 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) { - RTC_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); - } - // We should never use more than the default bandwidth for RTP-based - // data channels. Don't allow SDP to set the bandwidth, because - // that would give JS the opportunity to "break the Internet". - // See: https://code.google.com/p/chromium/issues/detail?id=280726 - if (media_type == cricket::MEDIA_TYPE_DATA && - cricket::IsRtpProtocol(protocol) && - b > cricket::kDataMaxBandwidth / 1000) { - rtc::StringBuilder description; - description << "RTP-based data channels may not send more than " - << cricket::kDataMaxBandwidth / 1000 << "kbps."; - return ParseFailed(line, description.str(), error); - } - // Prevent integer overflow. - b = std::min(b, INT_MAX / 1000); - media_desc->set_bandwidth(b * 1000); } + bandwidth_type = kApplicationSpecificBandwidth; + } else if (HasAttribute(line, kTransportSpecificBandwidth)) { + if (!GetValue(line, kTransportSpecificBandwidth, &bandwidth, error)) { + return false; + } + bandwidth_type = kTransportSpecificBandwidth; + } else { + continue; } - continue; + int b = 0; + 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 (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); + } + // We should never use more than the default bandwidth for RTP-based + // data channels. Don't allow SDP to set the bandwidth, because + // that would give JS the opportunity to "break the Internet". + // See: https://code.google.com/p/chromium/issues/detail?id=280726 + // Disallow TIAS since it shouldn't be generated for RTP data channels in + // the first place and provides another way to get around the limitation. + if (media_type == cricket::MEDIA_TYPE_DATA && + cricket::IsRtpProtocol(protocol) && + (b > cricket::kDataMaxBandwidth / 1000 || + bandwidth_type == kTransportSpecificBandwidth)) { + rtc::StringBuilder description; + description << "RTP-based data channels may not send more than " + << cricket::kDataMaxBandwidth / 1000 << "kbps."; + return ParseFailed(line, description.str(), error); + } + // Convert values. Prevent integer overflow. + if (bandwidth_type == kApplicationSpecificBandwidth) { + b = std::min(b, INT_MAX / 1000) * 1000; + } else { + b = std::min(b, INT_MAX); + } + media_desc->set_bandwidth(b); + media_desc->set_bandwidth_type(bandwidth_type); } // Parse the media level connection data. diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 7b83c86ab1..2a4c36d61f 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -2189,16 +2189,31 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithBundle) { TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithBandwidth) { VideoContentDescription* vcd = GetFirstVideoContentDescription(&desc_); - vcd->set_bandwidth(100 * 1000); + vcd->set_bandwidth(100 * 1000 + 755); // Integer division will drop the 755. + vcd->set_bandwidth_type("AS"); AudioContentDescription* acd = GetFirstAudioContentDescription(&desc_); - acd->set_bandwidth(50 * 1000); + acd->set_bandwidth(555); + acd->set_bandwidth_type("TIAS"); ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), jdesc_.session_version())); std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_bandwidth = kSdpFullString; InjectAfter("c=IN IP4 74.125.224.39\r\n", "b=AS:100\r\n", &sdp_with_bandwidth); - InjectAfter("c=IN IP4 74.125.127.126\r\n", "b=AS:50\r\n", + InjectAfter("c=IN IP4 74.125.127.126\r\n", "b=TIAS:555\r\n", + &sdp_with_bandwidth); + EXPECT_EQ(sdp_with_bandwidth, message); +} + +// Should default to b=AS if bandwidth_type isn't set. +TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithMissingBandwidthType) { + VideoContentDescription* vcd = GetFirstVideoContentDescription(&desc_); + vcd->set_bandwidth(100 * 1000); + ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), + jdesc_.session_version())); + std::string message = webrtc::SdpSerialize(jdesc_); + std::string sdp_with_bandwidth = kSdpFullString; + InjectAfter("c=IN IP4 74.125.224.39\r\n", "b=AS:100\r\n", &sdp_with_bandwidth); EXPECT_EQ(sdp_with_bandwidth, message); } @@ -2309,6 +2324,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithDataChannelAndBandwidth) { JsepSessionDescription jsep_desc(kDummyType); AddRtpDataChannel(); data_desc_->set_bandwidth(100 * 1000); + data_desc_->set_bandwidth_type("AS"); MakeDescriptionWithoutCandidates(&jsep_desc); std::string message = webrtc::SdpSerialize(jsep_desc); @@ -2612,6 +2628,23 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithBandwidth) { EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_bandwidth)); } +TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithTiasBandwidth) { + JsepSessionDescription jdesc_with_bandwidth(kDummyType); + std::string sdp_with_bandwidth = kSdpFullString; + InjectAfter("a=mid:video_content_name\r\na=sendrecv\r\n", "b=TIAS:100000\r\n", + &sdp_with_bandwidth); + InjectAfter("a=mid:audio_content_name\r\na=sendrecv\r\n", "b=TIAS:50000\r\n", + &sdp_with_bandwidth); + EXPECT_TRUE(SdpDeserialize(sdp_with_bandwidth, &jdesc_with_bandwidth)); + VideoContentDescription* vcd = GetFirstVideoContentDescription(&desc_); + vcd->set_bandwidth(100 * 1000); + AudioContentDescription* acd = GetFirstAudioContentDescription(&desc_); + acd->set_bandwidth(50 * 1000); + ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), + jdesc_.session_version())); + EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_bandwidth)); +} + TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithIceOptions) { JsepSessionDescription jdesc_with_ice_options(kDummyType); std::string sdp_with_ice_options = kSdpFullString;