diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index cb63b43362..ba04b5c715 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -11,7 +11,6 @@ #include "modules/rtp_rtcp/source/rtp_sender.h" #include -#include #include #include @@ -137,9 +136,6 @@ RTPSender::RTPSender( packet_history_(clock), flexfec_packet_history_(clock), // Statistics - send_delays_(), - max_delay_it_(send_delays_.end()), - sum_delays_ms_(0), rtp_stats_callback_(nullptr), total_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale), @@ -992,32 +988,12 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, return sent; } -void RTPSender::RecomputeMaxSendDelay() { - max_delay_it_ = send_delays_.begin(); - for (auto it = send_delays_.begin(); it != send_delays_.end(); ++it) { - if (it->second >= max_delay_it_->second) { - max_delay_it_ = it; - } - } -} - -bool RTPSender::ValidIterator() { - if (max_delay_it_ == send_delays_.end()) - return send_delays_.size() == 0; - for (auto it = send_delays_.begin(); it != send_delays_.end(); ++it) { - if (max_delay_it_ == it) { - return true; - } - } - return false; -} - void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) { if (!send_side_delay_observer_ || capture_time_ms <= 0) return; uint32_t ssrc; - int avg_delay_ms = 0; + int64_t avg_delay_ms = 0; int max_delay_ms = 0; { rtc::CritScope lock(&send_critsect_); @@ -1027,74 +1003,24 @@ void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) { } { rtc::CritScope cs(&statistics_crit_); - // Compute the max and average of the recent capture-to-send delays. - // The time complexity of the current approach depends on the distribution - // of the delay values. This could be done more efficiently. - - RTC_DCHECK(ValidIterator()); - // Remove elements older than kSendSideDelayWindowMs. - auto lower_bound = - send_delays_.lower_bound(now_ms - kSendSideDelayWindowMs); - for (auto it = send_delays_.begin(); it != lower_bound; ++it) { - if (max_delay_it_ == it) { - max_delay_it_ = send_delays_.end(); - } - sum_delays_ms_ -= it->second; + // TODO(holmer): Compute this iteratively instead. + send_delays_[now_ms] = now_ms - capture_time_ms; + send_delays_.erase( + send_delays_.begin(), + send_delays_.lower_bound(now_ms - kSendSideDelayWindowMs)); + int num_delays = 0; + for (auto it = send_delays_.upper_bound(now_ms - kSendSideDelayWindowMs); + it != send_delays_.end(); ++it) { + max_delay_ms = std::max(max_delay_ms, it->second); + avg_delay_ms += it->second; + ++num_delays; } - send_delays_.erase(send_delays_.begin(), lower_bound); - if (max_delay_it_ == send_delays_.end()) { - // Removed the previous max. Need to recompute. - RecomputeMaxSendDelay(); - } - - // Add the new element. - RTC_DCHECK_GE(now_ms, static_cast(0)); - RTC_DCHECK_LE(now_ms, std::numeric_limits::max() / 2); - RTC_DCHECK_GE(capture_time_ms, static_cast(0)); - RTC_DCHECK_LE(capture_time_ms, std::numeric_limits::max() / 2); - int64_t diff_ms = now_ms - capture_time_ms; - RTC_DCHECK_GE(diff_ms, static_cast(0)); - RTC_DCHECK_LE(diff_ms, - static_cast(std::numeric_limits::max())); - int new_send_delay = rtc::dchecked_cast(now_ms - capture_time_ms); - SendDelayMap::iterator it; - bool inserted; - std::tie(it, inserted) = - send_delays_.insert(std::make_pair(now_ms, new_send_delay)); - if (!inserted) { - // TODO(terelius): If we have multiple delay measurements during the same - // millisecond then we keep the most recent one. It is not clear that this - // is the right decision, but it preserves an earlier behavior. - int previous_send_delay = it->second; - sum_delays_ms_ -= previous_send_delay; - it->second = new_send_delay; - if (max_delay_it_ == it && new_send_delay < previous_send_delay) { - RecomputeMaxSendDelay(); - } - } - if (max_delay_it_ == send_delays_.end() || - it->second >= max_delay_it_->second) { - max_delay_it_ = it; - } - sum_delays_ms_ += new_send_delay; - - size_t num_delays = send_delays_.size(); - RTC_DCHECK_GE(diff_ms, static_cast(0)); - RTC_DCHECK_LE(diff_ms, - static_cast(std::numeric_limits::max())); - RTC_DCHECK(max_delay_it_ != send_delays_.end()); - max_delay_ms = rtc::dchecked_cast(max_delay_it_->second); - int64_t avg_ms = (sum_delays_ms_ + num_delays / 2) / num_delays; - RTC_DCHECK_GE(avg_ms, static_cast(0)); - RTC_DCHECK_LE(avg_ms, - static_cast(std::numeric_limits::max())); - avg_delay_ms = - rtc::dchecked_cast((sum_delays_ms_ + num_delays / 2) / num_delays); - - RTC_DCHECK(ValidIterator()); + if (num_delays == 0) + return; + avg_delay_ms = (avg_delay_ms + num_delays / 2) / num_delays; } - send_side_delay_observer_->SendSideDelayUpdated(avg_delay_ms, max_delay_ms, - ssrc); + send_side_delay_observer_->SendSideDelayUpdated( + rtc::dchecked_cast(avg_delay_ms), max_delay_ms, ssrc); } void RTPSender::UpdateOnSendPacket(int packet_id, diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 1793b7639f..7f66d24bfd 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -240,8 +240,6 @@ class RTPSender { const PacketOptions& options, const PacedPacketInfo& pacing_info); - bool ValidIterator() RTC_EXCLUSIVE_LOCKS_REQUIRED(statistics_crit_); - void RecomputeMaxSendDelay() RTC_EXCLUSIVE_LOCKS_REQUIRED(statistics_crit_); void UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms); void UpdateOnSendPacket(int packet_id, int64_t capture_time_ms, @@ -299,8 +297,6 @@ class RTPSender { // Statistics rtc::CriticalSection statistics_crit_; SendDelayMap send_delays_ RTC_GUARDED_BY(statistics_crit_); - SendDelayMap::const_iterator max_delay_it_ RTC_GUARDED_BY(statistics_crit_); - int64_t sum_delays_ms_ RTC_GUARDED_BY(statistics_crit_); FrameCounts frame_counts_ RTC_GUARDED_BY(statistics_crit_); StreamDataCounters rtp_stats_ RTC_GUARDED_BY(statistics_crit_); StreamDataCounters rtx_rtp_stats_ RTC_GUARDED_BY(statistics_crit_); diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 58b30c2fe6..d143c8700e 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -133,11 +133,6 @@ class MockTransportSequenceNumberAllocator MOCK_METHOD0(AllocateSequenceNumber, uint16_t()); }; -class MockSendSideDelayObserver : public SendSideDelayObserver { - public: - MOCK_METHOD3(SendSideDelayUpdated, void(int, int, uint32_t)); -}; - class MockSendPacketObserver : public SendPacketObserver { public: MOCK_METHOD3(OnSendPacket, void(uint16_t, int64_t, uint32_t)); @@ -490,71 +485,6 @@ TEST_P(RtpSenderTestWithoutPacer, NoAllocationIfNotRegistered) { SendGenericPayload(); } -TEST_P(RtpSenderTestWithoutPacer, OnSendSideDelayUpdated) { - testing::StrictMock send_side_delay_observer_; - rtp_sender_.reset( - new RTPSender(false, &fake_clock_, &transport_, nullptr, nullptr, nullptr, - nullptr, nullptr, nullptr, &send_side_delay_observer_, - &mock_rtc_event_log_, nullptr, nullptr, nullptr, false)); - rtp_sender_->SetSSRC(kSsrc); - - const uint8_t kPayloadType = 127; - const uint32_t kCaptureTimeMsToRtpTimestamp = 90; // 90 kHz clock - char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC"; - RTPVideoHeader video_header; - EXPECT_EQ(0, rtp_sender_->RegisterPayload(payload_name, kPayloadType, - 1000 * kCaptureTimeMsToRtpTimestamp, - 0, 1500)); - - // Send packet with 10 ms send-side delay. The average and max should be 10 - // ms. - EXPECT_CALL(send_side_delay_observer_, SendSideDelayUpdated(10, 10, kSsrc)) - .Times(1); - int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); - fake_clock_.AdvanceTimeMilliseconds(10); - EXPECT_TRUE(rtp_sender_->SendOutgoingData( - kVideoFrameKey, kPayloadType, - capture_time_ms * kCaptureTimeMsToRtpTimestamp, capture_time_ms, - kPayloadData, sizeof(kPayloadData), nullptr, &video_header, nullptr, - kDefaultExpectedRetransmissionTimeMs)); - - // Send another packet with 20 ms delay. The average - // and max should be 15 and 20 ms respectively. - EXPECT_CALL(send_side_delay_observer_, SendSideDelayUpdated(15, 20, kSsrc)) - .Times(1); - fake_clock_.AdvanceTimeMilliseconds(10); - EXPECT_TRUE(rtp_sender_->SendOutgoingData( - kVideoFrameKey, kPayloadType, - capture_time_ms * kCaptureTimeMsToRtpTimestamp, capture_time_ms, - kPayloadData, sizeof(kPayloadData), nullptr, &video_header, nullptr, - kDefaultExpectedRetransmissionTimeMs)); - - // Send another packet at the same time, which replaces the last packet. - // Since this packet has 0 ms delay, the average is now 5 ms and max is 10 ms. - // TODO(terelius): Is is not clear that this is the right behavior. - EXPECT_CALL(send_side_delay_observer_, SendSideDelayUpdated(5, 10, kSsrc)) - .Times(1); - capture_time_ms = fake_clock_.TimeInMilliseconds(); - EXPECT_TRUE(rtp_sender_->SendOutgoingData( - kVideoFrameKey, kPayloadType, - capture_time_ms * kCaptureTimeMsToRtpTimestamp, capture_time_ms, - kPayloadData, sizeof(kPayloadData), nullptr, &video_header, nullptr, - kDefaultExpectedRetransmissionTimeMs)); - - // Send a packet 1 second later. The earlier packets should have timed - // out, so both max and average should be the delay of this packet. - fake_clock_.AdvanceTimeMilliseconds(1000); - capture_time_ms = fake_clock_.TimeInMilliseconds(); - fake_clock_.AdvanceTimeMilliseconds(1); - EXPECT_CALL(send_side_delay_observer_, SendSideDelayUpdated(1, 1, kSsrc)) - .Times(1); - EXPECT_TRUE(rtp_sender_->SendOutgoingData( - kVideoFrameKey, kPayloadType, - capture_time_ms * kCaptureTimeMsToRtpTimestamp, capture_time_ms, - kPayloadData, sizeof(kPayloadData), nullptr, &video_header, nullptr, - kDefaultExpectedRetransmissionTimeMs)); -} - TEST_P(RtpSenderTestWithoutPacer, OnSendPacketUpdated) { EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber,