diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index bf06e23686..693fef0822 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -160,7 +160,6 @@ struct AudioOptions { void SetAll(const AudioOptions& change) { echo_cancellation.SetFrom(change.echo_cancellation); auto_gain_control.SetFrom(change.auto_gain_control); - rx_auto_gain_control.SetFrom(change.rx_auto_gain_control); noise_suppression.SetFrom(change.noise_suppression); highpass_filter.SetFrom(change.highpass_filter); stereo_swapping.SetFrom(change.stereo_swapping); @@ -181,10 +180,6 @@ struct AudioOptions { tx_agc_digital_compression_gain.SetFrom( change.tx_agc_digital_compression_gain); tx_agc_limiter.SetFrom(change.tx_agc_limiter); - rx_agc_target_dbov.SetFrom(change.rx_agc_target_dbov); - rx_agc_digital_compression_gain.SetFrom( - change.rx_agc_digital_compression_gain); - rx_agc_limiter.SetFrom(change.rx_agc_limiter); recording_sample_rate.SetFrom(change.recording_sample_rate); playout_sample_rate.SetFrom(change.playout_sample_rate); dscp.SetFrom(change.dscp); @@ -194,7 +189,6 @@ struct AudioOptions { bool operator==(const AudioOptions& o) const { return echo_cancellation == o.echo_cancellation && auto_gain_control == o.auto_gain_control && - rx_auto_gain_control == o.rx_auto_gain_control && noise_suppression == o.noise_suppression && highpass_filter == o.highpass_filter && stereo_swapping == o.stereo_swapping && @@ -213,9 +207,6 @@ struct AudioOptions { tx_agc_target_dbov == o.tx_agc_target_dbov && tx_agc_digital_compression_gain == o.tx_agc_digital_compression_gain && tx_agc_limiter == o.tx_agc_limiter && - rx_agc_target_dbov == o.rx_agc_target_dbov && - rx_agc_digital_compression_gain == o.rx_agc_digital_compression_gain && - rx_agc_limiter == o.rx_agc_limiter && recording_sample_rate == o.recording_sample_rate && playout_sample_rate == o.playout_sample_rate && dscp == o.dscp && @@ -227,7 +218,6 @@ struct AudioOptions { ost << "AudioOptions {"; ost << ToStringIfSet("aec", echo_cancellation); ost << ToStringIfSet("agc", auto_gain_control); - ost << ToStringIfSet("rx_agc", rx_auto_gain_control); ost << ToStringIfSet("ns", noise_suppression); ost << ToStringIfSet("hf", highpass_filter); ost << ToStringIfSet("swap", stereo_swapping); @@ -248,10 +238,6 @@ struct AudioOptions { ost << ToStringIfSet("tx_agc_digital_compression_gain", tx_agc_digital_compression_gain); ost << ToStringIfSet("tx_agc_limiter", tx_agc_limiter); - ost << ToStringIfSet("rx_agc_target_dbov", rx_agc_target_dbov); - ost << ToStringIfSet("rx_agc_digital_compression_gain", - rx_agc_digital_compression_gain); - ost << ToStringIfSet("rx_agc_limiter", rx_agc_limiter); ost << ToStringIfSet("recording_sample_rate", recording_sample_rate); ost << ToStringIfSet("playout_sample_rate", playout_sample_rate); ost << ToStringIfSet("dscp", dscp); @@ -265,8 +251,6 @@ struct AudioOptions { Settable echo_cancellation; // Audio processing to adjust the sensitivity of the local mic dynamically. Settable auto_gain_control; - // Audio processing to apply gain to the remote audio. - Settable rx_auto_gain_control; // Audio processing to filter out background noise. Settable noise_suppression; // Audio processing to remove background noise of lower frequencies. @@ -291,9 +275,6 @@ struct AudioOptions { Settable tx_agc_target_dbov; Settable tx_agc_digital_compression_gain; Settable tx_agc_limiter; - Settable rx_agc_target_dbov; - Settable rx_agc_digital_compression_gain; - Settable rx_agc_limiter; Settable recording_sample_rate; Settable playout_sample_rate; // Set DSCP value for packet sent from audio channel. diff --git a/talk/media/webrtc/fakewebrtcvoiceengine.h b/talk/media/webrtc/fakewebrtcvoiceengine.h index 672c5626f9..10b5357eae 100644 --- a/talk/media/webrtc/fakewebrtcvoiceengine.h +++ b/talk/media/webrtc/fakewebrtcvoiceengine.h @@ -208,8 +208,6 @@ class FakeWebRtcVoiceEngine opus_dtx(false), red(false), nack(false), - rx_agc_enabled(false), - rx_agc_mode(webrtc::kAgcDefault), cn8_type(13), cn16_type(105), dtmf_type(106), @@ -224,7 +222,6 @@ class FakeWebRtcVoiceEngine neteq_capacity(-1), neteq_fast_accelerate(false) { memset(&send_codec, 0, sizeof(send_codec)); - memset(&rx_agc_config, 0, sizeof(rx_agc_config)); } bool external_transport; bool send; @@ -238,9 +235,6 @@ class FakeWebRtcVoiceEngine bool opus_dtx; bool red; bool nack; - bool rx_agc_enabled; - webrtc::AgcModes rx_agc_mode; - webrtc::AgcConfig rx_agc_config; int cn8_type; int cn16_type; int dtmf_type; @@ -1001,27 +995,12 @@ class FakeWebRtcVoiceEngine WEBRTC_STUB(SetRxNsStatus, (int channel, bool enable, webrtc::NsModes mode)); WEBRTC_STUB(GetRxNsStatus, (int channel, bool& enabled, webrtc::NsModes& mode)); - WEBRTC_FUNC(SetRxAgcStatus, (int channel, bool enable, - webrtc::AgcModes mode)) { - channels_[channel]->rx_agc_enabled = enable; - channels_[channel]->rx_agc_mode = mode; - return 0; - } - WEBRTC_FUNC(GetRxAgcStatus, (int channel, bool& enabled, - webrtc::AgcModes& mode)) { - enabled = channels_[channel]->rx_agc_enabled; - mode = channels_[channel]->rx_agc_mode; - return 0; - } - - WEBRTC_FUNC(SetRxAgcConfig, (int channel, webrtc::AgcConfig config)) { - channels_[channel]->rx_agc_config = config; - return 0; - } - WEBRTC_FUNC(GetRxAgcConfig, (int channel, webrtc::AgcConfig& config)) { - config = channels_[channel]->rx_agc_config; - return 0; - } + WEBRTC_STUB(SetRxAgcStatus, (int channel, bool enable, + webrtc::AgcModes mode)); + WEBRTC_STUB(GetRxAgcStatus, (int channel, bool& enabled, + webrtc::AgcModes& mode)); + WEBRTC_STUB(SetRxAgcConfig, (int channel, webrtc::AgcConfig config)); + WEBRTC_STUB(GetRxAgcConfig, (int channel, webrtc::AgcConfig& config)); WEBRTC_STUB(RegisterRxVadObserver, (int, webrtc::VoERxVadCallback&)); WEBRTC_STUB(DeRegisterRxVadObserver, (int channel)); diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 54fac221d8..89397b4822 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -148,6 +148,18 @@ const char kAecDumpByAudioOptionFilename[] = "/sdcard/audio.aecdump"; const char kAecDumpByAudioOptionFilename[] = "audio.aecdump"; #endif +bool ValidateStreamParams(const StreamParams& sp) { + if (sp.ssrcs.empty()) { + LOG(LS_ERROR) << "No SSRCs in stream parameters: " << sp.ToString(); + return false; + } + if (sp.ssrcs.size() > 1) { + LOG(LS_ERROR) << "Multiple SSRCs in stream parameters: " << sp.ToString(); + return false; + } + return true; +} + // Dumps an AudioCodec in RFC 2327-ish format. std::string ToString(const AudioCodec& codec) { std::stringstream ss; @@ -221,6 +233,19 @@ bool FindCodec(const std::vector& codecs, return false; } +bool VerifyUniquePayloadTypes(const std::vector& codecs) { + if (codecs.empty()) { + return true; + } + std::vector payload_types; + for (const AudioCodec& codec : codecs) { + payload_types.push_back(codec.id); + } + std::sort(payload_types.begin(), payload_types.end()); + auto it = std::unique(payload_types.begin(), payload_types.end()); + return it == payload_types.end(); +} + bool IsNackEnabled(const AudioCodec& codec) { return codec.HasFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty)); @@ -1445,9 +1470,6 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { // Check if DSCP value is changed from previous. bool dscp_option_changed = (options_.dscp != options.dscp); - // TODO(xians): Add support to set different options for different send - // streams after we support multiple APMs. - // We retain all of the existing options, and apply the given ones // on top. This means there is no way to "clear" options such that // they go back to the engine default. @@ -1461,55 +1483,6 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { } } - // Receiver-side auto gain control happens per channel, so set it here from - // options. Note that, like conference mode, setting it on the engine won't - // have the desired effect, since voice channels don't inherit options from - // the media engine when those options are applied per-channel. - bool rx_auto_gain_control; - if (options.rx_auto_gain_control.Get(&rx_auto_gain_control)) { - if (engine()->voe()->processing()->SetRxAgcStatus( - voe_channel(), rx_auto_gain_control, - webrtc::kAgcFixedDigital) == -1) { - LOG_RTCERR1(SetRxAgcStatus, rx_auto_gain_control); - return false; - } else { - LOG(LS_VERBOSE) << "Rx auto gain set to " << rx_auto_gain_control - << " with mode " << webrtc::kAgcFixedDigital; - } - } - if (options.rx_agc_target_dbov.IsSet() || - options.rx_agc_digital_compression_gain.IsSet() || - options.rx_agc_limiter.IsSet()) { - webrtc::AgcConfig config; - // If only some of the options are being overridden, get the current - // settings for the channel and bail if they aren't available. - if (!options.rx_agc_target_dbov.IsSet() || - !options.rx_agc_digital_compression_gain.IsSet() || - !options.rx_agc_limiter.IsSet()) { - if (engine()->voe()->processing()->GetRxAgcConfig( - voe_channel(), config) != 0) { - LOG(LS_ERROR) << "Failed to get default rx agc configuration for " - << "channel " << voe_channel() << ". Since not all rx " - << "agc options are specified, unable to safely set rx " - << "agc options."; - return false; - } - } - config.targetLeveldBOv = - options.rx_agc_target_dbov.GetWithDefaultIfUnset( - config.targetLeveldBOv); - config.digitalCompressionGaindB = - options.rx_agc_digital_compression_gain.GetWithDefaultIfUnset( - config.digitalCompressionGaindB); - config.limiterEnable = options.rx_agc_limiter.GetWithDefaultIfUnset( - config.limiterEnable); - if (engine()->voe()->processing()->SetRxAgcConfig( - voe_channel(), config) == -1) { - LOG_RTCERR4(SetRxAgcConfig, voe_channel(), config.targetLeveldBOv, - config.digitalCompressionGaindB, config.limiterEnable); - return false; - } - } if (dscp_option_changed) { rtc::DiffServCodePoint dscp = rtc::DSCP_DEFAULT; if (options_.dscp.GetWithDefaultIfUnset(false)) @@ -1518,9 +1491,7 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { LOG(LS_WARNING) << "Failed to set DSCP settings for audio channel"; } } - RecreateAudioReceiveStreams(); - LOG(LS_INFO) << "Set voice channel options. Current options: " << options_.ToString(); return true; @@ -1528,9 +1499,14 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { bool WebRtcVoiceMediaChannel::SetRecvCodecs( const std::vector& codecs) { - RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Set the payload types to be used for incoming media. - LOG(LS_INFO) << "Setting receive voice codecs:"; + LOG(LS_INFO) << "Setting receive voice codecs."; + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + + if (!VerifyUniquePayloadTypes(codecs)) { + LOG(LS_ERROR) << "Codec payload types overlap."; + return false; + } std::vector new_codecs; // Find all new codecs. We allow adding new codecs but don't allow changing @@ -2229,17 +2205,18 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); LOG(LS_INFO) << "AddRecvStream: " << sp.ToString(); - rtc::CritScope lock(&receive_channels_cs_); - - if (!VERIFY(sp.ssrcs.size() == 1)) - return false; - uint32_t ssrc = sp.first_ssrc(); - - if (ssrc == 0) { - LOG(LS_WARNING) << "AddRecvStream with 0 ssrc is not supported."; + if (!ValidateStreamParams(sp)) { return false; } + uint32_t ssrc = sp.first_ssrc(); + if (ssrc == 0) { + LOG(LS_WARNING) << "AddRecvStream with ssrc==0 is not supported."; + return false; + } + + rtc::CritScope lock(&receive_channels_cs_); + if (receive_channels_.find(ssrc) != receive_channels_.end()) { LOG(LS_ERROR) << "Stream already exists with ssrc " << ssrc; return false; @@ -2667,16 +2644,19 @@ void WebRtcVoiceMediaChannel::OnRtcpReceived( return; } - // If it is a sender report, find the channel that is listening. + // If it is a sender report, find the receive channel that is listening. bool has_sent_to_default_channel = false; if (type == kRtcpTypeSR) { - int which_channel = - GetReceiveChannelId(ParseSsrc(packet->data(), packet->size(), true)); - if (which_channel != -1) { + uint32_t ssrc = 0; + if (!GetRtcpSsrc(packet->data(), packet->size(), &ssrc)) { + return; + } + int recv_channel_id = GetReceiveChannelId(ssrc); + if (recv_channel_id != -1) { engine()->voe()->network()->ReceivedRTCPPacket( - which_channel, packet->data(), packet->size()); + recv_channel_id, packet->data(), packet->size()); - if (IsDefaultChannel(which_channel)) + if (IsDefaultChannel(recv_channel_id)) has_sent_to_default_channel = true; } } diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 70318d14f7..413a969438 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -2219,23 +2219,6 @@ TEST_F(WebRtcVoiceEngineTestFake, TxAgcConfigViaOptions) { EXPECT_EQ(13, agc_config.targetLeveldBOv); } -TEST_F(WebRtcVoiceEngineTestFake, RxAgcConfigViaOptions) { - EXPECT_TRUE(SetupEngine()); - int channel_num = voe_.GetLastChannel(); - send_parameters_.options.rx_agc_target_dbov.Set(6); - send_parameters_.options.rx_agc_digital_compression_gain.Set(0); - send_parameters_.options.rx_agc_limiter.Set(true); - send_parameters_.options.rx_auto_gain_control.Set(true); - EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); - - webrtc::AgcConfig agc_config; - EXPECT_EQ(0, engine_.voe()->processing()->GetRxAgcConfig( - channel_num, agc_config)); - EXPECT_EQ(6, agc_config.targetLeveldBOv); - EXPECT_EQ(0, agc_config.digitalCompressionGaindB); - EXPECT_TRUE(agc_config.limiterEnable); -} - TEST_F(WebRtcVoiceEngineTestFake, SampleRatesViaOptions) { EXPECT_TRUE(SetupEngine()); cricket::AudioOptions options;