Using AudioOption to enable audio network adaptor.

webrtc/media/base/mediachannel.h

Index: webrtc/media/base/mediachannel.h
diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h
index 781a46d90c63122bc6504ca02f97cf3ecefa742a..b35981d2829fafde271d9f56d35f4cbebb0649f4 100644
--- a/webrtc/media/base/mediachannel.h
+++ b/webrtc/media/base/mediachannel.h
@@ -168,6 +168,8 @@ struct AudioOptions {
     SetFrom(&recording_sample_rate, change.recording_sample_rate);
     SetFrom(&playout_sample_rate, change.playout_sample_rate);
     SetFrom(&combined_audio_video_bwe, change.combined_audio_video_bwe);
+    SetFrom(&audio_network_adaptor, change.audio_network_adaptor);
+    SetFrom(&audio_network_adaptor_config, change.audio_network_adaptor_config);
   }
 
   bool operator==(const AudioOptions& o) const {
@@ -195,7 +197,9 @@ struct AudioOptions {
            tx_agc_limiter == o.tx_agc_limiter &&
            recording_sample_rate == o.recording_sample_rate &&
            playout_sample_rate == o.playout_sample_rate &&
-           combined_audio_video_bwe == o.combined_audio_video_bwe;
+           combined_audio_video_bwe == o.combined_audio_video_bwe &&
+           audio_network_adaptor == o.audio_network_adaptor &&
+           audio_network_adaptor_config == o.audio_network_adaptor_config;
   }
   bool operator!=(const AudioOptions& o) const { return !(*this == o); }
 
@@ -227,6 +231,7 @@ struct AudioOptions {
     ost << ToStringIfSet("recording_sample_rate", recording_sample_rate);
     ost << ToStringIfSet("playout_sample_rate", playout_sample_rate);
     ost << ToStringIfSet("combined_audio_video_bwe", combined_audio_video_bwe);
+    ost << ToStringIfSet("audio_network_adaptor", audio_network_adaptor);

[the sun, 2016/10/25 09:25:46]

audio_network_adaptor_config missing?

[minyue-webrtc, 2016/10/27 14:33:10]

I don't know if to String will be useful for audio_network_adaptor_config
because it is not human readable. Do you think it is still good to add it?

[the sun, 2016/10/28 18:21:52]

Oh? What does it look like? Is it really not possible to understand anything
about the configuration by looking at the string? Then, why is it a string? :)

[minyue-webrtc, 2016/10/28 18:41:57]

It is a proto buffer serialized string. it is a std::string since proto buffer
c++ api uses it as default type.

[the sun, 2016/10/28 19:04:47]

Ah, in that case, just leave:
// The adaptor config is a serialized proto buffer and therefore not human
readable.
// ost << ToStringIfSet("audio_network_adaptor_config",
audio_network_adaptor_config);

     ost << "}";
     return ost.str();
   }
@@ -267,6 +272,10 @@ struct AudioOptions {
   // "googCombinedAudioVideoBwe", but not used anywhere. So delete it,
   // and check if any other AudioOptions members are unused.
   rtc::Optional<bool> combined_audio_video_bwe;
+  // Enable audio network adaptor.
+  rtc::Optional<bool> audio_network_adaptor;

[minyue-webrtc, 2016/10/24 10:57:11]

I initially wanted to let audio_network_adaptor_config.has_value to determine
whether audio network adaptor should be on or off. However, there is a normally
a third status for audio options (don't care / don't override). So I need to add
an optional<bool> here too.

[the sun, 2016/10/25 09:46:32]

How are you going to roll the feature out? If you're planning to use an
experiment (i.e. using the webrtc::field_trial:: API), could you just check that
in the WVoMC ctor? That would allow you to use just one field here.

[minyue-webrtc, 2016/10/27 14:33:10]

The feature is configured in a way that it is up to the application to decide
whether to turn it on and how to configure audio network adaptor.

[the sun, 2016/10/28 19:04:47]

Btw, what happened to your idea of only having the config string?

[minyue-webrtc, 2016/10/28 20:15:03]

I initially wanted to let audio_network_adaptor_config.has_value to determine
whether audio network adaptor should be on or off. However, there is a
a third status for audio options (don't care / don't override). So I need to add
an optional<bool> here too.

[the sun, 2016/10/28 20:33:08]

I probably misunderstood then - I thought you said since the config string is
always required when the adaptor is enabled, that you could use the empty string
("") as the third state. But I may be getting the logic wrong.

[minyue-webrtc, 2016/10/28 21:05:45]

If we use only a Option<string>, when a second set of options comes in, we may
not be easily distinguish between "don't care" and "turn off".  I cannot find
better solutions. In principle, Optional<bool> |audio_network_adaptor| must be
there, and string |audio_network_adaptor_config| is obligatory when
|audio_network_adaptor| is true.

But the obligatoriness of |audio_network_adaptor_config| can be changed in the
future, if a default config is introduced.

+  // Config string for audio network adaptor.
+  rtc::Optional<std::string> audio_network_adaptor_config;
 
  private:
   template <typename T>