From e1405ad0d164230c77c4ac10efd92db3ef2f9030 Mon Sep 17 00:00:00 2001 From: ossu Date: Mon, 23 Jan 2017 08:55:48 -0800 Subject: [PATCH] Removed double-special-casing of ISAC in libjingle and WebRtcVoE. webrtcvoiceengine.cc ensured that if the bitrate set for ISAC was 0, it was changed to -1 so that the codec could manage the bitrate itself. webrtcsdp.cc ensured that if the bitrate set for ISAC was 0, it was explicitly set to default values to avoid the codec's built in bitrate management. Eventually, there'll be no codec specific code like this in these layers. This is one step towards that goal. BUG=webrtc:5806 Review-Url: https://codereview.webrtc.org/2642923003 Cr-Commit-Position: refs/heads/master@{#16220} --- webrtc/media/engine/webrtcvoiceengine.cc | 6 ------ .../engine/webrtcvoiceengine_unittest.cc | 2 +- webrtc/pc/webrtcsdp.cc | 19 +------------------ webrtc/pc/webrtcsdp_unittest.cc | 13 +++---------- 4 files changed, 5 insertions(+), 35 deletions(-) diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index a3c5832fd9..9f7ea58d90 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -371,12 +371,6 @@ class WebRtcVoiceCodecs final { // Reset G722 sample rate to 16000 to match WebRTC. MaybeFixupG722(&voe_codec, 16000); - // Apply codec-specific settings. - if (IsCodec(codec, kIsacCodecName)) { - // If ISAC and an explicit bitrate is not specified, - // enable auto bitrate adjustment. - voe_codec.rate = (in.bitrate > 0) ? in.bitrate : -1; - } *out = voe_codec; } return true; diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 60389342a8..eb6f606cf2 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -1888,7 +1888,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsBitrate) { const auto& gcodec = GetSendStreamConfig(kSsrc1).send_codec_spec.codec_inst; EXPECT_EQ(103, gcodec.pltype); EXPECT_STREQ("ISAC", gcodec.plname); - EXPECT_EQ(-1, gcodec.rate); + EXPECT_EQ(32000, gcodec.rate); } parameters.codecs[0].bitrate = 28000; // bitrate == 28000 SetSendParameters(parameters); diff --git a/webrtc/pc/webrtcsdp.cc b/webrtc/pc/webrtcsdp.cc index d4003c2a02..ced85cb323 100644 --- a/webrtc/pc/webrtcsdp.cc +++ b/webrtc/pc/webrtcsdp.cc @@ -205,11 +205,6 @@ static const char kApplicationSpecificMaximum[] = "AS"; static const int kDefaultVideoClockrate = 90000; -// ISAC special-case. -static const char kIsacCodecName[] = "ISAC"; // From webrtcvoiceengine.cc -static const int kIsacWbDefaultRate = 32000; // From acm_common_defs.h -static const int kIsacSwbDefaultRate = 56000; // From acm_common_defs.h - static const char kDefaultSctpmapProtocol[] = "webrtc-datachannel"; // RTP payload type is in the 0-127 range. Use -1 to indicate "all" payload @@ -3098,21 +3093,9 @@ bool ParseRtpmapAttribute(const std::string& line, return false; } } - int bitrate = 0; - // The default behavior for ISAC (bitrate == 0) in webrtcvoiceengine.cc - // (specifically FindWebRtcCodec) is bandwidth-adaptive variable bitrate. - // The bandwidth adaptation doesn't always work well, so this code - // sets a fixed target bitrate instead. - if (_stricmp(encoding_name.c_str(), kIsacCodecName) == 0) { - if (clock_rate <= 16000) { - bitrate = kIsacWbDefaultRate; - } else { - bitrate = kIsacSwbDefaultRate; - } - } AudioContentDescription* audio_desc = static_cast(media_desc); - UpdateCodec(payload_type, encoding_name, clock_rate, bitrate, channels, + UpdateCodec(payload_type, encoding_name, clock_rate, 0, channels, audio_desc); } else if (media_type == cricket::MEDIA_TYPE_DATA) { DataContentDescription* data_desc = diff --git a/webrtc/pc/webrtcsdp_unittest.cc b/webrtc/pc/webrtcsdp_unittest.cc index 54eb22e11a..43774abf7d 100644 --- a/webrtc/pc/webrtcsdp_unittest.cc +++ b/webrtc/pc/webrtcsdp_unittest.cc @@ -1128,8 +1128,8 @@ class WebRtcSdpTest : public testing::Test { audio->set_protocol(cricket::kMediaProtocolSavpf); AudioCodec opus(111, "opus", 48000, 0, 2); audio->AddCodec(opus); - audio->AddCodec(AudioCodec(103, "ISAC", 16000, 32000, 1)); - audio->AddCodec(AudioCodec(104, "ISAC", 32000, 56000, 1)); + audio->AddCodec(AudioCodec(103, "ISAC", 16000, 0, 1)); + audio->AddCodec(AudioCodec(104, "ISAC", 32000, 0, 1)); return audio; } @@ -1664,13 +1664,6 @@ class WebRtcSdpTest : public testing::Test { cricket::AudioCodec codec = acd->codecs()[i]; VerifyCodecParameter(codec.params, "ptime", params.ptime); VerifyCodecParameter(codec.params, "maxptime", params.max_ptime); - if (codec.name == "ISAC") { - if (codec.clockrate == 16000) { - EXPECT_EQ(32000, codec.bitrate); - } else { - EXPECT_EQ(56000, codec.bitrate); - } - } } const ContentInfo* vc = GetFirstVideoContent(jdesc_output->description()); @@ -2275,7 +2268,7 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutRtpmap) { // the payload types (s) on the m= line. ref_codecs.push_back(AudioCodec(0, "PCMU", 8000, 0, 1)); ref_codecs.push_back(AudioCodec(18, "G729", 16000, 0, 1)); - ref_codecs.push_back(AudioCodec(103, "ISAC", 16000, 32000, 1)); + ref_codecs.push_back(AudioCodec(103, "ISAC", 16000, 0, 1)); EXPECT_EQ(ref_codecs, audio->codecs()); }