Make the audio channel communicate network state changes to the call.

webrtc/call/call.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 68 matching lines @@
 
   Stats GetStats() const override;
 
   DeliveryStatus DeliverPacket(MediaType media_type,
                                const uint8_t* packet,
                                size_t length,
                                const PacketTime& packet_time) override;
 
   void SetBitrateConfig(
       const webrtc::Call::Config::BitrateConfig& bitrate_config) override;
-  void SignalNetworkState(NetworkState state) override;
+
+  void SignalChannelNetworkState(MediaType media, NetworkState state) override;
 
   void OnSentPacket(const rtc::SentPacket& sent_packet) override;
 
   // Implements BitrateObserver.
   void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss,
                         int64_t rtt_ms) override;
 
  private:
   DeliveryStatus DeliverRtcp(MediaType media_type, const uint8_t* packet,
                              size_t length);
   DeliveryStatus DeliverRtp(MediaType media_type,
                             const uint8_t* packet,
                             size_t length,
                             const PacketTime& packet_time);
 
   void ConfigureSync(const std::string& sync_group)
       EXCLUSIVE_LOCKS_REQUIRED(receive_crit_);
 
   VoiceEngine* voice_engine() {
     internal::AudioState* audio_state =
         static_cast<internal::AudioState*>(config_.audio_state.get());
     if (audio_state)
       return audio_state->voice_engine();
     else
       return nullptr;
   }
 
   void UpdateSendHistograms() EXCLUSIVE_LOCKS_REQUIRED(&bitrate_crit_);
   void UpdateReceiveHistograms();
+  void UpdateAggregateNetworkState();
 
   Clock* const clock_;
 
   const int num_cpu_cores_;
   const rtc::scoped_ptr<ProcessThread> module_process_thread_;
   const rtc::scoped_ptr<ProcessThread> pacer_thread_;
   const rtc::scoped_ptr<CallStats> call_stats_;
   const rtc::scoped_ptr<BitrateAllocator> bitrate_allocator_;
   Call::Config config_;
   rtc::ThreadChecker configuration_thread_checker_;
 
-  bool network_enabled_;
+  NetworkState audio_network_state_;
+  NetworkState video_network_state_;
 
   rtc::scoped_ptr<RWLockWrapper> receive_crit_;
   // Audio and Video receive streams are owned by the client that creates them.
   std::map<uint32_t, AudioReceiveStream*> audio_receive_ssrcs_
       GUARDED_BY(receive_crit_);
   std::map<uint32_t, VideoReceiveStream*> video_receive_ssrcs_
       GUARDED_BY(receive_crit_);
   std::set<VideoReceiveStream*> video_receive_streams_
       GUARDED_BY(receive_crit_);
   std::map<std::string, AudioReceiveStream*> sync_stream_mapping_
@@ skipping 40 matching lines @@
 namespace internal {
 
 Call::Call(const Call::Config& config)
     : clock_(Clock::GetRealTimeClock()),
       num_cpu_cores_(CpuInfo::DetectNumberOfCores()),
       module_process_thread_(ProcessThread::Create("ModuleProcessThread")),
       pacer_thread_(ProcessThread::Create("PacerThread")),
       call_stats_(new CallStats(clock_)),
       bitrate_allocator_(new BitrateAllocator()),
       config_(config),
-      network_enabled_(true),
+      audio_network_state_(kNetworkUp),
+      video_network_state_(kNetworkUp),
       receive_crit_(RWLockWrapper::CreateRWLock()),
       send_crit_(RWLockWrapper::CreateRWLock()),
       received_video_bytes_(0),
       received_audio_bytes_(0),
       received_rtcp_bytes_(0),
       first_rtp_packet_received_ms_(-1),
       last_rtp_packet_received_ms_(-1),
       first_packet_sent_ms_(-1),
       estimated_send_bitrate_sum_kbits_(0),
       pacer_bitrate_sum_kbits_(0),
@@ skipping 107 matching lines @@
   // RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   return this;
 }
 
 webrtc::AudioSendStream* Call::CreateAudioSendStream(
     const webrtc::AudioSendStream::Config& config) {
   TRACE_EVENT0("webrtc", "Call::CreateAudioSendStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   AudioSendStream* send_stream = new AudioSendStream(
       config, config_.audio_state, congestion_controller_.get());
-  if (!network_enabled_)
-    send_stream->SignalNetworkState(kNetworkDown);
   {
     WriteLockScoped write_lock(*send_crit_);
     RTC_DCHECK(audio_send_ssrcs_.find(config.rtp.ssrc) ==
                audio_send_ssrcs_.end());
     audio_send_ssrcs_[config.rtp.ssrc] = send_stream;
   }
+  send_stream->SignalNetworkState(audio_network_state_);
+  UpdateAggregateNetworkState();
   return send_stream;
 }
 
 void Call::DestroyAudioSendStream(webrtc::AudioSendStream* send_stream) {
   TRACE_EVENT0("webrtc", "Call::DestroyAudioSendStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   RTC_DCHECK(send_stream != nullptr);
 
   send_stream->Stop();
 
   webrtc::internal::AudioSendStream* audio_send_stream =
       static_cast<webrtc::internal::AudioSendStream*>(send_stream);
   {
     WriteLockScoped write_lock(*send_crit_);
     size_t num_deleted = audio_send_ssrcs_.erase(
         audio_send_stream->config().rtp.ssrc);
     RTC_DCHECK(num_deleted == 1);
   }
+  UpdateAggregateNetworkState();
   delete audio_send_stream;
 }
 
 webrtc::AudioReceiveStream* Call::CreateAudioReceiveStream(
     const webrtc::AudioReceiveStream::Config& config) {
   TRACE_EVENT0("webrtc", "Call::CreateAudioReceiveStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   AudioReceiveStream* receive_stream = new AudioReceiveStream(
       congestion_controller_.get(), config, config_.audio_state);
   {
     WriteLockScoped write_lock(*receive_crit_);
     RTC_DCHECK(audio_receive_ssrcs_.find(config.rtp.remote_ssrc) ==
                audio_receive_ssrcs_.end());
     audio_receive_ssrcs_[config.rtp.remote_ssrc] = receive_stream;
     ConfigureSync(config.sync_group);
   }
+  receive_stream->SignalNetworkState(audio_network_state_);
+  UpdateAggregateNetworkState();
   return receive_stream;
 }
 
 void Call::DestroyAudioReceiveStream(
     webrtc::AudioReceiveStream* receive_stream) {
   TRACE_EVENT0("webrtc", "Call::DestroyAudioReceiveStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   RTC_DCHECK(receive_stream != nullptr);
   webrtc::internal::AudioReceiveStream* audio_receive_stream =
       static_cast<webrtc::internal::AudioReceiveStream*>(receive_stream);
   {
     WriteLockScoped write_lock(*receive_crit_);
     size_t num_deleted = audio_receive_ssrcs_.erase(
         audio_receive_stream->config().rtp.remote_ssrc);
     RTC_DCHECK(num_deleted == 1);
     const std::string& sync_group = audio_receive_stream->config().sync_group;
     const auto it = sync_stream_mapping_.find(sync_group);
     if (it != sync_stream_mapping_.end() &&
         it->second == audio_receive_stream) {
       sync_stream_mapping_.erase(it);
       ConfigureSync(sync_group);
     }
   }
+  UpdateAggregateNetworkState();
   delete audio_receive_stream;
 }
 
 webrtc::VideoSendStream* Call::CreateVideoSendStream(
     const webrtc::VideoSendStream::Config& config,
     const VideoEncoderConfig& encoder_config) {
   TRACE_EVENT0("webrtc", "Call::CreateVideoSendStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
 
   // TODO(mflodman): Base the start bitrate on a current bandwidth estimate, if
   // the call has already started.
   VideoSendStream* send_stream = new VideoSendStream(
       num_cpu_cores_, module_process_thread_.get(), call_stats_.get(),
       congestion_controller_.get(), &remb_, bitrate_allocator_.get(), config,
       encoder_config, suspended_video_send_ssrcs_);
-
-  if (!network_enabled_)
-    send_stream->SignalNetworkState(kNetworkDown);
-
-  WriteLockScoped write_lock(*send_crit_);
-  for (uint32_t ssrc : config.rtp.ssrcs) {
-    RTC_DCHECK(video_send_ssrcs_.find(ssrc) == video_send_ssrcs_.end());
-    video_send_ssrcs_[ssrc] = send_stream;
+  {
+    WriteLockScoped write_lock(*send_crit_);
+    for (uint32_t ssrc : config.rtp.ssrcs) {
+      RTC_DCHECK(video_send_ssrcs_.find(ssrc) == video_send_ssrcs_.end());
+      video_send_ssrcs_[ssrc] = send_stream;
+    }
+    video_send_streams_.insert(send_stream);
+    send_stream->SignalNetworkState(video_network_state_);

[the sun, 2016/03/07 14:16:43]

Move SignalNetworkState(), logging and UpdateAggregateNetworkState() outside the
scope holding the lock (we want to hold the lock for as short duration as
possible).

Also, make it the order: signal, update, log since that is closer to what you do
for the audio streams.

[skvlad, 2016/03/07 19:20:56]

Done.

+    if (event_log_)
+      event_log_->LogVideoSendStreamConfig(config);
   }
-  video_send_streams_.insert(send_stream);
-
-  if (event_log_)
-    event_log_->LogVideoSendStreamConfig(config);
-
+  UpdateAggregateNetworkState();
   return send_stream;
 }
 
 void Call::DestroyVideoSendStream(webrtc::VideoSendStream* send_stream) {
   TRACE_EVENT0("webrtc", "Call::DestroyVideoSendStream");
   RTC_DCHECK(send_stream != nullptr);
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
 
   send_stream->Stop();
 
@@ skipping 14 matching lines @@
   RTC_CHECK(send_stream_impl != nullptr);
 
   VideoSendStream::RtpStateMap rtp_state = send_stream_impl->GetRtpStates();
 
   for (VideoSendStream::RtpStateMap::iterator it = rtp_state.begin();
        it != rtp_state.end();
        ++it) {
     suspended_video_send_ssrcs_[it->first] = it->second;
   }
 
+  UpdateAggregateNetworkState();
   delete send_stream_impl;
 }
 
 webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream(
     const webrtc::VideoReceiveStream::Config& config) {
   TRACE_EVENT0("webrtc", "Call::CreateVideoReceiveStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   VideoReceiveStream* receive_stream = new VideoReceiveStream(
       num_cpu_cores_, congestion_controller_.get(), config, voice_engine(),
       module_process_thread_.get(), call_stats_.get(), &remb_);
+  {
+    WriteLockScoped write_lock(*receive_crit_);
+    RTC_DCHECK(video_receive_ssrcs_.find(config.rtp.remote_ssrc) ==
+               video_receive_ssrcs_.end());
+    video_receive_ssrcs_[config.rtp.remote_ssrc] = receive_stream;
+    // TODO(pbos): Configure different RTX payloads per receive payload.
+    VideoReceiveStream::Config::Rtp::RtxMap::const_iterator it =
+        config.rtp.rtx.begin();
+    if (it != config.rtp.rtx.end())
+      video_receive_ssrcs_[it->second.ssrc] = receive_stream;
+    video_receive_streams_.insert(receive_stream);
 
-  WriteLockScoped write_lock(*receive_crit_);
-  RTC_DCHECK(video_receive_ssrcs_.find(config.rtp.remote_ssrc) ==
-             video_receive_ssrcs_.end());
-  video_receive_ssrcs_[config.rtp.remote_ssrc] = receive_stream;
-  // TODO(pbos): Configure different RTX payloads per receive payload.
-  VideoReceiveStream::Config::Rtp::RtxMap::const_iterator it =
-      config.rtp.rtx.begin();
-  if (it != config.rtp.rtx.end())
-    video_receive_ssrcs_[it->second.ssrc] = receive_stream;
-  video_receive_streams_.insert(receive_stream);
-
-  ConfigureSync(config.sync_group);
-
-  if (!network_enabled_)
-    receive_stream->SignalNetworkState(kNetworkDown);
+    ConfigureSync(config.sync_group);
+  }
+  receive_stream->SignalNetworkState(video_network_state_);

[the sun, 2016/03/07 14:16:43]

Like above: signal, update, log
There shouldn't be any reason to have different ordering.

[skvlad, 2016/03/07 19:20:56]

Done.

 
   if (event_log_)
     event_log_->LogVideoReceiveStreamConfig(config);
 
+  UpdateAggregateNetworkState();
   return receive_stream;
 }
 
 void Call::DestroyVideoReceiveStream(
     webrtc::VideoReceiveStream* receive_stream) {
   TRACE_EVENT0("webrtc", "Call::DestroyVideoReceiveStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   RTC_DCHECK(receive_stream != nullptr);
   VideoReceiveStream* receive_stream_impl = nullptr;
   {
     WriteLockScoped write_lock(*receive_crit_);
     // Remove all ssrcs pointing to a receive stream. As RTX retransmits on a
     // separate SSRC there can be either one or two.
     auto it = video_receive_ssrcs_.begin();
     while (it != video_receive_ssrcs_.end()) {
       if (it->second == static_cast<VideoReceiveStream*>(receive_stream)) {
         if (receive_stream_impl != nullptr)
           RTC_DCHECK(receive_stream_impl == it->second);
         receive_stream_impl = it->second;
         video_receive_ssrcs_.erase(it++);
       } else {
         ++it;
       }
     }
     video_receive_streams_.erase(receive_stream_impl);
     RTC_CHECK(receive_stream_impl != nullptr);
     ConfigureSync(receive_stream_impl->config().sync_group);
   }
+  UpdateAggregateNetworkState();
   delete receive_stream_impl;
 }
 
 Call::Stats Call::GetStats() const {
   // TODO(solenberg): Some test cases in EndToEndTest use this from a different
   // thread. Re-enable once that is fixed.
   // RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   Stats stats;
   // Fetch available send/receive bitrates.
   uint32_t send_bandwidth = 0;
@@ skipping 26 matching lines @@
           bitrate_config.max_bitrate_bps) {
     // Nothing new to set, early abort to avoid encoder reconfigurations.
     return;
   }
   config_.bitrate_config = bitrate_config;
   congestion_controller_->SetBweBitrates(bitrate_config.min_bitrate_bps,
                                          bitrate_config.start_bitrate_bps,
                                          bitrate_config.max_bitrate_bps);
 }
 
-void Call::SignalNetworkState(NetworkState state) {
+void Call::SignalChannelNetworkState(MediaType media,
+                                     NetworkState state) {
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
-  network_enabled_ = state == kNetworkUp;
-  congestion_controller_->SignalNetworkState(state);
+  if (media == MediaType::AUDIO)

[the sun, 2016/03/07 14:16:43]

is MediaType::DATA ok?
I would prefer a
switch (media) {
  case MediaType::AUDIO:
    ...
  case MediaType::VIDEO:
    ....
  default:
    RTC_NOTREACHED();
    break;
}

[skvlad, 2016/03/07 19:20:56]

Done.

+    audio_network_state_ = state;
+  if (media == MediaType::VIDEO)
+    video_network_state_ = state;
+
+  UpdateAggregateNetworkState();
   {
-    ReadLockScoped write_lock(*send_crit_);
+    ReadLockScoped read_lock(*send_crit_);
     for (auto& kv : audio_send_ssrcs_) {
-      kv.second->SignalNetworkState(state);
+      kv.second->SignalNetworkState(audio_network_state_);
     }
     for (auto& kv : video_send_ssrcs_) {
-      kv.second->SignalNetworkState(state);
+      kv.second->SignalNetworkState(video_network_state_);
     }
   }
   {
-    ReadLockScoped write_lock(*receive_crit_);
+    ReadLockScoped read_lock(*receive_crit_);
+    for (auto& kv : audio_receive_ssrcs_) {
+      kv.second->SignalNetworkState(audio_network_state_);
+    }
     for (auto& kv : video_receive_ssrcs_) {
-      kv.second->SignalNetworkState(state);
+      kv.second->SignalNetworkState(video_network_state_);
     }
   }
 }
 
+void Call::UpdateAggregateNetworkState() {
+  RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
+
+  bool haveVideo = false;
+  bool haveAudio = false;
+  {
+    ReadLockScoped read_lock(*send_crit_);
+    if (audio_send_ssrcs_.size() > 0) haveAudio = true;
+    if (video_send_ssrcs_.size() > 0) haveVideo = true;
+  }
+  {
+    ReadLockScoped read_lock(*receive_crit_);
+    if (audio_receive_ssrcs_.size() > 0) haveAudio = true;
+    if (video_receive_ssrcs_.size() > 0) haveVideo = true;
+  }
+
+  NetworkState aggregateState = kNetworkDown;
+  if ((!haveVideo && !haveAudio) ||

[the sun, 2016/03/07 14:16:43]

Hmm, was this a special case for the tests? If so, can the test be modified? Or
is there some other reason we should consider the network alway up when we have
no streams?

[skvlad, 2016/03/07 19:20:56]

This used to be a special case for the tests in the previous design where the
MediaChannels had to report their status - but it's not needed  any more. Even
in tests that don't create MediaChannels, there are send/receive streams - so
state tracking works well even there. I have removed this special case.

+      (haveVideo && video_network_state_ == kNetworkUp) ||
+      (haveAudio && audio_network_state_ == kNetworkUp)) {
+    aggregateState = kNetworkUp;
+  }
+
+  LOG(LS_INFO) << "UpdateAggregateNetworkState: aggregateState=" <<
+      (aggregateState == kNetworkUp ? "up" : "down");
+
+  congestion_controller_ -> SignalNetworkState(aggregateState);
+}
+
 void Call::OnSentPacket(const rtc::SentPacket& sent_packet) {
   if (first_packet_sent_ms_ == -1)
     first_packet_sent_ms_ = clock_->TimeInMilliseconds();
   congestion_controller_->OnSentPacket(sent_packet);
 }
 
 void Call::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss,
                             int64_t rtt_ms) {
   uint32_t allocated_bitrate_bps = bitrate_allocator_->OnNetworkChanged(
       target_bitrate_bps, fraction_loss, rtt_ms);
@@ skipping 162 matching lines @@
   // thread. Then this check can be enabled.
   // RTC_DCHECK(!configuration_thread_checker_.CalledOnValidThread());
   if (RtpHeaderParser::IsRtcp(packet, length))
     return DeliverRtcp(media_type, packet, length);
 
   return DeliverRtp(media_type, packet, length, packet_time);
 }
 
 }  // namespace internal
 }  // namespace webrtc