diff options
author | henrik.lundin@webrtc.org <henrik.lundin@webrtc.org> | 2014-11-07 12:25:00 +0000 |
---|---|---|
committer | henrik.lundin@webrtc.org <henrik.lundin@webrtc.org> | 2014-11-07 12:25:00 +0000 |
commit | f97800413f157c911aefbf5be167dbd4806a2323 (patch) | |
tree | 3c7d78d81b4c69cae88708e072ba9eee0bec0257 | |
parent | b3da49e39f95555c92519d7fcb42108339b2925e (diff) | |
download | talk-f97800413f157c911aefbf5be167dbd4806a2323.tar.gz |
Reapply "Advertise G722 as 8 kHz rather than 16 kHz""
This reverts r7653 and relands r7645. The reason for the original revert was that G722 disappeared from the SDP offer. This is now fixed. Also, a unit test was updated compared with the original change.
BUG=3951
TBR=pthatcher@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/27089004
git-svn-id: http://webrtc.googlecode.com/svn/trunk/talk@7662 4adac7df-926f-26a2-2b94-8c16560cd09d
-rw-r--r-- | media/webrtc/webrtcvoiceengine.cc | 31 | ||||
-rw-r--r-- | media/webrtc/webrtcvoiceengine.h | 1 | ||||
-rw-r--r-- | media/webrtc/webrtcvoiceengine_unittest.cc | 26 | ||||
-rw-r--r-- | session/media/mediasessionclient_unittest.cc | 12 |
4 files changed, 56 insertions, 14 deletions
diff --git a/media/webrtc/webrtcvoiceengine.cc b/media/webrtc/webrtcvoiceengine.cc index 95e16e4..74b3163 100644 --- a/media/webrtc/webrtcvoiceengine.cc +++ b/media/webrtc/webrtcvoiceengine.cc @@ -74,7 +74,7 @@ static const CodecPref kCodecPrefs[] = { { "ISAC", 32000, 1, 104, true }, { "CELT", 32000, 1, 109, true }, { "CELT", 32000, 2, 110, true }, - { "G722", 16000, 1, 9, false }, + { "G722", 8000, 1, 9, false }, { "ILBC", 8000, 1, 102, false }, { "PCMU", 8000, 1, 0, false }, { "PCMA", 8000, 1, 8, false }, @@ -110,6 +110,7 @@ static const int kDefaultAudioDeviceId = 0; static const char kIsacCodecName[] = "ISAC"; static const char kL16CodecName[] = "L16"; +static const char kG722CodecName[] = "G722"; // Parameter used for NACK. // This value is equivalent to 5 seconds of audio data at 20 ms per packet. @@ -485,12 +486,24 @@ static void GetOpusConfig(const AudioCodec& codec, webrtc::CodecInst* voe_codec, voe_codec->rate = GetOpusBitrate(codec, *max_playback_rate); } +// Changes RTP timestamp rate of G722. This is due to the "bug" in the RFC +// which says that G722 should be advertised as 8 kHz although it is a 16 kHz +// codec. +static void MaybeFixupG722(webrtc::CodecInst* voe_codec, int new_plfreq) { + if (_stricmp(voe_codec->plname, kG722CodecName) == 0) { + // If the ASSERT triggers, the codec definition in WebRTC VoiceEngine + // has changed, and this special case is no longer needed. + ASSERT(voe_codec->plfreq != new_plfreq); + voe_codec->plfreq = new_plfreq; + } +} + void WebRtcVoiceEngine::ConstructCodecs() { LOG(LS_INFO) << "WebRtc VoiceEngine codecs:"; int ncodecs = voe_wrapper_->codec()->NumOfCodecs(); for (int i = 0; i < ncodecs; ++i) { webrtc::CodecInst voe_codec; - if (voe_wrapper_->codec()->GetCodec(i, voe_codec) != -1) { + if (GetVoeCodec(i, voe_codec)) { // Skip uncompressed formats. if (_stricmp(voe_codec.plname, kL16CodecName) == 0) { continue; @@ -540,6 +553,15 @@ void WebRtcVoiceEngine::ConstructCodecs() { std::sort(codecs_.begin(), codecs_.end(), &AudioCodec::Preferable); } +bool WebRtcVoiceEngine::GetVoeCodec(int index, webrtc::CodecInst& codec) { + if (voe_wrapper_->codec()->GetCodec(index, codec) != -1) { + // Change the sample rate of G722 to 8000 to match SDP. + MaybeFixupG722(&codec, 8000); + return true; + } + return false; +} + WebRtcVoiceEngine::~WebRtcVoiceEngine() { LOG(LS_VERBOSE) << "WebRtcVoiceEngine::~WebRtcVoiceEngine"; if (voe_wrapper_->base()->DeRegisterVoiceEngineObserver() == -1) { @@ -1224,7 +1246,7 @@ bool WebRtcVoiceEngine::FindWebRtcCodec(const AudioCodec& in, int ncodecs = voe_wrapper_->codec()->NumOfCodecs(); for (int i = 0; i < ncodecs; ++i) { webrtc::CodecInst voe_codec; - if (voe_wrapper_->codec()->GetCodec(i, voe_codec) != -1) { + if (GetVoeCodec(i, voe_codec)) { AudioCodec codec(voe_codec.pltype, voe_codec.plname, voe_codec.plfreq, voe_codec.rate, voe_codec.channels, 0); bool multi_rate = IsCodecMultiRate(voe_codec); @@ -1243,6 +1265,9 @@ bool WebRtcVoiceEngine::FindWebRtcCodec(const AudioCodec& in, voe_codec.rate = in.bitrate; } + // Reset G722 sample rate to 16000 to match WebRTC. + MaybeFixupG722(&voe_codec, 16000); + // Apply codec-specific settings. if (IsIsac(codec)) { // If ISAC and an explicit bitrate is not specified, diff --git a/media/webrtc/webrtcvoiceengine.h b/media/webrtc/webrtcvoiceengine.h index f19059b..34b9f3c 100644 --- a/media/webrtc/webrtcvoiceengine.h +++ b/media/webrtc/webrtcvoiceengine.h @@ -199,6 +199,7 @@ class WebRtcVoiceEngine void Construct(); void ConstructCodecs(); + bool GetVoeCodec(int index, webrtc::CodecInst& codec); bool InitInternal(); bool EnsureSoundclipEngineInit(); void SetTraceFilter(int filter); diff --git a/media/webrtc/webrtcvoiceengine_unittest.cc b/media/webrtc/webrtcvoiceengine_unittest.cc index 5deabd2..5eb6e24 100644 --- a/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/media/webrtc/webrtcvoiceengine_unittest.cc @@ -52,14 +52,16 @@ static const cricket::AudioCodec kPcmuCodec(0, "PCMU", 8000, 64000, 1, 0); static const cricket::AudioCodec kIsacCodec(103, "ISAC", 16000, 32000, 1, 0); static const cricket::AudioCodec kCeltCodec(110, "CELT", 32000, 64000, 2, 0); static const cricket::AudioCodec kOpusCodec(111, "opus", 48000, 64000, 2, 0); +static const cricket::AudioCodec kG722CodecVoE(9, "G722", 16000, 64000, 1, 0); +static const cricket::AudioCodec kG722CodecSdp(9, "G722", 8000, 64000, 1, 0); static const cricket::AudioCodec kRedCodec(117, "red", 8000, 0, 1, 0); static const cricket::AudioCodec kCn8000Codec(13, "CN", 8000, 0, 1, 0); static const cricket::AudioCodec kCn16000Codec(105, "CN", 16000, 0, 1, 0); static const cricket::AudioCodec kTelephoneEventCodec(106, "telephone-event", 8000, 0, 1, 0); static const cricket::AudioCodec* const kAudioCodecs[] = { - &kPcmuCodec, &kIsacCodec, &kCeltCodec, &kOpusCodec, &kRedCodec, - &kCn8000Codec, &kCn16000Codec, &kTelephoneEventCodec, + &kPcmuCodec, &kIsacCodec, &kCeltCodec, &kOpusCodec, &kG722CodecVoE, + &kRedCodec, &kCn8000Codec, &kCn16000Codec, &kTelephoneEventCodec, }; const char kRingbackTone[] = "RIFF____WAVE____ABCD1234"; static uint32 kSsrc1 = 0x99; @@ -770,6 +772,20 @@ TEST_F(WebRtcVoiceEngineTestFake, DontResetSetSendCodec) { EXPECT_EQ(1, voe_.GetNumSetSendCodecs()); } +// Verify that G722 is set with 16000 samples per second to WebRTC. +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecG722) { + EXPECT_TRUE(SetupEngine()); + int channel_num = voe_.GetLastChannel(); + std::vector<cricket::AudioCodec> codecs; + codecs.push_back(kG722CodecSdp); + EXPECT_TRUE(channel_->SetSendCodecs(codecs)); + webrtc::CodecInst gcodec; + EXPECT_EQ(0, voe_.GetSendCodec(channel_num, gcodec)); + EXPECT_STREQ("G722", gcodec.plname); + EXPECT_EQ(1, gcodec.channels); + EXPECT_EQ(16000, gcodec.plfreq); +} + // Test that if clockrate is not 48000 for opus, we fail. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusBadClockrate) { EXPECT_TRUE(SetupEngine()); @@ -3208,7 +3224,7 @@ TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) { EXPECT_TRUE(engine.FindCodec( cricket::AudioCodec(96, "PCMA", 8000, 0, 1, 0))); EXPECT_TRUE(engine.FindCodec( - cricket::AudioCodec(96, "G722", 16000, 0, 1, 0))); + cricket::AudioCodec(96, "G722", 8000, 0, 1, 0))); EXPECT_TRUE(engine.FindCodec( cricket::AudioCodec(96, "red", 8000, 0, 1, 0))); EXPECT_TRUE(engine.FindCodec( @@ -3225,7 +3241,7 @@ TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) { EXPECT_TRUE(engine.FindCodec( cricket::AudioCodec(8, "", 8000, 0, 1, 0))); // PCMA EXPECT_TRUE(engine.FindCodec( - cricket::AudioCodec(9, "", 16000, 0, 1, 0))); // G722 + cricket::AudioCodec(9, "", 8000, 0, 1, 0))); // G722 EXPECT_TRUE(engine.FindCodec( cricket::AudioCodec(13, "", 8000, 0, 1, 0))); // CN // Check sample/bitrate matching. @@ -3248,7 +3264,7 @@ TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) { EXPECT_EQ(103, it->id); } else if (it->name == "ISAC" && it->clockrate == 32000) { EXPECT_EQ(104, it->id); - } else if (it->name == "G722" && it->clockrate == 16000) { + } else if (it->name == "G722" && it->clockrate == 8000) { EXPECT_EQ(9, it->id); } else if (it->name == "telephone-event") { EXPECT_EQ(126, it->id); diff --git a/session/media/mediasessionclient_unittest.cc b/session/media/mediasessionclient_unittest.cc index 3e8a90f..eb052ef 100644 --- a/session/media/mediasessionclient_unittest.cc +++ b/session/media/mediasessionclient_unittest.cc @@ -61,7 +61,7 @@ static const cricket::AudioCodec kAudioCodecs[] = { cricket::AudioCodec(119, "ISACLC", 16000, 40000, 1, 16), cricket::AudioCodec(99, "speex", 16000, 22000, 1, 15), cricket::AudioCodec(97, "IPCMWB", 16000, 80000, 1, 14), - cricket::AudioCodec(9, "G722", 16000, 64000, 1, 13), + cricket::AudioCodec(9, "G722", 8000, 64000, 1, 13), cricket::AudioCodec(102, "iLBC", 8000, 13300, 1, 12), cricket::AudioCodec(98, "speex", 8000, 11000, 1, 11), cricket::AudioCodec(3, "GSM", 8000, 13000, 1, 10), @@ -81,7 +81,7 @@ static const cricket::AudioCodec kAudioCodecs[] = { static const cricket::AudioCodec kAudioCodecsDifferentPreference[] = { cricket::AudioCodec(104, "ISAC", 32000, -1, 1, 17), cricket::AudioCodec(97, "IPCMWB", 16000, 80000, 1, 14), - cricket::AudioCodec(9, "G722", 16000, 64000, 1, 13), + cricket::AudioCodec(9, "G722", 8000, 64000, 1, 13), cricket::AudioCodec(119, "ISACLC", 16000, 40000, 1, 16), cricket::AudioCodec(103, "ISAC", 16000, -1, 1, 18), cricket::AudioCodec(99, "speex", 16000, 22000, 1, 15), @@ -197,7 +197,7 @@ const std::string kGingleInitiate( " <payload-type xmlns='http://www.google.com/session/phone' " \ " id='97' name='IPCMWB' clockrate='16000' bitrate='80000' /> " \ " <payload-type xmlns='http://www.google.com/session/phone' " \ - " id='9' name='G722' clockrate='16000' bitrate='64000' /> " \ + " id='9' name='G722' clockrate='8000' bitrate='64000' /> " \ " <payload-type xmlns='http://www.google.com/session/phone' " \ " id='102' name='iLBC' clockrate='8000' bitrate='13300' />" \ " <payload-type xmlns='http://www.google.com/session/phone' " \ @@ -248,7 +248,7 @@ const std::string kJingleInitiate( " <parameter name='bitrate' value='80000'/> " \ " </payload-type> " \ " <payload-type " \ - " id='9' name='G722' clockrate='16000'> " \ + " id='9' name='G722' clockrate='8000'> " \ " <parameter name='bitrate' value='64000'/> " \ " </payload-type> " \ " <payload-type " \ @@ -1918,7 +1918,7 @@ class MediaSessionClientTest : public sigslot::has_slots<> { e = NextFromPayloadType(e); ASSERT_TRUE(e != NULL); codec = AudioCodecFromPayloadType(e); - VerifyAudioCodec(codec, 9, "G722", 16000, 64000, 1); + VerifyAudioCodec(codec, 9, "G722", 8000, 64000, 1); e = NextFromPayloadType(e); ASSERT_TRUE(e != NULL); @@ -2112,7 +2112,7 @@ class MediaSessionClientTest : public sigslot::has_slots<> { codec = AudioCodecFromPayloadType(e); ASSERT_EQ(9, codec.id); ASSERT_EQ("G722", codec.name); - ASSERT_EQ(16000, codec.clockrate); + ASSERT_EQ(8000, codec.clockrate); ASSERT_EQ(64000, codec.bitrate); ASSERT_EQ(1, codec.channels); |