From 418dd0b96ad4101bbd39d90d939aac0275273f42 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Fri, 22 Feb 2019 10:58:49 +0100 Subject: [PATCH] Stop using special RTT value for DelayBasedBwe. There are two RTT values reported to GoogCC. They come from the same source initially but one is calculated and smoothed in the video call stats. However, there's not really any technical reasons why this value should be received via the stats, this has just been maintained for legacy reasons. Experiments shows no real difference between the modes, therefore the stats-reported RTT is removed in this CL as a cleanup. Bug: None Change-Id: If1462d6c91570ffb883ecef2ba034f04a571c9b5 Reviewed-on: https://webrtc-review.googlesource.com/c/123883 Reviewed-by: Christoffer Rodbro Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#26833} --- call/call.cc | 3 --- call/rtp_transport_controller_send.cc | 17 ----------------- call/rtp_transport_controller_send.h | 4 ---- call/rtp_transport_controller_send_interface.h | 2 -- call/test/mock_rtp_transport_controller_send.h | 1 - .../goog_cc/goog_cc_network_control.cc | 11 ++++------- 6 files changed, 4 insertions(+), 34 deletions(-) diff --git a/call/call.cc b/call/call.cc index 5baf4250ef..38fca35e75 100644 --- a/call/call.cc +++ b/call/call.cc @@ -484,8 +484,6 @@ Call::~Call() { module_process_thread_->DeRegisterModule(call_stats_.get()); module_process_thread_->Stop(); call_stats_->DeregisterStatsObserver(&receive_side_cc_); - call_stats_->DeregisterStatsObserver( - transport_send_->GetCallStatsObserver()); } int64_t first_sent_packet_ms = transport_send_->GetFirstPacketTimeMs(); @@ -517,7 +515,6 @@ void Call::RegisterRateObserver() { transport_send_ptr_->RegisterTargetTransferRateObserver(this); call_stats_->RegisterStatsObserver(&receive_side_cc_); - call_stats_->RegisterStatsObserver(transport_send_->GetCallStatsObserver()); module_process_thread_->RegisterModule( receive_side_cc_.GetRemoteBitrateEstimator(true), RTC_FROM_HERE); diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 60f3c2d80f..1152029197 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -184,9 +184,6 @@ void RtpTransportControllerSend::SetPacingFactor(float pacing_factor) { void RtpTransportControllerSend::SetQueueTimeLimit(int limit_ms) { pacer_.SetQueueTimeLimit(limit_ms); } -CallStatsObserver* RtpTransportControllerSend::GetCallStatsObserver() { - return this; -} void RtpTransportControllerSend::RegisterPacketFeedbackObserver( PacketFeedbackObserver* observer) { transport_feedback_adapter_.RegisterPacketFeedbackObserver(observer); @@ -442,20 +439,6 @@ void RtpTransportControllerSend::OnTransportFeedback( transport_feedback_adapter_.GetOutstandingData().bytes()); } -void RtpTransportControllerSend::OnRttUpdate(int64_t avg_rtt_ms, - int64_t max_rtt_ms) { - int64_t now_ms = clock_->TimeInMilliseconds(); - RoundTripTimeUpdate report; - report.receive_time = Timestamp::ms(now_ms); - report.round_trip_time = TimeDelta::ms(avg_rtt_ms); - report.smoothed = true; - task_queue_.PostTask([this, report]() { - RTC_DCHECK_RUN_ON(&task_queue_); - if (controller_) - PostUpdates(controller_->OnRoundTripTimeUpdate(report)); - }); -} - void RtpTransportControllerSend::MaybeCreateControllers() { RTC_DCHECK(!controller_); RTC_DCHECK(!control_handler_); diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index c1b576e90e..bbff3705ba 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -42,7 +42,6 @@ class RtcEventLog; class RtpTransportControllerSend final : public RtpTransportControllerSendInterface, public RtcpBandwidthObserver, - public CallStatsObserver, public TransportFeedbackObserver { public: RtpTransportControllerSend( @@ -81,7 +80,6 @@ class RtpTransportControllerSend final void SetKeepAliveConfig(const RtpKeepAliveConfig& config); void SetPacingFactor(float pacing_factor) override; void SetQueueTimeLimit(int limit_ms) override; - CallStatsObserver* GetCallStatsObserver() override; void RegisterPacketFeedbackObserver( PacketFeedbackObserver* observer) override; void DeRegisterPacketFeedbackObserver( @@ -116,8 +114,6 @@ class RtpTransportControllerSend final const PacedPacketInfo& pacing_info) override; void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override; - // Implements CallStatsObserver interface - void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; private: void MaybeCreateControllers() RTC_RUN_ON(task_queue_); void UpdateInitialConstraints(TargetRateConstraints new_contraints) diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index e8c1fccdd0..e0401a110e 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -134,8 +134,6 @@ class RtpTransportControllerSendInterface { virtual void SetPacingFactor(float pacing_factor) = 0; virtual void SetQueueTimeLimit(int limit_ms) = 0; - virtual CallStatsObserver* GetCallStatsObserver() = 0; - virtual void RegisterPacketFeedbackObserver( PacketFeedbackObserver* observer) = 0; virtual void DeRegisterPacketFeedbackObserver( diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index dd9fc85bf8..415b27abd5 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -52,7 +52,6 @@ class MockRtpTransportControllerSend MOCK_METHOD3(SetAllocatedSendBitrateLimits, void(int, int, int)); MOCK_METHOD1(SetPacingFactor, void(float)); MOCK_METHOD1(SetQueueTimeLimit, void(int)); - MOCK_METHOD0(GetCallStatsObserver, CallStatsObserver*()); MOCK_METHOD1(RegisterPacketFeedbackObserver, void(PacketFeedbackObserver*)); MOCK_METHOD1(DeRegisterPacketFeedbackObserver, void(PacketFeedbackObserver*)); MOCK_METHOD1(RegisterTargetTransferRateObserver, diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc index c5466f7a43..4c6b20bb42 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -255,14 +255,11 @@ NetworkControlUpdate GoogCcNetworkController::OnRemoteBitrateReport( NetworkControlUpdate GoogCcNetworkController::OnRoundTripTimeUpdate( RoundTripTimeUpdate msg) { - if (packet_feedback_only_) + if (packet_feedback_only_ || msg.smoothed) return NetworkControlUpdate(); - if (msg.smoothed) { - if (delay_based_bwe_) - delay_based_bwe_->OnRttUpdate(msg.round_trip_time); - } else { - bandwidth_estimation_->UpdateRtt(msg.round_trip_time, msg.receive_time); - } + if (delay_based_bwe_) + delay_based_bwe_->OnRttUpdate(msg.round_trip_time); + bandwidth_estimation_->UpdateRtt(msg.round_trip_time, msg.receive_time); return NetworkControlUpdate(); }