Move Ownership of RtpModules to VideoSendStream.

webrtc/video/video_send_stream.cc

 /*
  *  Copyright (c) 2013 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 16 matching lines @@
 #include "webrtc/video/call_stats.h"
 #include "webrtc/video/video_capture_input.h"
 #include "webrtc/video/vie_remb.h"
 #include "webrtc/video_send_stream.h"
 
 namespace webrtc {
 
 class RtcpIntraFrameObserver;
 class TransportFeedbackObserver;
 
+static const int kMinSendSidePacketHistorySize = 600;
+
+namespace {
+
+std::vector<RtpRtcp*> CreateRtpRtcpModules(
+    Transport* outgoing_transport,
+    RtcpIntraFrameObserver* intra_frame_callback,
+    RtcpBandwidthObserver* bandwidth_callback,
+    TransportFeedbackObserver* transport_feedback_callback,
+    RtcpRttStats* rtt_stats,
+    RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer,
+    RtpPacketSender* paced_sender,
+    TransportSequenceNumberAllocator* transport_sequence_number_allocator,
+    BitrateStatisticsObserver* send_bitrate_observer,
+    FrameCountObserver* send_frame_count_observer,
+    SendSideDelayObserver* send_side_delay_observer,
+    size_t num_modules) {
+  RTC_DCHECK_GT(num_modules, 0u);
+  RtpRtcp::Configuration configuration;
+  ReceiveStatistics* null_receive_statistics = configuration.receive_statistics;
+  configuration.audio = false;
+  configuration.receiver_only = false;
+  configuration.receive_statistics = null_receive_statistics;
+  configuration.outgoing_transport = outgoing_transport;
+  configuration.intra_frame_callback = intra_frame_callback;
+  configuration.rtt_stats = rtt_stats;
+  configuration.rtcp_packet_type_counter_observer =
+      rtcp_packet_type_counter_observer;
+  configuration.paced_sender = paced_sender;
+  configuration.transport_sequence_number_allocator =
+      transport_sequence_number_allocator;
+  configuration.send_bitrate_observer = send_bitrate_observer;
+  configuration.send_frame_count_observer = send_frame_count_observer;
+  configuration.send_side_delay_observer = send_side_delay_observer;
+  configuration.bandwidth_callback = bandwidth_callback;
+  configuration.transport_feedback_callback = transport_feedback_callback;
+
+  std::vector<RtpRtcp*> modules;
+  for (size_t i = 0; i < num_modules; ++i) {
+    RtpRtcp* rtp_rtcp = RtpRtcp::CreateRtpRtcp(configuration);
+    rtp_rtcp->SetSendingStatus(false);
+    rtp_rtcp->SetSendingMediaStatus(false);
+    rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
+    modules.push_back(rtp_rtcp);
+  }
+  return modules;
+}
+
+}  // namespace
+
 std::string
 VideoSendStream::Config::EncoderSettings::ToString() const {
   std::stringstream ss;
   ss << "{payload_name: " << payload_name;
   ss << ", payload_type: " << payload_type;
   ss << ", encoder: " << (encoder ? "(VideoEncoder)" : "nullptr");
   ss << '}';
   return ss.str();
 }
 
@@ skipping 129 matching lines @@
       remb_(remb),
       encoder_thread_(EncoderThreadFunction, this, "EncoderThread"),
       encoder_wakeup_event_(false, false),
       stop_encoder_thread_(0),
       overuse_detector_(
           Clock::GetRealTimeClock(),
           GetCpuOveruseOptions(config.encoder_settings.full_overuse_time),
           this,
           config.post_encode_callback,
           &stats_proxy_),
-      vie_channel_(config.send_transport,
-                   module_process_thread_,
-                   &payload_router_,
-                   nullptr,
-                   &encoder_feedback_,
-                   congestion_controller_->GetBitrateController()
-                       ->CreateRtcpBandwidthObserver(),
-                   congestion_controller_->GetTransportFeedbackObserver(),
-                   nullptr,
-                   call_stats_->rtcp_rtt_stats(),
-                   congestion_controller_->pacer(),
-                   congestion_controller_->packet_router(),
-                   config_.rtp.ssrcs.size(),
-                   true),
-      vie_receiver_(vie_channel_.vie_receiver()),
       vie_encoder_(num_cpu_cores,
                    config_.rtp.ssrcs,
                    module_process_thread_,
                    &stats_proxy_,
                    config.pre_encode_callback,
                    &overuse_detector_,
                    congestion_controller_->pacer(),
                    &payload_router_),
       vcm_(vie_encoder_.vcm()),
-      rtp_rtcp_modules_(vie_channel_.rtp_rtcp()),
+      rtp_rtcp_modules_(CreateRtpRtcpModules(
+          config.send_transport,
+          &encoder_feedback_,
+          congestion_controller_->GetBitrateController()
+              ->CreateRtcpBandwidthObserver(),

[pbos-webrtc, 2016/04/07 16:02:08]

This one leaks, perhaps it should go into a scoped_ptr?

[perkj_webrtc, 2016/04/08 10:59:04]

Done.

+          congestion_controller_->GetTransportFeedbackObserver(),
+          call_stats_->rtcp_rtt_stats(),
+          &stats_proxy_,
+          congestion_controller_->pacer(),
+          congestion_controller_->packet_router(),
+          &stats_proxy_,

[pbos-webrtc, 2016/04/07 16:02:08]

Since this function is only used here can you replace the four &stats_proxy_'s
here with one SendStatisticsProxy argument to CreateRtpRtcpModules?

[perkj_webrtc, 2016/04/08 10:59:04]

Done.

+          &stats_proxy_,
+          &stats_proxy_,
+          config_.rtp.ssrcs.size())),
+      payload_router_(rtp_rtcp_modules_),
       input_(&encoder_wakeup_event_,
              config_.local_renderer,
              &stats_proxy_,
              &overuse_detector_) {
   LOG(LS_INFO) << "VideoSendStream: " << config_.ToString();
 
   RTC_DCHECK(!config_.rtp.ssrcs.empty());
   RTC_DCHECK(module_process_thread_);
   RTC_DCHECK(call_stats_);
   RTC_DCHECK(congestion_controller_);
   RTC_DCHECK(remb_);
 
-  payload_router_.Init(rtp_rtcp_modules_);
   RTC_CHECK(vie_encoder_.Init());
   encoder_feedback_.Init(config_.rtp.ssrcs, &vie_encoder_);
-  RTC_CHECK(vie_channel_.Init() == 0);
 
-  vcm_->RegisterProtectionCallback(vie_channel_.vcm_protection_callback());
+  // RTP/RTCP initialization.
+  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
+    module_process_thread_->RegisterModule(rtp_rtcp);
+    congestion_controller_->packet_router()->AddRtpModule(rtp_rtcp);
+  }
 
-  call_stats_->RegisterStatsObserver(vie_channel_.GetStatsObserver());
+  payload_router_.SetSendingRtpModules(1);

[pbos-webrtc, 2016/04/07 16:02:08]

Should this just default to 1 within PayloadRouter?

[perkj_webrtc, 2016/04/08 10:59:04]

Done.

+
+  vcm_->RegisterProtectionCallback(this);
+
+  //  NOW! This only seems to be used receive side from looking at the code??

[pbos-webrtc, 2016/04/07 16:02:08]

Stefan, do you know what these CallStats are?

+  //  call_stats_->RegisterStatsObserver(vie_channel_.GetStatsObserver());
 
   for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) {
     const std::string& extension = config_.rtp.extensions[i].name;
     int id = config_.rtp.extensions[i].id;
     // One-byte-extension local identifiers are in the range 1-14 inclusive.
     RTC_DCHECK_GE(id, 1);
     RTC_DCHECK_LE(id, 14);
     RTC_DCHECK(RtpExtension::IsSupportedForVideo(extension));
     for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
       RTC_CHECK_EQ(0, rtp_rtcp->RegisterSendRtpHeaderExtension(
@@ skipping 12 matching lines @@
   // a waste of bandwidth since FEC packets still have to be transmitted. Note
   // that this is not the case with FLEXFEC.
   if (enable_protection_nack &&
       !PayloadTypeSupportsSkippingFecPackets(
           config_.encoder_settings.payload_name)) {
     LOG(LS_WARNING) << "Transmitting payload type without picture ID using"
                        "NACK+FEC is a waste of bandwidth since FEC packets "
                        "also have to be retransmitted. Disabling FEC.";
     enable_protection_fec = false;
   }
+
+  // Set to valid uint8_ts to be castable later without signed overflows.
+  uint8_t payload_type_red = 0;

[pbos-webrtc, 2016/04/07 16:02:08]

Can you break out this block into a separate method?

[perkj_webrtc, 2016/04/08 10:59:04]

Done.

+  uint8_t payload_type_fec = 0;
   // TODO(changbin): Should set RTX for RED mapping in RTP sender in future.
-  vie_channel_.SetProtectionMode(enable_protection_nack, enable_protection_fec,
-                                 config_.rtp.fec.red_payload_type,
-                                 config_.rtp.fec.ulpfec_payload_type);
+  // Validate payload types. If either RED or FEC payload types are set then
+  // both should be. If FEC is enabled then they both have to be set.
+  if (enable_protection_fec || config_.rtp.fec.red_payload_type != -1 ||
+      config_.rtp.fec.ulpfec_payload_type != -1) {
+    RTC_DCHECK_GE(config_.rtp.fec.red_payload_type, 0);
+    RTC_DCHECK_GE(config_.rtp.fec.ulpfec_payload_type, 0);
+    RTC_DCHECK_LE(config_.rtp.fec.red_payload_type, 127);
+    RTC_DCHECK_LE(config_.rtp.fec.ulpfec_payload_type, 127);
+    payload_type_red = static_cast<uint8_t>(config_.rtp.fec.red_payload_type);
+    payload_type_fec =
+        static_cast<uint8_t>(config_.rtp.fec.ulpfec_payload_type);
+  } else {
+    // Payload types unset.
+    RTC_DCHECK_EQ(config_.rtp.fec.red_payload_type, -1);
+    RTC_DCHECK_EQ(config_.rtp.fec.ulpfec_payload_type, -1);
+  }
+
+  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
+    // Set NACK.
+    rtp_rtcp->SetStorePacketsStatus(
+        enable_protection_nack || congestion_controller_->pacer(),
+        kMinSendSidePacketHistorySize);
+    // Set FEC.
+    for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
+      rtp_rtcp->SetGenericFECStatus(enable_protection_fec, payload_type_red,
+                                    payload_type_fec);
+    }
+  }
+
   vie_encoder_.SetProtectionMethod(enable_protection_nack,
                                    enable_protection_fec);
 
   ConfigureSsrcs();
 
   // TODO(pbos): Should we set CNAME on all RTP modules?
   rtp_rtcp_modules_.front()->SetCNAME(config_.rtp.c_name.c_str());
   // 28 to match packet overhead in ModuleRtpRtcpImpl.
   static const size_t kRtpPacketSizeOverhead = 28;
   RTC_DCHECK_LE(config_.rtp.max_packet_size, 0xFFFFu + kRtpPacketSizeOverhead);
@@ skipping 11 matching lines @@
   RTC_DCHECK(config.encoder_settings.encoder);
   RTC_DCHECK_GE(config.encoder_settings.payload_type, 0);
   RTC_DCHECK_LE(config.encoder_settings.payload_type, 127);
   RTC_CHECK_EQ(0, vie_encoder_.RegisterExternalEncoder(
                       config.encoder_settings.encoder,
                       config.encoder_settings.payload_type,
                       config.encoder_settings.internal_source));
 
   ReconfigureVideoEncoder(encoder_config);
 
-  vie_channel_.RegisterSendSideDelayObserver(&stats_proxy_);
-
   if (config_.post_encode_callback)
     vie_encoder_.RegisterPostEncodeImageCallback(&encoded_frame_proxy_);
 
   if (config_.suspend_below_min_bitrate) {
     vcm_->SuspendBelowMinBitrate();
     bitrate_allocator_->EnforceMinBitrate(false);
   }
 
-  vie_channel_.RegisterRtcpPacketTypeCounterObserver(&stats_proxy_);
-  vie_channel_.RegisterSendBitrateObserver(&stats_proxy_);
-  vie_channel_.RegisterSendFrameCountObserver(&stats_proxy_);
-
   module_process_thread_->RegisterModule(&overuse_detector_);
 
   encoder_thread_.Start();
   encoder_thread_.SetPriority(rtc::kHighPriority);
 }
 
 VideoSendStream::~VideoSendStream() {
   LOG(LS_INFO) << "~VideoSendStream: " << config_.ToString();
 
   Stop();
 
   // Stop the encoder thread permanently.
   rtc::AtomicOps::ReleaseStore(&stop_encoder_thread_, 1);
   encoder_wakeup_event_.Set();
   encoder_thread_.Stop();
 
   // This needs to happen after stopping the encoder thread,
   // since the encoder thread calls AddObserver.
   bitrate_allocator_->RemoveObserver(this);
 
   module_process_thread_->DeRegisterModule(&overuse_detector_);
-  vie_channel_.RegisterSendFrameCountObserver(nullptr);
-  vie_channel_.RegisterSendBitrateObserver(nullptr);
-  vie_channel_.RegisterRtcpPacketTypeCounterObserver(nullptr);
 
   vie_encoder_.DeRegisterExternalEncoder(config_.encoder_settings.payload_type);
 
-  call_stats_->DeregisterStatsObserver(vie_channel_.GetStatsObserver());
+  // call_stats_->DeregisterStatsObserver(vie_channel_.GetStatsObserver());
   rtp_rtcp_modules_[0]->SetREMBStatus(false);
   remb_->RemoveRembSender(rtp_rtcp_modules_[0]);
 
-  // ViEChannel outlives ViEEncoder so remove encoder from feedback before
-  // destruction.
-  encoder_feedback_.TearDown();
+  // What is this???????
+  //  congestion_controller_->GetRemoteBitrateEstimator(false)->RemoveStream(

[pbos-webrtc, 2016/04/07 16:02:08]

Looks removable, don't think we need to remote estimate anything from a sender's
perspective. Stefan?

+  //      vie_receiver_->GetRemoteSsrc());
 
-  congestion_controller_->GetRemoteBitrateEstimator(false)->RemoveStream(
-      vie_receiver_->GetRemoteSsrc());
+  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
+    congestion_controller_->packet_router()->RemoveRtpModule(rtp_rtcp);
+    module_process_thread_->DeRegisterModule(rtp_rtcp);
+    delete rtp_rtcp;
+  }
 }
 
 VideoCaptureInput* VideoSendStream::Input() {
   return &input_;
 }
 
 void VideoSendStream::Start() {
   if (payload_router_.active())
     return;
   vie_encoder_.Pause();
   payload_router_.set_active(true);
   // Was not already started, trigger a keyframe.
   vie_encoder_.SendKeyFrame();
   vie_encoder_.Restart();
-  vie_receiver_->StartReceive();
 }
 
 void VideoSendStream::Stop() {
   if (!payload_router_.active())
     return;
   // TODO(pbos): Make sure the encoder stops here.
   payload_router_.set_active(false);
-  vie_receiver_->StopReceive();
 }
 
 bool VideoSendStream::EncoderThreadFunction(void* obj) {
   static_cast<VideoSendStream*>(obj)->EncoderProcess();
   // We're done, return false to abort.
   return false;
 }
 
 void VideoSendStream::EncoderProcess() {
   while (true) {
@@ skipping 144 matching lines @@
   video_codec.maxFramerate = streams[0].max_framerate;
 
   video_codec.startBitrate =
       bitrate_allocator_->AddObserver(this,
                                       video_codec.minBitrate * 1000,
                                       video_codec.maxBitrate * 1000) / 1000;
   vie_encoder_.SetEncoder(video_codec, config.min_transmit_bitrate_bps);
 }
 
 bool VideoSendStream::DeliverRtcp(const uint8_t* packet, size_t length) {
-  return vie_receiver_->DeliverRtcp(packet, length);
+  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
+    rtp_rtcp->IncomingRtcpPacket(packet, length);
+  return true;
 }
 
 VideoSendStream::Stats VideoSendStream::GetStats() {
   return stats_proxy_.GetStats();
 }
 
 void VideoSendStream::OveruseDetected() {
   if (config_.overuse_callback)
     config_.overuse_callback->OnLoadUpdate(LoadObserver::kOveruse);
 }
@@ skipping 44 matching lines @@
       rtp_rtcp->SetRtxSendPayloadType(config_.rtp.fec.red_rtx_payload_type,
                                       config_.rtp.fec.red_payload_type);
     }
   }
 }
 
 std::map<uint32_t, RtpState> VideoSendStream::GetRtpStates() const {
   std::map<uint32_t, RtpState> rtp_states;
   for (size_t i = 0; i < config_.rtp.ssrcs.size(); ++i) {
     uint32_t ssrc = config_.rtp.ssrcs[i];
-    rtp_states[ssrc] = vie_channel_.GetRtpStateForSsrc(ssrc);
+    for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {

[pbos-webrtc, 2016/04/07 16:02:08]

Can't we use rtp_rtcp_modules_[i]->GetRtpStateForSsrc(ssrc) directly? instead of
mapping from index to SSRC and then go through all modules?

Maybe remove and say GetRtpState and GetRtxState if we no longer need to provide
SSRC as input to the module?

[perkj_webrtc, 2016/04/08 10:59:04]

hum.... right... 

+      if (rtp_rtcp->GetRtpStateForSsrc(ssrc, &rtp_states[ssrc]))
+        break;
+    }
   }
 
   for (size_t i = 0; i < config_.rtp.rtx.ssrcs.size(); ++i) {
     uint32_t ssrc = config_.rtp.rtx.ssrcs[i];
-    rtp_states[ssrc] = vie_channel_.GetRtpStateForSsrc(ssrc);
+    for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
+      if (rtp_rtcp->GetRtpStateForSsrc(ssrc, &rtp_states[ssrc]))
+        break;
+    }
   }
 
   return rtp_states;
 }
 
 void VideoSendStream::SignalNetworkState(NetworkState state) {
   // When network goes up, enable RTCP status before setting transmission state.
   // When it goes down, disable RTCP afterwards. This ensures that any packets
   // sent due to the network state changed will not be dropped.
   if (state == kNetworkUp) {
@@ skipping 10 matching lines @@
 int VideoSendStream::GetPaddingNeededBps() const {
   return vie_encoder_.GetPaddingNeededBps();
 }
 
 void VideoSendStream::OnBitrateUpdated(uint32_t bitrate_bps,
                                        uint8_t fraction_loss,
                                        int64_t rtt) {
   vie_encoder_.OnBitrateUpdated(bitrate_bps, fraction_loss, rtt);
 }
 
+
+int VideoSendStream::ProtectionRequest(const FecProtectionParams* delta_params,
+                                       const FecProtectionParams* key_params,
+                                       uint32_t* sent_video_rate_bps,
+                                       uint32_t* sent_nack_rate_bps,
+                                       uint32_t* sent_fec_rate_bps) {
+  *sent_video_rate_bps = 0;
+  *sent_nack_rate_bps = 0;
+  *sent_fec_rate_bps = 0;
+  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
+    uint32_t not_used = 0;
+    uint32_t module_video_rate = 0;
+    uint32_t module_fec_rate = 0;
+    uint32_t module_nack_rate = 0;
+    rtp_rtcp->SetFecParameters(delta_params, key_params);
+    rtp_rtcp->BitrateSent(&not_used, &module_video_rate, &module_fec_rate,
+                          &module_nack_rate);
+    *sent_video_rate_bps += module_video_rate;
+    *sent_nack_rate_bps += module_nack_rate;
+    *sent_fec_rate_bps += module_fec_rate;
+  }
+  return 0;
+}
+
 }  // namespace internal
 }  // namespace webrtc