Pick a matching CN codec, rather than the first CN codec.
It seems to me that we're currently just picking the first CN codec, rather than the one that matches the clock rate of the voice codec. The only test I've gotten to fail by changing this behavior is the one that's also changed in this CL, which explicitly expects a CN codec to be chosen even though there's none matching. BUG=webrtc:7282 Review-Url: https://codereview.webrtc.org/2707133007 Cr-Commit-Position: refs/heads/master@{#16979}
This commit is contained in:
parent
6ebaa6a283
commit
0c4b849b27
@ -1995,32 +1995,33 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
|
||||
|
||||
// Loop through the codecs list again to find the CN codec.
|
||||
// TODO(solenberg): Break out into a separate function?
|
||||
for (const AudioCodec& codec : codecs) {
|
||||
for (const AudioCodec& cn_codec : codecs) {
|
||||
// Ignore codecs we don't know about. The negotiation step should prevent
|
||||
// this, but double-check to be sure.
|
||||
webrtc::CodecInst voe_codec = {0};
|
||||
if (!WebRtcVoiceEngine::ToCodecInst(codec, &voe_codec)) {
|
||||
LOG(LS_WARNING) << "Unknown codec " << ToString(codec);
|
||||
if (!WebRtcVoiceEngine::ToCodecInst(cn_codec, &voe_codec)) {
|
||||
LOG(LS_WARNING) << "Unknown codec " << ToString(cn_codec);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (IsCodec(codec, kCnCodecName)) {
|
||||
if (IsCodec(cn_codec, kCnCodecName) &&
|
||||
cn_codec.clockrate == codec->clockrate) {
|
||||
// Turn voice activity detection/comfort noise on if supported.
|
||||
// Set the wideband CN payload type appropriately.
|
||||
// (narrowband always uses the static payload type 13).
|
||||
int cng_plfreq = -1;
|
||||
switch (codec.clockrate) {
|
||||
switch (cn_codec.clockrate) {
|
||||
case 8000:
|
||||
case 16000:
|
||||
case 32000:
|
||||
cng_plfreq = codec.clockrate;
|
||||
cng_plfreq = cn_codec.clockrate;
|
||||
break;
|
||||
default:
|
||||
LOG(LS_WARNING) << "CN frequency " << codec.clockrate
|
||||
LOG(LS_WARNING) << "CN frequency " << cn_codec.clockrate
|
||||
<< " not supported.";
|
||||
continue;
|
||||
}
|
||||
send_codec_spec.cng_payload_type = codec.id;
|
||||
send_codec_spec.cng_payload_type = cn_codec.id;
|
||||
send_codec_spec.cng_plfreq = cng_plfreq;
|
||||
break;
|
||||
}
|
||||
|
||||
@ -1244,8 +1244,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecs) {
|
||||
EXPECT_EQ(48000, send_codec_spec.codec_inst.rate);
|
||||
EXPECT_STREQ("ISAC", send_codec_spec.codec_inst.plname);
|
||||
EXPECT_NE(send_codec_spec.codec_inst.plfreq, send_codec_spec.cng_plfreq);
|
||||
EXPECT_EQ(13, send_codec_spec.cng_payload_type);
|
||||
EXPECT_EQ(webrtc::kFreq8000Hz, send_codec_spec.cng_plfreq);
|
||||
EXPECT_EQ(-1, send_codec_spec.cng_payload_type);
|
||||
EXPECT_FALSE(channel_->CanInsertDtmf());
|
||||
}
|
||||
|
||||
@ -2350,9 +2349,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsWithMultipleSendStreams) {
|
||||
const auto& send_codec_spec =
|
||||
call_.GetAudioSendStream(ssrc)->GetConfig().send_codec_spec;
|
||||
EXPECT_STREQ("PCMU", send_codec_spec.codec_inst.plname);
|
||||
EXPECT_NE(send_codec_spec.codec_inst.plfreq, send_codec_spec.cng_plfreq);
|
||||
EXPECT_EQ(97, send_codec_spec.cng_payload_type);
|
||||
EXPECT_EQ(webrtc::kFreq16000Hz, send_codec_spec.cng_plfreq);
|
||||
EXPECT_EQ(-1, send_codec_spec.cng_payload_type);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user