From ccd4b2aec88c79c531254fd31611ec741c77738f Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Tue, 23 Apr 2013 15:58:23 +0000 Subject: [PATCH] Add a default RTT to CallStats and use different values for buffered/real-time mode. BUG=1613 Review URL: https://webrtc-codereview.appspot.com/1326007 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3888 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../modules/rtp_rtcp/source/rtcp_receiver.cc | 20 +------------ .../modules/rtp_rtcp/source/rtcp_receiver.h | 5 ---- .../modules/rtp_rtcp/source/rtp_receiver.cc | 3 +- webrtc/modules/rtp_rtcp/source/rtp_receiver.h | 5 ++++ .../modules/rtp_rtcp/source/rtp_rtcp_config.h | 1 + .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 13 +++----- webrtc/video_engine/call_stats.cc | 18 ++++++----- webrtc/video_engine/call_stats.h | 3 ++ webrtc/video_engine/call_stats_unittest.cc | 30 +++++++++++++++++++ webrtc/video_engine/vie_defines.h | 2 ++ webrtc/video_engine/vie_rtp_rtcp_impl.cc | 3 +- 11 files changed, 60 insertions(+), 43 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 3b9659a599..b39043af6d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -54,8 +54,7 @@ RTCPReceiver::RTCPReceiver(const int32_t id, Clock* clock, _receivedInfoMap(), _packetTimeOutMS(0), _lastReceivedRrMs(0), - _lastIncreasedSequenceNumberMs(0), - _rtt(0) { + _lastIncreasedSequenceNumberMs(0) { memset(&_remoteSenderInfo, 0, sizeof(_remoteSenderInfo)); WEBRTC_TRACE(kTraceMemory, kTraceRtpRtcp, id, "%s created", __FUNCTION__); } @@ -211,23 +210,6 @@ int32_t RTCPReceiver::RTT(const uint32_t remoteSSRC, return 0; } -uint16_t RTCPReceiver::RTT() const { - CriticalSectionScoped lock(_criticalSectionRTCPReceiver); - if (!_receivedReportBlockMap.empty()) { - return 0; - } - return _rtt; -} - -int RTCPReceiver::SetRTT(uint16_t rtt) { - CriticalSectionScoped lock(_criticalSectionRTCPReceiver); - if (!_receivedReportBlockMap.empty()) { - return -1; - } - _rtt = rtt; - return 0; -} - int32_t RTCPReceiver::NTP(uint32_t *ReceivedNTPsecs, uint32_t *ReceivedNTPfrac, diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 0833b4a573..d92318925a 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -237,11 +237,6 @@ protected: // The time we last received an RTCP RR telling we have ssuccessfully // delivered RTP packet to the remote side. int64_t _lastIncreasedSequenceNumberMs; - - // Externally set RTT. This value can only be used if there are no valid - // RTT estimates. - uint16_t _rtt; - }; } // namespace webrtc #endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_RECEIVER_H_ diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc index 7a811659a5..2855a147e9 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc @@ -95,7 +95,8 @@ RTPReceiver::RTPReceiver(const int32_t id, max_reordering_threshold_(kDefaultMaxReorderingThreshold), rtx_(false), ssrc_rtx_(0), - payload_type_rtx_(-1) { + payload_type_rtx_(-1), + rtt_ms_(kInitialReceiveSideRtt) { assert(incoming_audio_messages_callback && incoming_messages_callback && incoming_payload_callback); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver.h index eaaab79c68..998a62d33d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver.h @@ -158,6 +158,10 @@ class RTPReceiver : public Bitrate { const uint16_t bytes, const bool old_packet); + void set_rtt_ms(uint32_t rtt_ms) { rtt_ms_ = rtt_ms; } + + uint32_t rtt_ms() const { return rtt_ms_; } + private: // Returns whether RED is configured with payload_type. bool REDPayloadType(const int8_t payload_type) const; @@ -237,6 +241,7 @@ class RTPReceiver : public Bitrate { bool rtx_; uint32_t ssrc_rtx_; int payload_type_rtx_; + uint32_t rtt_ms_; }; } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h index 408354056a..c42f1de057 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h @@ -23,6 +23,7 @@ enum { NACK_BYTECOUNT_SIZE = 60}; // size of our NACK history enum { kSendSideNackListSizeSanity = 20000 }; enum { kDefaultMaxReorderingThreshold = 50 }; // In sequence numbers. enum { kRtcpMaxNackFields = 253 }; +enum { kInitialReceiveSideRtt = 100 }; enum { RTCP_INTERVAL_VIDEO_MS = 1000 }; enum { RTCP_INTERVAL_AUDIO_MS = 5000 }; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index bda7f12186..96d186e13e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -1224,9 +1224,9 @@ int32_t ModuleRtpRtcpImpl::ResetRTT(const uint32_t remote_ssrc) { return rtcp_receiver_.ResetRTT(remote_ssrc); } -void ModuleRtpRtcpImpl:: SetRtt(uint32_t rtt) { +void ModuleRtpRtcpImpl::SetRtt(uint32_t rtt) { WEBRTC_TRACE(kTraceModuleCall, kTraceRtpRtcp, id_, "SetRtt(rtt: %u)", rtt); - rtcp_receiver_.SetRTT(static_cast(rtt)); + rtp_receiver_->set_rtt_ms(rtt); } // Reset RTP statistics. @@ -1545,13 +1545,8 @@ int32_t ModuleRtpRtcpImpl::SendNACK(const uint16_t* nack_list, id_, "SendNACK(size:%u)", size); - uint16_t avg_rtt = 0; - rtcp_receiver_.RTT(rtp_receiver_->SSRC(), NULL, &avg_rtt, NULL, NULL); - - int64_t wait_time = 5 + ((avg_rtt * 3) >> 1); // 5 + RTT * 1.5. - if (wait_time == 5) { - wait_time = 100; // During startup we don't have an RTT. - } + // 5 + RTT * 1.5. + int64_t wait_time = 5 + ((rtp_receiver_->rtt_ms() * 3) / 2); const int64_t now = clock_->TimeInMilliseconds(); const int64_t time_limit = now - wait_time; uint16_t nackLength = size; diff --git a/webrtc/video_engine/call_stats.cc b/webrtc/video_engine/call_stats.cc index 233d536ab5..823c33602b 100644 --- a/webrtc/video_engine/call_stats.cc +++ b/webrtc/video_engine/call_stats.cc @@ -23,6 +23,7 @@ namespace webrtc { const int kRttTimeoutMs = 1500; // Time interval for updating the observers. const int kUpdateIntervalMs = 1000; +const uint32_t kInitialRttMs = 200; class RtcpObserver : public RtcpRttObserver { public: @@ -42,7 +43,9 @@ class RtcpObserver : public RtcpRttObserver { CallStats::CallStats() : crit_(CriticalSectionWrapper::CreateCriticalSection()), rtcp_rtt_observer_(new RtcpObserver(this)), - last_process_time_(TickTime::MillisecondTimestamp()) { + last_process_time_(TickTime::MillisecondTimestamp()), + last_reported_rtt_(kInitialRttMs), + rtt_report_received_(false) { } CallStats::~CallStats() { @@ -73,13 +76,13 @@ int32_t CallStats::Process() { if (it->rtt > max_rtt) max_rtt = it->rtt; } - - // If there is a valid rtt, update all observers. if (max_rtt > 0) { - for (std::list::iterator it = observers_.begin(); - it != observers_.end(); ++it) { - (*it)->OnRttUpdate(max_rtt); - } + last_reported_rtt_ = max_rtt; + } + // If there is a valid rtt, update all observers. + for (std::list::iterator it = observers_.begin(); + it != observers_.end(); ++it) { + (*it)->OnRttUpdate(last_reported_rtt_); } last_process_time_ = time_now; return 0; @@ -114,6 +117,7 @@ void CallStats::OnRttUpdate(uint32_t rtt) { CriticalSectionScoped cs(crit_.get()); int64_t time_now = TickTime::MillisecondTimestamp(); reports_.push_back(RttTime(rtt, time_now)); + rtt_report_received_ = true; } } // namespace webrtc diff --git a/webrtc/video_engine/call_stats.h b/webrtc/video_engine/call_stats.h index 5fd93a7624..378e1ba15a 100644 --- a/webrtc/video_engine/call_stats.h +++ b/webrtc/video_engine/call_stats.h @@ -68,6 +68,9 @@ class CallStats : public Module { // Observers getting stats reports. std::list observers_; + uint32_t last_reported_rtt_; + bool rtt_report_received_; + DISALLOW_COPY_AND_ASSIGN(CallStats); }; diff --git a/webrtc/video_engine/call_stats_unittest.cc b/webrtc/video_engine/call_stats_unittest.cc index 4c14699738..83d1c82903 100644 --- a/webrtc/video_engine/call_stats_unittest.cc +++ b/webrtc/video_engine/call_stats_unittest.cc @@ -22,6 +22,8 @@ using ::testing::Return; namespace webrtc { +enum { kDefaultRttMs = 200 }; + class MockStatsObserver : public CallStatsObserver { public: MockStatsObserver() {} @@ -176,4 +178,32 @@ TEST_F(CallStatsTest, ChangeRtt) { call_stats_->DeregisterStatsObserver(&stats_observer); } +TEST_F(CallStatsTest, NoRttUpdates) { + MockStatsObserver stats_observer; + call_stats_->RegisterStatsObserver(&stats_observer); + + // Advance clock to be ready for an update. + TickTime::AdvanceFakeClock(1000); + EXPECT_CALL(stats_observer, OnRttUpdate(kDefaultRttMs)) + .Times(1); + call_stats_->Process(); + + RtcpRttObserver* rtcp_observer = call_stats_->rtcp_rtt_observer(); + const int kNewRtt = 50; + // Report an RTT and verify that it replaces the default. + rtcp_observer->OnRttUpdate(kNewRtt); + TickTime::AdvanceFakeClock(1000); + EXPECT_CALL(stats_observer, OnRttUpdate(kNewRtt)) + .Times(1); + call_stats_->Process(); + + TickTime::AdvanceFakeClock(1500); + // The last reported RTT should still be reported when all reports have + // timed out. + EXPECT_CALL(stats_observer, OnRttUpdate(kNewRtt)) + .Times(1); + call_stats_->Process(); + + call_stats_->DeregisterStatsObserver(&stats_observer); +} } // namespace webrtc diff --git a/webrtc/video_engine/vie_defines.h b/webrtc/video_engine/vie_defines.h index 189a0e96d1..1a3095dea5 100644 --- a/webrtc/video_engine/vie_defines.h +++ b/webrtc/video_engine/vie_defines.h @@ -76,6 +76,8 @@ enum { kViEDefaultRenderDelayMs = 10 }; // ViERTP_RTCP enum { kSendSidePacketHistorySize = 600 }; +enum { kDefaultBufferingRtt = 20 }; +enum { kDefaultRealtimeRtt = 200 }; // NACK enum { kMaxPacketAgeToNack = 450 }; // In sequence numbers. diff --git a/webrtc/video_engine/vie_rtp_rtcp_impl.cc b/webrtc/video_engine/vie_rtp_rtcp_impl.cc index 4233894dff..32dcf0c225 100644 --- a/webrtc/video_engine/vie_rtp_rtcp_impl.cc +++ b/webrtc/video_engine/vie_rtp_rtcp_impl.cc @@ -631,7 +631,7 @@ int ViERTP_RTCPImpl::SetSenderBufferingMode(int video_channel, } int ViERTP_RTCPImpl::SetReceiverBufferingMode(int video_channel, - int target_delay_ms) { + int target_delay_ms) { WEBRTC_TRACE(kTraceApiCall, kTraceVideo, ViEId(shared_data_->instance_id(), video_channel), "%s(channel: %d, receiver target_delay: %d)", @@ -645,7 +645,6 @@ int ViERTP_RTCPImpl::SetReceiverBufferingMode(int video_channel, shared_data_->SetLastError(kViERtpRtcpInvalidChannelId); return -1; } - // Update the channel with buffering mode settings. if (vie_channel->SetReceiverBufferingMode(target_delay_ms) != 0) { WEBRTC_TRACE(kTraceError, kTraceVideo,