Revert "Change ReceiveStatistics reaction to large sequence numbers jumps"

This reverts commit c4f120130f495e9726bf221356642de69125f4a2.

Reason for revert: breaks downstream tests due to zero max reordering for audio receive channels

Original change's description:
> Change ReceiveStatistics reaction to large sequence numbers jumps
> 
> Consider stream restart when two sequential packets arrived far from
> previous packets' sequence numbers.
> instead of resetting on single one.
> For packet loss calculation ignore sequence number gap during reset.
> 
> Bug: webrtc:9445, b/38179459
> Change-Id: I0c2717ef8f9ec182b280ae757b5582f56d9afcef
> Reviewed-on: https://webrtc-review.googlesource.com/c/111962
> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
> Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#25890}

TBR=danilchap@webrtc.org,asapersson@webrtc.org

Change-Id: Icc9f4d86d9f0b07f0fa2f3d443f9a90aa91f5e21
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:9445, b/38179459
Reviewed-on: https://webrtc-review.googlesource.com/c/113067
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25897}
This commit is contained in:
Danil Chapovalov
2018-12-04 17:16:10 +00:00
committed by Commit Bot
parent 6353fbc237
commit 7e0299e245
3 changed files with 61 additions and 295 deletions

View File

@ -48,10 +48,11 @@ StreamStatisticianImpl::StreamStatisticianImpl(
last_receive_time_ms_(0), last_receive_time_ms_(0),
last_received_timestamp_(0), last_received_timestamp_(0),
received_seq_first_(0), received_seq_first_(0),
received_seq_max_(-1), received_seq_max_(0),
received_seq_wraps_(0),
last_report_inorder_packets_(0), last_report_inorder_packets_(0),
last_report_old_packets_(0), last_report_old_packets_(0),
last_report_seq_max_(-1), last_report_seq_max_(0),
rtcp_callback_(rtcp_callback), rtcp_callback_(rtcp_callback),
rtp_callback_(rtp_callback) {} rtp_callback_(rtp_callback) {}
@ -63,75 +64,54 @@ void StreamStatisticianImpl::OnRtpPacket(const RtpPacketReceived& packet) {
rtp_callback_->DataCountersUpdated(counters, ssrc_); rtp_callback_->DataCountersUpdated(counters, ssrc_);
} }
bool StreamStatisticianImpl::UpdateOutOfOrder(const RtpPacketReceived& packet,
int64_t sequence_number,
int64_t now_ms) {
RTC_DCHECK_EQ(sequence_number,
seq_unwrapper_.UnwrapWithoutUpdate(packet.SequenceNumber()));
// Check if |packet| is second packet of a stream restart.
if (received_seq_out_of_order_) {
uint16_t expected_sequence_number = *received_seq_out_of_order_ + 1;
received_seq_out_of_order_ = absl::nullopt;
if (packet.SequenceNumber() == expected_sequence_number) {
// Ignore sequence number gap caused by stream restart for next packet
// loss calculation.
last_report_seq_max_ = sequence_number;
last_report_inorder_packets_ = receive_counters_.transmitted.packets -
receive_counters_.retransmitted.packets;
// As final part of stream restart consider |packet| is not out of order.
return false;
}
}
if (std::abs(sequence_number - received_seq_max_) >
max_reordering_threshold_) {
// Sequence number gap looks too large, wait until next packet to check
// for a stream restart.
received_seq_out_of_order_ = packet.SequenceNumber();
return true;
}
if (sequence_number > received_seq_max_)
return false;
// Old out of order packet, may be retransmit.
if (enable_retransmit_detection_ && IsRetransmitOfOldPacket(packet, now_ms))
receive_counters_.retransmitted.AddPacket(packet);
return true;
}
StreamDataCounters StreamStatisticianImpl::UpdateCounters( StreamDataCounters StreamStatisticianImpl::UpdateCounters(
const RtpPacketReceived& packet) { const RtpPacketReceived& packet) {
rtc::CritScope cs(&stream_lock_); rtc::CritScope cs(&stream_lock_);
RTC_DCHECK_EQ(ssrc_, packet.Ssrc()); RTC_DCHECK_EQ(ssrc_, packet.Ssrc());
uint16_t sequence_number = packet.SequenceNumber();
bool in_order =
// First packet is always in order.
last_receive_time_ms_ == 0 ||
IsNewerSequenceNumber(sequence_number, received_seq_max_) ||
// If we have a restart of the remote side this packet is still in order.
!IsNewerSequenceNumber(sequence_number,
received_seq_max_ - max_reordering_threshold_);
int64_t now_ms = clock_->TimeInMilliseconds(); int64_t now_ms = clock_->TimeInMilliseconds();
incoming_bitrate_.Update(packet.size(), now_ms); incoming_bitrate_.Update(packet.size(), now_ms);
receive_counters_.transmitted.AddPacket(packet); receive_counters_.transmitted.AddPacket(packet);
if (!in_order && enable_retransmit_detection_ &&
IsRetransmitOfOldPacket(packet, now_ms)) {
receive_counters_.retransmitted.AddPacket(packet);
}
int64_t sequence_number = if (receive_counters_.transmitted.packets == 1) {
seq_unwrapper_.UnwrapWithoutUpdate(packet.SequenceNumber()); received_seq_first_ = packet.SequenceNumber();
if (!ReceivedRtpPacket()) {
received_seq_first_ = sequence_number;
last_report_seq_max_ = sequence_number - 1;
receive_counters_.first_packet_time_ms = now_ms; receive_counters_.first_packet_time_ms = now_ms;
} else if (UpdateOutOfOrder(packet, sequence_number, now_ms)) {
return receive_counters_;
} }
// In order packet.
received_seq_max_ = sequence_number;
seq_unwrapper_.UpdateLast(sequence_number);
// If new time stamp and more than one in-order packet received, calculate // Count only the new packets received. That is, if packets 1, 2, 3, 5, 4, 6
// new jitter statistics. // are received, 4 will be ignored.
if (packet.Timestamp() != last_received_timestamp_ && if (in_order) {
(receive_counters_.transmitted.packets - // Wrong if we use RetransmitOfOldPacket.
receive_counters_.retransmitted.packets) > 1) { if (receive_counters_.transmitted.packets > 1 &&
UpdateJitter(packet, now_ms); received_seq_max_ > packet.SequenceNumber()) {
// Wrap around detected.
received_seq_wraps_++;
}
// New max.
received_seq_max_ = packet.SequenceNumber();
// If new time stamp and more than one in-order packet received, calculate
// new jitter statistics.
if (packet.Timestamp() != last_received_timestamp_ &&
(receive_counters_.transmitted.packets -
receive_counters_.retransmitted.packets) > 1) {
UpdateJitter(packet, now_ms);
}
last_received_timestamp_ = packet.Timestamp();
last_receive_time_ms_ = now_ms;
} }
last_received_timestamp_ = packet.Timestamp();
last_receive_time_ms_ = now_ms;
return receive_counters_; return receive_counters_;
} }
@ -183,7 +163,9 @@ bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics,
bool reset) { bool reset) {
{ {
rtc::CritScope cs(&stream_lock_); rtc::CritScope cs(&stream_lock_);
if (!ReceivedRtpPacket()) { if (received_seq_first_ == 0 &&
receive_counters_.transmitted.payload_bytes == 0) {
// We have not received anything.
return false; return false;
} }
@ -214,7 +196,9 @@ bool StreamStatisticianImpl::GetActiveStatisticsAndReset(
// Not active. // Not active.
return false; return false;
} }
if (!ReceivedRtpPacket()) { if (received_seq_first_ == 0 &&
receive_counters_.transmitted.payload_bytes == 0) {
// We have not received anything.
return false; return false;
} }
@ -228,9 +212,19 @@ bool StreamStatisticianImpl::GetActiveStatisticsAndReset(
RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() { RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() {
RtcpStatistics stats; RtcpStatistics stats;
if (last_report_inorder_packets_ == 0) {
// First time we send a report.
last_report_seq_max_ = received_seq_first_ - 1;
}
// Calculate fraction lost. // Calculate fraction lost.
int64_t exp_since_last = received_seq_max_ - last_report_seq_max_; uint16_t exp_since_last = (received_seq_max_ - last_report_seq_max_);
RTC_DCHECK_GE(exp_since_last, 0);
if (last_report_seq_max_ > received_seq_max_) {
// Can we assume that the seq_num can't go decrease over a full RTCP period?
exp_since_last = 0;
}
// Number of received RTP packets since last report, counts all packets but // Number of received RTP packets since last report, counts all packets but
// not re-transmissions. // not re-transmissions.
@ -267,7 +261,7 @@ RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() {
cumulative_loss_ += missing; cumulative_loss_ += missing;
stats.packets_lost = cumulative_loss_; stats.packets_lost = cumulative_loss_;
stats.extended_highest_sequence_number = stats.extended_highest_sequence_number =
static_cast<uint32_t>(received_seq_max_); (received_seq_wraps_ << 16) + received_seq_max_;
// Note: internal jitter value is in Q4 and needs to be scaled by 1/16. // Note: internal jitter value is in Q4 and needs to be scaled by 1/16.
stats.jitter = jitter_q4_ >> 4; stats.jitter = jitter_q4_ >> 4;

View File

@ -17,8 +17,6 @@
#include <map> #include <map>
#include <vector> #include <vector>
#include "absl/types/optional.h"
#include "modules/include/module_common_types_public.h"
#include "rtc_base/criticalsection.h" #include "rtc_base/criticalsection.h"
#include "rtc_base/rate_statistics.h" #include "rtc_base/rate_statistics.h"
#include "rtc_base/thread_annotations.h" #include "rtc_base/thread_annotations.h"
@ -60,18 +58,7 @@ class StreamStatisticianImpl : public StreamStatistician,
RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
void UpdateJitter(const RtpPacketReceived& packet, int64_t receive_time_ms) void UpdateJitter(const RtpPacketReceived& packet, int64_t receive_time_ms)
RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
// Updates StreamStatistician for out of order packets.
// Returns true if packet considered to be out of order.
bool UpdateOutOfOrder(const RtpPacketReceived& packet,
int64_t sequence_number,
int64_t now_ms)
RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
// Updates StreamStatistician for incoming packets.
StreamDataCounters UpdateCounters(const RtpPacketReceived& packet); StreamDataCounters UpdateCounters(const RtpPacketReceived& packet);
// Checks if this StreamStatistician received any rtp packets.
bool ReceivedRtpPacket() const RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_) {
return received_seq_max_ >= 0;
}
const uint32_t ssrc_; const uint32_t ssrc_;
Clock* const clock_; Clock* const clock_;
@ -87,13 +74,9 @@ class StreamStatisticianImpl : public StreamStatistician,
int64_t last_receive_time_ms_ RTC_GUARDED_BY(&stream_lock_); int64_t last_receive_time_ms_ RTC_GUARDED_BY(&stream_lock_);
uint32_t last_received_timestamp_ RTC_GUARDED_BY(&stream_lock_); uint32_t last_received_timestamp_ RTC_GUARDED_BY(&stream_lock_);
SequenceNumberUnwrapper seq_unwrapper_ RTC_GUARDED_BY(&stream_lock_); uint16_t received_seq_first_ RTC_GUARDED_BY(&stream_lock_);
int64_t received_seq_first_ RTC_GUARDED_BY(&stream_lock_); uint16_t received_seq_max_ RTC_GUARDED_BY(&stream_lock_);
int64_t received_seq_max_ RTC_GUARDED_BY(&stream_lock_); uint16_t received_seq_wraps_ RTC_GUARDED_BY(&stream_lock_);
// Assume that the other side restarted when there are two sequential packets
// with large jump from received_seq_max_.
absl::optional<uint16_t> received_seq_out_of_order_
RTC_GUARDED_BY(&stream_lock_);
// Current counter values. // Current counter values.
StreamDataCounters receive_counters_ RTC_GUARDED_BY(&stream_lock_); StreamDataCounters receive_counters_ RTC_GUARDED_BY(&stream_lock_);
@ -101,7 +84,7 @@ class StreamStatisticianImpl : public StreamStatistician,
// Counter values when we sent the last report. // Counter values when we sent the last report.
uint32_t last_report_inorder_packets_ RTC_GUARDED_BY(&stream_lock_); uint32_t last_report_inorder_packets_ RTC_GUARDED_BY(&stream_lock_);
uint32_t last_report_old_packets_ RTC_GUARDED_BY(&stream_lock_); uint32_t last_report_old_packets_ RTC_GUARDED_BY(&stream_lock_);
int64_t last_report_seq_max_ RTC_GUARDED_BY(&stream_lock_); uint16_t last_report_seq_max_ RTC_GUARDED_BY(&stream_lock_);
RtcpStatistics last_reported_statistics_ RTC_GUARDED_BY(&stream_lock_); RtcpStatistics last_reported_statistics_ RTC_GUARDED_BY(&stream_lock_);
// stream_lock_ shouldn't be held when calling callbacks. // stream_lock_ shouldn't be held when calling callbacks.

View File

@ -295,217 +295,6 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) {
EXPECT_EQ(177u, statistics.jitter); EXPECT_EQ(177u, statistics.jitter);
} }
TEST_F(ReceiveStatisticsTest, SimpleLossComputation) {
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(3);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(4);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(5);
receive_statistics_->OnRtpPacket(packet1_);
RtcpStatistics statistics;
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
// 20% = 51/255.
EXPECT_EQ(51u, statistics.fraction_lost);
EXPECT_EQ(1, statistics.packets_lost);
}
TEST_F(ReceiveStatisticsTest, LossComputationWithReordering) {
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(3);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(2);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(5);
receive_statistics_->OnRtpPacket(packet1_);
RtcpStatistics statistics;
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
// 20% = 51/255.
EXPECT_EQ(51u, statistics.fraction_lost);
EXPECT_EQ(1, statistics.packets_lost);
}
TEST_F(ReceiveStatisticsTest, LossComputationWithDuplicates) {
// Lose 2 packets, but also receive 1 duplicate. Should actually count as
// only 1 packet being lost.
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(4);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(4);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(5);
receive_statistics_->OnRtpPacket(packet1_);
RtcpStatistics statistics;
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
// 20% = 51/255.
EXPECT_EQ(51u, statistics.fraction_lost);
EXPECT_EQ(1, statistics.packets_lost);
}
TEST_F(ReceiveStatisticsTest, LossComputationWithSequenceNumberWrapping) {
// First, test loss computation over a period that included a sequence number
// rollover.
packet1_.SetSequenceNumber(0xfffd);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(0);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(0xfffe);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
// Only one packet was actually lost, 0xffff.
RtcpStatistics statistics;
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
// 20% = 51/255.
EXPECT_EQ(51u, statistics.fraction_lost);
EXPECT_EQ(1, statistics.packets_lost);
// Now test losing one packet *after* the rollover.
packet1_.SetSequenceNumber(3);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
// 50% = 127/255.
EXPECT_EQ(127u, statistics.fraction_lost);
EXPECT_EQ(2, statistics.packets_lost);
}
TEST_F(ReceiveStatisticsTest, StreamRestartDoesntCountAsLoss) {
RtcpStatistics statistics;
receive_statistics_->SetMaxReorderingThreshold(200);
packet1_.SetSequenceNumber(0);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(400);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(0, statistics.fraction_lost);
EXPECT_EQ(0, statistics.packets_lost);
packet1_.SetSequenceNumber(401);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(0, statistics.fraction_lost);
EXPECT_EQ(0, statistics.packets_lost);
}
TEST_F(ReceiveStatisticsTest, CountsLossAfterStreamRestart) {
RtcpStatistics statistics;
receive_statistics_->SetMaxReorderingThreshold(200);
packet1_.SetSequenceNumber(0);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(400);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(401);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(403);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(1, statistics.packets_lost);
}
TEST_F(ReceiveStatisticsTest, StreamCanRestartAtSequenceNumberWrapAround) {
RtcpStatistics statistics;
receive_statistics_->SetMaxReorderingThreshold(200);
packet1_.SetSequenceNumber(0xffff - 401);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(0xffff - 400);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(0xffff);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(0);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(2);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(1, statistics.packets_lost);
}
TEST_F(ReceiveStatisticsTest, StreamRestartNeedsTwoConsecutivePackets) {
RtcpStatistics statistics;
receive_statistics_->SetMaxReorderingThreshold(200);
packet1_.SetSequenceNumber(400);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(401);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
packet1_.SetSequenceNumber(3);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(401u, statistics.extended_highest_sequence_number);
packet1_.SetSequenceNumber(4);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(4u, statistics.extended_highest_sequence_number);
}
TEST_F(ReceiveStatisticsTest, WrapsAroundExtendedHighestSequenceNumber) {
RtcpStatistics statistics;
packet1_.SetSequenceNumber(0xffff);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(0xffffu, statistics.extended_highest_sequence_number);
// Wrap around.
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(0x10001u, statistics.extended_highest_sequence_number);
// Should be treated as out of order; shouldn't increment highest extended
// sequence number.
packet1_.SetSequenceNumber(0x10000 - 6);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(0x10001u, statistics.extended_highest_sequence_number);
// Receive a couple packets then wrap around again.
receive_statistics_->SetMaxReorderingThreshold(200);
for (int i = 10; i < 0xffff; i += 150) {
packet1_.SetSequenceNumber(i);
receive_statistics_->OnRtpPacket(packet1_);
}
packet1_.SetSequenceNumber(1);
receive_statistics_->OnRtpPacket(packet1_);
receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics,
true);
EXPECT_EQ(0x20001u, statistics.extended_highest_sequence_number);
}
class RtpTestCallback : public StreamDataCountersCallback { class RtpTestCallback : public StreamDataCountersCallback {
public: public:
RtpTestCallback() RtpTestCallback()