diff --git a/call/call.cc b/call/call.cc index f6aa6d3248..a9e81c4bdc 100644 --- a/call/call.cc +++ b/call/call.cc @@ -268,6 +268,8 @@ class Call : public webrtc::Call, NetworkState audio_network_state_; NetworkState video_network_state_; + rtc::CriticalSection aggregate_network_up_crit_; + bool aggregate_network_up_ RTC_GUARDED_BY(aggregate_network_up_crit_); std::unique_ptr receive_crit_; // Audio, Video, and FlexFEC receive streams are owned by the client that @@ -413,6 +415,7 @@ Call::Call(const Call::Config& config, config_(config), audio_network_state_(kNetworkDown), video_network_state_(kNetworkDown), + aggregate_network_up_(false), receive_crit_(RWLockWrapper::CreateRWLock()), send_crit_(RWLockWrapper::CreateRWLock()), event_log_(config.event_log), @@ -909,7 +912,14 @@ Call::Stats Call::GetStats() const { &ssrcs, &recv_bandwidth); stats.send_bandwidth_bps = send_bandwidth; stats.recv_bandwidth_bps = recv_bandwidth; - stats.pacer_delay_ms = transport_send_->GetPacerQueuingDelayMs(); + // TODO(srte): It is unclear if we only want to report queues if network is + // available. + { + rtc::CritScope cs(&aggregate_network_up_crit_); + stats.pacer_delay_ms = + aggregate_network_up_ ? transport_send_->GetPacerQueuingDelayMs() : 0; + } + stats.rtt_ms = call_stats_->rtcp_rtt_stats()->LastProcessedRtt(); { rtc::CritScope cs(&bitrate_crit_); @@ -1016,16 +1026,17 @@ void Call::UpdateAggregateNetworkState() { have_video = true; } - NetworkState aggregate_state = kNetworkDown; - if ((have_video && video_network_state_ == kNetworkUp) || - (have_audio && audio_network_state_ == kNetworkUp)) { - aggregate_state = kNetworkUp; - } + bool aggregate_network_up = + ((have_video && video_network_state_ == kNetworkUp) || + (have_audio && audio_network_state_ == kNetworkUp)); RTC_LOG(LS_INFO) << "UpdateAggregateNetworkState: aggregate_state=" - << (aggregate_state == kNetworkUp ? "up" : "down"); - - transport_send_->OnNetworkAvailability(aggregate_state == kNetworkUp); + << (aggregate_network_up ? "up" : "down"); + { + rtc::CritScope cs(&aggregate_network_up_crit_); + aggregate_network_up_ = aggregate_network_up; + } + transport_send_->OnNetworkAvailability(aggregate_network_up); } void Call::OnSentPacket(const rtc::SentPacket& sent_packet) { diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 5554127923..80df7654ea 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -145,10 +145,10 @@ bool RtpTransportControllerSend::AvailableBandwidth(uint32_t* bandwidth) const { return send_side_cc_->AvailableBandwidth(bandwidth); } int64_t RtpTransportControllerSend::GetPacerQueuingDelayMs() const { - return send_side_cc_->GetPacerQueuingDelayMs(); + return pacer_.QueueInMs(); } int64_t RtpTransportControllerSend::GetFirstPacketTimeMs() const { - return send_side_cc_->GetFirstPacketTimeMs(); + return pacer_.FirstSentPacketTimeMs(); } void RtpTransportControllerSend::EnablePeriodicAlrProbing(bool enable) { send_side_cc_->EnablePeriodicAlrProbing(enable); diff --git a/modules/congestion_controller/include/send_side_congestion_controller.h b/modules/congestion_controller/include/send_side_congestion_controller.h index 1f579f8c45..7093da4b3d 100644 --- a/modules/congestion_controller/include/send_side_congestion_controller.h +++ b/modules/congestion_controller/include/send_side_congestion_controller.h @@ -88,8 +88,8 @@ class SendSideCongestionController RTC_DEPRECATED RtcpBandwidthObserver* GetBandwidthObserver() const; bool AvailableBandwidth(uint32_t* bandwidth) const override; - int64_t GetPacerQueuingDelayMs() const override; - int64_t GetFirstPacketTimeMs() const override; + virtual int64_t GetPacerQueuingDelayMs() const; + virtual int64_t GetFirstPacketTimeMs() const; TransportFeedbackObserver* GetTransportFeedbackObserver() override; diff --git a/modules/congestion_controller/include/send_side_congestion_controller_interface.h b/modules/congestion_controller/include/send_side_congestion_controller_interface.h index 0ff3f96277..6ea0d06c7d 100644 --- a/modules/congestion_controller/include/send_side_congestion_controller_interface.h +++ b/modules/congestion_controller/include/send_side_congestion_controller_interface.h @@ -59,8 +59,6 @@ class SendSideCongestionControllerInterface : public CallStatsObserver, size_t transport_overhead_bytes_per_packet) = 0; virtual RtcpBandwidthObserver* GetBandwidthObserver() = 0; virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0; - virtual int64_t GetPacerQueuingDelayMs() const = 0; - virtual int64_t GetFirstPacketTimeMs() const = 0; virtual TransportFeedbackObserver* GetTransportFeedbackObserver() = 0; virtual void EnablePeriodicAlrProbing(bool enable) = 0; virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0; diff --git a/modules/congestion_controller/rtp/include/send_side_congestion_controller.h b/modules/congestion_controller/rtp/include/send_side_congestion_controller.h index b69f5c0a82..e3f0c60ec2 100644 --- a/modules/congestion_controller/rtp/include/send_side_congestion_controller.h +++ b/modules/congestion_controller/rtp/include/send_side_congestion_controller.h @@ -95,8 +95,6 @@ class SendSideCongestionController RtcpBandwidthObserver* GetBandwidthObserver() override; bool AvailableBandwidth(uint32_t* bandwidth) const override; - int64_t GetPacerQueuingDelayMs() const override; - int64_t GetFirstPacketTimeMs() const override; TransportFeedbackObserver* GetTransportFeedbackObserver() override; diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller.cc b/modules/congestion_controller/rtp/send_side_congestion_controller.cc index 844d2b1895..64b8d43c61 100644 --- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc +++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc @@ -398,17 +398,6 @@ void SendSideCongestionController::UpdateStreamsConfig() { controller_->OnStreamsConfig(streams_config_); } -int64_t SendSideCongestionController::GetPacerQueuingDelayMs() const { - // TODO(srte): This should be made less synchronous. Now it grabs a lock in - // the pacer just for stats usage. Some kind of push interface might make - // sense. - return network_available_ ? pacer_->QueueInMs() : 0; -} - -int64_t SendSideCongestionController::GetFirstPacketTimeMs() const { - return pacer_->FirstSentPacketTimeMs(); -} - TransportFeedbackObserver* SendSideCongestionController::GetTransportFeedbackObserver() { return this; diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc b/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc index 579c645088..fa5d6ab642 100644 --- a/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc +++ b/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc @@ -331,22 +331,6 @@ TEST_F(SendSideCongestionControllerTest, controller_->Process(); } -TEST_F(SendSideCongestionControllerTest, GetPacerQueuingDelayMs) { - EXPECT_CALL(observer_, OnNetworkChanged(_, _, _, _)).Times(AtLeast(1)); - - const int64_t kQueueTimeMs = 123; - EXPECT_CALL(*pacer_, QueueInMs()).WillRepeatedly(Return(kQueueTimeMs)); - EXPECT_EQ(kQueueTimeMs, controller_->GetPacerQueuingDelayMs()); - - // Expect zero pacer delay when network is down. - controller_->SignalNetworkState(kNetworkDown); - EXPECT_EQ(0, controller_->GetPacerQueuingDelayMs()); - - // Network is up, pacer delay should be reported. - controller_->SignalNetworkState(kNetworkUp); - EXPECT_EQ(kQueueTimeMs, controller_->GetPacerQueuingDelayMs()); -} - TEST_F(SendSideCongestionControllerTest, GetProbingInterval) { clock_.AdvanceTimeMilliseconds(25); controller_->Process();