Keep disabled RtpRtcp modules registered.

webrtc/video/vie_channel.cc

 /*
  *  Copyright (c) 2012 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 17 matching lines @@
 #include "webrtc/modules/video_coding/include/video_coding.h"
 #include "webrtc/modules/video_processing/include/video_processing.h"
 #include "webrtc/modules/video_render/video_render_defines.h"
 #include "webrtc/system_wrappers/include/metrics.h"
 #include "webrtc/video/call_stats.h"
 #include "webrtc/video/payload_router.h"
 #include "webrtc/video/receive_statistics_proxy.h"
 
 namespace webrtc {
 
-const int kMinSendSidePacketHistorySize = 600;
-const int kMaxPacketAgeToNack = 450;
-const int kMaxNackListSize = 250;
+static const int kMinSendSidePacketHistorySize = 600;
+static const int kMaxPacketAgeToNack = 450;
+static const int kMaxNackListSize = 250;
 
 // Helper class receiving statistics callbacks.
 class ChannelStatsObserver : public CallStatsObserver {
  public:
   explicit ChannelStatsObserver(ViEChannel* owner) : owner_(owner) {}
   virtual ~ChannelStatsObserver() {}
 
   // Implements StatsObserver.
   virtual void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
     owner_->OnRttUpdate(avg_rtt_ms, max_rtt_ms);
@@ skipping 44 matching lines @@
       vie_receiver_(vcm_, remote_bitrate_estimator, this),
       stats_observer_(new ChannelStatsObserver(this)),
       receive_stats_callback_(nullptr),
       incoming_video_stream_(nullptr),
       intra_frame_observer_(intra_frame_observer),
       rtt_stats_(rtt_stats),
       paced_sender_(paced_sender),
       packet_router_(packet_router),
       bandwidth_observer_(bandwidth_observer),
       transport_feedback_observer_(transport_feedback_observer),
-      nack_history_size_sender_(kMinSendSidePacketHistorySize),
       max_nack_reordering_threshold_(kMaxPacketAgeToNack),
       pre_render_callback_(NULL),
       last_rtt_ms_(0),
       rtp_rtcp_modules_(
           CreateRtpRtcpModules(!sender,
                                vie_receiver_.GetReceiveStatistics(),
                                transport,
                                intra_frame_observer_,
                                bandwidth_observer_.get(),
                                transport_feedback_observer_,
@@ skipping 16 matching lines @@
     RTC_DCHECK(vcm_);
     vcm_->SetNackSettings(kMaxNackListSize, max_nack_reordering_threshold_, 0);
   }
 }
 
 int32_t ViEChannel::Init() {
   static const int kDefaultRenderDelayMs = 10;
   module_process_thread_->RegisterModule(vie_receiver_.GetReceiveStatistics());
 
   // RTP/RTCP initialization.
-  module_process_thread_->RegisterModule(rtp_rtcp_modules_[0]);
+  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
+    module_process_thread_->RegisterModule(rtp_rtcp);

[stefan-webrtc, 2016/02/24 14:17:16]

Should these be deregistered too?

[pbos-webrtc, 2016/02/24 14:44:25]

They are deregistered in the destructor.

 
   rtp_rtcp_modules_[0]->SetKeyFrameRequestMethod(kKeyFrameReqPliRtcp);
   if (paced_sender_) {
     for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
-      rtp_rtcp->SetStorePacketsStatus(true, nack_history_size_sender_);
+      rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize);
   }
   packet_router_->AddRtpModule(rtp_rtcp_modules_[0], sender_);
   if (sender_) {
     send_payload_router_->SetSendingRtpModules(
         std::vector<RtpRtcp*>(1, rtp_rtcp_modules_[0]));
     RTC_DCHECK(!send_payload_router_->active());
   } else {
     if (vcm_->RegisterReceiveCallback(this) != 0) {
       return -1;
     }
@@ skipping 11 matching lines @@
   module_process_thread_->DeRegisterModule(
       vie_receiver_.GetReceiveStatistics());
   if (sender_) {
     send_payload_router_->SetSendingRtpModules(std::vector<RtpRtcp*>());
   }
   for (size_t i = 0; i < num_active_rtp_rtcp_modules_; ++i)
     packet_router_->RemoveRtpModule(rtp_rtcp_modules_[i], sender_);
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
     module_process_thread_->DeRegisterModule(rtp_rtcp);
     delete rtp_rtcp;
   }

[stefan-webrtc, 2016/02/24 15:04:38]

You may want to deregister these before deregistering
vie_receiver_.GetReceiveStatistics() for symmetry.

[pbos-webrtc, 2016/02/24 15:12:27]

Not sure what implications that have since RTP modules are deleted after this. I
intend to move RTP modules outside this class in the future and will do
appropriate cleanup then.

 }
 
 void ViEChannel::UpdateHistograms() {
   int64_t now = Clock::GetRealTimeClock()->TimeInMilliseconds();
 
   if (sender_) {
     RtcpPacketTypeCounter rtcp_counter;
     GetSendRtcpPacketTypeCounter(&rtcp_counter);
     int64_t elapsed_sec = rtcp_counter.TimeSinceFirstPacketInMs(now) / 1000;
     if (elapsed_sec > metrics::kMinRunTimeInSeconds) {
@@ skipping 169 matching lines @@
   // modules can be deleted after this step.
   vie_receiver_.RegisterRtpRtcpModules(registered_modules);
 
   // Update the packet and payload routers with the sending RtpRtcp modules.
   send_payload_router_->SetSendingRtpModules(registered_modules);
 
   if (router_was_active)
     send_payload_router_->set_active(true);
 
   // Deregister previously registered modules.
-  for (size_t i = num_active_modules; i < num_prev_active_modules; ++i) {
-    module_process_thread_->DeRegisterModule(rtp_rtcp_modules_[i]);
-    packet_router_->RemoveRtpModule(rtp_rtcp_modules_[i], sender_);
-  }
+  for (size_t i = num_active_modules; i < num_prev_active_modules; ++i)
+    packet_router_->RemoveRtpModule(rtp_rtcp_modules_[i], true);
   // Register new active modules.
-  for (size_t i = num_prev_active_modules; i < num_active_modules; ++i) {
-    module_process_thread_->RegisterModule(rtp_rtcp_modules_[i]);
-    packet_router_->AddRtpModule(rtp_rtcp_modules_[i], sender_);
-  }
+  for (size_t i = num_prev_active_modules; i < num_active_modules; ++i)
+    packet_router_->AddRtpModule(rtp_rtcp_modules_[i], true);
   return 0;
 }
 
 void ViEChannel::SetProtectionMode(bool enable_nack,
                                    bool enable_fec,
                                    int payload_type_red,
                                    int payload_type_fec) {
   // 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_fec || payload_type_red != -1 || payload_type_fec != -1) {
@@ skipping 32 matching lines @@
 }
 
 void ViEChannel::ProcessNACKRequest(const bool enable) {
   if (enable) {
     // Turn on NACK.
     if (rtp_rtcp_modules_[0]->RTCP() == RtcpMode::kOff)
       return;
     vie_receiver_.SetNackStatus(true, max_nack_reordering_threshold_);
 
     for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
-      rtp_rtcp->SetStorePacketsStatus(true, nack_history_size_sender_);
+      rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize);
 
     if (!sender_) {
       vcm_->RegisterPacketRequestCallback(this);
       // Don't introduce errors when NACK is enabled.
       vcm_->SetDecodeErrorMode(kNoErrors);
     }
   } else {
     if (!sender_) {
       vcm_->RegisterPacketRequestCallback(nullptr);
       for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
@@ skipping 405 matching lines @@
   rtc::CritScope lock(&crit_);
   receive_stats_callback_ = receive_statistics_proxy;
 }
 
 void ViEChannel::SetIncomingVideoStream(
     IncomingVideoStream* incoming_video_stream) {
   rtc::CritScope lock(&crit_);
   incoming_video_stream_ = incoming_video_stream;
 }
 }  // namespace webrtc