Remove WebRtcVideoEngine::SetVoiceEngine.

Instead enforcing that a voice engine is set on construction. Apart from
simplifying the class this permits tracing to be set up in the
constructor without worrying about racing sets from SetVoiceEngine
later.

BUG=
R=tommi@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/44489004

Cr-Commit-Position: refs/heads/master@{#8555}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8555 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/media/base/fakemediaengine.h

@@ -860,7 +860,9 @@ class FakeVoiceEngine : public FakeBaseEngine {
 
 class FakeVideoEngine : public FakeBaseEngine {
  public:
-  FakeVideoEngine() : capture_(false), processor_(NULL) {
+  FakeVideoEngine() : FakeVideoEngine(nullptr) {}
+  explicit FakeVideoEngine(FakeVoiceEngine* voice)
+      : capture_(false), processor_(NULL) {
     // Add a fake video codec. Note that the name must not be "" as there are
     // sanity checks against that.
     codecs_.push_back(VideoCodec(0, "fake_video_codec", 0, 0, 0, 0));

talk/media/base/mediaengine.h

@@ -172,7 +172,7 @@ class MediaEngineFactory {
 template<class VOICE, class VIDEO>
 class CompositeMediaEngine : public MediaEngineInterface {
  public:
-  CompositeMediaEngine() {}
+  CompositeMediaEngine() : video_(&voice_) {}
   virtual ~CompositeMediaEngine() {}
   virtual bool Init(rtc::Thread* worker_thread) {
     if (!voice_.Init(worker_thread))

talk/media/base/videoengine_unittest.h

@@ -100,7 +100,7 @@ inline int TimeBetweenSend(const cricket::VideoCodec& codec) {
 template<class T>
 class VideoEngineOverride : public T {
  public:
-  VideoEngineOverride() {
+  VideoEngineOverride() : T(nullptr) {
   }
   virtual ~VideoEngineOverride() {
   }

talk/media/webrtc/webrtcmediaengine.cc

@@ -49,7 +49,6 @@ class WebRtcMediaEngine :
       WebRtcVideoEncoderFactory* encoder_factory,
       WebRtcVideoDecoderFactory* decoder_factory) {
     voice_.SetAudioDeviceModule(adm, adm_sc);
-    video_.SetVoiceEngine(&voice_);
     video_.SetExternalEncoderFactory(encoder_factory);
     video_.SetExternalDecoderFactory(decoder_factory);
   }
@@ -66,7 +65,6 @@ class WebRtcMediaEngine2 :
     voice_.SetAudioDeviceModule(adm, adm_sc);
     video_.SetExternalDecoderFactory(decoder_factory);
     video_.SetExternalEncoderFactory(encoder_factory);
-    video_.SetVoiceEngine(&voice_);
   }
 };
 #endif  // WEBRTC_CHROMIUM_BUILD

talk/media/webrtc/webrtcvideoengine.cc

@@ -1066,8 +1066,8 @@ static bool GetCpuOveruseOptions(const VideoOptions& options,
   return true;
 }
 
-WebRtcVideoEngine::WebRtcVideoEngine() {
-  Construct(new ViEWrapper(), new ViETraceWrapper(), NULL,
+WebRtcVideoEngine::WebRtcVideoEngine(WebRtcVoiceEngine* voice_engine) {
+  Construct(new ViEWrapper(), new ViETraceWrapper(), voice_engine,
       new rtc::CpuMonitor(NULL));
 }
 
@@ -1473,15 +1473,6 @@ void WebRtcVideoEngine::UnregisterChannel(WebRtcVideoMediaChannel *channel) {
                   channels_.end());
 }
 
-bool WebRtcVideoEngine::SetVoiceEngine(WebRtcVoiceEngine* voice_engine) {
-  if (initialized_) {
-    LOG(LS_WARNING) << "SetVoiceEngine can not be called after Init";
-    return false;
-  }
-  voice_engine_ = voice_engine;
-  return true;
-}
-
 bool WebRtcVideoEngine::EnableTimedRender() {
   if (initialized_) {
     LOG(LS_WARNING) << "EnableTimedRender can not be called after Init";

talk/media/webrtc/webrtcvideoengine.h

@@ -98,7 +98,7 @@ class WebRtcVideoEngine : public sigslot::has_slots<>,
                           public webrtc::TraceCallback {
  public:
   // Creates the WebRtcVideoEngine with internal VideoCaptureModule.
-  WebRtcVideoEngine();
+  explicit WebRtcVideoEngine(WebRtcVoiceEngine* voice_engine);
   // For testing purposes. Allows the WebRtcVoiceEngine,
   // ViEWrapper and CpuMonitor to be mocks.
   // TODO(juberti): Remove the 3-arg ctor once fake tracing is implemented.
@@ -131,8 +131,6 @@ class WebRtcVideoEngine : public sigslot::has_slots<>,
 
   sigslot::repeater2<VideoCapturer*, CaptureState> SignalCaptureStateChange;
 
-  // Set the VoiceEngine for A/V sync. This can only be called before Init.
-  bool SetVoiceEngine(WebRtcVoiceEngine* voice_engine);
   // Set a WebRtcVideoDecoderFactory for external decoding. Video engine does
   // not take the ownership of |decoder_factory|. The caller needs to make sure
   // that |decoder_factory| outlives the video engine.

talk/media/webrtc/webrtcvideoengine2.cc

@@ -315,9 +315,9 @@ void DefaultUnsignalledSsrcHandler::SetDefaultRenderer(
   }
 }
 
-WebRtcVideoEngine2::WebRtcVideoEngine2()
+WebRtcVideoEngine2::WebRtcVideoEngine2(WebRtcVoiceEngine* voice_engine)
     : worker_thread_(NULL),
-      voice_engine_(NULL),
+      voice_engine_(voice_engine),
       default_codec_format_(kDefaultVideoMaxWidth,
                             kDefaultVideoMaxHeight,
                             FPS_TO_INTERVAL(kDefaultVideoMaxFramerate),
@@ -524,15 +524,6 @@ bool WebRtcVideoEngine2::CanSendCodec(const VideoCodec& requested,
   return out->width > 0 && out->height > 0;
 }
 
-bool WebRtcVideoEngine2::SetVoiceEngine(WebRtcVoiceEngine* voice_engine) {
-  if (initialized_) {
-    LOG(LS_WARNING) << "SetVoiceEngine can not be called after Init";
-    return false;
-  }
-  voice_engine_ = voice_engine;
-  return true;
-}
-
 // Ignore spammy trace messages, mostly from the stats API when we haven't
 // gotten RTCP info yet from the remote side.
 bool WebRtcVideoEngine2::ShouldIgnoreTrace(const std::string& trace) {

talk/media/webrtc/webrtcvideoengine2.h

@@ -111,7 +111,7 @@ class WebRtcCallFactory {
 // WebRtcVideoEngine2 is used for the new native WebRTC Video API (webrtc:1667).
 class WebRtcVideoEngine2 : public sigslot::has_slots<> {
  public:
-  WebRtcVideoEngine2();
+  explicit WebRtcVideoEngine2(WebRtcVoiceEngine* voice_engine);
   virtual ~WebRtcVideoEngine2();
 
   // Used for testing to be able to check and use the webrtc::Call config.
@@ -145,9 +145,6 @@ class WebRtcVideoEngine2 : public sigslot::has_slots<> {
   // This is currently ignored.
   sigslot::repeater2<VideoCapturer*, CaptureState> SignalCaptureStateChange;
 
-  // Set the VoiceEngine for A/V sync. This can only be called before Init.
-  bool SetVoiceEngine(WebRtcVoiceEngine* voice_engine);
-
   bool FindCodec(const VideoCodec& in);
   bool CanSendCodec(const VideoCodec& in,
                     const VideoCodec& current,

talk/media/webrtc/webrtcvideoengine2_unittest.cc

@@ -363,7 +363,9 @@ void FakeCall::SignalNetworkState(webrtc::Call::NetworkState state) {
 
 class WebRtcVideoEngine2Test : public ::testing::Test {
  public:
-  WebRtcVideoEngine2Test() {
+  WebRtcVideoEngine2Test() : WebRtcVideoEngine2Test(nullptr) {}
+  WebRtcVideoEngine2Test(WebRtcVoiceEngine* voice_engine)
+      : engine_(voice_engine) {
     std::vector<VideoCodec> engine_codecs = engine_.codecs();
     assert(!engine_codecs.empty());
     bool codec_set = false;
@@ -408,6 +410,9 @@ class WebRtcVideoEngine2Test : public ::testing::Test {
       cricket::WebRtcVideoDecoderFactory* decoder_factory,
       const std::vector<VideoCodec>& codecs);
 
+  // Used in WebRtcVideoEngine2VoiceTest, but defined here so it's properly
+  // initialized when the constructor is called.
+  WebRtcVoiceEngine voice_engine_;
   WebRtcVideoEngine2 engine_;
   VideoCodec default_codec_;
   VideoCodec default_red_codec_;
@@ -415,17 +420,20 @@ class WebRtcVideoEngine2Test : public ::testing::Test {
   VideoCodec default_rtx_codec_;
 };
 
-TEST_F(WebRtcVideoEngine2Test, ConfiguresAvSyncForFirstReceiveChannel) {
+class WebRtcVideoEngine2VoiceTest : public WebRtcVideoEngine2Test {
+ public:
+  WebRtcVideoEngine2VoiceTest() : WebRtcVideoEngine2Test(&voice_engine_) {}
+};
+
+TEST_F(WebRtcVideoEngine2VoiceTest, ConfiguresAvSyncForFirstReceiveChannel) {
   FakeCallFactory call_factory;
   engine_.SetCallFactory(&call_factory);
 
-  WebRtcVoiceEngine voice_engine;
-  engine_.SetVoiceEngine(&voice_engine);
-  voice_engine.Init(rtc::Thread::Current());
+  voice_engine_.Init(rtc::Thread::Current());
   engine_.Init(rtc::Thread::Current());
 
   rtc::scoped_ptr<VoiceMediaChannel> voice_channel(
-      voice_engine.CreateChannel());
+      voice_engine_.CreateChannel());
   ASSERT_TRUE(voice_channel.get() != NULL);
   WebRtcVoiceMediaChannel* webrtc_voice_channel =
       static_cast<WebRtcVoiceMediaChannel*>(voice_channel.get());
@@ -438,8 +446,8 @@ TEST_F(WebRtcVideoEngine2Test, ConfiguresAvSyncForFirstReceiveChannel) {
 
   webrtc::Call::Config call_config = fake_call->GetConfig();
 
-  ASSERT_TRUE(voice_engine.voe()->engine() != NULL);
-  ASSERT_EQ(voice_engine.voe()->engine(), call_config.voice_engine);
+  ASSERT_TRUE(voice_engine_.voe()->engine() != NULL);
+  ASSERT_EQ(voice_engine_.voe()->engine(), call_config.voice_engine);
 
   EXPECT_TRUE(channel->AddRecvStream(StreamParams::CreateLegacy(kSsrc)));
   EXPECT_TRUE(channel->AddRecvStream(StreamParams::CreateLegacy(kSsrc + 1)));
@@ -2282,7 +2290,7 @@ TEST_F(WebRtcVideoChannel2Test, TranslatesSenderBitrateStatsCorrectly) {
 class WebRtcVideoEngine2SimulcastTest : public testing::Test {
  public:
   WebRtcVideoEngine2SimulcastTest()
-      : engine_codecs_(engine_.codecs()) {
+      : engine_(nullptr), engine_codecs_(engine_.codecs()) {
     assert(!engine_codecs_.empty());
 
     bool codec_set = false;