Switch from using internal constraints to use explicit PeerConnectionFactoryInterface::SetOptions t…

content/renderer/media/media_stream_dependency_factory.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/media_stream_dependency_factory.h"
 
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 284 matching lines @@
                                &native_video_constraints).get());
     source_observer->AddSource(source_data->video_source());
   }
 
   // Do additional source initialization if the audio source is a valid
   // microphone or tab audio.
   RTCMediaConstraints native_audio_constraints(audio_constraints);
   ApplyFixedAudioConstraints(&native_audio_constraints);
   WebKit::WebVector<WebKit::WebMediaStreamTrack> audio_tracks;
   web_stream->audioTracks(audio_tracks);
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kEnableWebRtcAecRecordings)) {
-    native_audio_constraints.AddOptional(
-        RTCMediaConstraints::kInternalAecDump, "true");
-  }
   for (size_t i = 0; i < audio_tracks.size(); ++i) {
     const WebKit::WebMediaStreamSource& source = audio_tracks[i].source();
     MediaStreamSourceExtraData* source_data =
         static_cast<MediaStreamSourceExtraData*>(source.extraData());
     if (!source_data) {
       // TODO(henrika): Implement support for sources from remote MediaStreams.
       NOTIMPLEMENTED();
       continue;
     }
 
@@ skipping 184 matching lines @@
     WebKit::WebMediaStreamTrack writable_track = track;
     writable_track.setSourceProvider(NULL);
     return native_stream->RemoveTrack(native_stream->FindAudioTrack(track_id));
   }
 
   CHECK_EQ(type, WebKit::WebMediaStreamSource::TypeVideo);
   return native_stream->RemoveTrack(native_stream->FindVideoTrack(track_id));
 }
 
 bool MediaStreamDependencyFactory::CreatePeerConnectionFactory() {
+  DCHECK(!pc_factory_.get());
+  DCHECK(!audio_device_.get());
   DVLOG(1) << "MediaStreamDependencyFactory::CreatePeerConnectionFactory()";
-  if (!pc_factory_.get()) {
-    DCHECK(!audio_device_.get());
-    audio_device_ = new WebRtcAudioDeviceImpl();
 
-    scoped_ptr<cricket::WebRtcVideoDecoderFactory> decoder_factory;
-    scoped_ptr<cricket::WebRtcVideoEncoderFactory> encoder_factory;
+  audio_device_ = new WebRtcAudioDeviceImpl();

[Ami GONE FROM CHROMIUM, 2013/10/23 21:33:11]

nit: it would be clearer in my opinion to make this a local  scoped_refptr, and
then assign the local value to audio_device_ around l.550, i.e. only if
pc_factory_ is being set to something non-NULL.

I'd also move this "new" statement to as close as the first use of the var (the
webrtc::CreatePeerConnectionFactory() at l.539) as possible.

[perkj_chrome, 2013/11/01 09:12:54]

good points. Done.

 
-    const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
-    scoped_refptr<RendererGpuVideoAcceleratorFactories> gpu_factories =
-        RenderThreadImpl::current()->GetGpuFactories();
+  scoped_ptr<cricket::WebRtcVideoDecoderFactory> decoder_factory;
+  scoped_ptr<cricket::WebRtcVideoEncoderFactory> encoder_factory;
+
+  const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
+  scoped_refptr<RendererGpuVideoAcceleratorFactories> gpu_factories =
+      RenderThreadImpl::current()->GetGpuFactories();
 #if !defined(GOOGLE_TV)
-    if (!cmd_line->HasSwitch(switches::kDisableWebRtcHWDecoding)) {
-      if (gpu_factories)
-        decoder_factory.reset(new RTCVideoDecoderFactory(gpu_factories));
-    }
+  if (!cmd_line->HasSwitch(switches::kDisableWebRtcHWDecoding)) {
+    if (gpu_factories)
+      decoder_factory.reset(new RTCVideoDecoderFactory(gpu_factories));
+  }
 #else
-    // PeerConnectionFactory will hold the ownership of this
-    // VideoDecoderFactory.
-    decoder_factory.reset(decoder_factory_tv_ = new RTCVideoDecoderFactoryTv());
+  // PeerConnectionFactory will hold the ownership of this
+  // VideoDecoderFactory.
+  decoder_factory.reset(decoder_factory_tv_ = new RTCVideoDecoderFactoryTv());
 #endif
 
-    if (!cmd_line->HasSwitch(switches::kDisableWebRtcHWEncoding)) {
-      if (gpu_factories)
-        encoder_factory.reset(new RTCVideoEncoderFactory(gpu_factories));
-    }
+  if (!cmd_line->HasSwitch(switches::kDisableWebRtcHWEncoding)) {
+    if (gpu_factories)
+      encoder_factory.reset(new RTCVideoEncoderFactory(gpu_factories));
+  }
 
-    scoped_refptr<webrtc::PeerConnectionFactoryInterface> factory(
-        webrtc::CreatePeerConnectionFactory(worker_thread_,
-                                            signaling_thread_,
-                                            audio_device_.get(),
-                                            encoder_factory.release(),
-                                            decoder_factory.release()));
-    if (factory.get())
-      pc_factory_ = factory;
-    else
-      audio_device_ = NULL;
+

[Ami GONE FROM CHROMIUM, 2013/10/23 21:33:11]

delete extra \n ?

[perkj_chrome, 2013/11/01 09:12:54]

Done.

+  scoped_refptr<webrtc::PeerConnectionFactoryInterface> factory(
+      webrtc::CreatePeerConnectionFactory(worker_thread_,
+                                          signaling_thread_,
+                                          audio_device_.get(),
+                                          encoder_factory.release(),
+                                          decoder_factory.release()));
+  if (!factory.get()) {
+    audio_device_ = NULL;
+    return false;
   }
-  return pc_factory_.get() != NULL;
+
+  pc_factory_ = factory;
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  PeerConnectionFactoryInterface::Options factory_options;

[Ami GONE FROM CHROMIUM, 2013/10/23 21:33:11]

Probably obvious but this can't land until 55263923 rolls into codesite webrtc
and chromium DEPS rolls.

[perkj_chrome, 2013/11/01 09:12:54]

Done.

+  factory_options.enable_aec_dump =
+      cmd_line->HasSwitch(switches::kEnableWebRtcAecRecordings);
+  factory_options.disable_sctp_data_channels=

[Ami GONE FROM CHROMIUM, 2013/10/23 21:33:11]

s/=/ =/

[perkj_chrome, 2013/11/01 09:12:54]

Done.

+      cmd_line->HasSwitch(switches::kDisableSCTPDataChannels);
+  pc_factory_->SetOptions(factory_options);
+  return true;
 }
 
 bool MediaStreamDependencyFactory::PeerConnectionFactoryCreated() {
   return pc_factory_.get() != NULL;
 }
 
 scoped_refptr<webrtc::PeerConnectionInterface>
 MediaStreamDependencyFactory::CreatePeerConnection(
     const webrtc::PeerConnectionInterface::IceServers& ice_servers,
     const webrtc::MediaConstraintsInterface* constraints,
@@ skipping 272 matching lines @@
   }
 
   // Add the capturer to the WebRtcAudioDeviceImpl if it is a new capturer.
   if (is_new_capturer)
     GetWebRtcAudioDevice()->AddAudioCapturer(capturer);
 
   return capturer;
 }
 
 }  // namespace content