Initialize VideoEncoder objects asynchronously.

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 128 matching lines @@
       num_active_rtp_rtcp_modules_(1) {
   vie_receiver_.SetRtpRtcpModule(rtp_rtcp_modules_[0]);
   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]);
-
   rtp_rtcp_modules_[0]->SetKeyFrameRequestMethod(kKeyFrameReqPliRtcp);
   if (paced_sender_) {
     for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
       rtp_rtcp->SetStorePacketsStatus(true, nack_history_size_sender_);
   }
   packet_router_->AddRtpModule(rtp_rtcp_modules_[0]);
   if (sender_) {
-    std::list<RtpRtcp*> send_rtp_modules(1, rtp_rtcp_modules_[0]);
-    send_payload_router_->SetSendingRtpModules(send_rtp_modules);
+    send_payload_router_->SetSendingRtpModules(
+        std::vector<RtpRtcp*>(1, rtp_rtcp_modules_[0]));
     RTC_DCHECK(!send_payload_router_->active());
   }
   if (vcm_->RegisterReceiveCallback(this) != 0) {
     return -1;
   }
   vcm_->RegisterFrameTypeCallback(this);
   vcm_->RegisterReceiveStatisticsCallback(this);
   vcm_->RegisterDecoderTimingCallback(this);
   vcm_->SetRenderDelay(kDefaultRenderDelayMs);
 
   module_process_thread_->RegisterModule(vcm_);
   module_process_thread_->RegisterModule(&vie_sync_);
+  module_process_thread_->RegisterModule(this);
 
   return 0;
 }
 
+// TODO(pbos): Consider atomics, this might block.
+int64_t ViEChannel::TimeUntilNextProcess() {
+  size_t active_modules;
+  {
+    CriticalSectionScoped cs(crit_.get());
+    active_modules = num_active_rtp_rtcp_modules_;
+  }
+
+  int64_t min_time = rtp_rtcp_modules_[0]->TimeUntilNextProcess();
+  for (size_t i = 1; i < active_modules; ++i) {
+    int64_t time = rtp_rtcp_modules_[i]->TimeUntilNextProcess();
+    if (time < min_time)
+      min_time = time;
+  }
+  return min_time;
+}
+
+// TODO(pbos): Consider atomics, this might block.
+int32_t ViEChannel::Process() {
+  size_t active_modules;
+  {
+    CriticalSectionScoped cs(crit_.get());
+    active_modules = num_active_rtp_rtcp_modules_;
+  }
+
+  for (size_t i = 0; i < active_modules; ++i) {
+    rtp_rtcp_modules_[i]->Process();
+  }
+  return 0;
+}

[stefan-webrtc, 2016/01/19 12:02:10]

Could you enlighten why this was necessary to change in this CL?

[pbos-webrtc, 2016/01/19 15:27:58]

I can only add/remove modules from the construction thread, but I need to turn
on/off RTP modules on the fly from the encoder thread.

[stefan-webrtc, 2016/01/20 12:22:48]

I don't think I fully understand. How is the process thread involved with the
construction thread and the encoder thread?

Is it because you want to avoid having to register/deregister modules on the
process thread for some reason? Seems like we are basically duplicating code
that the ProcessThread aldready has?

[stefan-webrtc, 2016/01/27 09:06:43]

Ping

+
 ViEChannel::~ViEChannel() {
   UpdateHistograms();
   // Make sure we don't get more callbacks from the RTP module.
+  module_process_thread_->DeRegisterModule(this);
   module_process_thread_->DeRegisterModule(
       vie_receiver_.GetReceiveStatistics());
   module_process_thread_->DeRegisterModule(vcm_);
   module_process_thread_->DeRegisterModule(&vie_sync_);
-  send_payload_router_->SetSendingRtpModules(std::list<RtpRtcp*>());
+  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]);
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
     module_process_thread_->DeRegisterModule(rtp_rtcp);
     delete rtp_rtcp;
   }
   if (!sender_)
     StopDecodeThread();
   // Release modules.
   VideoCodingModule::Destroy(vcm_);
@@ skipping 136 matching lines @@
       if (vie_receiver_.IsFecEnabled()) {
         RTC_HISTOGRAM_COUNTS_SPARSE_10000(
             "WebRTC.Video.FecBitrateReceivedInKbps",
             static_cast<int>(rtp_rtx.fec.TotalBytes() * 8 / elapsed_sec /
                              1000));
       }
     }
   }
 }
 
-int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec,
-                                 bool new_stream) {
+void ViEChannel::SetSendCodec(const VideoCodec& video_codec) {

[stefan-webrtc, 2016/01/19 12:02:10]

Should we check what thread this is being called on?

[pbos-webrtc, 2016/01/19 15:27:58]

It'd be interesting to inject thread checkers from VideoSendStream that'd be
shared across objects (encoder_thread_checker). I think that sounds like a
follow-up though?

[stefan-webrtc, 2016/01/20 12:22:48]

Can't you have a separate thread checker?

+  // Here crit_ is used to synchronize between this method, StartSend and
+  // StopSend and not end up in an inconsistent sending state.
+  CriticalSectionScoped cs(crit_.get());
   RTC_DCHECK(sender_);
-  if (video_codec.codecType == kVideoCodecRED ||
-      video_codec.codecType == kVideoCodecULPFEC) {
-    LOG_F(LS_ERROR) << "Not a valid send codec " << video_codec.codecType;
-    return -1;
-  }
-  if (kMaxSimulcastStreams < video_codec.numberOfSimulcastStreams) {
-    LOG_F(LS_ERROR) << "Incorrect config "
-                    << video_codec.numberOfSimulcastStreams;
-    return -1;
-  }
+  RTC_DCHECK(video_codec.codecType != kVideoCodecRED);
+  RTC_DCHECK(video_codec.codecType != kVideoCodecULPFEC);
+  RTC_DCHECK_LE(video_codec.numberOfSimulcastStreams, kMaxSimulcastStreams);
   // Update the RTP module with the settings.
   // Stop and Start the RTP module -> trigger new SSRC, if an SSRC hasn't been
   // set explicitly.
   // The first layer is always active, so the first module can be checked for
   // sending status.
-  bool is_sending = rtp_rtcp_modules_[0]->Sending();
   bool router_was_active = send_payload_router_->active();
   send_payload_router_->set_active(false);
-  send_payload_router_->SetSendingRtpModules(std::list<RtpRtcp*>());
+  send_payload_router_->SetSendingRtpModules(std::vector<RtpRtcp*>());
 
+  size_t num_prev_active_modules = num_active_rtp_rtcp_modules_;
+  num_active_rtp_rtcp_modules_ = video_codec.numberOfSimulcastStreams > 0
+                                     ? video_codec.numberOfSimulcastStreams
+                                     : 1;
   std::vector<RtpRtcp*> registered_modules;
-  std::vector<RtpRtcp*> deregistered_modules;
-  size_t num_active_modules = video_codec.numberOfSimulcastStreams > 0
-                                   ? video_codec.numberOfSimulcastStreams
-                                   : 1;
-  size_t num_prev_active_modules;
-  {
-    // Cache which modules are active so StartSend can know which ones to start.
-    CriticalSectionScoped cs(crit_.get());
-    num_prev_active_modules = num_active_rtp_rtcp_modules_;
-    num_active_rtp_rtcp_modules_ = num_active_modules;
-  }
-  for (size_t i = 0; i < num_active_modules; ++i)
+  for (size_t i = 0; i < num_active_rtp_rtcp_modules_; ++i)
     registered_modules.push_back(rtp_rtcp_modules_[i]);
 
-  for (size_t i = num_active_modules; i < rtp_rtcp_modules_.size(); ++i)
-    deregistered_modules.push_back(rtp_rtcp_modules_[i]);
-
   // Disable inactive modules.
-  for (RtpRtcp* rtp_rtcp : deregistered_modules) {
+  for (size_t i = num_active_rtp_rtcp_modules_; i < rtp_rtcp_modules_.size();
+       ++i) {
+    RtpRtcp* const rtp_rtcp = rtp_rtcp_modules_[i];
     rtp_rtcp->SetSendingStatus(false);
     rtp_rtcp->SetSendingMediaStatus(false);
   }
 
   // Configure active modules.
+  bool is_sending = rtp_rtcp_modules_[0]->Sending();
   for (RtpRtcp* rtp_rtcp : registered_modules) {
     rtp_rtcp->DeRegisterSendPayload(video_codec.plType);
-    if (rtp_rtcp->RegisterSendPayload(video_codec) != 0) {
-      return -1;
-    }
+    RTC_CHECK_EQ(0, rtp_rtcp->RegisterSendPayload(video_codec));
     rtp_rtcp->SetSendingStatus(is_sending);
     rtp_rtcp->SetSendingMediaStatus(is_sending);
   }
 
-  // |RegisterSimulcastRtpRtcpModules| resets all old weak pointers and old
-  // modules can be deleted after this step.
   vie_receiver_.RegisterRtpRtcpModules(registered_modules);
 
-  // Update the packet and payload routers with the sending RtpRtcp modules.
-  if (sender_) {
-    std::list<RtpRtcp*> active_send_modules;
-    for (RtpRtcp* rtp_rtcp : registered_modules)
-      active_send_modules.push_back(rtp_rtcp);
-    send_payload_router_->SetSendingRtpModules(active_send_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]);
+  for (size_t i = num_active_rtp_rtcp_modules_; i < num_prev_active_modules;
+       ++i) {
     packet_router_->RemoveRtpModule(rtp_rtcp_modules_[i]);
   }
   // 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]);
+  for (size_t i = num_prev_active_modules; i < num_active_rtp_rtcp_modules_;
+       ++i) {
     packet_router_->AddRtpModule(rtp_rtcp_modules_[i]);
   }
-  return 0;
+
+  // 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);
 }

[stefan-webrtc, 2016/01/19 12:02:10]

When reading the changes for this method I can't see much that actually did
change. Can you please summarize what you have done here? Did you only remove
registration/deregistration of rtp modules?

[pbos-webrtc, 2016/01/19 15:27:58]

Yes, and move crit_ to cover the entire function (so that it remains
synchronized with StartSend and StopSend).

[stefan-webrtc, 2016/01/20 12:22:48]

Ok!

 
 int32_t ViEChannel::SetReceiveCodec(const VideoCodec& video_codec) {
   RTC_DCHECK(!sender_);
   if (!vie_receiver_.SetReceiveCodec(video_codec)) {
     return -1;
   }
 
   if (video_codec.codecType != kVideoCodecRED &&
       video_codec.codecType != kVideoCodecULPFEC) {
     // Register codec type with VCM, but do not register RED or ULPFEC.
@@ skipping 459 matching lines @@
   for (size_t i = 0; i < num_active_rtp_rtcp_modules_; ++i) {
     RtpRtcp* rtp_rtcp = rtp_rtcp_modules_[i];
     rtp_rtcp->SetSendingMediaStatus(true);
     rtp_rtcp->SetSendingStatus(true);
   }
   send_payload_router_->set_active(true);
   return 0;
 }
 
 int32_t ViEChannel::StopSend() {
+  // Acquire crit_ to synchronize sending status with StartSend and
+  // SetSendCodec.
+  CriticalSectionScoped cs(crit_.get());
   send_payload_router_->set_active(false);
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
     rtp_rtcp->SetSendingMediaStatus(false);
 
   if (!rtp_rtcp_modules_[0]->Sending()) {
     return -1;
   }
 
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
     rtp_rtcp->SetSendingStatus(false);
@@ skipping 296 matching lines @@
   CriticalSectionScoped cs(crit_.get());
   receive_stats_callback_ = receive_statistics_proxy;
 }
 
 void ViEChannel::SetIncomingVideoStream(
     IncomingVideoStream* incoming_video_stream) {
   CriticalSectionScoped cs(crit_.get());
   incoming_video_stream_ = incoming_video_stream;
 }
 }  // namespace webrtc