Combining SetVideoSend and SetSource into one method.

webrtc/media/engine/webrtcvideoengine2.cc

 /*
  *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 1001 matching lines @@
     rtc::CritScope stream_lock(&stream_crit_);
     for (const auto& kv : send_streams_) {
       kv.second->SetSend(send);
     }
   }
   sending_ = send;
   return true;
 }
 
 // TODO(nisse): The enable argument was used for mute logic which has
-// been moved to VideoBroadcaster. So delete this method, and use
-// SetOptions instead.
-bool WebRtcVideoChannel2::SetVideoSend(uint32_t ssrc, bool enable,
-                                       const VideoOptions* options) {
+// been moved to VideoBroadcaster. So remove the argument from this
+// method.
+bool WebRtcVideoChannel2::SetVideoSend(
+    uint32_t ssrc,
+    bool enable,
+    const VideoOptions* options,
+    rtc::VideoSourceInterface<cricket::VideoFrame>* source) {
   TRACE_EVENT0("webrtc", "SetVideoSend");
+  RTC_DCHECK(ssrc != 0);
   LOG(LS_INFO) << "SetVideoSend (ssrc= " << ssrc << ", enable = " << enable
-               << "options: " << (options ? options->ToString() : "nullptr")
-               << ").";
+               << ", options: " << (options ? options->ToString() : "nullptr")
+               << ", source = " << (source != nullptr ? "(source)" : "NULL")

[pbos-webrtc, 2016/05/14 00:42:39]

"nullptr"

[Taylor Brandstetter, 2016/05/16 19:03:00]

Done.

+               << ")";
 
-  if (enable && options) {
-    SetOptions(ssrc, *options);
+  rtc::CritScope stream_lock(&stream_crit_);
+  const auto& kv = send_streams_.find(ssrc);
+  if (kv == send_streams_.end()) {
+    // Allow unknown ssrc only if source is null.
+    RTC_CHECK(source == nullptr);
+    LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc;

[pbos-webrtc, 2016/05/14 00:42:39]

This log will never happen, do RTC_CHECK(...) <<

[Taylor Brandstetter, 2016/05/16 19:03:00]

I believe it actually can happen due to this issue:
https://bugs.chromium.org/p/webrtc/issues/detail?id=4908

Hence the workaround to only RTC_CHECK if the source is not nullptr.

[pbos-webrtc, 2016/05/16 19:04:50]

Oh, totally misread this. Carry on. :)

+    return false;
   }
-  return true;
+
+  return kv->second->SetVideoSend(enable, options, source);
 }
 
 bool WebRtcVideoChannel2::ValidateSendSsrcAvailability(
     const StreamParams& sp) const {
   for (uint32_t ssrc : sp.ssrcs) {
     if (send_ssrcs_.find(ssrc) != send_ssrcs_.end()) {
       LOG(LS_ERROR) << "Send stream with SSRC '" << ssrc << "' already exists.";
       return false;
     }
   }
@@ skipping 278 matching lines @@
   // Get send stream bitrate stats.
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32_t, WebRtcVideoSendStream*>::iterator stream =
            send_streams_.begin();
        stream != send_streams_.end(); ++stream) {
     stream->second->FillBandwidthEstimationInfo(&bwe_info);
   }
   video_media_info->bw_estimations.push_back(bwe_info);
 }
 
-void WebRtcVideoChannel2::SetSource(
-    uint32_t ssrc,
-    rtc::VideoSourceInterface<cricket::VideoFrame>* source) {
-  LOG(LS_INFO) << "SetSource: " << ssrc << " -> "
-               << (source ? "(source)" : "NULL");
-  RTC_DCHECK(ssrc != 0);
-
-  rtc::CritScope stream_lock(&stream_crit_);
-  const auto& kv = send_streams_.find(ssrc);
-  if (kv == send_streams_.end()) {
-    // Allow unknown ssrc only if source is null.
-    RTC_CHECK(source == nullptr);
-  } else {
-    kv->second->SetSource(source);
-  }
-}
-
 void WebRtcVideoChannel2::OnPacketReceived(
     rtc::CopyOnWriteBuffer* packet,
     const rtc::PacketTime& packet_time) {
   const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp,
                                               packet_time.not_before);
   const webrtc::PacketReceiver::DeliveryStatus delivery_result =
       call_->Receiver()->DeliverPacket(
           webrtc::MediaType::VIDEO,
           packet->cdata(), packet->size(),
           webrtc_packet_time);
@@ skipping 66 matching lines @@
       webrtc::MediaType::VIDEO,
       ready ? webrtc::kNetworkUp : webrtc::kNetworkDown);
 }
 
 void WebRtcVideoChannel2::OnNetworkRouteChanged(
     const std::string& transport_name,
     const rtc::NetworkRoute& network_route) {
   call_->OnNetworkRouteChanged(transport_name, network_route);
 }
 
-// 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
   MediaChannel::SetOption(NetworkInterface::ST_RTP,
                           rtc::Socket::OPT_RCVBUF,
                           kVideoRtpBufferSize);
 
   // Speculative change to increase the outbound socket buffer size.
   // In b/15152257, we are seeing a significant number of packets discarded
   // due to lack of socket buffer space, although it's not yet clear what the
@@ skipping 148 matching lines @@
   // Not sending, abort after reconfiguration. Reconfiguration should still
   // occur to permit sending this input as quickly as possible once we start
   // sending (without having to reconfigure then).
   if (!sending_) {
     return;
   }
 
   stream_->Input()->IncomingCapturedFrame(video_frame);
 }
 
-void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSource(
+bool WebRtcVideoChannel2::WebRtcVideoSendStream::SetVideoSend(
+    bool enable,
+    const VideoOptions* options,
     rtc::VideoSourceInterface<cricket::VideoFrame>* source) {
-  TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::SetSource");
+  TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::SetVideoSend");
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (!source && !source_)
-    return;
-  DisconnectSource();
+  bool source_changing = source_ != source;
+  if (source_changing) {
+    DisconnectSource();
+  }
 
   {
     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_ = rtc::Optional<int64_t>();
+    if (enable && options) {
+      VideoOptions old_options = parameters_.options;
+      parameters_.options.SetAll(*options);
+      // Reconfigure encoder settings on the naext frame or stream
+      // recreation if the options changed.
+      if (parameters_.options != old_options) {
+        pending_encoder_reconfiguration_ = true;
+      }
+    }
 
-    if (source == NULL) {
-      if (stream_ != NULL) {
+    if (source_changing) {
+      // Reset timestamps to realign new incoming frames to a webrtc timestamp.
+      // A new source may have a different timestamp delta than the previous
+      // one.
+      first_frame_timestamp_ms_ = rtc::Optional<int64_t>();
+
+      if (source == nullptr && stream_ != nullptr) {
         LOG(LS_VERBOSE) << "Disabling capturer, sending black frame.";
         // Force this black frame not to be dropped due to timestamp order
         // check. As IncomingCapturedFrame will drop the frame if this frame's
         // timestamp is less than or equal to last frame's timestamp, it is
         // necessary to give this black frame a larger timestamp than the
         // previous one.
         last_frame_timestamp_ms_ += 1;
         stream_->Input()->IncomingCapturedFrame(
             CreateBlackFrame(last_dimensions_.width, last_dimensions_.height,
                              last_frame_timestamp_ms_, last_rotation_));
-
-
       }
+      source_ = source;
     }
   }
-  source_ = source;
+
   // |source_->AddOrUpdateSink| may not be called while holding |lock_| since
   // that might cause a lock order inversion.
-  if (source_) {
+  if (source_changing && source_ != nullptr) {

[pbos-webrtc, 2016/05/14 00:42:39]

&& source_

[Taylor Brandstetter, 2016/05/16 19:03:00]

Done.

     source_->AddOrUpdateSink(this, sink_wants_);
   }
+  return true;
 }
 
 void WebRtcVideoChannel2::WebRtcVideoSendStream::DisconnectSource() {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   if (source_ == NULL) {
     return;
   }
 
   // |source_->RemoveSink| may not be called while holding |lock_| since
   // that might cause a lock order inversion.
   source_->RemoveSink(this);
   source_ = nullptr;
-  // Reset |cpu_restricted_counter_| if the capturer is changed. It is not
+  // Reset |cpu_restricted_counter_| if the source is changed. It is not
   // possible to know if the video resolution is restricted by CPU usage after
-  // the capturer is changed since the next capturer might be screen capture
+  // the source is changed since the next source might be screen capture
   // with another resolution and frame rate.
   cpu_restricted_counter_ = 0;
 }
 
 const std::vector<uint32_t>&
 WebRtcVideoChannel2::WebRtcVideoSendStream::GetSsrcs() const {
   return ssrcs_;
 }
 
-void WebRtcVideoChannel2::WebRtcVideoSendStream::SetOptions(
-    const VideoOptions& options) {
-  rtc::CritScope cs(&lock_);
-
-  VideoOptions old_options = parameters_.options;
-  parameters_.options.SetAll(options);
-  // Reconfigure encoder settings on the next frame or stream
-  // recreation if the options changed.
-  if (parameters_.options != old_options) {
-    pending_encoder_reconfiguration_ = true;
-  }
-}
-
 webrtc::VideoCodecType CodecTypeFromName(const std::string& name) {
   if (CodecNamesEq(name, kVp8CodecName)) {
     return webrtc::kVideoCodecVP8;
   } else if (CodecNamesEq(name, kVp9CodecName)) {
     return webrtc::kVideoCodecVP9;
   } else if (CodecNamesEq(name, kH264CodecName)) {
     return webrtc::kVideoCodecH264;
   }
   return webrtc::kVideoCodecUnknown;
 }
@@ skipping 115 matching lines @@
       SetCodec(*parameters_.codec_settings);
       recreate_stream = false;  // SetCodec has already recreated the stream.
     }
     if (recreate_stream) {
       LOG(LS_INFO)
           << "RecreateWebRtcStream (send) because of SetSendParameters";
       RecreateWebRtcStream();
     }
   }  // release |lock_|
 
-  // |capturer_->AddOrUpdateSink| may not be called while holding |lock_| since
+  // |source_->AddOrUpdateSink| may not be called while holding |lock_| since
   // that might cause a lock order inversion.
   if (params.rtp_header_extensions) {
     sink_wants_.rotation_applied = !ContainsHeaderExtension(
         *params.rtp_header_extensions, kRtpVideoRotationHeaderExtension);
     if (source_) {
       source_->AddOrUpdateSink(this, sink_wants_);
     }
   }
 }
 
@@ skipping 174 matching lines @@
     if (parameters_.options.is_screencast.value_or(false))
       return;
 
     rtc::Optional<int> max_pixel_count;
     rtc::Optional<int> max_pixel_count_step_up;
     if (load == kOveruse) {
       if (cpu_restricted_counter_ >= kMaxCpuDowngrades) {
         return;
       }
       // The input video frame size will have a resolution with less than or
-      // equal to |max_pixel_count| depending on how the capturer can scale the
+      // equal to |max_pixel_count| depending on how the source can scale the
       // input frame size.
       max_pixel_count = rtc::Optional<int>(
           (last_dimensions_.height * last_dimensions_.width * 3) / 5);
       // Increase |number_of_cpu_adapt_changes_| if
       // sink_wants_.max_pixel_count will be changed since
-      // last time |capturer_->AddOrUpdateSink| was called. That is, this will
-      // result in a new request for the capturer to change resolution.
+      // last time |source_->AddOrUpdateSink| was called. That is, this will
+      // result in a new request for the source to change resolution.
       if (!sink_wants_.max_pixel_count ||
           *sink_wants_.max_pixel_count > *max_pixel_count) {
         ++number_of_cpu_adapt_changes_;
         ++cpu_restricted_counter_;
       }
     } else {
       RTC_DCHECK(load == kUnderuse);
       // The input video frame size will have a resolution with "one step up"
       // pixels than |max_pixel_count_step_up| where "one step up" depends on
-      // how the capturer can scale the input frame size.
+      // how the source can scale the input frame size.
       max_pixel_count_step_up = rtc::Optional<int>(last_dimensions_.height *
                                                    last_dimensions_.width);
       // Increase |number_of_cpu_adapt_changes_| if
       // sink_wants_.max_pixel_count_step_up will be changed since
-      // last time |capturer_->AddOrUpdateSink| was called. That is, this will
-      // result in a new request for the capturer to change resolution.
+      // last time |source_->AddOrUpdateSink| was called. That is, this will
+      // result in a new request for the source to change resolution.
       if (sink_wants_.max_pixel_count ||
           (sink_wants_.max_pixel_count_step_up &&
            *sink_wants_.max_pixel_count_step_up < *max_pixel_count_step_up)) {
         ++number_of_cpu_adapt_changes_;
         --cpu_restricted_counter_;
       }
     }
     sink_wants_.max_pixel_count = max_pixel_count;
     sink_wants_.max_pixel_count_step_up = max_pixel_count_step_up;
   }
@@ skipping 530 matching lines @@
         rtx_mapping[video_codecs[i].codec.id] !=
             fec_settings.red_payload_type) {
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket