From 8034614b817810f33ad193d8fa5c460bc6c28276 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Wed, 27 Apr 2016 14:17:10 -0700 Subject: [PATCH] Cap the send bitrate for opus and iSAC before passing down to VoE. The voice engine expects send bitrates no more than the maximum for the codec. For example, 510kbps for opus. So if "b=AS" sets a maximum above the codec maximum, WebRtcVoiceEngine needs to cap it. BUG=603690 Review-Url: https://codereview.webrtc.org/1920123002 Cr-Commit-Position: refs/heads/master@{#12537} --- webrtc/media/engine/webrtcvoiceengine.cc | 76 ++++++++++++------- webrtc/media/engine/webrtcvoiceengine.h | 6 +- .../engine/webrtcvoiceengine_unittest.cc | 40 +++++----- 3 files changed, 72 insertions(+), 50 deletions(-) diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index c11993d550..63241332a3 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -84,6 +84,9 @@ const int kOpusBitrateFb = 32000; const int kOpusMinBitrate = 6000; const int kOpusMaxBitrate = 510000; +// iSAC bitrate should be <= 56000. +const int kIsacMaxBitrate = 56000; + // Default audio dscp value. // See http://tools.ietf.org/html/rfc2474 for details. // See also http://tools.ietf.org/html/draft-jennings-rtcweb-qos-00 @@ -349,6 +352,16 @@ class WebRtcVoiceCodecs final { return false; } + static int MaxBitrateBps(const webrtc::CodecInst& codec) { + for (size_t i = 0; i < arraysize(kCodecPrefs); ++i) { + if (IsCodec(codec, kCodecPrefs[i].name) && + kCodecPrefs[i].clockrate == codec.plfreq) { + return kCodecPrefs[i].max_bitrate_bps; + } + } + return 0; + } + // If the AudioCodec param kCodecParamPTime is set, then we will set it to // codec pacsize if it's valid, or we will pick the next smallest value we // support. @@ -419,6 +432,7 @@ class WebRtcVoiceCodecs final { int payload_type; bool is_multi_rate; int packet_sizes_ms[kMaxNumPacketSize]; + int max_bitrate_bps; }; // Note: keep the supported packet sizes in ascending order. static const CodecPref kCodecPrefs[12]; @@ -485,19 +499,19 @@ class WebRtcVoiceCodecs final { }; const WebRtcVoiceCodecs::CodecPref WebRtcVoiceCodecs::kCodecPrefs[12] = { - { kOpusCodecName, 48000, 2, 111, true, { 10, 20, 40, 60 } }, - { kIsacCodecName, 16000, 1, 103, true, { 30, 60 } }, - { kIsacCodecName, 32000, 1, 104, true, { 30 } }, - // G722 should be advertised as 8000 Hz because of the RFC "bug". - { kG722CodecName, 8000, 1, 9, false, { 10, 20, 30, 40, 50, 60 } }, - { kIlbcCodecName, 8000, 1, 102, false, { 20, 30, 40, 60 } }, - { kPcmuCodecName, 8000, 1, 0, false, { 10, 20, 30, 40, 50, 60 } }, - { kPcmaCodecName, 8000, 1, 8, false, { 10, 20, 30, 40, 50, 60 } }, - { kCnCodecName, 32000, 1, 106, false, { } }, - { kCnCodecName, 16000, 1, 105, false, { } }, - { kCnCodecName, 8000, 1, 13, false, { } }, - { kRedCodecName, 8000, 1, 127, false, { } }, - { kDtmfCodecName, 8000, 1, 126, false, { } }, + {kOpusCodecName, 48000, 2, 111, true, {10, 20, 40, 60}, kOpusMaxBitrate}, + {kIsacCodecName, 16000, 1, 103, true, {30, 60}, kIsacMaxBitrate}, + {kIsacCodecName, 32000, 1, 104, true, {30}, kIsacMaxBitrate}, + // G722 should be advertised as 8000 Hz because of the RFC "bug". + {kG722CodecName, 8000, 1, 9, false, {10, 20, 30, 40, 50, 60}}, + {kIlbcCodecName, 8000, 1, 102, false, {20, 30, 40, 60}}, + {kPcmuCodecName, 8000, 1, 0, false, {10, 20, 30, 40, 50, 60}}, + {kPcmaCodecName, 8000, 1, 8, false, {10, 20, 30, 40, 50, 60}}, + {kCnCodecName, 32000, 1, 106, false, {}}, + {kCnCodecName, 16000, 1, 105, false, {}}, + {kCnCodecName, 8000, 1, 13, false, {}}, + {kRedCodecName, 8000, 1, 127, false, {}}, + {kDtmfCodecName, 8000, 1, 126, false, {}}, }; } // namespace { @@ -1368,7 +1382,7 @@ bool WebRtcVoiceMediaChannel::SetSendParameters( } } - if (!SetSendBitrate(params.max_bandwidth_bps)) { + if (!SetMaxSendBitrate(params.max_bandwidth_bps)) { return false; } return SetOptions(params.options); @@ -1748,7 +1762,7 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( } } } - // TODO(solenberg): SetSendBitrate() yields another call to SetSendCodec(). + // TODO(solenberg): SetMaxSendBitrate() yields another call to SetSendCodec(). // Check if it is possible to fuse with the previous call in this function. SetChannelParameters(channel, rtp_parameters); @@ -2384,9 +2398,9 @@ bool WebRtcVoiceMediaChannel::MuteStream(uint32_t ssrc, bool muted) { return true; } -bool WebRtcVoiceMediaChannel::SetSendBitrate(int bps) { - LOG(LS_INFO) << "WebRtcVoiceMediaChannel::SetSendBitrate."; - send_bitrate_bps_ = bps; +bool WebRtcVoiceMediaChannel::SetMaxSendBitrate(int bps) { + LOG(LS_INFO) << "WebRtcVoiceMediaChannel::SetMaxSendBitrate."; + max_send_bitrate_bps_ = bps; for (const auto& kv : send_streams_) { if (!SetChannelParameters(kv.second->channel(), @@ -2403,17 +2417,18 @@ bool WebRtcVoiceMediaChannel::SetChannelParameters( RTC_CHECK_EQ(1UL, parameters.encodings.size()); // TODO(deadbeef): Handle setting parameters with a list of codecs in a // different order (which should change the send codec). - return SetSendBitrate( - channel, - MinPositive(send_bitrate_bps_, parameters.encodings[0].max_bitrate_bps)); + return SetMaxSendBitrate( + channel, MinPositive(max_send_bitrate_bps_, + parameters.encodings[0].max_bitrate_bps)); } -bool WebRtcVoiceMediaChannel::SetSendBitrate(int channel, int bps) { +bool WebRtcVoiceMediaChannel::SetMaxSendBitrate(int channel, int bps) { // Bitrate is auto by default. // TODO(bemasc): Fix this so that if SetMaxSendBandwidth(50) is followed by // SetMaxSendBandwith(0), the second call removes the previous limit. - if (bps <= 0) + if (bps <= 0) { return true; + } if (!HasSendCodec()) { LOG(LS_INFO) << "The send codec has not been set up yet. " @@ -2426,10 +2441,13 @@ bool WebRtcVoiceMediaChannel::SetSendBitrate(int channel, int bps) { if (is_multi_rate) { // If codec is multi-rate then just set the bitrate. - codec.rate = bps; + int max_bitrate_bps = WebRtcVoiceCodecs::MaxBitrateBps(codec); + codec.rate = std::min(bps, max_bitrate_bps); + LOG(LS_INFO) << "Setting codec " << codec.plname << " to bitrate " << bps + << " bps."; if (!SetSendCodec(channel, codec)) { - LOG(LS_INFO) << "Failed to set codec " << codec.plname << " to bitrate " - << bps << " bps."; + LOG(LS_ERROR) << "Failed to set codec " << codec.plname << " to bitrate " + << bps << " bps."; return false; } return true; @@ -2438,9 +2456,9 @@ bool WebRtcVoiceMediaChannel::SetSendBitrate(int channel, int bps) { // then fail. If codec is not multi-rate and |bps| exceeds or equal the // fixed bitrate then ignore. if (bps < codec.rate) { - LOG(LS_INFO) << "Failed to set codec " << codec.plname - << " to bitrate " << bps << " bps" - << ", requires at least " << codec.rate << " bps."; + LOG(LS_ERROR) << "Failed to set codec " << codec.plname << " to bitrate " + << bps << " bps" + << ", requires at least " << codec.rate << " bps."; return false; } return true; diff --git a/webrtc/media/engine/webrtcvoiceengine.h b/webrtc/media/engine/webrtcvoiceengine.h index fd46dee898..1cc5504304 100644 --- a/webrtc/media/engine/webrtcvoiceengine.h +++ b/webrtc/media/engine/webrtcvoiceengine.h @@ -228,10 +228,10 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, bool IsDefaultRecvStream(uint32_t ssrc) { return default_recv_ssrc_ == static_cast(ssrc); } - bool SetSendBitrate(int bps); + bool SetMaxSendBitrate(int bps); bool SetChannelParameters(int channel, const webrtc::RtpParameters& parameters); - bool SetSendBitrate(int channel, int bps); + bool SetMaxSendBitrate(int channel, int bps); bool HasSendCodec() const { return send_codec_spec_.codec_inst.pltype != -1; } @@ -243,7 +243,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, WebRtcVoiceEngine* const engine_ = nullptr; std::vector send_codecs_; std::vector recv_codecs_; - int send_bitrate_bps_ = 0; + int max_send_bitrate_bps_ = 0; AudioOptions options_; rtc::Optional dtmf_payload_type_; bool desired_playout_ = false; diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index d3437265a1..9f9ab47242 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -218,10 +218,10 @@ class WebRtcVoiceEngineTestFake : public testing::Test { // |max_bitrate| is a parameter to set to SetMaxSendBandwidth(). // |expected_result| is the expected result from SetMaxSendBandwidth(). // |expected_bitrate| is the expected audio bitrate afterward. - void TestSendBandwidth(const cricket::AudioCodec& codec, - int max_bitrate, - bool expected_result, - int expected_bitrate) { + void TestMaxSendBandwidth(const cricket::AudioCodec& codec, + int max_bitrate, + bool expected_result, + int expected_bitrate) { cricket::AudioSendParameters parameters; parameters.codecs.push_back(codec); parameters.max_bandwidth_bps = max_bitrate; @@ -756,13 +756,13 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendBandwidthAuto) { // bitrate is <= 0. // ISAC, default bitrate == 32000. - TestSendBandwidth(kIsacCodec, 0, true, 32000); + TestMaxSendBandwidth(kIsacCodec, 0, true, 32000); // PCMU, default bitrate == 64000. - TestSendBandwidth(kPcmuCodec, -1, true, 64000); + TestMaxSendBandwidth(kPcmuCodec, -1, true, 64000); // opus, default bitrate == 64000. - TestSendBandwidth(kOpusCodec, -1, true, 64000); + TestMaxSendBandwidth(kOpusCodec, -1, true, 64000); } TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthMultiRateAsCaller) { @@ -771,12 +771,16 @@ TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthMultiRateAsCaller) { // Test that the bitrate of a multi-rate codec is always the maximum. // ISAC, default bitrate == 32000. - TestSendBandwidth(kIsacCodec, 128000, true, 128000); - TestSendBandwidth(kIsacCodec, 16000, true, 16000); + TestMaxSendBandwidth(kIsacCodec, 40000, true, 40000); + TestMaxSendBandwidth(kIsacCodec, 16000, true, 16000); + // Rates above the max (56000) should be capped. + TestMaxSendBandwidth(kIsacCodec, 100000, true, 56000); // opus, default bitrate == 64000. - TestSendBandwidth(kOpusCodec, 96000, true, 96000); - TestSendBandwidth(kOpusCodec, 48000, true, 48000); + TestMaxSendBandwidth(kOpusCodec, 96000, true, 96000); + TestMaxSendBandwidth(kOpusCodec, 48000, true, 48000); + // Rates above the max (510000) should be capped. + TestMaxSendBandwidth(kOpusCodec, 600000, true, 510000); } TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthFixedRateAsCaller) { @@ -786,13 +790,13 @@ TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthFixedRateAsCaller) { // if it's bigger than the fixed rate. // PCMU, fixed bitrate == 64000. - TestSendBandwidth(kPcmuCodec, 0, true, 64000); - TestSendBandwidth(kPcmuCodec, 1, false, 64000); - TestSendBandwidth(kPcmuCodec, 128000, true, 64000); - TestSendBandwidth(kPcmuCodec, 32000, false, 64000); - TestSendBandwidth(kPcmuCodec, 64000, true, 64000); - TestSendBandwidth(kPcmuCodec, 63999, false, 64000); - TestSendBandwidth(kPcmuCodec, 64001, true, 64000); + TestMaxSendBandwidth(kPcmuCodec, 0, true, 64000); + TestMaxSendBandwidth(kPcmuCodec, 1, false, 64000); + TestMaxSendBandwidth(kPcmuCodec, 128000, true, 64000); + TestMaxSendBandwidth(kPcmuCodec, 32000, false, 64000); + TestMaxSendBandwidth(kPcmuCodec, 64000, true, 64000); + TestMaxSendBandwidth(kPcmuCodec, 63999, false, 64000); + TestMaxSendBandwidth(kPcmuCodec, 64001, true, 64000); } TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthMultiRateAsCallee) {