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@7662 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc
index 95e16e4..74b3163 100644
--- a/talk/media/webrtc/webrtcvoiceengine.cc
+++ b/talk/media/webrtc/webrtcvoiceengine.cc
@@ -74,7 +74,7 @@
{ "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 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 @@
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 @@
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 @@
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 @@
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/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h
index f19059b..34b9f3c 100644
--- a/talk/media/webrtc/webrtcvoiceengine.h
+++ b/talk/media/webrtc/webrtcvoiceengine.h
@@ -199,6 +199,7 @@
void Construct();
void ConstructCodecs();
+ bool GetVoeCodec(int index, webrtc::CodecInst& codec);
bool InitInternal();
bool EnsureSoundclipEngineInit();
void SetTraceFilter(int filter);
diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc
index 5deabd2..5eb6e24 100644
--- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc
+++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc
@@ -52,14 +52,16 @@
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 @@
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 @@
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 @@
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 @@
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/talk/session/media/mediasessionclient_unittest.cc b/talk/session/media/mediasessionclient_unittest.cc
index 3e8a90f..eb052ef 100644
--- a/talk/session/media/mediasessionclient_unittest.cc
+++ b/talk/session/media/mediasessionclient_unittest.cc
@@ -61,7 +61,7 @@
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 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 @@
" <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 @@
" <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 @@
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 @@
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);
