From 63b345441a995665c1cdd0329b65f895675874ff Mon Sep 17 00:00:00 2001 From: solenberg Date: Tue, 29 Sep 2015 06:06:31 -0700 Subject: [PATCH] Simplify handling of options in WebRtcVoiceMediaEngine. Also removes unnecessary typedef ChannelList. BUG=webrtc:4690 Review URL: https://codereview.webrtc.org/1364753002 Cr-Commit-Position: refs/heads/master@{#10107} --- talk/media/webrtc/webrtcvoiceengine.cc | 63 ++++++++------------------ talk/media/webrtc/webrtcvoiceengine.h | 35 +++----------- 2 files changed, 25 insertions(+), 73 deletions(-) diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 5130232bca..2f73cdd58f 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -582,32 +582,6 @@ bool WebRtcVoiceEngine::SetOptions(const AudioOptions& options) { return true; } -bool WebRtcVoiceEngine::SetOptionOverrides(const AudioOptions& overrides) { - LOG(LS_INFO) << "Setting option overrides: " << overrides.ToString(); - if (!ApplyOptions(overrides)) { - return false; - } - option_overrides_ = overrides; - return true; -} - -bool WebRtcVoiceEngine::ClearOptionOverrides() { - LOG(LS_INFO) << "Clearing option overrides."; - AudioOptions options = options_; - // Only call ApplyOptions if |options_overrides_| contains overrided options. - // ApplyOptions affects NS, AGC other options that is shared between - // all WebRtcVoiceEngineChannels. - if (option_overrides_ == AudioOptions()) { - return true; - } - - if (!ApplyOptions(options)) { - return false; - } - option_overrides_ = AudioOptions(); - return true; -} - // AudioOptions defaults are set in InitInternal (for options with corresponding // MediaEngineInterface flags) and in SetOptions(int) for flagless options. bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { @@ -1245,18 +1219,16 @@ bool WebRtcVoiceEngine::FindChannelAndSsrc( return false; } -void WebRtcVoiceEngine::RegisterChannel(WebRtcVoiceMediaChannel *channel) { +void WebRtcVoiceEngine::RegisterChannel(WebRtcVoiceMediaChannel* channel) { rtc::CritScope lock(&channels_cs_); channels_.push_back(channel); } -void WebRtcVoiceEngine::UnregisterChannel(WebRtcVoiceMediaChannel *channel) { +void WebRtcVoiceEngine::UnregisterChannel(WebRtcVoiceMediaChannel* channel) { rtc::CritScope lock(&channels_cs_); - ChannelList::iterator i = std::find(channels_.begin(), - channels_.end(), - channel); - if (i != channels_.end()) { - channels_.erase(i); + auto it = std::find(channels_.begin(), channels_.end(), channel); + if (it != channels_.end()) { + channels_.erase(it); } } @@ -1507,13 +1479,11 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { options_.SetAll(options); if (send_ != SEND_NOTHING) { - if (!engine()->SetOptionOverrides(options_)) { + if (!engine()->ApplyOptions(options_)) { LOG(LS_WARNING) << - "Failed to engine SetOptionOverrides during channel SetOptions."; + "Failed to apply engine options during channel SetOptions."; return false; } - } else { - // Will be interpreted when appropriate. } // Receiver-side auto gain control happens per channel, so set it here from @@ -2075,19 +2045,24 @@ bool WebRtcVoiceMediaChannel::ChangeSend(SendFlags send) { return true; } - // Change the settings on each send channel. - if (send == SEND_MICROPHONE) - engine()->SetOptionOverrides(options_); + // Apply channel specific options. + if (send == SEND_MICROPHONE) { + engine()->ApplyOptions(options_); + } // Change the settings on each send channel. for (const auto& ch : send_channels_) { - if (!ChangeSend(ch.second->channel(), send)) + if (!ChangeSend(ch.second->channel(), send)) { return false; + } } - // Clear up the options after stopping sending. - if (send == SEND_NOTHING) - engine()->ClearOptionOverrides(); + // Clear up the options after stopping sending. Since we may previously have + // applied the channel specific options, now apply the original options stored + // in WebRtcVoiceEngine. + if (send == SEND_NOTHING) { + engine()->ApplyOptions(engine()->GetOptions()); + } send_ = send; return true; diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 5266f55550..7f0e89c13e 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -92,8 +92,8 @@ class WebRtcVoiceEngine // For tracking WebRtc channels. Needed because we have to pause them // all when switching devices. // May only be called by WebRtcVoiceMediaChannel. - void RegisterChannel(WebRtcVoiceMediaChannel *channel); - void UnregisterChannel(WebRtcVoiceMediaChannel *channel); + void RegisterChannel(WebRtcVoiceMediaChannel* channel); + void UnregisterChannel(WebRtcVoiceMediaChannel* channel); // Called by WebRtcVoiceMediaChannel to set a gain offset from // the default AGC target level. @@ -112,32 +112,16 @@ class WebRtcVoiceEngine int CreateMediaVoiceChannel(); private: - typedef std::vector ChannelList; - void Construct(); void ConstructCodecs(); bool GetVoeCodec(int index, webrtc::CodecInst* codec); bool InitInternal(); void SetTraceFilter(int filter); void SetTraceOptions(const std::string& options); - // Applies either options or overrides. Every option that is "set" - // will be applied. Every option not "set" will be ignored. This - // allows us to selectively turn on and off different options easily - // at any time. + // Every option that is "set" will be applied. Every option not "set" will be + // ignored. This allows us to selectively turn on and off different options + // easily at any time. bool ApplyOptions(const AudioOptions& options); - // Overrides, when set, take precedence over the options on a - // per-option basis. For example, if AGC is set in options and AEC - // is set in overrides, AGC and AEC will be both be set. Overrides - // can also turn off options. For example, if AGC is set to "on" in - // options and AGC is set to "off" in overrides, the result is that - // AGC will be off until different overrides are applied or until - // the overrides are cleared. Only one set of overrides is present - // at a time (they do not "stack"). And when the overrides are - // cleared, the media engine's state reverts back to the options set - // via SetOptions. This allows us to have both "persistent options" - // (the normal options) and "temporary options" (overrides). - bool SetOptionOverrides(const AudioOptions& options); - bool ClearOptionOverrides(); // webrtc::TraceCallback: void Print(webrtc::TraceLevel level, const char* trace, int length) override; @@ -169,7 +153,7 @@ class WebRtcVoiceEngine bool is_dumping_aec_; std::vector codecs_; std::vector rtp_header_extensions_; - ChannelList channels_; + std::vector channels_; // channels_ can be read from WebRtc callback thread. We need a lock on that // callback as well as the RegisterChannel/UnregisterChannel. rtc::CriticalSection channels_cs_; @@ -178,14 +162,7 @@ class WebRtcVoiceEngine webrtc::Config voe_config_; bool initialized_; - // See SetOptions and SetOptionOverrides for a description of the - // difference between options and overrides. - // options_ are the base options, which combined with the - // option_overrides_, create the current options being used. - // options_ is stored so that when option_overrides_ is cleared, we - // can restore the options_ without the option_overrides. AudioOptions options_; - AudioOptions option_overrides_; // Cache received extended_filter_aec, delay_agnostic_aec and experimental_ns // values, and apply them in case they are missing in the audio options. We