diff --git a/talk/app/webrtc/mediaconstraintsinterface.cc b/talk/app/webrtc/mediaconstraintsinterface.cc index 6f88d793a..0ecadd691 100644 --- a/talk/app/webrtc/mediaconstraintsinterface.cc +++ b/talk/app/webrtc/mediaconstraintsinterface.cc @@ -119,6 +119,7 @@ const char MediaConstraintsInterface::kHighBitrate[] = const char MediaConstraintsInterface::kVeryHighBitrate[] = "googVeryHighBitrate"; const char MediaConstraintsInterface::kPayloadPadding[] = "googPayloadPadding"; +const char MediaConstraintsInterface::kOpusFec[] = "googOpusFec"; // Set |value| to the value associated with the first appearance of |key|, or diff --git a/talk/app/webrtc/mediaconstraintsinterface.h b/talk/app/webrtc/mediaconstraintsinterface.h index 0db39a667..36257db71 100644 --- a/talk/app/webrtc/mediaconstraintsinterface.h +++ b/talk/app/webrtc/mediaconstraintsinterface.h @@ -134,6 +134,11 @@ class MediaConstraintsInterface { static const char kVeryHighBitrate[]; // googVeryHighBitrate static const char kPayloadPadding[]; // googPayloadPadding + // PeerConnection codec constraint keys. This should be combined with the + // values above. + // kOpusFec controls whether we ask the other side to turn on FEC for Opus. + static const char kOpusFec[]; // googOpusFec + // The prefix of internal-only constraints whose JS set values should be // stripped by Chrome before passed down to Libjingle. static const char kInternalConstraintPrefix[]; diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 3ba1943de..1b1c28781 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -615,6 +615,10 @@ bool WebRtcSession::Initialize( cricket::VideoOptions::HIGH); } + SetOptionFromOptionalConstraint(constraints, + MediaConstraintsInterface::kOpusFec, + &audio_options_.opus_fec); + const cricket::VideoCodec default_codec( JsepSessionDescription::kDefaultVideoCodecId, JsepSessionDescription::kDefaultVideoCodecName, diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index 2d65daf59..34d2deff7 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -182,6 +182,7 @@ struct AudioOptions { recording_sample_rate.SetFrom(change.recording_sample_rate); playout_sample_rate.SetFrom(change.playout_sample_rate); dscp.SetFrom(change.dscp); + opus_fec.SetFrom(change.opus_fec); } bool operator==(const AudioOptions& o) const { @@ -207,7 +208,8 @@ struct AudioOptions { rx_agc_limiter == o.rx_agc_limiter && recording_sample_rate == o.recording_sample_rate && playout_sample_rate == o.playout_sample_rate && - dscp == o.dscp; + dscp == o.dscp && + opus_fec == o.opus_fec; } std::string ToString() const { @@ -238,6 +240,7 @@ struct AudioOptions { ost << ToStringIfSet("recording_sample_rate", recording_sample_rate); ost << ToStringIfSet("playout_sample_rate", playout_sample_rate); ost << ToStringIfSet("dscp", dscp); + ost << ToStringIfSet("opus_fec", opus_fec); ost << "}"; return ost.str(); } @@ -275,6 +278,8 @@ struct AudioOptions { Settable playout_sample_rate; // Set DSCP value for packet sent from audio channel. Settable dscp; + // Set Opus FEC + Settable opus_fec; }; // Options that can be applied to a VideoMediaChannel or a VideoMediaEngine. diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 785cdf11c..f1460a6cb 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -237,6 +237,7 @@ static AudioOptions GetDefaultEngineOptions() { options.experimental_aec.Set(false); options.experimental_ns.Set(false); options.aec_dump.Set(false); + options.opus_fec.Set(false); return options; } @@ -399,12 +400,8 @@ static bool IsIsac(const AudioCodec& codec) { // True if params["stereo"] == "1" static bool IsOpusStereoEnabled(const AudioCodec& codec) { - CodecParameterMap::const_iterator param = - codec.params.find(kCodecParamStereo); - if (param == codec.params.end()) { - return false; - } - return param->second == kParamValueTrue; + int value; + return codec.GetParam(kCodecParamStereo, &value) && value == 1; } static bool IsValidOpusBitrate(int bitrate) { @@ -426,14 +423,20 @@ static int GetOpusBitrateFromParams(const AudioCodec& codec) { return bitrate; } -// True if params["useinbandfec"] == "1" +// Return true params[kCodecParamUseInbandFec] == kParamValueTrue, false +// otherwise. static bool IsOpusFecEnabled(const AudioCodec& codec) { - CodecParameterMap::const_iterator param = - codec.params.find(kCodecParamUseInbandFec); - if (param == codec.params.end()) - return false; + int value; + return codec.GetParam(kCodecParamUseInbandFec, &value) && value == 1; +} - return param->second == kParamValueTrue; +// Set params[kCodecParamUseInbandFec]. Caller should make sure codec is Opus. +static void SetOpusFec(AudioCodec *codec, bool opus_fec) { + if (opus_fec) { + codec->params[kCodecParamUseInbandFec] = kParamValueTrue; + } else { + codec->params.erase(kCodecParamUseInbandFec); + } } void WebRtcVoiceEngine::ConstructCodecs() { @@ -480,6 +483,7 @@ void WebRtcVoiceEngine::ConstructCodecs() { } // TODO(hellner): Add ptime, sprop-stereo, stereo and useinbandfec // when they can be set to values other than the default. + SetOpusFec(&codec, false); } codecs_.push_back(codec); } else { @@ -905,6 +909,16 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { } } + bool opus_fec = false; + if (options.opus_fec.Get(&opus_fec)) { + LOG(LS_INFO) << "Opus FEC is enabled? " << opus_fec; + for (std::vector::iterator it = codecs_.begin(); + it != codecs_.end(); ++it) { + if (IsOpus(*it)) + SetOpusFec(&(*it), opus_fec); + } + } + return true; } @@ -2022,7 +2036,7 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( voe_codec.rate = bitrate_from_params; } - // If FEC is enabled. + // For Opus, we also enable inband FEC if it is requested. if (IsOpusFecEnabled(*it)) { LOG(LS_INFO) << "Enabling Opus FEC on channel " << channel; #ifdef USE_WEBRTC_DEV_BRANCH diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 80a50c555..58893b98a 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -1146,7 +1146,7 @@ TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamEnableNack) { #ifdef USE_WEBRTC_DEV_BRANCH // Test that without useinbandfec, Opus FEC is off. -TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecNoOpusFEC) { +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecNoOpusFec) { EXPECT_TRUE(SetupEngine()); int channel_num = voe_.GetLastChannel(); std::vector codecs; @@ -1157,7 +1157,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecNoOpusFEC) { } // Test that with useinbandfec=0, Opus FEC is off. -TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusDisableFEC) { +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusDisableFec) { EXPECT_TRUE(SetupEngine()); int channel_num = voe_.GetLastChannel(); std::vector codecs; @@ -1174,7 +1174,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusDisableFEC) { } // Test that with useinbandfec=1, Opus FEC is on. -TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusEnableFEC) { +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusEnableFec) { EXPECT_TRUE(SetupEngine()); int channel_num = voe_.GetLastChannel(); std::vector codecs; @@ -1191,7 +1191,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusEnableFEC) { } // Test that with useinbandfec=1, stereo=1, Opus FEC is on. -TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusEnableFECStereo) { +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusEnableFecStereo) { EXPECT_TRUE(SetupEngine()); int channel_num = voe_.GetLastChannel(); std::vector codecs; @@ -1209,7 +1209,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusEnableFECStereo) { } // Test that with non-Opus, codec FEC is off. -TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecIsacNoFEC) { +TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecIsacNoFec) { EXPECT_TRUE(SetupEngine()); int channel_num = voe_.GetLastChannel(); std::vector codecs; @@ -1219,6 +1219,31 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecIsacNoFEC) { } #endif // USE_WEBRTC_DEV_BRANCH +// Test AudioOptions controls whether opus FEC is supported in codec list. +TEST_F(WebRtcVoiceEngineTestFake, OpusFecViaOptions) { + EXPECT_TRUE(SetupEngine()); + std::vector codecs = engine_.codecs(); + int value; + for (std::vector::const_iterator it = codecs.begin(); + it != codecs.end(); ++it) { + if (_stricmp(it->name.c_str(), cricket::kOpusCodecName) == 0) { + EXPECT_FALSE(it->GetParam(cricket::kCodecParamUseInbandFec, &value)); + } + } + + cricket::AudioOptions options; + options.opus_fec.Set(true); + EXPECT_TRUE(engine_.SetOptions(options)); + codecs = engine_.codecs(); + for (std::vector::const_iterator it = codecs.begin(); + it != codecs.end(); ++it) { + if (_stricmp(it->name.c_str(), cricket::kOpusCodecName) == 0) { + EXPECT_TRUE(it->GetParam(cricket::kCodecParamUseInbandFec, &value)); + EXPECT_EQ(1, value); + } + } +} + // Test that we can apply CELT with stereo mode but fail with mono mode. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCelt) { EXPECT_TRUE(SetupEngine());