Combining SetVideoSend and SetSource into one method.

webrtc/media/engine/webrtcvideoengine2.cc

Index: webrtc/media/engine/webrtcvideoengine2.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index 9f74a86a8480a1ec7b47948090a875ed2ce6a537..8cc3067a0abfcdc487521ccbd7e8a99fae6bfc2e 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -998,21 +998,27 @@ bool WebRtcVideoChannel2::SetSend(bool send) {
   return true;
 }
 
-bool WebRtcVideoChannel2::SetVideoSend(uint32_t ssrc, bool enable,
-                                       const VideoOptions* options) {
+bool WebRtcVideoChannel2::SetVideoSend(uint32_t ssrc,
+                                       bool enable,
+                                       const VideoOptions* options,
+                                       VideoCapturer* capturer) {
   TRACE_EVENT0("webrtc", "SetVideoSend");
+  RTC_DCHECK(ssrc != 0);
   LOG(LS_INFO) << "SetVideoSend (ssrc= " << ssrc << ", enable = " << enable
                << "options: " << (options ? options->ToString() : "nullptr")
-               << ").";
+               << ", capturer = "
+               << (capturer != nullptr ? "(capturer)" : "NULL") << ")";
 
   // TODO(solenberg): The state change should be fully rolled back if any one of
   //                  these calls fail.
-  if (!MuteStream(ssrc, !enable)) {
+  rtc::CritScope stream_lock(&stream_crit_);
+  const auto& kv = send_streams_.find(ssrc);
+  if (kv == send_streams_.end()) {
+    LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc;
     return false;
   }
-  if (enable && options) {
-    SetOptions(ssrc, *options);
-  }
+
+  kv->second->SetVideoSend(enable, options, capturer);
   return true;
 }
 
@@ -1312,24 +1318,6 @@ void WebRtcVideoChannel2::FillBandwidthEstimationStats(
   video_media_info->bw_estimations.push_back(bwe_info);
 }
 
-bool WebRtcVideoChannel2::SetCapturer(uint32_t ssrc, VideoCapturer* capturer) {
-  LOG(LS_INFO) << "SetCapturer: " << ssrc << " -> "
-               << (capturer != NULL ? "(capturer)" : "NULL");
-  RTC_DCHECK(ssrc != 0);
-  {
-    rtc::CritScope stream_lock(&stream_crit_);
-    const auto& kv = send_streams_.find(ssrc);
-    if (kv == send_streams_.end()) {
-      LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc;
-      return false;
-    }
-    if (!kv->second->SetCapturer(capturer)) {
-      return false;
-    }
-  }
-  return true;
-}
-
 void WebRtcVideoChannel2::OnPacketReceived(
     rtc::CopyOnWriteBuffer* packet,
     const rtc::PacketTime& packet_time) {
@@ -1410,34 +1398,6 @@ void WebRtcVideoChannel2::OnReadyToSend(bool ready) {
       ready ? webrtc::kNetworkUp : webrtc::kNetworkDown);
 }
 
-bool WebRtcVideoChannel2::MuteStream(uint32_t ssrc, bool mute) {
-  LOG(LS_VERBOSE) << "MuteStream: " << ssrc << " -> "
-                  << (mute ? "mute" : "unmute");
-  RTC_DCHECK(ssrc != 0);
-  rtc::CritScope stream_lock(&stream_crit_);
-  const auto& kv = send_streams_.find(ssrc);
-  if (kv == send_streams_.end()) {
-    LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc;
-    return false;
-  }
-
-  kv->second->MuteStream(mute);
-  return true;
-}
-
-// TODO(pbos): Remove SetOptions in favor of SetSendParameters.
-void WebRtcVideoChannel2::SetOptions(uint32_t ssrc,
-                                     const VideoOptions& options) {
-  LOG(LS_INFO) << "SetOptions: ssrc " << ssrc << ": " << options.ToString();
-
-  rtc::CritScope stream_lock(&stream_crit_);
-  const auto& kv = send_streams_.find(ssrc);
-  if (kv == send_streams_.end()) {
-    return;
-  }
-  kv->second->SetOptions(options);
-}
-
 void WebRtcVideoChannel2::SetInterface(NetworkInterface* iface) {
   MediaChannel::SetInterface(iface);
   // Set the RTP recv/send buffer to a bigger size
@@ -1610,23 +1570,40 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::OnFrame(
   stream_->Input()->IncomingCapturedFrame(video_frame);
 }
 
-bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetCapturer(
+bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetVideoSend(
+    bool enable,
+    const VideoOptions* options,
     VideoCapturer* capturer) {
-  TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::SetCapturer");
+  TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::SetVideoSend");
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
-  if (!DisconnectCapturer() && capturer == NULL) {
-    return false;
+
+  bool capturer_changing = capturer_ != capturer;
+  if (capturer_changing) {
+    if (!DisconnectCapturer() && capturer == nullptr) {
+      return false;
+    }
   }
 
   {
     rtc::CritScope cs(&lock_);
 
-    // Reset timestamps to realign new incoming frames to a webrtc timestamp. A
-    // new capturer may have a different timestamp delta than the previous one.
-    first_frame_timestamp_ms_ = 0;
+    muted_ = !enable;
+    if (enable && options) {
+      VideoOptions old_options = parameters_.options;
+      parameters_.options.SetAll(*options);
+      if (parameters_.options != old_options) {
+        // Reconfigure encoder settings on the next frame or stream recreation.
+        pending_encoder_reconfiguration_ = true;
+      }
+    }
+
+    if (capturer_changing) {
+      // Reset timestamps to realign new incoming frames to a webrtc timestamp.
+      // A new capturer may have a different timestamp delta than the previous
+      // one.
+      first_frame_timestamp_ms_ = 0;
 
-    if (capturer == NULL) {
-      if (stream_ != NULL) {
+      if (capturer == nullptr && stream_ != nullptr) {
         LOG(LS_VERBOSE) << "Disabling capturer, sending black frame.";
         webrtc::VideoFrame black_frame;
 
@@ -1642,23 +1619,18 @@ bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetCapturer(
         black_frame.set_render_time_ms(last_frame_timestamp_ms_);
         stream_->Input()->IncomingCapturedFrame(black_frame);
       }
-
-      capturer_ = NULL;
-      return true;

[Taylor Brandstetter, 2016/03/29 22:30:35]

I got rid of this early return and added "if (capturer_changing && capturer_ !=
nullptr)" below because I thought that makes the method's flow more clear.

+      capturer_ = capturer;
     }
   }
-  capturer_ = capturer;
+
   // |capturer_->AddOrUpdateSink| may not be called while holding |lock_| since
   // that might cause a lock order inversion.
-  capturer_->AddOrUpdateSink(this, sink_wants_);
+  if (capturer_changing && capturer_ != nullptr) {
+    capturer_->AddOrUpdateSink(this, sink_wants_);
+  }
   return true;
 }
 
-void WebRtcVideoChannel2::WebRtcVideoSendStream::MuteStream(bool mute) {
-  rtc::CritScope cs(&lock_);
-  muted_ = mute;
-}
-
 bool WebRtcVideoChannel2::WebRtcVideoSendStream::DisconnectCapturer() {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   if (capturer_ == NULL) {
@@ -1682,16 +1654,6 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::GetSsrcs() const {
   return ssrcs_;
 }
 
-void WebRtcVideoChannel2::WebRtcVideoSendStream::SetOptions(
-    const VideoOptions& options) {
-  rtc::CritScope cs(&lock_);
-
-  parameters_.options.SetAll(options);
-  // Reconfigure encoder settings on the next frame or stream
-  // recreation.
-  pending_encoder_reconfiguration_ = true;
-}
-
 webrtc::VideoCodecType CodecTypeFromName(const std::string& name) {
   if (CodecNamesEq(name, kVp8CodecName)) {
     return webrtc::kVideoCodecVP8;