Simplify handling of options in WebRtcVoiceMediaEngine.

talk/media/webrtc/webrtcvoiceengine.cc

 /*
  * libjingle
  * Copyright 2004 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 568 matching lines @@
 }
 
 bool WebRtcVoiceEngine::SetOptions(const AudioOptions& options) {
   if (!ApplyOptions(options)) {
     return false;
   }
   options_ = 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) {
   AudioOptions options = options_in;  // The options are modified below.
   // kEcConference is AEC with high suppression.
   webrtc::EcModes ec_mode = webrtc::kEcConference;
   webrtc::AecmModes aecm_mode = webrtc::kAecmSpeakerphone;
   webrtc::AgcModes agc_mode = webrtc::kAgcAdaptiveAnalog;
   webrtc::NsModes ns_mode = webrtc::kNsHighSuppression;
   bool aecm_comfort_noise = false;
@@ skipping 613 matching lines @@
   return false;
 }
 
 void WebRtcVoiceEngine::RegisterChannel(WebRtcVoiceMediaChannel *channel) {
   rtc::CritScope lock(&channels_cs_);
   channels_.push_back(channel);
 }
 
 void WebRtcVoiceEngine::UnregisterChannel(WebRtcVoiceMediaChannel *channel) {
   rtc::CritScope lock(&channels_cs_);
-  ChannelList::iterator i = std::find(channels_.begin(),
-                                      channels_.end(),
-                                      channel);
+  auto i = std::find(channels_.begin(), channels_.end(), channel);

[pbos-webrtc, 2015/09/23 13:41:34]

s/i/it/

[the sun, 2015/09/23 15:11:28]

Done.

   if (i != channels_.end()) {
     channels_.erase(i);
   }
 }
 
 // Adjusts the default AGC target level by the specified delta.
 // NB: If we start messing with other config fields, we'll want
 // to save the current webrtc::AgcConfig as well.
 bool WebRtcVoiceEngine::AdjustAgcLevel(int delta) {
   webrtc::AgcConfig config = default_agc_config_;
@@ skipping 232 matching lines @@
 
   // 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.
   options_.SetAll(options);
 
   if (send_ != SEND_NOTHING) {
-    if (!engine()->SetOptionOverrides(options_)) {
-      LOG(LS_WARNING) <<
-          "Failed to engine SetOptionOverrides during channel SetOptions.";
+    if (!engine()->ApplyOptions(options_)) {
       return false;
     }
-  } else {
-    // Will be interpreted when appropriate.
   }
 
   // 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,
@@ skipping 542 matching lines @@
 bool WebRtcVoiceMediaChannel::ResumeSend() {
   return ChangeSend(desired_send_);
 }
 
 bool WebRtcVoiceMediaChannel::ChangeSend(SendFlags send) {
   if (send_ == send) {
     return true;
   }
 
   // Change the settings on each send channel.
-  if (send == SEND_MICROPHONE)
-    engine()->SetOptionOverrides(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();
+  if (send == SEND_NOTHING) {
+    engine()->ApplyOptions(engine()->GetOptions());

[pbos-webrtc, 2015/09/23 13:41:34]

Why is this not a no-op?

[the sun, 2015/09/23 15:11:28]

Because the engine options_ can be different from the channel options_. Yes, it
is confusing. This CL makes it a little less confusing.

[pbos-webrtc, 2015/09/23 15:13:14]

But you're applying engine's GetOptions to engine? Or are there separate
engine()s? Put a comment here?

+  }
 
   send_ = send;
   return true;
 }
 
 bool WebRtcVoiceMediaChannel::ChangeSend(int channel, SendFlags send) {
   if (send == SEND_MICROPHONE) {
     if (engine()->voe()->base()->StartSend(channel) == -1) {
       LOG_RTCERR1(StartSend, channel);
       return false;
@@ skipping 1184 matching lines @@
       LOG(LS_WARNING) << "Unknown codec " << ToString(codec);
       return false;
     }
   }
   return true;
 }
 
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VOICE