Non-constraint interfaces for all constrainable interfaces

webrtc/api/localaudiosource.cc

Index: webrtc/api/localaudiosource.cc
diff --git a/webrtc/api/localaudiosource.cc b/webrtc/api/localaudiosource.cc
index 4b961cf2a9791c218d3156ebdc8d2361a3d7d297..5fcfe73779389146809201e3cf5fa7e95b98ba79 100644
--- a/webrtc/api/localaudiosource.cc
+++ b/webrtc/api/localaudiosource.cc
@@ -77,6 +77,15 @@ rtc::scoped_refptr<LocalAudioSource> LocalAudioSource::Create(
   return source;
 }
 
+rtc::scoped_refptr<LocalAudioSource> LocalAudioSource::Create(
+    const PeerConnectionFactoryInterface::Options& options,
+    const cricket::AudioOptions* audio_options) {
+  rtc::scoped_refptr<LocalAudioSource> source(
+      new rtc::RefCountedObject<LocalAudioSource>());
+  source->Initialize(options, audio_options);
+  return source;
+}
+
 void LocalAudioSource::Initialize(
     const PeerConnectionFactoryInterface::Options& options,
     const MediaConstraintsInterface* constraints) {
@@ -93,4 +102,14 @@ void LocalAudioSource::Initialize(
   source_state_ = kLive;
 }
 
+void LocalAudioSource::Initialize(
+    const PeerConnectionFactoryInterface::Options& options,
+    const cricket::AudioOptions* audio_options) {
+  if (!audio_options)

[perkj_webrtc, 2016/02/23 11:40:18]

I think you should allow audio_options == nullptr

ie- source_state_ = kLive.

Even better- make sure source_state_(kLive) in the ctor and remove setting it
here and in the default Initialize.

[hta-webrtc, 2016/02/23 14:30:34]

The constructor sets source_state_(kInitializing). I thought this code path
strange, but didn't want to change behavior, so I reproduced the previous
behavior, even though it's bizarre. TODO?

[nisse-webrtc, 2016/02/23 15:11:02]

If it's possible to delete all code dealing with a state of "existing, but
somehow invalid", that would be nice.

I'm not sure what the conventions are in our code, but it seems a static Create
method which does new + Initialize is used when initialization needs to call
virtual functions, which doesn't quite work in the constructor. But when doing
things this way, the constructor is usually not public, and hopefully subclasses
can be trusted to do correct initialization so we don't need code to check for
misuse or track the unititialized state.

[perkj_webrtc, 2016/03/01 08:41:26]

It is probably correct in the base class since for video the state changes
depending on how it went to parse the contraints and start the camera. But here
I dont think it make sence. The LocalAudioSource should set it to someting that
make sence. So please fix it in your new code. 
Also- if audio_options is a pointer - I would have suspected that null is ok and
therefore have the state be kLive even if it is null.

[hta-webrtc, 2016/03/02 09:33:40]

It is actually not present in the base class. It's a private field of the
LocalAudioSource class.

If the logic above holds, it serves no purpose and should be removed. OK. Done.

+    return;
+
+  options_ = *audio_options;
+  source_state_ = kLive;
+}
+
 }  // namespace webrtc